krb5 commit [krb5-1.19]: Load certs when checking pkinit_identities values
Greg Hudson
ghudson at mit.edu
Wed Feb 17 15:44:30 EST 2021
https://github.com/krb5/krb5/commit/2a3a4088ecafea68d0a91a48c4abc3c11cdf59d8
commit 2a3a4088ecafea68d0a91a48c4abc3c11cdf59d8
Author: Ken Hornstein <kenh at cmf.nrl.navy.mil>
Date: Wed Jan 27 21:21:19 2021 -0500
Load certs when checking pkinit_identities values
Move the crypto_load_certs() probe from pkinit_identity_initialize()
to process_option_identity(). This will attempt to load a certificate
for each pkinit_identities value, and if the certificate load fails to
move to the next line.
For PKCS11, return an error if pkinit_open_session() fails, but do not
fail in pkinit_open_session() just because identity prompts are
deferred.
[ghudson at mit.edu: added test case; moved cert probe to
process_option_identity(); rewrote commit message]
(cherry picked from commit 13ae08e70a05768d4f65978ce1a8d4e16fec0d35)
ticket: 8984
version_fixed: 1.19.1
doc/admin/conf_files/krb5_conf.rst | 7 ++--
src/plugins/preauth/pkinit/pkinit_crypto_openssl.c | 10 ++----
src/plugins/preauth/pkinit/pkinit_identity.c | 30 ++++++++++----------
src/tests/t_pkinit.py | 7 ++++
4 files changed, 29 insertions(+), 25 deletions(-)
diff --git a/doc/admin/conf_files/krb5_conf.rst b/doc/admin/conf_files/krb5_conf.rst
index cb17a84..6751759 100644
--- a/doc/admin/conf_files/krb5_conf.rst
+++ b/doc/admin/conf_files/krb5_conf.rst
@@ -1126,10 +1126,9 @@ PKINIT krb5.conf options
**pkinit_identities**
Specifies the location(s) to be used to find the user's X.509
identity information. If this option is specified multiple times,
- the first valid value is used; this can be used to specify an
- environment variable (with **ENV:**\ *envvar*) followed by a
- default value. Note that these values are not used if the user
- specifies **X509_user_identity** on the command line.
+ each value is attempted in order until certificates are found.
+ Note that these values are not used if the user specifies
+ **X509_user_identity** on the command line.
**pkinit_kdc_hostname**
The presence of this option indicates that the client is willing
diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
index d7d1593..e5940a5 100644
--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
+++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
@@ -3748,7 +3748,7 @@ pkinit_open_session(krb5_context context,
pkinit_set_deferred_id(&cctx->deferred_ids,
p11name, tinfo.flags, NULL);
free(p11name);
- return KRB5KRB_ERR_GENERIC;
+ return 0;
}
/* Look up a responder-supplied password for the token. */
password = pkinit_find_deferred_id(cctx->deferred_ids, p11name);
@@ -4552,11 +4552,9 @@ pkinit_get_certs_pkcs11(krb5_context context,
id_cryptoctx->slotid = idopts->slotid;
id_cryptoctx->pkcs11_method = 1;
- if (pkinit_open_session(context, id_cryptoctx)) {
- pkiDebug("can't open pkcs11 session\n");
- if (!id_cryptoctx->defer_id_prompt)
- return KRB5KDC_ERR_PREAUTH_FAILED;
- }
+ r = pkinit_open_session(context, id_cryptoctx);
+ if (r != 0)
+ return r;
if (id_cryptoctx->defer_id_prompt) {
/*
* We need to reset all of the PKCS#11 state, so that the next time we
diff --git a/src/plugins/preauth/pkinit/pkinit_identity.c b/src/plugins/preauth/pkinit/pkinit_identity.c
index b89c5d0..4046b15 100644
--- a/src/plugins/preauth/pkinit/pkinit_identity.c
+++ b/src/plugins/preauth/pkinit/pkinit_identity.c
@@ -378,7 +378,7 @@ process_option_identity(krb5_context context,
pkinit_req_crypto_context req_cryptoctx,
pkinit_identity_opts *idopts,
pkinit_identity_crypto_context id_cryptoctx,
- const char *value)
+ krb5_principal princ, const char *value)
{
const char *residual;
int idtype;
@@ -424,7 +424,7 @@ process_option_identity(krb5_context context,
switch (idtype) {
case IDTYPE_ENVVAR:
return process_option_identity(context, plg_cryptoctx, req_cryptoctx,
- idopts, id_cryptoctx,
+ idopts, id_cryptoctx, princ,
secure_getenv(residual));
break;
case IDTYPE_FILE:
@@ -450,7 +450,16 @@ process_option_identity(krb5_context context,
retval = EINVAL;
break;
}
- return retval;
+ if (retval)
+ return retval;
+
+ retval = crypto_load_certs(context, plg_cryptoctx, req_cryptoctx, idopts,
+ id_cryptoctx, princ, TRUE);
+ if (retval)
+ return retval;
+
+ crypto_free_cert_info(context, plg_cryptoctx, req_cryptoctx, id_cryptoctx);
+ return 0;
}
static krb5_error_code
@@ -525,12 +534,13 @@ pkinit_identity_initialize(krb5_context context,
if (idopts->identity != NULL) {
retval = process_option_identity(context, plg_cryptoctx,
req_cryptoctx, idopts,
- id_cryptoctx, idopts->identity);
+ id_cryptoctx, princ,
+ idopts->identity);
} else if (idopts->identity_alt != NULL) {
for (i = 0; retval != 0 && idopts->identity_alt[i] != NULL; i++) {
retval = process_option_identity(context, plg_cryptoctx,
req_cryptoctx, idopts,
- id_cryptoctx,
+ id_cryptoctx, princ,
idopts->identity_alt[i]);
}
} else {
@@ -540,16 +550,6 @@ pkinit_identity_initialize(krb5_context context,
pkiDebug("%s: no user identity options specified\n", __FUNCTION__);
goto errout;
}
- if (retval)
- goto errout;
-
- retval = crypto_load_certs(context, plg_cryptoctx, req_cryptoctx,
- idopts, id_cryptoctx, princ, TRUE);
- if (retval)
- goto errout;
-
- crypto_free_cert_info(context, plg_cryptoctx, req_cryptoctx,
- id_cryptoctx);
} else {
/* We're the anonymous principal. */
retval = 0;
diff --git a/src/tests/t_pkinit.py b/src/tests/t_pkinit.py
index f224383..aee4da2 100755
--- a/src/tests/t_pkinit.py
+++ b/src/tests/t_pkinit.py
@@ -183,6 +183,13 @@ realm.kinit(realm.user_princ,
realm.klist(realm.user_princ)
realm.run([kvno, realm.host_princ])
+# Try using multiple configured pkinit_identities, to make sure we
+# fall back to the second one when the first one cannot be read.
+id_conf = {'realms': {'$realm': {'pkinit_identities': [file_identity + 'X',
+ file_identity]}}}
+id_env = realm.special_env('idconf', False, krb5_conf=id_conf)
+realm.kinit(realm.user_princ, expected_trace=msgs, env=id_env)
+
# Try again using RSA instead of DH.
mark('FILE identity, no password, RSA')
realm.kinit(realm.user_princ,
More information about the cvs-krb5
mailing list