rpms/krb5
7
0
Fork 0
Code Issues Pull Requests Releases Activity

Add hostname-based ccselect module

Browse Source 
Also update certauth EKU stuff

Resolves: #1463665
This commit is contained in:
Robbie Harwood 2017-09-05 18:16:56 +00:00
parent 8f0349dc3e
commit f6b653fac2
7 changed files with 2023 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
Add-PKINIT-test-case-for-generic-client-cert.patch Normal file
View File

@ -0,0 +1,51 @@
From 22e89e4e2d2819b7371efb848be525914b2750e8 Mon Sep 17 00:00:00 2001
From: Greg Hudson <ghudson@mit.edu>
Date: Fri, 25 Aug 2017 12:39:14 -0400
Subject: [PATCH] Add PKINIT test case for generic client cert
In t_pkinit.py, add a test case where a client cert with no extensions
is authorized via subject and issuer using a pkinit_cert_match string
attribute.
ticket: 8562
(cherry picked from commit 8c5d50888aab554239fd51306e79c5213833c898)
[rharwood@redhat.com: backport around dbmatch module]
---
src/tests/t_pkinit.py | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/src/tests/t_pkinit.py b/src/tests/t_pkinit.py
index e943f4974..fa5c5199e 100755
--- a/src/tests/t_pkinit.py
+++ b/src/tests/t_pkinit.py
@@ -26,6 +26,7 @@ user_enc_p12 = os.path.join(certs, 'user-enc.p12')
user_upn_p12 = os.path.join(certs, 'user-upn.p12')
user_upn2_p12 = os.path.join(certs, 'user-upn2.p12')
user_upn3_p12 = os.path.join(certs, 'user-upn3.p12')
+generic_p12 = os.path.join(certs, 'generic.p12')
path = os.path.join(os.getcwd(), 'testdir', 'tmp-pkinit-certs')
path_enc = os.path.join(os.getcwd(), 'testdir', 'tmp-pkinit-certs-enc')
@@ -65,6 +66,7 @@ p12_identity = 'PKCS12:%s' % user_p12
p12_upn_identity = 'PKCS12:%s' % user_upn_p12
p12_upn2_identity = 'PKCS12:%s' % user_upn2_p12
p12_upn3_identity = 'PKCS12:%s' % user_upn3_p12
+p12_generic_identity = 'PKCS12:%s' % generic_p12
p12_enc_identity = 'PKCS12:%s' % user_enc_p12
p11_identity = 'PKCS11:soft-pkcs11.so'
p11_token_identity = ('PKCS11:module_name=soft-pkcs11.so:'
@@ -284,6 +286,14 @@ realm.run(['./responder', '-X', 'X509_user_identity=%s' % p12_enc_identity,
realm.klist(realm.user_princ)
realm.run([kvno, realm.host_princ])
+# Authorize a client cert with no PKINIT extensions using subject and
+# issuer. (Relies on EKU checking being turned off.)
+rule = '&&<SUBJECT>CN=user$<ISSUER>O=MIT,'
+realm.run([kadminl, 'setstr', realm.user_princ, 'pkinit_cert_match', rule])
+realm.kinit(realm.user_princ,
+ flags=['-X', 'X509_user_identity=%s' % p12_generic_identity])
+realm.klist(realm.user_princ)
+
if not have_soft_pkcs11:
skip_rest('PKINIT PKCS11 tests', 'soft-pkcs11.so not found')

293
Add-hostname-based-ccselect-module.patch Normal file
View File

@ -0,0 +1,293 @@
From 624060dabcc06ea40847ffd98c9b05c66e65d6ba Mon Sep 17 00:00:00 2001
From: Robbie Harwood <rharwood@redhat.com>
Date: Wed, 23 Aug 2017 17:25:17 -0400
Subject: [PATCH] Add hostname-based ccselect module
The hostname module selects the ccache whose realm is the longest
parent domain tail of the uppercase server hostname.
[ghudson@mit.edu: minor edits]
ticket: 8613 (new)
(cherry picked from commit a4ddc6cf576b4155e6b994307902567f26f752b2)
---
doc/admin/conf_files/krb5_conf.rst | 4 +
src/lib/krb5/ccache/Makefile.in | 3 +
src/lib/krb5/ccache/cc-int.h | 4 +
src/lib/krb5/ccache/ccselect.c | 5 ++
src/lib/krb5/ccache/ccselect_hostname.c | 146 ++++++++++++++++++++++++++++++++
src/tests/gssapi/t_ccselect.py | 9 ++
6 files changed, 171 insertions(+)
create mode 100644 src/lib/krb5/ccache/ccselect_hostname.c
diff --git a/doc/admin/conf_files/krb5_conf.rst b/doc/admin/conf_files/krb5_conf.rst
index c0e4349c0..5f1de2e50 100644
--- a/doc/admin/conf_files/krb5_conf.rst
+++ b/doc/admin/conf_files/krb5_conf.rst
@@ -744,6 +744,10 @@ disabled with the disable tag):
Uses the service realm to guess an appropriate cache from the
collection
+**hostname**
+ If the service principal is host-based, uses the service hostname
+ to guess an appropriate cache from the collection
+
.. _pwqual:
pwqual interface
diff --git a/src/lib/krb5/ccache/Makefile.in b/src/lib/krb5/ccache/Makefile.in
index 5ac870728..f84cf793e 100644
--- a/src/lib/krb5/ccache/Makefile.in
+++ b/src/lib/krb5/ccache/Makefile.in
@@ -34,6 +34,7 @@ STLIBOBJS= \
ccdefops.o \
ccmarshal.o \
ccselect.o \
+ ccselect_hostname.o \
ccselect_k5identity.o \
ccselect_realm.o \
cc_dir.o \
@@ -52,6 +53,7 @@ OBJS= $(OUTPRE)ccbase.$(OBJEXT) \
$(OUTPRE)ccdefops.$(OBJEXT) \
$(OUTPRE)ccmarshal.$(OBJEXT) \
$(OUTPRE)ccselect.$(OBJEXT) \
+ $(OUTPRE)ccselect_hostname.$(OBJEXT) \
$(OUTPRE)ccselect_k5identity.$(OBJEXT) \
$(OUTPRE)ccselect_realm.$(OBJEXT) \
$(OUTPRE)cc_dir.$(OBJEXT) \
@@ -70,6 +72,7 @@ SRCS= $(srcdir)/ccbase.c \
$(srcdir)/ccdefops.c \
$(srcdir)/ccmarshal.c \
$(srcdir)/ccselect.c \
+ $(srcdir)/ccselect_hostname.c \
$(srcdir)/ccselect_k5identity.c \
$(srcdir)/ccselect_realm.c \
$(srcdir)/cc_dir.c \
diff --git a/src/lib/krb5/ccache/cc-int.h b/src/lib/krb5/ccache/cc-int.h
index ee9b5e0e9..d920367ce 100644
--- a/src/lib/krb5/ccache/cc-int.h
+++ b/src/lib/krb5/ccache/cc-int.h
@@ -123,6 +123,10 @@ k5_cccol_force_unlock(void);
krb5_error_code
krb5int_fcc_new_unique(krb5_context context, char *template, krb5_ccache *id);
+krb5_error_code
+ccselect_hostname_initvt(krb5_context context, int maj_ver, int min_ver,
+ krb5_plugin_vtable vtable);
+
krb5_error_code
ccselect_realm_initvt(krb5_context context, int maj_ver, int min_ver,
krb5_plugin_vtable vtable);
diff --git a/src/lib/krb5/ccache/ccselect.c b/src/lib/krb5/ccache/ccselect.c
index ee4b83a9b..393d39733 100644
--- a/src/lib/krb5/ccache/ccselect.c
+++ b/src/lib/krb5/ccache/ccselect.c
@@ -71,6 +71,11 @@ load_modules(krb5_context context)
if (ret != 0)
goto cleanup;
+ ret = k5_plugin_register(context, PLUGIN_INTERFACE_CCSELECT, "hostname",
+ ccselect_hostname_initvt);
+ if (ret != 0)
+ goto cleanup;
+
ret = k5_plugin_load_all(context, PLUGIN_INTERFACE_CCSELECT, &modules);
if (ret != 0)
goto cleanup;
diff --git a/src/lib/krb5/ccache/ccselect_hostname.c b/src/lib/krb5/ccache/ccselect_hostname.c
new file mode 100644
index 000000000..475cfabae
--- /dev/null
+++ b/src/lib/krb5/ccache/ccselect_hostname.c
@@ -0,0 +1,146 @@
+/* -*- mode: c; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/* lib/krb5/ccache/ccselect_hostname.c - hostname ccselect module */
+/*
+ * Copyright (C) 2017 by Red Hat, Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
+ * OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "k5-int.h"
+#include "cc-int.h"
+#include <ctype.h>
+#include <krb5/ccselect_plugin.h>
+
+/* Swap a and b, using tmp as an intermediate. */
+#define SWAP(a, b, tmp) \
+ tmp = a; \
+ a = b; \
+ b = tmp;
+
+static krb5_error_code
+hostname_init(krb5_context context, krb5_ccselect_moddata *data_out,
+ int *priority_out)
+{
+ *data_out = NULL;
+ *priority_out = KRB5_CCSELECT_PRIORITY_HEURISTIC;
+ return 0;
+}
+
+static krb5_error_code
+hostname_choose(krb5_context context, krb5_ccselect_moddata data,
+ krb5_principal server, krb5_ccache *ccache_out,
+ krb5_principal *princ_out)
+{
+ krb5_error_code ret;
+ char *p, *host = NULL;
+ size_t hostlen;
+ krb5_cccol_cursor col_cursor;
+ krb5_ccache ccache, tmp_ccache, best_ccache = NULL;
+ krb5_principal princ, tmp_princ, best_princ = NULL;
+ krb5_data domain;
+
+ *ccache_out = NULL;
+ *princ_out = NULL;
+
+ if (server->type != KRB5_NT_SRV_HST || server->length < 2)
+ return KRB5_PLUGIN_NO_HANDLE;
+
+ /* Compute upper-case hostname. */
+ hostlen = server->data[1].length;
+ host = k5memdup0(server->data[1].data, hostlen, &ret);
+ if (host == NULL)
+ return ret;
+ for (p = host; *p != '\0'; p++) {
+ if (islower(*p))
+ *p = toupper(*p);
+ }
+
+ /* Scan the collection for a cache with a client principal whose realm is
+ * the longest tail of the server hostname. */
+ ret = krb5_cccol_cursor_new(context, &col_cursor);
+ if (ret)
+ goto done;
+
+ for (ret = krb5_cccol_cursor_next(context, col_cursor, &ccache);
+ ret == 0 && ccache != NULL;
+ ret = krb5_cccol_cursor_next(context, col_cursor, &ccache)) {
+ ret = krb5_cc_get_principal(context, ccache, &princ);
+ if (ret) {
+ krb5_cc_close(context, ccache);
+ break;
+ }
+
+ /* Check for a longer match than we have. */
+ domain = make_data(host, hostlen);
+ while (best_princ == NULL ||
+ best_princ->realm.length < domain.length) {
+ if (data_eq(princ->realm, domain)) {
+ SWAP(best_ccache, ccache, tmp_ccache);
+ SWAP(best_princ, princ, tmp_princ);
+ break;
+ }
+
+ /* Try the next parent domain. */
+ p = memchr(domain.data, '.', domain.length);
+ if (p == NULL)
+ break;
+ domain = make_data(p + 1, hostlen - (p + 1 - host));
+ }
+
+ if (ccache != NULL)
+ krb5_cc_close(context, ccache);
+ krb5_free_principal(context, princ);
+ }
+
+ krb5_cccol_cursor_free(context, &col_cursor);
+
+ if (best_ccache != NULL) {
+ *ccache_out = best_ccache;
+ *princ_out = best_princ;
+ } else {
+ ret = KRB5_PLUGIN_NO_HANDLE;
+ }
+
+done:
+ free(host);
+ return ret;
+}
+
+krb5_error_code
+ccselect_hostname_initvt(krb5_context context, int maj_ver, int min_ver,
+ krb5_plugin_vtable vtable)
+{
+ krb5_ccselect_vtable vt;
+
+ if (maj_ver != 1)
+ return KRB5_PLUGIN_VER_NOTSUPP;
+ vt = (krb5_ccselect_vtable)vtable;
+ vt->name = "hostname";
+ vt->init = hostname_init;
+ vt->choose = hostname_choose;
+ return 0;
+}
diff --git a/src/tests/gssapi/t_ccselect.py b/src/tests/gssapi/t_ccselect.py
index 668a2cc62..3503f9269 100755
--- a/src/tests/gssapi/t_ccselect.py
+++ b/src/tests/gssapi/t_ccselect.py
@@ -33,6 +33,7 @@ host1 = 'p:' + r1.host_princ
host2 = 'p:' + r2.host_princ
foo = 'foo.krbtest.com'
foo2 = 'foo.krbtest2.com'
+foobar = "foo.bar.krbtest.com"
# These strings specify the target as a GSS name. The resulting
# principal will have the host-based type, with the referral realm
@@ -42,6 +43,7 @@ foo2 = 'foo.krbtest2.com'
# single component.
gssserver = 'h:host@' + foo
gssserver2 = 'h:host@' + foo2
+gssserver_bar = 'h:host@' + foobar
gsslocal = 'h:host@localhost'
# refserver specifies the target as a principal in the referral realm.
@@ -77,10 +79,12 @@ r1.addprinc('host/localhost')
r2.addprinc('host/localhost')
r1.addprinc('host/' + foo)
r2.addprinc('host/' + foo2)
+r1.addprinc('host/' + foobar)
r1.extract_keytab('host/localhost', r1.keytab)
r2.extract_keytab('host/localhost', r2.keytab)
r1.extract_keytab('host/' + foo, r1.keytab)
r2.extract_keytab('host/' + foo2, r2.keytab)
+r1.extract_keytab('host/' + foobar, r1.keytab)
# Get tickets for one user in each realm (zaphod will be primary).
r1.kinit(alice, password('alice'))
@@ -128,6 +132,11 @@ output = r2.run(['./t_ccselect', gsslocal])
if output != (zaphod + '\n'):
fail('zaphod not chosen via default realm fallback')
+# Check that realm ccselect fallback works correctly
+r1.run(['./t_ccselect', gssserver_bar], expected_msg=alice)
+r2.kinit(zaphod, password('zaphod'))
+r1.run(['./t_ccselect', gssserver_bar], expected_msg=alice)
+
# Get a second cred in r1 (bob will be primary).
r1.kinit(bob, password('bob'))

1120
Add-test-cert-with-no-extensions.patch Normal file
View File

File diff suppressed because it is too large Load Diff

422
Convert-some-pkiDebug-messages-to-TRACE-macros.patch Normal file
View File

@ -0,0 +1,422 @@
From 4dcab7d706331b469678f3a516cd67fffd331058 Mon Sep 17 00:00:00 2001
From: Matt Rogers <mrogers@redhat.com>
Date: Wed, 29 Mar 2017 10:35:13 -0400
Subject: [PATCH] Convert some pkiDebug messages to TRACE macros
ticket: 8568 (new)
(cherry picked from commit 9852862a83952a94300adfafa3e333f43396ec33)
---
src/plugins/preauth/pkinit/pkinit_crypto_openssl.c | 46 ++++++---------
src/plugins/preauth/pkinit/pkinit_identity.c | 3 -
src/plugins/preauth/pkinit/pkinit_matching.c | 1 +
src/plugins/preauth/pkinit/pkinit_srv.c | 24 ++++----
src/plugins/preauth/pkinit/pkinit_trace.h | 68 +++++++++++++++++++++-
5 files changed, 97 insertions(+), 45 deletions(-)
diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
index 90c30dbf5..70e230ec2 100644
--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
+++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
@@ -2320,7 +2320,6 @@ crypto_check_cert_eku(krb5_context context,
X509_NAME_oneline(X509_get_subject_name(reqctx->received_cert),
buf, sizeof(buf));
- pkiDebug("%s: looking for EKUs in cert = %s\n", __FUNCTION__, buf);
if ((i = X509_get_ext_by_NID(reqctx->received_cert,
NID_ext_key_usage, -1)) >= 0) {
@@ -2354,7 +2353,6 @@ crypto_check_cert_eku(krb5_context context,
if (found_eku) {
ASN1_BIT_STRING *usage = NULL;
- pkiDebug("%s: found acceptable EKU, checking for digitalSignature\n", __FUNCTION__);
/* check that digitalSignature KeyUsage is present */
X509_check_ca(reqctx->received_cert);
@@ -2363,12 +2361,10 @@ crypto_check_cert_eku(krb5_context context,
if (!ku_reject(reqctx->received_cert,
X509v3_KU_DIGITAL_SIGNATURE)) {
- pkiDebug("%s: found digitalSignature KU\n",
- __FUNCTION__);
+ TRACE_PKINIT_EKU(context);
*valid_eku = 1;
} else
- pkiDebug("%s: didn't find digitalSignature KU\n",
- __FUNCTION__);
+ TRACE_PKINIT_EKU_NO_KU(context);
}
ASN1_BIT_STRING_free(usage);
}
@@ -4317,8 +4313,7 @@ pkinit_get_certs_pkcs12(krb5_context context,
fp = fopen(idopts->cert_filename, "rb");
if (fp == NULL) {
- pkiDebug("Failed to open PKCS12 file '%s', error %d\n",
- idopts->cert_filename, errno);
+ TRACE_PKINIT_PKCS_OPEN_FAIL(context, idopts->cert_filename, errno);
goto cleanup;
}
set_cloexec_file(fp);
@@ -4326,8 +4321,7 @@ pkinit_get_certs_pkcs12(krb5_context context,
p12 = d2i_PKCS12_fp(fp, NULL);
fclose(fp);
if (p12 == NULL) {
- pkiDebug("Failed to decode PKCS12 file '%s' contents\n",
- idopts->cert_filename);
+ TRACE_PKINIT_PKCS_DECODE_FAIL(context, idopts->cert_filename);
goto cleanup;
}
/*
@@ -4345,7 +4339,7 @@ pkinit_get_certs_pkcs12(krb5_context context,
char *p12name = reassemble_pkcs12_name(idopts->cert_filename);
const char *tmp;
- pkiDebug("Initial PKCS12_parse with no password failed\n");
+ TRACE_PKINIT_PKCS_PARSE_FAIL_FIRST(context);
if (id_cryptoctx->defer_id_prompt) {
/* Supply the identity name to be passed to the responder. */
@@ -4386,14 +4380,14 @@ pkinit_get_certs_pkcs12(krb5_context context,
NULL, NULL, 1, &kprompt);
k5int_set_prompt_types(context, 0);
if (r) {
- pkiDebug("Failed to prompt for PKCS12 password");
+ TRACE_PKINIT_PKCS_PROMPT_FAIL(context);
goto cleanup;
}
}
ret = PKCS12_parse(p12, rdat.data, &y, &x, NULL);
if (ret == 0) {
- pkiDebug("Second PKCS12_parse with password failed\n");
+ TRACE_PKINIT_PKCS_PARSE_FAIL_SECOND(context);
goto cleanup;
}
}
@@ -4516,8 +4510,7 @@ pkinit_get_certs_fs(krb5_context context,
}
if (idopts->key_filename == NULL) {
- pkiDebug("%s: failed to get user's private key location\n",
- __FUNCTION__);
+ TRACE_PKINIT_NO_PRIVKEY(context);
goto cleanup;
}
@@ -4545,8 +4538,7 @@ pkinit_get_certs_dir(krb5_context context,
char *dirname, *suf;
if (idopts->cert_filename == NULL) {
- pkiDebug("%s: failed to get user's certificate directory location\n",
- __FUNCTION__);
+ TRACE_PKINIT_NO_CERT(context);
return ENOENT;
}
@@ -4590,8 +4582,7 @@ pkinit_get_certs_dir(krb5_context context,
retval = pkinit_load_fs_cert_and_key(context, id_cryptoctx,
certname, keyname, i);
if (retval == 0) {
- pkiDebug("%s: Successfully loaded cert (and key) for %s\n",
- __FUNCTION__, dentry->d_name);
+ TRACE_PKINIT_LOADED_CERT(context, dentry->d_name);
i++;
}
else
@@ -4599,8 +4590,7 @@ pkinit_get_certs_dir(krb5_context context,
}
if (!id_cryptoctx->defer_id_prompt && i == 0) {
- pkiDebug("%s: No cert/key pairs found in directory '%s'\n",
- __FUNCTION__, idopts->cert_filename);
+ TRACE_PKINIT_NO_CERT_AND_KEY(context, idopts->cert_filename);
retval = ENOENT;
goto cleanup;
}
@@ -5370,9 +5360,7 @@ crypto_cert_select_default(krb5_context context,
goto errout;
}
if (cert_count != 1) {
- pkiDebug("%s: ERROR: There are %d certs to choose from, "
- "but there must be exactly one.\n",
- __FUNCTION__, cert_count);
+ TRACE_PKINIT_NO_DEFAULT_CERT(context, cert_count);
retval = EINVAL;
goto errout;
}
@@ -5520,7 +5508,7 @@ load_cas_and_crls(krb5_context context,
switch(catype) {
case CATYPE_ANCHORS:
if (sk_X509_num(ca_certs) == 0) {
- pkiDebug("no anchors in file, %s\n", filename);
+ TRACE_PKINIT_NO_CA_ANCHOR(context, filename);
if (id_cryptoctx->trustedCAs == NULL)
sk_X509_free(ca_certs);
} else {
@@ -5530,7 +5518,7 @@ load_cas_and_crls(krb5_context context,
break;
case CATYPE_INTERMEDIATES:
if (sk_X509_num(ca_certs) == 0) {
- pkiDebug("no intermediates in file, %s\n", filename);
+ TRACE_PKINIT_NO_CA_INTERMEDIATE(context, filename);
if (id_cryptoctx->intermediateCAs == NULL)
sk_X509_free(ca_certs);
} else {
@@ -5540,7 +5528,7 @@ load_cas_and_crls(krb5_context context,
break;
case CATYPE_CRLS:
if (sk_X509_CRL_num(ca_crls) == 0) {
- pkiDebug("no crls in file, %s\n", filename);
+ TRACE_PKINIT_NO_CRL(context, filename);
if (id_cryptoctx->revoked == NULL)
sk_X509_CRL_free(ca_crls);
} else {
@@ -5626,14 +5614,14 @@ crypto_load_cas_and_crls(krb5_context context,
int catype,
char *id)
{
- pkiDebug("%s: called with idtype %s and catype %s\n",
- __FUNCTION__, idtype2string(idtype), catype2string(catype));
switch (idtype) {
case IDTYPE_FILE:
+ TRACE_PKINIT_LOAD_FROM_FILE(context);
return load_cas_and_crls(context, plg_cryptoctx, req_cryptoctx,
id_cryptoctx, catype, id);
break;
case IDTYPE_DIR:
+ TRACE_PKINIT_LOAD_FROM_DIR(context);
return load_cas_and_crls_dir(context, plg_cryptoctx, req_cryptoctx,
id_cryptoctx, catype, id);
break;
diff --git a/src/plugins/preauth/pkinit/pkinit_identity.c b/src/plugins/preauth/pkinit/pkinit_identity.c
index a897efa25..737552e85 100644
--- a/src/plugins/preauth/pkinit/pkinit_identity.c
+++ b/src/plugins/preauth/pkinit/pkinit_identity.c
@@ -608,7 +608,6 @@ pkinit_identity_prompt(krb5_context context,
retval = pkinit_cert_matching(context, plg_cryptoctx,
req_cryptoctx, id_cryptoctx, princ);
if (retval) {
- pkiDebug("%s: No matching certificate found\n", __FUNCTION__);
crypto_free_cert_info(context, plg_cryptoctx, req_cryptoctx,
id_cryptoctx);
goto errout;
@@ -621,8 +620,6 @@ pkinit_identity_prompt(krb5_context context,
retval = crypto_cert_select_default(context, plg_cryptoctx,
req_cryptoctx, id_cryptoctx);
if (retval) {
- pkiDebug("%s: Failed while selecting default certificate\n",
- __FUNCTION__);
crypto_free_cert_info(context, plg_cryptoctx, req_cryptoctx,
id_cryptoctx);
goto errout;
diff --git a/src/plugins/preauth/pkinit/pkinit_matching.c b/src/plugins/preauth/pkinit/pkinit_matching.c
index a50c50c8d..cad4c2b9a 100644
--- a/src/plugins/preauth/pkinit/pkinit_matching.c
+++ b/src/plugins/preauth/pkinit/pkinit_matching.c
@@ -812,6 +812,7 @@ pkinit_cert_matching(krb5_context context,
goto cleanup;
}
} else {
+ TRACE_PKINIT_NO_MATCHING_CERT(context);
retval = ENOENT; /* XXX */
goto cleanup;
}
diff --git a/src/plugins/preauth/pkinit/pkinit_srv.c b/src/plugins/preauth/pkinit/pkinit_srv.c
index 32ca122f2..9c6e96c9e 100644
--- a/src/plugins/preauth/pkinit/pkinit_srv.c
+++ b/src/plugins/preauth/pkinit/pkinit_srv.c
@@ -188,6 +188,7 @@ verify_client_san(krb5_context context,
plgctx->opts->allow_upn ? &upns : NULL,
NULL);
if (retval == ENOENT) {
+ TRACE_PKINIT_SERVER_NO_SAN(context);
goto out;
} else if (retval) {
pkiDebug("%s: error from retrieve_certificate_sans()\n", __FUNCTION__);
@@ -224,7 +225,7 @@ verify_client_san(krb5_context context,
krb5_free_unparsed_name(context, san_string);
#endif
if (cb->match_client(context, rock, princs[i])) {
- pkiDebug("%s: pkinit san match found\n", __FUNCTION__);
+ TRACE_PKINIT_SERVER_MATCHING_SAN_FOUND(context);
*valid_san = 1;
retval = 0;
goto out;
@@ -252,7 +253,7 @@ verify_client_san(krb5_context context,
krb5_free_unparsed_name(context, san_string);
#endif
if (cb->match_client(context, rock, upns[i])) {
- pkiDebug("%s: upn san match found\n", __FUNCTION__);
+ TRACE_PKINIT_SERVER_MATCHING_UPN_FOUND(context);
*valid_san = 1;
retval = 0;
goto out;
@@ -300,7 +301,7 @@ verify_client_eku(krb5_context context,
*eku_accepted = 0;
if (plgctx->opts->require_eku == 0) {
- pkiDebug("%s: configuration requests no EKU checking\n", __FUNCTION__);
+ TRACE_PKINIT_SERVER_EKU_SKIP(context);
*eku_accepted = 1;
retval = 0;
goto out;
@@ -364,6 +365,7 @@ authorize_cert(krb5_context context, certauth_handle *certauth_modules,
ret = KRB5_PLUGIN_NO_HANDLE;
for (i = 0; certauth_modules != NULL && certauth_modules[i] != NULL; i++) {
h = certauth_modules[i];
+ TRACE_PKINIT_SERVER_CERT_AUTH(context, h->vt.name);
ret = h->vt.authorize(context, h->moddata, cert, cert_len, client,
&opts, db_ent, &ais);
if (ret == 0)
@@ -449,7 +451,7 @@ pkinit_server_verify_padata(krb5_context context,
switch ((int)data->pa_type) {
case KRB5_PADATA_PK_AS_REQ:
- pkiDebug("processing KRB5_PADATA_PK_AS_REQ\n");
+ TRACE_PKINIT_SERVER_PADATA_VERIFY(context);
retval = k5int_decode_krb5_pa_pk_as_req(&k5data, &reqp);
if (retval) {
pkiDebug("decode_krb5_pa_pk_as_req failed\n");
@@ -472,7 +474,7 @@ pkinit_server_verify_padata(krb5_context context,
break;
case KRB5_PADATA_PK_AS_REP_OLD:
case KRB5_PADATA_PK_AS_REQ_OLD:
- pkiDebug("processing KRB5_PADATA_PK_AS_REQ_OLD\n");
+ TRACE_PKINIT_SERVER_PADATA_VERIFY_OLD(context);
retval = k5int_decode_krb5_pa_pk_as_req_draft9(&k5data, &reqp9);
if (retval) {
pkiDebug("decode_krb5_pa_pk_as_req_draft9 failed\n");
@@ -500,7 +502,7 @@ pkinit_server_verify_padata(krb5_context context,
goto cleanup;
}
if (retval) {
- pkiDebug("pkcs7_signeddata_verify failed\n");
+ TRACE_PKINIT_SERVER_PADATA_VERIFY_FAIL(context);
goto cleanup;
}
if (is_signed) {
@@ -830,7 +832,7 @@ pkinit_server_return_padata(krb5_context context,
return ENOENT;
}
- pkiDebug("pkinit_return_padata: entered!\n");
+ TRACE_PKINIT_SERVER_RETURN_PADATA(context);
reqctx = (pkinit_kdc_req_context)modreq;
if (encrypting_key->contents) {
@@ -1463,8 +1465,7 @@ pkinit_san_authorize(krb5_context context, krb5_certauth_moddata moddata,
return ret;
if (!valid_san) {
- pkiDebug("%s: did not find an acceptable SAN in user certificate\n",
- __FUNCTION__);
+ TRACE_PKINIT_SERVER_SAN_REJECT(context);
return KRB5KDC_ERR_CLIENT_NAME_MISMATCH;
}
@@ -1490,8 +1491,7 @@ pkinit_eku_authorize(krb5_context context, krb5_certauth_moddata moddata,
return ret;
if (!valid_eku) {
- pkiDebug("%s: did not find an acceptable EKU in user certificate\n",
- __FUNCTION__);
+ TRACE_PKINIT_SERVER_EKU_REJECT(context);
return KRB5KDC_ERR_INCONSISTENT_KEY_PURPOSE;
}
@@ -1617,7 +1617,7 @@ pkinit_server_plugin_init(krb5_context context,
return ENOMEM;
for (i = 0, j = 0; i < numrealms; i++) {
- pkiDebug("%s: processing realm '%s'\n", __FUNCTION__, realmnames[i]);
+ TRACE_PKINIT_SERVER_INIT_REALM(context, realmnames[i]);
retval = pkinit_server_plugin_init_realm(context, realmnames[i], &plgctx);
if (retval == 0 && plgctx != NULL)
realm_contexts[j++] = plgctx;
diff --git a/src/plugins/preauth/pkinit/pkinit_trace.h b/src/plugins/preauth/pkinit/pkinit_trace.h
index 458d0961e..6abe28c0c 100644
--- a/src/plugins/preauth/pkinit/pkinit_trace.h
+++ b/src/plugins/preauth/pkinit/pkinit_trace.h
@@ -52,7 +52,7 @@
#define TRACE_PKINIT_CLIENT_REP_CHECKSUM_FAIL(c, expected, received) \
TRACE(c, "PKINIT client checksum mismatch: expected {cksum}, " \
"received {cksum}", expected, received)
-#define TRACE_PKINIT_CLIENT_REP_DH(c) \
+#define TRACE_PKINIT_CLIENT_REP_DH(c) \
TRACE(c, "PKINIT client verified DH reply")
#define TRACE_PKINIT_CLIENT_REP_DH_FAIL(c) \
TRACE(c, "PKINIT client could not verify DH reply")
@@ -91,6 +91,72 @@
#define TRACE_PKINIT_OPENSSL_ERROR(c, msg) \
TRACE(c, "PKINIT OpenSSL error: {str}", msg)
+#define TRACE_PKINIT_SERVER_CERT_AUTH(c, modname) \
+ TRACE(c, "PKINIT server authorizing cert with module {str}", \
+ modname)
+#define TRACE_PKINIT_SERVER_EKU_REJECT(c) \
+ TRACE(c, "PKINIT server found no acceptable EKU in client cert")
+#define TRACE_PKINIT_SERVER_EKU_SKIP(c) \
+ TRACE(c, "PKINIT server skipping EKU check due to configuration")
+#define TRACE_PKINIT_SERVER_INIT_REALM(c, realm) \
+ TRACE(c, "PKINIT server initializing realm {str}", realm)
+#define TRACE_PKINIT_SERVER_MATCHING_UPN_FOUND(c) \
+ TRACE(c, "PKINIT server found a matching UPN SAN in client cert")
+#define TRACE_PKINIT_SERVER_MATCHING_SAN_FOUND(c) \
+ TRACE(c, "PKINIT server found a matching SAN in client cert")
+#define TRACE_PKINIT_SERVER_NO_SAN(c) \
+ TRACE(c, "PKINIT server found no SAN in client cert")
+#define TRACE_PKINIT_SERVER_PADATA_VERIFY(c) \
+ TRACE(c, "PKINIT server verifying KRB5_PADATA_PK_AS_REQ")
+#define TRACE_PKINIT_SERVER_PADATA_VERIFY_OLD(c) \
+ TRACE(c, "PKINIT server verifying KRB5_PADATA_PK_AS_REQ_OLD")
+#define TRACE_PKINIT_SERVER_PADATA_VERIFY_FAIL(c) \
+ TRACE(c, "PKINIT server failed to verify PA data")
+#define TRACE_PKINIT_SERVER_RETURN_PADATA(c) \
+ TRACE(c, "PKINIT server returning PA data")
+#define TRACE_PKINIT_SERVER_SAN_REJECT(c) \
+ TRACE(c, "PKINIT server found no acceptable SAN in client cert")
+
+#define TRACE_PKINIT_EKU(c) \
+ TRACE(c, "PKINIT found acceptable EKU and digitalSignature KU")
+#define TRACE_PKINIT_EKU_NO_KU(c) \
+ TRACE(c, "PKINIT found acceptable EKU but no digitalSignature KU")
+#define TRACE_PKINIT_LOADED_CERT(c, name) \
+ TRACE(c, "PKINIT loaded cert and key for {str}", name)
+#define TRACE_PKINIT_LOAD_FROM_FILE(c) \
+ TRACE(c, "PKINIT loading CA certs and CRLs from FILE")
+#define TRACE_PKINIT_LOAD_FROM_DIR(c) \
+ TRACE(c, "PKINIT loading CA certs and CRLs from DIR")
+#define TRACE_PKINIT_NO_CA_ANCHOR(c, file) \
+ TRACE(c, "PKINIT no anchor CA in file {str}", file)
+#define TRACE_PKINIT_NO_CA_INTERMEDIATE(c, file) \
+ TRACE(c, "PKINIT no intermediate CA in file {str}", file)
+#define TRACE_PKINIT_NO_CERT(c) \
+ TRACE(c, "PKINIT no certificate provided")
+#define TRACE_PKINIT_NO_CERT_AND_KEY(c, dirname) \
+ TRACE(c, "PKINIT no cert and key pair found in directory {str}", \
+ dirname)
+#define TRACE_PKINIT_NO_CRL(c, file) \
+ TRACE(c, "PKINIT no CRL in file {str}", file)
+#define TRACE_PKINIT_NO_DEFAULT_CERT(c, count) \
+ TRACE(c, "PKINIT error: There are {int} certs, but there must " \
+ "be exactly one.", count)
+#define TRACE_PKINIT_NO_MATCHING_CERT(c) \
+ TRACE(c, "PKINIT no matching certificate found")
+#define TRACE_PKINIT_NO_PRIVKEY(c) \
+ TRACE(c, "PKINIT no private key provided")
+#define TRACE_PKINIT_PKCS_DECODE_FAIL(c, name) \
+ TRACE(c, "PKINIT failed to decode PKCS12 file {str} contents", name)
+#define TRACE_PKINIT_PKCS_OPEN_FAIL(c, name, err) \
+ TRACE(c, "PKINIT failed to open PKCS12 file {str}: err {errno}", \
+ name, err)
+#define TRACE_PKINIT_PKCS_PARSE_FAIL_FIRST(c) \
+ TRACE(c, "PKINIT initial PKCS12_parse with no password failed")
+#define TRACE_PKINIT_PKCS_PARSE_FAIL_SECOND(c) \
+ TRACE(c, "PKINIT second PKCS12_parse with password failed")
+#define TRACE_PKINIT_PKCS_PROMPT_FAIL(c) \
+ TRACE(c, "PKINIT failed to prompt for PKCS12 password")
+
#define TRACE_CERTAUTH_VTINIT_FAIL(c, ret) \
TRACE(c, "certauth module failed to init vtable: {kerr}", ret)
#define TRACE_CERTAUTH_INIT_FAIL(c, name, ret) \

124
Fix-certauth-built-in-module-returns.patch Normal file
View File

@ -0,0 +1,124 @@
From d507d9a78e12418f83c6db6e22052543f3e5db37 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

31
Make-certauth-eku-module-restrictive-only.patch
View File

@ -1,31 +0,0 @@
From 2b1a91087b668ab1021f1ca461b8210e7e015c8a Mon Sep 17 00:00:00 2001
From: Greg Hudson <ghudson@mit.edu>
Date: Thu, 24 Aug 2017 11:11:46 -0400
Subject: [PATCH] Make certauth eku module restrictive-only
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.
ticket: 8561
(cherry picked from commit aca6fd6bc07934a90a18a70116ea3b620228950a)
---
src/plugins/preauth/pkinit/pkinit_srv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/plugins/preauth/pkinit/pkinit_srv.c b/src/plugins/preauth/pkinit/pkinit_srv.c
index 32ca122f2..d7a604c80 100644
--- a/src/plugins/preauth/pkinit/pkinit_srv.c
+++ b/src/plugins/preauth/pkinit/pkinit_srv.c
@@ -1495,7 +1495,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

15
krb5.spec
View File

@ -18,7 +18,7 @@ Summary: The Kerberos network authentication system
Name: krb5
Version: 1.15.1
# for prerelease, should be e.g., 0.3.beta2%{?dist}
Release: 25%{?dist}
Release: 27%{?dist}
# - Maybe we should explode from the now-available-to-everybody tarball instead?
# http://web.mit.edu/kerberos/dist/krb5/1.13/krb5-1.13.2-signed.tar
# - The sources below are stored in a lookaside cache. Upload with
@ -91,7 +91,11 @@ Patch62: Fix-more-time-manipulations-for-y2038.patch
Patch63: Use-krb5_timestamp-where-appropriate.patch
Patch64: Add-KDC-policy-pluggable-interface.patch
Patch65: Fix-bugs-in-kdcpolicy-commit.patch
Patch66: Make-certauth-eku-module-restrictive-only.patch
Patch66: Convert-some-pkiDebug-messages-to-TRACE-macros.patch
Patch67: Fix-certauth-built-in-module-returns.patch
Patch68: Add-test-cert-with-no-extensions.patch
Patch69: Add-PKINIT-test-case-for-generic-client-cert.patch
Patch70: Add-hostname-based-ccselect-module.patch
License: MIT
URL: http://web.mit.edu/kerberos/www/
@ -743,6 +747,13 @@ exit 0
%{_libdir}/libkadm5srv_mit.so.*
%changelog
* Tue Sep 05 2017 Robbie Harwood <rharwood@redhat.com> - 1.15.1-27
- Add hostname-based ccselect module
- Resolves: #1463665
* Tue Sep 05 2017 Robbie Harwood <rharwood@redhat.com> - 1.15.1-26
- Backport upstream certauth EKU fixes
* Fri Aug 25 2017 Robbie Harwood <rharwood@redhat.com> - 1.15.1-25
- Backport certauth eku security fix