krb5 commit: Deindent crypto_retrieve_X509_sans()

Greg Hudson ghudson at mit.edu
Thu Jan 5 11:45:36 EST 2017


https://github.com/krb5/krb5/commit/c6b772523db9d7791ee1c56eb512c4626556a4e7
commit c6b772523db9d7791ee1c56eb512c4626556a4e7
Author: Greg Hudson <ghudson at mit.edu>
Date:   Wed Jan 4 11:33:57 2017 -0500

    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.

 src/plugins/preauth/pkinit/pkinit_crypto_openssl.c |  198 ++++++++++----------
 1 files changed, 101 insertions(+), 97 deletions(-)

diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
index 5100c3b..2ea2a49 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)


More information about the cvs-krb5 mailing list