From 1b679b12ed90ae86e19212c13472703089b5970e Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Wed, 12 Oct 2011 17:16:35 -0400 Subject: [PATCH 150/150] - stop guessing about library names and just use PR_GetLibraryName() (ghudson) - use snprintf/strlcat/strlcpy even for buffers whose size we know is large enough - crypto_pwfn(): check for out-of-memory allocating the prompt string - when creating a list of trusted certifiers, don't require that they be roots --- src/plugins/preauth/pkinit/pkinit_crypto_nss.c | 109 ++++++++++++++--------- 1 files changed, 66 insertions(+), 43 deletions(-) diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_nss.c b/src/plugins/preauth/pkinit/pkinit_crypto_nss.c index 2feb512..4b97d08 100644 --- a/src/plugins/preauth/pkinit/pkinit_crypto_nss.c +++ b/src/plugins/preauth/pkinit/pkinit_crypto_nss.c @@ -86,15 +86,8 @@ * include a friendly name. */ #define PKCS12_PREFIX "pkinit-pkcs12" -/* The name of the NSSPEM module. This is some OS-specific guesswork. */ -#if defined(_WIN32) -#define PKINIT_PEMOBJPREFIX "" -#elif TARGET_OS_MAC -#define PKINIT_PEMOBJPREFIX "lib" -#else -#define PKINIT_PEMOBJPREFIX "lib" -#endif -#define PEM_MODULE PKINIT_PEMOBJPREFIX "nsspem" PKINIT_DYNOBJEXT +/* The library name of the NSSPEM module. */ +#define PEM_MODULE "nsspem" /* Forward declaration. */ static krb5_error_code cert_retrieve_cert_sans(krb5_context context, @@ -562,6 +555,7 @@ crypto_pwfn(const char *what, PRBool retry, void *arg) krb5_prompt_type prompt_types[2]; krb5_data reply; char *text, *answer; + size_t text_size; void *data; /* We only want to be called once. */ @@ -575,8 +569,13 @@ crypto_pwfn(const char *what, PRBool retry, void *arg) return NULL; /* Set up the prompt. */ - text = PORT_ArenaZAlloc(id->pool, strlen(what) + 100); - snprintf(text, strlen(what) + 100, "Password for %s", what); + text_size = strlen(what) + 100; + text = PORT_ArenaZAlloc(id->pool, text_size); + if (text == NULL) { + pkiDebug("out of memory"); + return NULL; + } + snprintf(text, text_size, "Password for %s", what); memset(&prompt, 0, sizeof(prompt)); prompt.prompt = text; prompt.hidden = 1; @@ -669,6 +668,7 @@ static PK11SlotInfo * crypto_get_p12_slot(struct _pkinit_identity_crypto_context *id) { char *configdir, *spec; + size_t spec_size; int attempts; if (id->id_p12_slot == NULL) { @@ -691,14 +691,15 @@ crypto_get_p12_slot(struct _pkinit_identity_crypto_context *id) } } #endif - spec = PORT_ArenaZAlloc(id->pool, - strlen("configDir='' flags=readOnly") + - strlen(configdir) + 1); + spec_size = strlen("configDir='' flags=readOnly") + + strlen(configdir) + 1; + spec = PORT_ArenaZAlloc(id->pool, spec_size); if (spec != NULL) { if (strcmp(configdir, DEFAULT_CONFIGDIR) != 0) - sprintf(spec, "configDir='%s'", configdir); + snprintf(spec, spec_size, "configDir='%s'", configdir); else - sprintf(spec, "configDir='%s' flags=readOnly", configdir); + snprintf(spec, spec_size, "configDir='%s' flags=readOnly", + configdir); id->id_p12_slot = SECMOD_OpenUserDB(spec); } #ifdef PKCS12_HACK @@ -2039,6 +2040,7 @@ crypto_load_pkcs11(krb5_context context, SECMODModule **id_modules, *module; PK11SlotInfo *slot; char *spec; + size_t spec_size; const char *label, *id, *slotname, *tokenname; SECStatus status; int i, j; @@ -2047,12 +2049,11 @@ crypto_load_pkcs11(krb5_context context, return SECFailure; /* Build the module spec. */ - spec = PORT_ArenaZAlloc(id_cryptoctx->pool, - strlen("library=''") + - strlen(idopts->p11_module_name) * 2 + 1); + spec_size = strlen("library=''") + strlen(idopts->p11_module_name) * 2 + 1; + spec = PORT_ArenaZAlloc(id_cryptoctx->pool, spec_size); if (spec == NULL) return SECFailure; - strcpy(spec, "library=\""); + strlcpy(spec, "library=\"", spec_size); j = strlen(spec); for (i = 0; idopts->p11_module_name[i] != '\0'; i++) { if (strchr("\"", idopts->p11_module_name[i]) != NULL) @@ -2060,7 +2061,7 @@ crypto_load_pkcs11(krb5_context context, spec[j++] = idopts->p11_module_name[i]; } spec[j++] = '\0'; - strcat(spec, "\""); + strlcat(spec, "\"", spec_size); /* Count the number of modules we've already loaded. */ if (id_cryptoctx->id_modules != NULL) { @@ -2134,16 +2135,35 @@ static PK11SlotInfo * crypto_get_pem_slot(struct _pkinit_identity_crypto_context *id) { PK11SlotInfo *slot; + char *pem_module_name, *spec; + size_t spec_size; if (id->pem_module == NULL) { - id->pem_module = SECMOD_LoadUserModule("library=" PEM_MODULE, - NULL, PR_FALSE); + pem_module_name = PR_GetLibraryName(NULL, PEM_MODULE); + if (pem_module_name == NULL) { + pkiDebug("%s: error determining library name for %s\n", + __FUNCTION__, PEM_MODULE); + return NULL; + } + spec_size = strlen("library=") + strlen(pem_module_name) + 1; + spec = malloc(spec_size); + if (spec == NULL) { + pkiDebug("%s: out of memory building spec for %s\n", + __FUNCTION__, pem_module_name); + PR_FreeLibraryName(pem_module_name); + return NULL; + } + snprintf(spec, spec_size, "library=%s", pem_module_name); + id->pem_module = SECMOD_LoadUserModule(spec, NULL, PR_FALSE); if (id->pem_module == NULL) - pkiDebug("%s: error loading %s\n", __FUNCTION__, PEM_MODULE); - else if (!id->pem_module->loaded) - pkiDebug("%s: error really loading %s\n", __FUNCTION__, PEM_MODULE); + pkiDebug("%s: error loading %s\n", __FUNCTION__, pem_module_name); + else if (!id->pem_module->loaded) + pkiDebug("%s: error really loading %s\n", __FUNCTION__, + pem_module_name); else SECMOD_UpdateSlotList(id->pem_module); + free(spec); + PR_FreeLibraryName(pem_module_name); } if ((id->pem_module != NULL) && id->pem_module->loaded) { if (id->pem_module->slotCount != 0) @@ -2168,6 +2188,7 @@ crypto_nickname_c_cb(SECItem *old_nickname, PRBool *cancel, void *arg) CERTCertificate *leaf; char *old_name, *new_name, *p; SECItem *new_nickname, tmp; + size_t new_name_size; int i; leaf = arg; @@ -2181,10 +2202,11 @@ crypto_nickname_c_cb(SECItem *old_nickname, PRBool *cancel, void *arg) new_nickname = NULL; if (old_nickname == NULL) { old_name = leaf->subjectName; - new_name = PR_Malloc(strlen(PKCS12_PREFIX ": #1") + - strlen(old_name) + 1); + new_name_size = strlen(PKCS12_PREFIX ": #1") + strlen(old_name) + 1; + new_name = PR_Malloc(new_name_size); if (new_name != NULL) { - sprintf(new_name, PKCS12_PREFIX ": %s #1", old_name); + snprintf(new_name, new_name_size, PKCS12_PREFIX ": %s #1", + old_name); tmp.data = (unsigned char *) new_name; tmp.len = strlen(new_name) + 1; new_nickname = SECITEM_DupItem(&tmp); @@ -2197,16 +2219,19 @@ crypto_nickname_c_cb(SECItem *old_nickname, PRBool *cancel, void *arg) p = strrchr(old_name, '#'); i = (p ? atoi(p + 1) : 0) + 1; old_name = leaf->subjectName; - new_name = PR_Malloc(strlen(PKCS12_PREFIX ": #") + - strlen(old_name) + 3 * sizeof(i) + 1); + new_name_size = strlen(PKCS12_PREFIX ": #") + + strlen(old_name) + 3 * sizeof(i) + 1; + new_name = PR_Malloc(new_name_size); } else { old_name = leaf->subjectName; - new_name = PR_Malloc(strlen("pkinit-pkcs12: #1") + - strlen(old_name) + 1); + new_name_size = strlen(PKCS12_PREFIX ": #1") + + strlen(old_name) + 1; + new_name = PR_Malloc(new_name_size); i = 1; } if (new_name != NULL) { - sprintf(new_name, "pkinit-pkcs12: %s #%d", old_name, i); + snprintf(new_name, new_name_size, PKCS12_PREFIX ": %s #%d", + old_name, i); tmp.data = (unsigned char *) new_name; tmp.len = strlen(new_name) + 1; new_nickname = SECITEM_DupItem(&tmp); @@ -2630,7 +2655,7 @@ crypto_load_dir(krb5_context context, continue; } i = strlen(key); - strcpy(key + i - 4, ".key"); + memcpy(key + i - 4, ".key", 5); } /* Try loading the key and file as a pair. */ if (crypto_load_files(context, @@ -2659,18 +2684,19 @@ crypto_load_nssdb(krb5_context context, { PK11SlotInfo *userdb, **id_userdbs; char *p; + size_t spec_size; int i, j; if (configdir == NULL) return ENOENT; /* Build the spec. */ - p = PORT_ArenaZAlloc(id_cryptoctx->pool, - strlen("configDir='' flags=readOnly") + - strlen(configdir) * 2 + 1); + spec_size = strlen("configDir='' flags=readOnly") + + strlen(configdir) * 2 + 1; + p = PORT_ArenaZAlloc(id_cryptoctx->pool, spec_size); if (p == NULL) return ENOMEM; - strcpy(p, "configDir='"); + strlcpy(p, "configDir='", spec_size); j = strlen(p); for (i = 0; configdir[i] != '\0'; i++) { if (configdir[i] == '\'') @@ -2679,7 +2705,7 @@ crypto_load_nssdb(krb5_context context, p[j++] = configdir[i]; } p[j++] = '\0'; - strcat(p, "' flags=readOnly"); + strlcat(p, "' flags=readOnly", spec_size); /* Count the number of modules we've already loaded. */ if (id_cryptoctx->id_userdbs != NULL) { @@ -3491,9 +3517,6 @@ pkinit_create_td_trusted_certifiers(krb5_context context, (node->cert != NULL) && !CERT_LIST_END(node, sclist); node = CERT_LIST_NEXT(node)) { - /* If it's not a root, we can't trust it. Right? */ - if (!cert->isRoot) - continue; /* If we have no trust for it, we can't trust it. */ if (cert->trust == NULL) continue; -- 1.7.6.4