krb5 commit: Fix certauth built-in module returns

Greg Hudson <[email protected]>
Newsgroups gmane.comp.encryption.kerberos.cvs
Message-ID <[email protected]>
https://github.com/krb5/krb5/commit/07243f85a760fb37f0622d7ff0177db3f19ab025
commit 07243f85a760fb37f0622d7ff0177db3f19ab025
Author: Greg Hudson <[email protected]>
Date:   Thu Aug 24 11:11:46 2017 -0400

    Fix certauth built-in module returns
    
    The PKINIT certauth eku module should never authoritatively authorize
    a certificate, because an extended key usage does not establish a
    relationship between the certificate and any specific user; it only
    establishes that the certificate was created for PKINIT client
    authentication.  Therefore, pkinit_eku_authorize() should return
    KRB5_PLUGIN_NO_HANDLE on success, not 0.
    
    The certauth san module should pass if it does not find any SANs of
    the types it can match against; the presence of other types of SANs
    should not cause it to explicitly deny a certificate.  Check for an
    empty result from crypto_retrieve_cert_sans() in verify_client_san(),
    instead of returning ENOENT from crypto_retrieve_cert_sans() when
    there are no SANs at all.
    
    ticket: 8561

 src/plugins/preauth/pkinit/pkinit_crypto_openssl.c |   39 +++++++++-----------
 src/plugins/preauth/pkinit/pkinit_srv.c            |   14 +++++---
 2 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
index b583ff0..f7640ba 100644
--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
+++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
@@ -2125,7 +2125,6 @@ crypto_retrieve_X509_sans(krb5_context context,
 
     if (!(ext = X509_get_ext(cert, l)) || !(ialt = X509V3_EXT_d2i(ext))) {
         pkiDebug("%s: found no subject alt name extensions\n", __FUNCTION__);
-        retval = ENOENT;
         goto cleanup;
     }
     num_sans = sk_GENERAL_NAME_num(ialt);
@@ -2228,31 +2227,29 @@ crypto_retrieve_X509_sans(krb5_context context,
     sk_GENERAL_NAME_pop_free(ialt, GENERAL_NAME_free);
 
     retval = 0;
-    if (princs)
+    if (princs != NULL && *princs != NULL) {
         *princs_ret = princs;
-    if (upns)
+        princs = NULL;
+    }
+    if (upns != NULL && *upns != NULL) {
         *upn_ret = upns;
-    if (dnss)
+        upns = NULL;
+    }
+    if (dnss != NULL && *dnss != NULL) {
         *dns_ret = dnss;
+        dnss = NULL;
+    }
 
 cleanup:
-    if (retval) {
-        if (princs != NULL) {
-            for (i = 0; princs[i] != NULL; i++)
-                krb5_free_principal(context, princs[i]);
-            free(princs);
-        }
-        if (upns != NULL) {
-            for (i = 0; upns[i] != NULL; i++)
-                krb5_free_principal(context, upns[i]);
-            free(upns);
-        }
-        if (dnss != NULL) {
-            for (i = 0; dnss[i] != NULL; i++)
-                free(dnss[i]);
-            free(dnss);
-        }
-    }
+    for (i = 0; princs != NULL && princs[i] != NULL; i++)
+        krb5_free_principal(context, princs[i]);
+    free(princs);
+    for (i = 0; upns != NULL && upns[i] != NULL; i++)
+        krb5_free_principal(context, upns[i]);
+    free(upns);
+    for (i = 0; dnss != NULL && dnss[i] != NULL; i++)
+        free(dnss[i]);
+    free(dnss);
     return retval;
 }
 
diff --git a/src/plugins/preauth/pkinit/pkinit_srv.c b/src/plugins/preauth/pkinit/pkinit_srv.c
index 5da8892..7210fc1 100644
--- a/src/plugins/preauth/pkinit/pkinit_srv.c
+++ b/src/plugins/preauth/pkinit/pkinit_srv.c
@@ -187,14 +187,18 @@ verify_client_san(krb5_context context,
                                        &princs,
                                        plgctx->opts->allow_upn ? &upns : NULL,
                                        NULL);
-    if (retval == ENOENT) {
-        TRACE_PKINIT_SERVER_NO_SAN(context);
-        goto out;
-    } else if (retval) {
+    if (retval) {
         pkiDebug("%s: error from retrieve_certificate_sans()\n", __FUNCTION__);
         retval = KRB5KDC_ERR_CLIENT_NAME_MISMATCH;
         goto out;
     }
+
+    if (princs == NULL && upns == NULL) {
+        TRACE_PKINIT_SERVER_NO_SAN(context);
+        retval = ENOENT;
+        goto out;
+    }
+
     /* XXX Verify this is consistent with client side XXX */
 #if 0
     retval = call_san_checking_plugins(context, plgctx, reqctx, princs,
@@ -1497,7 +1501,7 @@ pkinit_eku_authorize(krb5_context context, krb5_certauth_moddata moddata,
         return KRB5KDC_ERR_INCONSISTENT_KEY_PURPOSE;
     }
 
-    return 0;
+    return KRB5_PLUGIN_NO_HANDLE;
 }
 
 static krb5_error_code
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.