From 54c5bec8deb2d4e972795e37273ad17a0b1e2f4f Mon Sep 17 00:00:00 2001

From: Greg Hudson <ghudson@mit.edu>

Date: Wed, 4 Jan 2017 11:33:57 -0500

Subject: [PATCH] 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.

(cherry picked from commit c6b772523db9d7791ee1c56eb512c4626556a4e7)

(cherry picked from commit 23086ac768a32db1e40a9b63684dbcfd76aba033)

---

 .../preauth/pkinit/pkinit_crypto_openssl.c    | 224 +++++++++---------

 1 file changed, 114 insertions(+), 110 deletions(-)

diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c

index bc6e7662e..8def8c542 100644

--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c

+++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c

@@ -2101,11 +2101,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__);

@@ -2121,118 +2131,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__);

+    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);

+

+    /* 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;

         }

-        num_sans = sk_GENERAL_NAME_num(ialt);

-

-        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;

-            }

-        }

-        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;

-            }

-        }

-

-        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) {

-#ifdef DEBUG_ASN1

-                    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++;

-                    }

-                } 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;

-            default:

-                pkiDebug("%s: SAN type = %d expecting %d\n",

-                         __FUNCTION__, gen->type, GEN_OTHERNAME);

-            }

-        }

-        sk_GENERAL_NAME_pop_free(ialt, GENERAL_NAME_free);

     }

+    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;

+        }

+    }

+

+    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) {

+#ifdef DEBUG_ASN1

+                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++;

+                }

+            } 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;

+        default:

+            pkiDebug("%s: SAN type = %d expecting %d\n", __FUNCTION__,

+                     gen->type, GEN_OTHERNAME);

+        }

+    }

+    sk_GENERAL_NAME_pop_free(ialt, GENERAL_NAME_free);

     retval = 0;

     if (princs)