crypt32: Fix certificate adding

- Implement add disposition in CertAddCertificateContextToStore,
  rather than in each store.
- Add a few more tests.

dlls/crypt32/store.c

@@ -126,11 +126,14 @@ struct _WINE_CERT_CONTEXT;
 typedef struct _WINE_CERT_CONTEXT * (*EnumCertFunc)
  (struct WINE_CRYPTCERTSTORE *store, struct _WINE_CERT_CONTEXT *pPrev);
 
-/* Called to add a new certificate context to a store.  If ppStoreContext is
+/* Called to add a certificate context to a store.  If toReplace is not NULL,
- * not NULL, the added context should be returned in *ppStoreContext.
+ * context replaces toReplace in the store, and access checks should not be
+ * performed.  Otherwise context is a new context, and it should only be
+ * added if the store allows it.  If ppStoreContext is not NULL, the added
+ * context should be returned in *ppStoreContext.
  */
 typedef BOOL (*AddCertFunc)(struct WINE_CRYPTCERTSTORE *store,
- struct _WINE_CERT_CONTEXT *context, DWORD dwAddDisposition,
+ struct _WINE_CERT_CONTEXT *context, struct _WINE_CERT_CONTEXT *toReplace,
  PCCERT_CONTEXT *ppStoreContext);
 
 typedef enum _CertStoreType {
@@ -174,7 +177,7 @@ typedef struct _WINE_CERT_CONTEXT
     LONG         ref;
     ContextType  type;
 } WINE_CERT_CONTEXT, *PWINE_CERT_CONTEXT;
-typedef const struct _WINE_CERT_CONTEXT PCWINE_CERT_CONTEXT;
+typedef const struct _WINE_CERT_CONTEXT *PCWINE_CERT_CONTEXT;
 
 typedef struct _WINE_CERT_CONTEXT_DATA
 {
@@ -317,83 +320,38 @@ static void CRYPT_InitCertRef(PWINE_CERT_CONTEXT_LINK ref,
 }
 
 static BOOL CRYPT_MemAddCert(PWINECRYPT_CERTSTORE store,
- PWINE_CERT_CONTEXT cert, DWORD dwAddDisposition,
+ PWINE_CERT_CONTEXT cert, PWINE_CERT_CONTEXT toReplace,
  PCCERT_CONTEXT *ppStoreContext)
 {
     WINE_MEMSTORE *ms = (WINE_MEMSTORE *)store;
-    BOOL add = FALSE, ret;
+    PWINE_CERT_LIST_ENTRY entry = CryptMemAlloc(sizeof(WINE_CERT_LIST_ENTRY));
-    PCCERT_CONTEXT existing = NULL;
+    BOOL ret;
 
-    TRACE("(%p, %p, %ld, %p)\n", store, cert, dwAddDisposition, ppStoreContext);
+    TRACE("(%p, %p, %p, %p)\n", store, cert, toReplace, ppStoreContext);
 
-    if (dwAddDisposition != CERT_STORE_ADD_ALWAYS)
+    if (entry)
     {
-        BYTE hashToAdd[20];
+        PWINE_CERT_LIST_ENTRY existing = (PWINE_CERT_LIST_ENTRY)toReplace;
-        DWORD size = sizeof(hashToAdd);
 
-        ret = CRYPT_GetCertificateContextProperty((PWINE_CERT_CONTEXT)cert,
+        TRACE("adding %p\n", entry);
-         CERT_HASH_PROP_ID, hashToAdd, &size);
+        CRYPT_InitCertRef(&entry->cert, (PWINE_CERT_CONTEXT)cert, store);
-        if (ret)
+        EnterCriticalSection(&ms->cs);
-        {
-            CRYPT_HASH_BLOB blob = { sizeof(hashToAdd), hashToAdd };
-
-            existing = CertFindCertificateInStore(store,
-             cert->cert.dwCertEncodingType, 0, CERT_FIND_SHA1_HASH, &blob,
-             NULL);
-        }
-    }
-    switch (dwAddDisposition)
-    {
-    case CERT_STORE_ADD_ALWAYS:
-        add = TRUE;
-        break;
-    case CERT_STORE_ADD_NEW:
-    {
         if (existing)
         {
-            TRACE("found matching certificate, not adding\n");
+            entry->entry.prev = existing->entry.prev;
-            SetLastError(CRYPT_E_EXISTS);
+            entry->entry.next = existing->entry.next;
-            add = FALSE;
+            entry->entry.prev->next = &entry->entry;
+            entry->entry.next->prev = &entry->entry;
+            existing->entry.prev = existing->entry.next = &existing->entry;
+            CertFreeCertificateContext((PCCERT_CONTEXT)existing);
         }
         else
-            add = TRUE;
-        break;
-    }
-    case CERT_STORE_ADD_REPLACE_EXISTING:
-    {
-        add = TRUE;
-        if (existing)
-        {
-            TRACE("found matching certificate, replacing\n");
-            CertDeleteCertificateFromStore(existing);
-        }
-        break;
-    }
-    default:
-        FIXME("Unimplemented add disposition %ld\n", dwAddDisposition);
-        add = FALSE;
-    }
-    if (existing)
-        CertFreeCertificateContext(existing);
-    if (add)
-    {
-        PWINE_CERT_LIST_ENTRY entry = CryptMemAlloc(
-         sizeof(WINE_CERT_LIST_ENTRY));
-
-        if (entry)
-        {
-            TRACE("adding %p\n", entry);
-            CRYPT_InitCertRef(&entry->cert, (PWINE_CERT_CONTEXT)cert, store);
-            EnterCriticalSection(&ms->cs);
             list_add_tail(&ms->certs, &entry->entry);
-            LeaveCriticalSection(&ms->cs);
+        LeaveCriticalSection(&ms->cs);
-            if (ppStoreContext)
+        if (ppStoreContext)
-                *ppStoreContext =
+            *ppStoreContext =
-                 CertDuplicateCertificateContext((PCCERT_CONTEXT)entry);
+             CertDuplicateCertificateContext((PCCERT_CONTEXT)entry);
-            ret = TRUE;
+        ret = TRUE;
-        }
-        else
-            ret = FALSE;
     }
     else
         ret = FALSE;
@@ -506,30 +464,78 @@ static WINECRYPT_CERTSTORE *CRYPT_MemOpenStore(HCRYPTPROV hCryptProv,
     return (PWINECRYPT_CERTSTORE)store;
 }
 
+static PWINE_COLLECTION_CERT_CONTEXT CRYPT_CollectionCreateContextFromChild(
+ PWINE_COLLECTIONSTORE store, PWINE_STORE_LIST_ENTRY storeEntry,
+ PWINE_CERT_CONTEXT child)
+{
+    PWINE_COLLECTION_CERT_CONTEXT ret =
+     CryptMemAlloc(sizeof(WINE_COLLECTION_CERT_CONTEXT));
+
+    if (ret)
+    {
+        CRYPT_InitCertRef((PWINE_CERT_CONTEXT_LINK)ret, child, store);
+        /* The child has already been addref'd, and CRYPT_InitCertRef does
+         * again, so free child once to get the ref count right.  (Not doing so
+         * will leak memory if the caller calls CertFreeCertificateContext
+         * rather than CertEnumCertificatesInStore.)
+         */
+        CertFreeCertificateContext((PCCERT_CONTEXT)child);
+        ret->storeEntry = storeEntry;
+    }
+    else
+        CertFreeCertificateContext((PCCERT_CONTEXT)child);
+    return ret;
+}
+
 static BOOL CRYPT_CollectionAddCert(PWINECRYPT_CERTSTORE store,
- PWINE_CERT_CONTEXT cert, DWORD dwAddDisposition,
+ PWINE_CERT_CONTEXT cert, PWINE_CERT_CONTEXT toReplace,
  PCCERT_CONTEXT *ppStoreContext)
 {
-    PWINE_COLLECTIONSTORE cs = (PWINE_COLLECTIONSTORE)store;
-    PWINE_STORE_LIST_ENTRY entry, next;
     BOOL ret;
+    PCCERT_CONTEXT childContext = NULL;
+    PWINE_STORE_LIST_ENTRY storeEntry = NULL;
 
-    TRACE("(%p, %p, %ld, %p)\n", store, cert, dwAddDisposition, ppStoreContext);
+    TRACE("(%p, %p, %p, %p)\n", store, cert, toReplace, ppStoreContext);
 
     ret = FALSE;
-    EnterCriticalSection(&cs->cs);
+    if (toReplace)
-    LIST_FOR_EACH_ENTRY_SAFE(entry, next, &cs->stores, WINE_STORE_LIST_ENTRY,
-     entry)
     {
-        if (entry->dwUpdateFlags & CERT_PHYSICAL_STORE_ADD_ENABLE_FLAG)
+        PWINE_COLLECTION_CERT_CONTEXT existing =
-        {
+         (PWINE_COLLECTION_CERT_CONTEXT)toReplace;
-            ret = entry->store->addCert(entry->store, cert, dwAddDisposition,
+
-             ppStoreContext);
+        storeEntry = existing->storeEntry;
-            break;
+        ret = storeEntry->store->addCert(storeEntry->store, cert,
-        }
+         existing->cert.linked, &childContext);
     }
-    LeaveCriticalSection(&cs->cs);
+    else
-    SetLastError(ret ? ERROR_SUCCESS : HRESULT_FROM_WIN32(ERROR_ACCESS_DENIED));
+    {
+        PWINE_COLLECTIONSTORE cs = (PWINE_COLLECTIONSTORE)store;
+        PWINE_STORE_LIST_ENTRY entry, next;
+
+        EnterCriticalSection(&cs->cs);
+        LIST_FOR_EACH_ENTRY_SAFE(entry, next, &cs->stores,
+         WINE_STORE_LIST_ENTRY, entry)
+        {
+            if (entry->dwUpdateFlags & CERT_PHYSICAL_STORE_ADD_ENABLE_FLAG)
+            {
+                storeEntry = entry;
+                ret = entry->store->addCert(entry->store, cert, NULL,
+                 &childContext);
+                break;
+            }
+        }
+        LeaveCriticalSection(&cs->cs);
+        if (!storeEntry)
+            SetLastError(HRESULT_FROM_WIN32(ERROR_ACCESS_DENIED));
+    }
+    if (ppStoreContext && childContext)
+    {
+        *ppStoreContext =
+         (PCCERT_CONTEXT)CRYPT_CollectionCreateContextFromChild(
+         (PWINE_COLLECTIONSTORE)store, storeEntry,
+         (PWINE_CERT_CONTEXT)childContext);
+    }
+    CertFreeCertificateContext(childContext);
     return ret;
 }
 
@@ -574,7 +580,8 @@ static PWINE_COLLECTION_CERT_CONTEXT CRYPT_CollectionAdvanceEnum(
     {
         /* Ref-counting funny business: "duplicate" (addref) the child, because
          * the CertFreeCertificateContext(pPrev) below can cause the ref count
-         * to become negative.  See comment below as well.
+         * to become negative.  See comment in
+         * CRYPT_CollectionCreateContextFromChild as well.
          */
         child = ((PWINE_COLLECTION_CERT_CONTEXT)pPrev)->cert.linked;
         CertDuplicateCertificateContext((PCCERT_CONTEXT)child);
@@ -587,22 +594,7 @@ static PWINE_COLLECTION_CERT_CONTEXT CRYPT_CollectionAdvanceEnum(
         child = storeEntry->store->enumCert((HCERTSTORE)storeEntry->store,
          NULL);
     if (child)
-    {
+        ret = CRYPT_CollectionCreateContextFromChild(store, storeEntry, child);
-        ret = CryptMemAlloc(sizeof(WINE_COLLECTION_CERT_CONTEXT));
-        if (ret)
-        {
-            CRYPT_InitCertRef((PWINE_CERT_CONTEXT_LINK)ret, child, store);
-            /* enumCert already addref'd once, and CRYPT_InitCertRef does again,
-             * so free child once to get the ref count right.  (Not doing so
-             * will leak memory if the caller calls CertFreeCertificateContext
-             * rather than CertEnumCertificatesInStore.)
-             */
-            CertFreeCertificateContext((PCCERT_CONTEXT)child);
-            ret->storeEntry = storeEntry;
-        }
-        else
-            CertFreeCertificateContext((PCCERT_CONTEXT)child);
-    }
     else
     {
         if (storeNext)
@@ -713,36 +705,40 @@ static void WINAPI CRYPT_ProvCloseStore(HCERTSTORE hCertStore, DWORD dwFlags)
 }
 
 static BOOL CRYPT_ProvAddCert(PWINECRYPT_CERTSTORE store,
- PWINE_CERT_CONTEXT cert, DWORD dwAddDisposition,
+ PWINE_CERT_CONTEXT cert, PWINE_CERT_CONTEXT toReplace,
  PCCERT_CONTEXT *ppStoreContext)
 {
     PWINE_PROVIDERSTORE ps = (PWINE_PROVIDERSTORE)store;
     BOOL ret;
 
-    TRACE("(%p, %p, %ld, %p)\n", store, cert, dwAddDisposition, ppStoreContext);
+    TRACE("(%p, %p, %p, %p)\n", store, cert, toReplace, ppStoreContext);
 
-    if (ps->hdr.dwOpenFlags & CERT_STORE_READONLY_FLAG)
+    if (toReplace)
-    {
+        ret = ps->memStore->addCert(ps->memStore, cert, toReplace,
-        SetLastError(ERROR_ACCESS_DENIED);
+         ppStoreContext);
-        ret = FALSE;
-    }
     else
     {
-        ret = TRUE;
+        if (ps->hdr.dwOpenFlags & CERT_STORE_READONLY_FLAG)
-        if (ps->provWriteCert)
-            ret = ps->provWriteCert(ps->hStoreProv, (PCCERT_CONTEXT)cert,
-             CERT_STORE_PROV_WRITE_ADD_FLAG);
-        if (ret)
         {
-            ret = ps->memStore->addCert(ps->memStore, cert,
+            SetLastError(ERROR_ACCESS_DENIED);
-             dwAddDisposition, ppStoreContext);
+            ret = FALSE;
-            /* dirty trick: replace the returned context's hCertStore with
+        }
-             * store.
+        else
-             */
+        {
-            if (ppStoreContext)
+            ret = TRUE;
-                (*(PCERT_CONTEXT *)ppStoreContext)->hCertStore = store;
+            if (ps->provWriteCert)
+                ret = ps->provWriteCert(ps->hStoreProv, (PCCERT_CONTEXT)cert,
+                 CERT_STORE_PROV_WRITE_ADD_FLAG);
+            if (ret)
+                ret = ps->memStore->addCert(ps->memStore, cert, NULL,
+                 ppStoreContext);
         }
     }
+    /* dirty trick: replace the returned context's hCertStore with
+     * store.
+     */
+    if (ppStoreContext)
+        (*(PCERT_CONTEXT *)ppStoreContext)->hCertStore = store;
     return ret;
 }
 
@@ -2094,43 +2090,84 @@ PCCERT_CONTEXT WINAPI CertDuplicateCertificateContext(
     return pCertContext;
 }
 
+static void CertContext_CopyProperties(PCCERT_CONTEXT to, PCCERT_CONTEXT from)
+{
+    PWINE_CERT_CONTEXT_DATA toData, fromData;
+
+    toData = CertContext_GetDataContext((PWINE_CERT_CONTEXT)to);
+    fromData = CertContext_GetDataContext((PWINE_CERT_CONTEXT)from);
+    ContextPropertyList_Copy(toData->properties, fromData->properties);
+}
+
 BOOL WINAPI CertAddCertificateContextToStore(HCERTSTORE hCertStore,
  PCCERT_CONTEXT pCertContext, DWORD dwAddDisposition,
  PCCERT_CONTEXT *ppStoreContext)
 {
     PWINECRYPT_CERTSTORE store = (PWINECRYPT_CERTSTORE)hCertStore;
-    PWINE_CERT_CONTEXT_DATA linked = CertContext_GetDataContext(
+    BOOL ret = TRUE;
-     (PWINE_CERT_CONTEXT)pCertContext);
+    PCCERT_CONTEXT toAdd = NULL, existing = NULL;
-    PWINE_CERT_CONTEXT cert;
-    BOOL ret;
 
     TRACE("(%p, %p, %08lx, %p)\n", hCertStore, pCertContext,
      dwAddDisposition, ppStoreContext);
 
-    /* FIXME: some tests needed to verify return codes */
+    if (dwAddDisposition != CERT_STORE_ADD_ALWAYS)
-    if (!store)
     {
-        SetLastError(ERROR_INVALID_PARAMETER);
+        BYTE hashToAdd[20];
-        return FALSE;
+        DWORD size = sizeof(hashToAdd);
-    }
+
-    if (store->dwMagic != WINE_CRYPTCERTSTORE_MAGIC)
+        ret = CRYPT_GetCertificateContextProperty(
-    {
+         (PWINE_CERT_CONTEXT)pCertContext, CERT_HASH_PROP_ID, hashToAdd, &size);
-        SetLastError(ERROR_INVALID_PARAMETER);
+        if (ret)
-        return FALSE;
+        {
+            CRYPT_HASH_BLOB blob = { sizeof(hashToAdd), hashToAdd };
+
+            existing = CertFindCertificateInStore(hCertStore,
+             pCertContext->dwCertEncodingType, 0, CERT_FIND_SHA1_HASH, &blob,
+             NULL);
+        }
     }
 
-    cert = CRYPT_CreateCertificateContext(pCertContext->dwCertEncodingType,
+    switch (dwAddDisposition)
-     pCertContext->pbCertEncoded, pCertContext->cbCertEncoded);
-    if (cert)
     {
-        PWINE_CERT_CONTEXT_DATA certData = CertContext_GetDataContext(cert);
+    case CERT_STORE_ADD_ALWAYS:
-
+        toAdd = CertDuplicateCertificateContext(pCertContext);
-        ContextPropertyList_Copy(certData->properties, linked->properties);
+        break;
-        ret = store->addCert(store, cert, dwAddDisposition, ppStoreContext);
+    case CERT_STORE_ADD_NEW:
-        CertFreeCertificateContext((PCCERT_CONTEXT)cert);
+        if (existing)
-    }
+        {
-    else
+            TRACE("found matching certificate, not adding\n");
+            SetLastError(CRYPT_E_EXISTS);
+            ret = FALSE;
+        }
+        else
+            toAdd = CertDuplicateCertificateContext(pCertContext);
+        break;
+    case CERT_STORE_ADD_REPLACE_EXISTING:
+        toAdd = CertDuplicateCertificateContext(pCertContext);
+        break;
+    case CERT_STORE_ADD_REPLACE_EXISTING_INHERIT_PROPERTIES:
+        toAdd = CertDuplicateCertificateContext(pCertContext);
+        if (existing)
+            CertContext_CopyProperties(toAdd, existing);
+        break;
+    case CERT_STORE_ADD_USE_EXISTING:
+        if (existing)
+            CertContext_CopyProperties(existing, pCertContext);
+        break;
+    default:
+        FIXME("Unimplemented add disposition %ld\n", dwAddDisposition);
         ret = FALSE;
+    }
+
+    if (toAdd)
+    {
+        ret = store->addCert(store, (PWINE_CERT_CONTEXT)toAdd,
+         (PWINE_CERT_CONTEXT)existing, ppStoreContext);
+        CertFreeCertificateContext(toAdd);
+    }
+    CertFreeCertificateContext(existing);
+
+    TRACE("returning %d\n", ret);
     return ret;
 }
 
@@ -2155,7 +2192,8 @@ BOOL WINAPI CertAddEncodedCertificateToStore(HCERTSTORE hCertStore,
 
         if (cert)
         {
-            ret = hcs->addCert(hcs, cert, dwAddDisposition, ppCertContext);
+            ret = CertAddCertificateContextToStore(hCertStore,
+             (PCCERT_CONTEXT)cert, dwAddDisposition, ppCertContext);
             CertFreeCertificateContext((PCCERT_CONTEXT)cert);
         }
         else

dlls/crypt32/tests/store.c

@@ -101,6 +101,8 @@ static const BYTE bigCert2[] = { 0x30, 0x7a, 0x02, 0x01, 0x01, 0x30, 0x02, 0x06,
 static const BYTE subjectName2[] = { 0x30, 0x15, 0x31, 0x13, 0x30, 0x11, 0x06,
  0x03, 0x55, 0x04, 0x03, 0x13, 0x0a, 0x41, 0x6c, 0x65, 0x78, 0x20, 0x4c, 0x61,
  0x6e, 0x67, 0x00 };
+static const BYTE bigCert2Hash[] = { 0x4a, 0x7f, 0x32, 0x1f, 0xcf, 0x3b, 0xc0,
+ 0x87, 0x48, 0x2b, 0xa1, 0x86, 0x54, 0x18, 0xe4, 0x3a, 0x0e, 0x53, 0x7e, 0x2b };
 static const BYTE certWithUsage[] = { 0x30, 0x81, 0x93, 0x02, 0x01, 0x01, 0x30,
  0x02, 0x06, 0x00, 0x30, 0x15, 0x31, 0x13, 0x30, 0x11, 0x06, 0x03, 0x55, 0x04,
  0x03, 0x13, 0x0a, 0x4a, 0x75, 0x61, 0x6e, 0x20, 0x4c, 0x61, 0x6e, 0x67, 0x00,
@@ -115,6 +117,110 @@ static const BYTE certWithUsage[] = { 0x30, 0x81, 0x93, 0x02, 0x01, 0x01, 0x30,
  0x03, 0x02, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x01 };
 static const BYTE serialNum[] = { 1 };
 
+static void testAddCert(void)
+{
+    HCERTSTORE store;
+
+    store = CertOpenStore(CERT_STORE_PROV_MEMORY, 0, 0,
+     CERT_STORE_CREATE_NEW_FLAG, NULL);
+    ok(store != NULL, "CertOpenStore failed: %ld\n", GetLastError());
+    if (store != NULL)
+    {
+        HCERTSTORE collection;
+        PCCERT_CONTEXT context;
+        BOOL ret;
+
+        ret = CertAddEncodedCertificateToStore(store, X509_ASN_ENCODING,
+         bigCert, sizeof(bigCert), CERT_STORE_ADD_ALWAYS, NULL);
+        ok(ret, "CertAddEncodedCertificateToStore failed: %08lx\n",
+         GetLastError());
+        ret = CertAddEncodedCertificateToStore(store, X509_ASN_ENCODING,
+         bigCert2, sizeof(bigCert2), CERT_STORE_ADD_NEW, NULL);
+        ok(ret, "CertAddEncodedCertificateToStore failed: %08lx\n",
+         GetLastError());
+        /* This has the same name as bigCert, so finding isn't done by name */
+        ret = CertAddEncodedCertificateToStore(store, X509_ASN_ENCODING,
+         certWithUsage, sizeof(certWithUsage), CERT_STORE_ADD_NEW, &context);
+        ok(ret, "CertAddEncodedCertificateToStore failed: %08lx\n",
+         GetLastError());
+        ok(context != NULL, "Expected a context\n");
+        if (context)
+        {
+            CRYPT_DATA_BLOB hash = { sizeof(bigCert2Hash),
+             (LPBYTE)bigCert2Hash };
+
+            CertDeleteCertificateFromStore(context);
+            /* Set the same hash as bigCert2, and try to readd it */
+            ret = CertSetCertificateContextProperty(context, CERT_HASH_PROP_ID,
+             0, &hash);
+            ok(ret, "CertSetCertificateContextProperty failed: %08lx\n",
+             GetLastError());
+            ret = CertAddCertificateContextToStore(store, context,
+             CERT_STORE_ADD_NEW, NULL);
+            /* The failure is a bit odd (CRYPT_E_ASN1_BADTAG), so just check
+             * that it fails.
+             */
+            ok(!ret, "Expected failure\n");
+            CertFreeCertificateContext(context);
+        }
+        context = CertCreateCertificateContext(X509_ASN_ENCODING, bigCert2,
+         sizeof(bigCert2));
+        ok(context != NULL, "Expected a context\n");
+        if (context)
+        {
+            /* Try to readd bigCert2 to the store */
+            ret = CertAddCertificateContextToStore(store, context,
+             CERT_STORE_ADD_NEW, NULL);
+            ok(!ret && GetLastError() == CRYPT_E_EXISTS,
+             "Expected CRYPT_E_EXISTS, got %08lx\n", GetLastError());
+            CertFreeCertificateContext(context);
+        }
+
+        /* FIXME: test whether adding a cert with the same subject name and
+         * serial number (but different otherwise) as an existing cert works.
+         */
+        collection = CertOpenStore(CERT_STORE_PROV_COLLECTION, 0, 0,
+         CERT_STORE_CREATE_NEW_FLAG, NULL);
+        ok(collection != NULL, "CertOpenStore failed: %08lx\n", GetLastError());
+        if (collection)
+        {
+            /* Add store to the collection, but disable updates */
+            CertAddStoreToCollection(collection, store, 0, 0);
+
+            context = CertCreateCertificateContext(X509_ASN_ENCODING, bigCert2,
+             sizeof(bigCert2));
+            ok(context != NULL, "Expected a context\n");
+            if (context)
+            {
+                /* Try to readd bigCert2 to the collection */
+                ret = CertAddCertificateContextToStore(collection, context,
+                 CERT_STORE_ADD_NEW, NULL);
+                ok(!ret && GetLastError() == CRYPT_E_EXISTS,
+                 "Expected CRYPT_E_EXISTS, got %08lx\n", GetLastError());
+                /* Replacing an existing certificate context is allowed, even
+                 * though updates to the collection aren't..
+                 */
+                ret = CertAddCertificateContextToStore(collection, context,
+                 CERT_STORE_ADD_REPLACE_EXISTING, NULL);
+                ok(ret, "CertAddCertificateContextToStore failed: %08lx\n",
+                 GetLastError());
+                /* but adding a new certificate isn't allowed. */
+                ret = CertAddCertificateContextToStore(collection, context,
+                 CERT_STORE_ADD_ALWAYS, NULL);
+                ok(!ret && GetLastError() ==
+                 HRESULT_FROM_WIN32(ERROR_ACCESS_DENIED),
+                 "Expected HRESULT_FROM_WIN32(ERROR_ACCESS_DENIED), got %08lx\n",
+                 GetLastError());
+                CertFreeCertificateContext(context);
+            }
+
+            CertCloseStore(collection, 0);
+        }
+
+        CertCloseStore(store, 0);
+    }
+}
+
 static void testDupCert(void)
 {
     HCERTSTORE store;
@@ -1592,6 +1698,7 @@ static void testAddSerialized(void)
 
 START_TEST(store)
 {
+    testAddCert();
     testDupCert();
     testFindCert();
     testGetSubjectCert();