From 41b9111b48e53bf7864ed1f134e0433b070fa900 Mon Sep 17 00:00:00 2001

From: Greg Hudson <ghudson@mit.edu>

Date: Thu, 24 Aug 2017 11:11:46 -0400

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

(cherry picked from commit 07243f85a760fb37f0622d7ff0177db3f19ab025)

---

 .../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 70e230ec2..7fa2efd21 100644

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

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

@@ -2137,7 +2137,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);

@@ -2240,31 +2239,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 9c6e96c9e..8e77606f8 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,

@@ -1495,7 +1499,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