You can not select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.

125 lines
4.4 KiB

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)
---
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 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