krb5 commit: Deindent crypto_retrieve_X509_sans()

Greg Hudson <[email protected]>
Newsgroups gmane.comp.encryption.kerberos.cvs
Message-ID <[email protected]>
https://github.com/krb5/krb5/commit/c6b772523db9d7791ee1c56eb512c4626556a4e7
commit c6b772523db9d7791ee1c56eb512c4626556a4e7
Author: Greg Hudson <[email protected]>
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)
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.