krb5 commit: Fix certauth built-in module returns

[Greg Hudson]

https://github.com/krb5/krb5/commit/07243f85a760fb37f0622d7ff0177db3f19ab025
commit 07243f85a760fb37f0622d7ff0177db3f19ab025
Author: Greg Hudson <ghudson at mit.edu>
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