From: Greg Hudson Date: Wed, 4 Jan 2017 16:33:57 +0000 (-0500) Subject: Deindent crypto_retrieve_X509_sans() X-Git-Tag: krb5-1.16-beta1~176 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c6b772523db9d7791ee1c56eb512c4626556a4e7;p=thirdparty%2Fkrb5.git Deindent crypto_retrieve_X509_sans() Fix some long lines in crypto_retrieve_X509_sans() by returning early if X509_get_ext_by_NID() returns a negative result. Also ensure that return parameters are always initialized. --- diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c index 5100c3bf8b..2ea2a4928b 100644 --- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c @@ -2097,11 +2097,21 @@ crypto_retrieve_X509_sans(krb5_context context, { krb5_error_code retval = EINVAL; char buf[DN_BUF_LEN]; - int p = 0, u = 0, d = 0, l; + int p = 0, u = 0, d = 0, ret = 0, l; krb5_principal *princs = NULL; krb5_principal *upns = NULL; unsigned char **dnss = NULL; - unsigned int i, num_found = 0; + unsigned int i, num_found = 0, num_sans = 0; + X509_EXTENSION *ext = NULL; + GENERAL_NAMES *ialt = NULL; + GENERAL_NAME *gen = NULL; + + if (princs_ret != NULL) + *princs_ret = NULL; + if (upn_ret != NULL) + *upn_ret = NULL; + if (dns_ret != NULL) + *dns_ret = NULL; if (princs_ret == NULL && upn_ret == NULL && dns_ret == NULL) { pkiDebug("%s: nowhere to return any values!\n", __FUNCTION__); @@ -2117,118 +2127,112 @@ crypto_retrieve_X509_sans(krb5_context context, buf, sizeof(buf)); pkiDebug("%s: looking for SANs in cert = %s\n", __FUNCTION__, buf); - if ((l = X509_get_ext_by_NID(cert, NID_subject_alt_name, -1)) >= 0) { - X509_EXTENSION *ext = NULL; - GENERAL_NAMES *ialt = NULL; - GENERAL_NAME *gen = NULL; - int ret = 0; - unsigned int num_sans = 0; + l = X509_get_ext_by_NID(cert, NID_subject_alt_name, -1); + if (l < 0) + return 0; - if (!(ext = X509_get_ext(cert, l)) || !(ialt = X509V3_EXT_d2i(ext))) { - pkiDebug("%s: found no subject alt name extensions\n", - __FUNCTION__); - goto cleanup; - } - num_sans = sk_GENERAL_NAME_num(ialt); + if (!(ext = X509_get_ext(cert, l)) || !(ialt = X509V3_EXT_d2i(ext))) { + pkiDebug("%s: found no subject alt name extensions\n", __FUNCTION__); + goto cleanup; + } + num_sans = sk_GENERAL_NAME_num(ialt); - pkiDebug("%s: found %d subject alt name extension(s)\n", - __FUNCTION__, num_sans); + pkiDebug("%s: found %d subject alt name extension(s)\n", __FUNCTION__, + num_sans); - /* OK, we're likely returning something. Allocate return values */ - if (princs_ret != NULL) { - princs = calloc(num_sans + 1, sizeof(krb5_principal)); - if (princs == NULL) { - retval = ENOMEM; - goto cleanup; - } + /* OK, we're likely returning something. Allocate return values */ + if (princs_ret != NULL) { + princs = calloc(num_sans + 1, sizeof(krb5_principal)); + if (princs == NULL) { + retval = ENOMEM; + goto cleanup; } - if (upn_ret != NULL) { - upns = calloc(num_sans + 1, sizeof(krb5_principal)); - if (upns == NULL) { - retval = ENOMEM; - goto cleanup; - } + } + if (upn_ret != NULL) { + upns = calloc(num_sans + 1, sizeof(krb5_principal)); + if (upns == NULL) { + retval = ENOMEM; + goto cleanup; } - if (dns_ret != NULL) { - dnss = calloc(num_sans + 1, sizeof(*dnss)); - if (dnss == NULL) { - retval = ENOMEM; - goto cleanup; - } + } + if (dns_ret != NULL) { + dnss = calloc(num_sans + 1, sizeof(*dnss)); + if (dnss == NULL) { + retval = ENOMEM; + goto cleanup; } + } - for (i = 0; i < num_sans; i++) { - krb5_data name = { 0, 0, NULL }; + for (i = 0; i < num_sans; i++) { + krb5_data name = { 0, 0, NULL }; - gen = sk_GENERAL_NAME_value(ialt, i); - switch (gen->type) { - case GEN_OTHERNAME: - name.length = gen->d.otherName->value->value.sequence->length; - name.data = (char *)gen->d.otherName->value->value.sequence->data; - if (princs != NULL - && OBJ_cmp(plgctx->id_pkinit_san, - gen->d.otherName->type_id) == 0) { + gen = sk_GENERAL_NAME_value(ialt, i); + switch (gen->type) { + case GEN_OTHERNAME: + name.length = gen->d.otherName->value->value.sequence->length; + name.data = (char *)gen->d.otherName->value->value.sequence->data; + if (princs != NULL && + OBJ_cmp(plgctx->id_pkinit_san, + gen->d.otherName->type_id) == 0) { #ifdef DEBUG_ASN1 - print_buffer_bin((unsigned char *)name.data, name.length, - "/tmp/pkinit_san"); + print_buffer_bin((unsigned char *)name.data, name.length, + "/tmp/pkinit_san"); #endif - ret = k5int_decode_krb5_principal_name(&name, &princs[p]); - if (ret) { - pkiDebug("%s: failed decoding pkinit san value\n", - __FUNCTION__); - } else { - p++; - num_found++; - } - } else if (upns != NULL - && OBJ_cmp(plgctx->id_ms_san_upn, - gen->d.otherName->type_id) == 0) { - /* Prevent abuse of embedded null characters. */ - if (memchr(name.data, '\0', name.length)) - break; - ret = krb5_parse_name_flags(context, name.data, - KRB5_PRINCIPAL_PARSE_ENTERPRISE, - &upns[u]); - if (ret) { - pkiDebug("%s: failed parsing ms-upn san value\n", - __FUNCTION__); - } else { - u++; - num_found++; - } + ret = k5int_decode_krb5_principal_name(&name, &princs[p]); + if (ret) { + pkiDebug("%s: failed decoding pkinit san value\n", + __FUNCTION__); } else { - pkiDebug("%s: unrecognized othername oid in SAN\n", + p++; + num_found++; + } + } else if (upns != NULL && + OBJ_cmp(plgctx->id_ms_san_upn, + gen->d.otherName->type_id) == 0) { + /* Prevent abuse of embedded null characters. */ + if (memchr(name.data, '\0', name.length)) + break; + ret = krb5_parse_name_flags(context, name.data, + KRB5_PRINCIPAL_PARSE_ENTERPRISE, + &upns[u]); + if (ret) { + pkiDebug("%s: failed parsing ms-upn san value\n", __FUNCTION__); - continue; + } else { + u++; + num_found++; } + } else { + pkiDebug("%s: unrecognized othername oid in SAN\n", + __FUNCTION__); + continue; + } - break; - case GEN_DNS: - if (dnss != NULL) { - /* Prevent abuse of embedded null characters. */ - if (memchr(gen->d.dNSName->data, '\0', - gen->d.dNSName->length)) - break; - pkiDebug("%s: found dns name = %s\n", - __FUNCTION__, gen->d.dNSName->data); - dnss[d] = (unsigned char *) - strdup((char *)gen->d.dNSName->data); - if (dnss[d] == NULL) { - pkiDebug("%s: failed to duplicate dns name\n", - __FUNCTION__); - } else { - d++; - num_found++; - } + break; + case GEN_DNS: + if (dnss != NULL) { + /* Prevent abuse of embedded null characters. */ + if (memchr(gen->d.dNSName->data, '\0', gen->d.dNSName->length)) + break; + pkiDebug("%s: found dns name = %s\n", __FUNCTION__, + gen->d.dNSName->data); + dnss[d] = (unsigned char *) + strdup((char *)gen->d.dNSName->data); + if (dnss[d] == NULL) { + pkiDebug("%s: failed to duplicate dns name\n", + __FUNCTION__); + } else { + d++; + num_found++; } - break; - default: - pkiDebug("%s: SAN type = %d expecting %d\n", - __FUNCTION__, gen->type, GEN_OTHERNAME); } + break; + default: + pkiDebug("%s: SAN type = %d expecting %d\n", __FUNCTION__, + gen->type, GEN_OTHERNAME); } - sk_GENERAL_NAME_pop_free(ialt, GENERAL_NAME_free); } + sk_GENERAL_NAME_pop_free(ialt, GENERAL_NAME_free); retval = 0; if (princs)