From 961546a38bcdc2bf6c2cd2fe4d406bc74b8cdadc Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Mon, 1 Nov 2010 12:24:26 -0400 Subject: [PATCH 086/150] - if we fail to load the pem module, don't try to refresh its slot list - don't add duplicate certs to lists - assume IDTYPE_NSS means an NSS database, rather than just assuming an IDTYPE_DIR means both NSS and its original meaning --- src/plugins/preauth/pkinit/pkinit_crypto_nss.c | 150 +++++++++++++++--------- 1 files changed, 93 insertions(+), 57 deletions(-) diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_nss.c b/src/plugins/preauth/pkinit/pkinit_crypto_nss.c index fb46496..3210348 100644 --- a/src/plugins/preauth/pkinit/pkinit_crypto_nss.c +++ b/src/plugins/preauth/pkinit/pkinit_crypto_nss.c @@ -233,7 +233,7 @@ struct _pkinit_identity_crypto_context { PLArenaPool *pool; SECMODModule *pem_module; SECMODModule **id_modules; - PK11SlotInfo **id_slots; + PK11SlotInfo **id_userdbs; CERTCertList *id_certs, *ca_certs, *other_certs; SECKEYPrivateKeyList *id_keys; CERTCertificate *id_cert; @@ -634,8 +634,9 @@ pkinit_init_identity_crypto(pkinit_identity_crypto_context *id_cryptoctx) if (id->pem_module == NULL) { pkiDebug("%s: error loading libnsspem.so\n", __FUNCTION__); + } else { + SECMOD_UpdateSlotList(id->pem_module); } - SECMOD_UpdateSlotList(id->pem_module); *id_cryptoctx = id; return 0; } @@ -662,10 +663,10 @@ pkinit_fini_identity_crypto(pkinit_identity_crypto_context id_cryptoctx) CERT_DestroyCertList(id_cryptoctx->ca_certs); SECKEY_DestroyPrivateKeyList(id_cryptoctx->id_keys); CERT_DestroyCertList(id_cryptoctx->id_certs); - if (id_cryptoctx->id_slots != NULL) { + if (id_cryptoctx->id_userdbs != NULL) { int i; - for (i = 0; id_cryptoctx->id_slots[i] != NULL; i++) { - SECMOD_CloseUserDB(id_cryptoctx->id_slots[i]); + for (i = 0; id_cryptoctx->id_userdbs[i] != NULL; i++) { + SECMOD_CloseUserDB(id_cryptoctx->id_userdbs[i]); } } if (id_cryptoctx->id_modules != NULL) { @@ -1737,6 +1738,25 @@ create_krb5_trustedCertifiers(krb5_context context, return 0; } +static SECStatus +cert_maybe_add_to_list(CERTCertList *list, CERTCertificate *cert) +{ + CERTCertListNode *node; + for (node = CERT_LIST_HEAD(list); + (node != NULL) && + (node->cert != NULL) && + !CERT_LIST_END(node, list); + node = CERT_LIST_NEXT(node)) { + if (SECITEM_ItemsAreEqual(&node->cert->derCert, + &cert->derCert)) { + /* Don't add the duplicate. */ + CERT_DestroyCertificate(cert); + return SECSuccess; + } + } + return CERT_AddCertToListTail(list, cert); +} + /* Load certificates for which we have private keys from the slot. */ static int cert_load_certs_with_keys_from_slot(krb5_context context, @@ -1804,7 +1824,7 @@ cert_load_certs_with_keys_from_slot(krb5_context context, /* DestroyCertList frees all of the certs in the list, * so we need to create a copy that it can own. */ cert = CERT_DupCertificate(cnode->cert); - if (CERT_AddCertToListTail(id->id_certs, cert) != SECSuccess) { + if (cert_maybe_add_to_list(id->id_certs, cert) != SECSuccess) { status = ENOMEM; } if (SECKEY_AddPrivateKeyToListTail(id->id_keys, @@ -2036,11 +2056,7 @@ crypto_load_files(krb5_context context, } } if ((status == SECSuccess) && (certfile != NULL)) { - if (cert_self) { - before = PK11_ListCertsInSlot(slot); - } else { - before = NULL; - } + before = PK11_ListCertsInSlot(slot); n_attrs = 0; crypto_set_attributes(&attrs[n_attrs++], CKA_CLASS, &certclass, sizeof(certclass)); @@ -2058,44 +2074,56 @@ crypto_load_files(krb5_context context, __FUNCTION__, cert_mark_trusted ? "CA " : "", certfile); - status = SECFailure; - } else { status = SECSuccess; + } else { + status = SECFailure; } - if (cert_self) { - /* Add any certs which are in the slot now, but which - * weren't before, and for which we have keys, to the - * list of possible identity certs. */ - after = PK11_ListCertsInSlot(slot); - if (after != NULL) { - for (anode = CERT_LIST_HEAD(after); - (anode != NULL) && - (anode->cert != NULL) && - !CERT_LIST_END(anode, after); - anode = CERT_LIST_NEXT(anode)) { - match = PR_FALSE; - if (before != NULL) { - for (bnode = CERT_LIST_HEAD(before); - (bnode != NULL) && - (bnode->cert != NULL) && - !CERT_LIST_END(bnode, after); - bnode = CERT_LIST_NEXT(bnode)) { - if (SECITEM_ItemsAreEqual(&anode->cert->derCert, - &bnode->cert->derCert)) { - match = PR_TRUE; - break; - } + /* Add any certs which are in the slot now, but which + * weren't before, and for which we have keys, to the + * right list of certs. */ + after = PK11_ListCertsInSlot(slot); + if (after != NULL) { + for (anode = CERT_LIST_HEAD(after); + (anode != NULL) && + (anode->cert != NULL) && + !CERT_LIST_END(anode, after); + anode = CERT_LIST_NEXT(anode)) { + match = PR_FALSE; + if (before != NULL) { + for (bnode = CERT_LIST_HEAD(before); + (bnode != NULL) && + (bnode->cert != NULL) && + !CERT_LIST_END(bnode, before); + bnode = CERT_LIST_NEXT(bnode)) { + if (SECITEM_ItemsAreEqual(&anode->cert->derCert, + &bnode->cert->derCert)) { + match = PR_TRUE; + break; } } - if (!match) { - cert = CERT_DupCertificate(anode->cert); - if (CERT_AddCertToListTail(id_cryptoctx->id_certs, cert) != SECSuccess) { + } + if (!match) { + cert = CERT_DupCertificate(anode->cert); + if (cert_self) { + /* Add to the identity list. */ + if (cert_maybe_add_to_list(id_cryptoctx->id_certs, cert) != SECSuccess) { + status = SECFailure; + } + } else + if (cert_mark_trusted) { + /* Add to the CA list. */ + if (cert_maybe_add_to_list(id_cryptoctx->ca_certs, cert) != SECSuccess) { + status = SECFailure; + } + } else { + /* Add to the "other" list. */ + if (cert_maybe_add_to_list(id_cryptoctx->other_certs, cert) != SECSuccess) { status = SECFailure; } } } - CERT_DestroyCertList(after); } + CERT_DestroyCertList(after); } if (before != NULL) { CERT_DestroyCertList(before); @@ -2194,7 +2222,7 @@ crypto_load_certdb(krb5_context context, const char *configdir, pkinit_identity_crypto_context id_cryptoctx) { - PK11SlotInfo *slot, **id_slots; + PK11SlotInfo *userdb, **id_userdbs; char *p; int i, j; @@ -2221,8 +2249,8 @@ crypto_load_certdb(krb5_context context, strcat(p, "' flags=readOnly"); /* Count the number of modules we've already loaded. */ - if (id_cryptoctx->id_slots != NULL) { - for (i = 0; id_cryptoctx->id_slots[i] != NULL; i++) { + if (id_cryptoctx->id_userdbs!= NULL) { + for (i = 0; id_cryptoctx->id_userdbs[i] != NULL; i++) { continue; } } else { @@ -2230,15 +2258,15 @@ crypto_load_certdb(krb5_context context, } /* Allocate a bigger list. */ - id_slots = PORT_ArenaZAlloc(id_cryptoctx->pool, - sizeof(id_slots[0]) * (i + 2)); + id_userdbs = PORT_ArenaZAlloc(id_cryptoctx->pool, + sizeof(id_userdbs[0]) * (i + 2)); for (j = 0; j < i; j++) { - id_slots[j] = id_cryptoctx->id_slots[j]; + id_userdbs[j] = id_cryptoctx->id_userdbs[j]; } /* Actually load the module. */ - slot = SECMOD_OpenUserDB(p); - if (slot == NULL) { + userdb = SECMOD_OpenUserDB(p); + if (userdb == NULL) { pkiDebug("%s: error loading NSS cert database \"%s\"", __FUNCTION__, configdir); return ENOENT; @@ -2246,12 +2274,12 @@ crypto_load_certdb(krb5_context context, pkiDebug("%s: opened NSS database \"%s\"\n", __FUNCTION__, configdir); /* Add us to the list and set the new list. */ - id_slots[i++] = slot; - id_slots[i] = NULL; - id_cryptoctx->id_slots = id_slots; + id_userdbs[i++] = userdb; + id_userdbs[i] = NULL; + id_cryptoctx->id_userdbs = id_userdbs; /* Load the keys from the database. */ - return cert_load_certs_with_keys_from_slot(context, id_cryptoctx, slot); + return cert_load_certs_with_keys_from_slot(context, id_cryptoctx, userdb); } /* Load up a certificate and associated key. */ @@ -2285,16 +2313,20 @@ crypto_load_certs(krb5_context context, } return 0; break; - case IDTYPE_DIR: + case IDTYPE_NSS: status = crypto_load_certdb(context, plg_cryptoctx, req_cryptoctx, idopts->cert_filename, id_cryptoctx); if (status != SECSuccess) { - pkiDebug("%s: error loading certdb \"%s\"\n", + pkiDebug("%s: error loading NSS certdb \"%s\"\n", __FUNCTION__, idopts->cert_filename); + return ENOMEM; } + return 0; + break; + case IDTYPE_DIR: status = crypto_load_dir(context, plg_cryptoctx, req_cryptoctx, @@ -2726,16 +2758,20 @@ crypto_load_cas_and_crls(krb5_context context, } return 0; break; - case IDTYPE_DIR: + case IDTYPE_NSS: status = crypto_load_certdb(context, plg_cryptoctx, req_cryptoctx, idopts->cert_filename, id_cryptoctx); if (status != SECSuccess) { - pkiDebug("%s: error loading CA certdb \"%s\"\n", + pkiDebug("%s: error loading NSS certdb \"%s\"\n", __FUNCTION__, idopts->cert_filename); + return ENOMEM; } + return 0; + break; + case IDTYPE_DIR: status = crypto_load_dir(context, plg_cryptoctx, req_cryptoctx, @@ -3019,7 +3055,7 @@ pkinit_create_td_trusted_certifiers(krb5_context context, /* DestroyCertList frees all of the certs in the list, * so we need to create a copy that it can own. */ cert = CERT_DupCertificate(node->cert); - if (CERT_AddCertToListTail(clist, cert) != SECSuccess) { + if (cert_maybe_add_to_list(clist, cert) != SECSuccess) { status = ENOMEM; } else { i++; -- 1.7.6.4