Blob Blame History Raw
From c7c702a9fee22a0f5173d94d8b1d5c2fac975f5c Mon Sep 17 00:00:00 2001
From: Greg Hudson <ghudson@mit.edu>
Date: Thu, 22 Mar 2018 20:07:17 -0400
Subject: [PATCH] Return UPN SANs as strings

(cherry picked from commit fd3c824e3be56a1fa77d140fd7e93934bfd6e565)
---
 src/plugins/preauth/pkinit/pkinit_crypto.h    |  4 +--
 .../preauth/pkinit/pkinit_crypto_openssl.c    | 28 +++++++------------
 src/plugins/preauth/pkinit/pkinit_matching.c  | 16 ++---------
 src/plugins/preauth/pkinit/pkinit_srv.c       | 21 +++++++++-----
 4 files changed, 29 insertions(+), 40 deletions(-)

diff --git a/src/plugins/preauth/pkinit/pkinit_crypto.h b/src/plugins/preauth/pkinit/pkinit_crypto.h
index c14f4456a..b6e4e0ac3 100644
--- a/src/plugins/preauth/pkinit/pkinit_crypto.h
+++ b/src/plugins/preauth/pkinit/pkinit_crypto.h
@@ -101,7 +101,7 @@ typedef struct _pkinit_cert_matching_data {
     unsigned int ku_bits;   /* key usage information */
     unsigned int eku_bits;  /* extended key usage information */
     krb5_principal *sans;   /* Null-terminated array of PKINIT SANs */
-    krb5_principal *upns;   /* Null-terimnated array of UPN SANs */
+    char **upns;	    /* Null-terimnated array of UPN SANs */
 } pkinit_cert_matching_data;
 
 /*
@@ -253,7 +253,7 @@ krb5_error_code crypto_retrieve_cert_sans
 		    if non-NULL, a null-terminated array of
 		    id-pkinit-san values found in the certificate
 		    are returned */
-	krb5_principal **upn_sans,			/* OUT
+	char ***upn_sans,				/* OUT
 		    if non-NULL, a null-terminated array of
 		    id-ms-upn-san values found in the certificate
 		    are returned */
diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
index a38738f45..3f106973c 100644
--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
+++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
@@ -29,6 +29,7 @@
  * SUCH DAMAGES.
  */
 
+#include "k5-int.h"
 #include "pkinit_crypto_openssl.h"
 #include "k5-buf.h"
 #include <dlfcn.h>
@@ -2095,15 +2096,14 @@ crypto_retrieve_X509_sans(krb5_context context,
                           pkinit_plg_crypto_context plgctx,
                           pkinit_req_crypto_context reqctx,
                           X509 *cert,
-                          krb5_principal **princs_ret,
-                          krb5_principal **upn_ret,
+                          krb5_principal **princs_ret, char ***upn_ret,
                           unsigned char ***dns_ret)
 {
     krb5_error_code retval = EINVAL;
     char buf[DN_BUF_LEN];
     int p = 0, u = 0, d = 0, ret = 0, l;
     krb5_principal *princs = NULL;
-    krb5_principal *upns = NULL;
+    char **upns = NULL;
     unsigned char **dnss = NULL;
     unsigned int i, num_found = 0, num_sans = 0;
     X509_EXTENSION *ext = NULL;
@@ -2153,7 +2153,7 @@ crypto_retrieve_X509_sans(krb5_context context,
         }
     }
     if (upn_ret != NULL) {
-        upns = calloc(num_sans + 1, sizeof(krb5_principal));
+        upns = calloc(num_sans + 1, sizeof(*upns));
         if (upns == NULL) {
             retval = ENOMEM;
             goto cleanup;
@@ -2196,16 +2196,9 @@ crypto_retrieve_X509_sans(krb5_context context,
                 /* 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++;
-                }
+                upns[u] = k5memdup0(name.data, name.length, &ret);
+                if (upns[u] == NULL)
+                    goto cleanup;
             } else {
                 pkiDebug("%s: unrecognized othername oid in SAN\n",
                          __FUNCTION__);
@@ -2257,7 +2250,7 @@ cleanup:
         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[i]);
     free(upns);
     for (i = 0; dnss != NULL && dnss[i] != NULL; i++)
         free(dnss[i]);
@@ -2281,8 +2274,7 @@ crypto_retrieve_cert_sans(krb5_context context,
                           pkinit_plg_crypto_context plgctx,
                           pkinit_req_crypto_context reqctx,
                           pkinit_identity_crypto_context idctx,
-                          krb5_principal **princs_ret,
-                          krb5_principal **upn_ret,
+                          krb5_principal **princs_ret, char ***upn_ret,
                           unsigned char ***dns_ret)
 {
     krb5_error_code retval = EINVAL;
@@ -5111,7 +5103,7 @@ crypto_cert_free_matching_data(krb5_context context,
         krb5_free_principal(context, md->sans[i]);
     free(md->sans);
     for (i = 0; md->upns != NULL && md->upns[i] != NULL; i++)
-        krb5_free_principal(context, md->upns[i]);
+        free(md->upns[i]);
     free(md->upns);
     free(md);
 }
diff --git a/src/plugins/preauth/pkinit/pkinit_matching.c b/src/plugins/preauth/pkinit/pkinit_matching.c
index fe1e0f386..d929fb3c0 100644
--- a/src/plugins/preauth/pkinit/pkinit_matching.c
+++ b/src/plugins/preauth/pkinit/pkinit_matching.c
@@ -490,11 +490,7 @@ component_match(krb5_context context,
                     break;
             }
             for (i = 0; md->upns != NULL && md->upns[i] != NULL; i++) {
-                krb5_unparse_name_flags(context, md->upns[i],
-                                        KRB5_PRINCIPAL_UNPARSE_NO_REALM,
-                                        &princ_string);
-                match = regexp_match(context, rc, princ_string);
-                krb5_free_unparsed_name(context, princ_string);
+                match = regexp_match(context, rc, md->upns[i]);
                 if (match)
                     break;
             }
@@ -584,14 +580,8 @@ check_all_certs(krb5_context context,
             pkiDebug("%s: PKINIT san: '%s'\n", __FUNCTION__, san_string);
             krb5_free_unparsed_name(context, san_string);
         }
-        for (j = 0; md->upns != NULL && md->upns[j] != NULL; j++) {
-            char *san_string;
-            krb5_unparse_name_flags(context, md->upns[j],
-                                    KRB5_PRINCIPAL_UNPARSE_NO_REALM,
-                                    &san_string);
-            pkiDebug("%s: UPN san: '%s'\n", __FUNCTION__, san_string);
-            krb5_free_unparsed_name(context, san_string);
-        }
+        for (j = 0; md->upns != NULL && md->upns[j] != NULL; j++)
+            pkiDebug("%s: UPN san: '%s'\n", __FUNCTION__, md->upns[j]);
 #endif
         certs_checked++;
         for (rc = rs->crs; rc != NULL; rc = rc->next) {
diff --git a/src/plugins/preauth/pkinit/pkinit_srv.c b/src/plugins/preauth/pkinit/pkinit_srv.c
index 143d331a2..42ad45fe4 100644
--- a/src/plugins/preauth/pkinit/pkinit_srv.c
+++ b/src/plugins/preauth/pkinit/pkinit_srv.c
@@ -174,8 +174,9 @@ verify_client_san(krb5_context context,
                   int *valid_san)
 {
     krb5_error_code retval;
-    krb5_principal *princs = NULL;
-    krb5_principal *upns = NULL;
+    krb5_principal *princs = NULL, upn;
+    krb5_boolean match;
+    char **upns = NULL;
     int i;
 #ifdef DEBUG_SAN_INFO
     char *client_string = NULL, *san_string;
@@ -251,12 +252,18 @@ verify_client_san(krb5_context context,
     pkiDebug("%s: Checking upn sans\n", __FUNCTION__);
     for (i = 0; upns[i] != NULL; i++) {
 #ifdef DEBUG_SAN_INFO
-        krb5_unparse_name(context, upns[i], &san_string);
         pkiDebug("%s: Comparing client '%s' to upn san value '%s'\n",
-                 __FUNCTION__, client_string, san_string);
-        krb5_free_unparsed_name(context, san_string);
+                 __FUNCTION__, client_string, upns[i]);
 #endif
-        if (cb->match_client(context, rock, upns[i])) {
+        retval = krb5_parse_name_flags(context, upns[i],
+                                       KRB5_PRINCIPAL_PARSE_ENTERPRISE, &upn);
+        if (retval) {
+            /* XXX trace */
+            continue;
+        }
+        match = cb->match_client(context, rock, upn);
+        krb5_free_principal(context, upn);
+        if (match) {
             TRACE_PKINIT_SERVER_MATCHING_UPN_FOUND(context);
             *valid_san = 1;
             retval = 0;
@@ -282,7 +289,7 @@ out:
     }
     if (upns != NULL) {
         for (i = 0; upns[i] != NULL; i++)
-            krb5_free_principal(context, upns[i]);
+            free(upns[i]);
         free(upns);
     }
 #ifdef DEBUG_SAN_INFO