From 0f94f224f16f196d8d3fb56cfcf4a65bdd0f20c7 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 cf2f16294..3949eb9c2 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