From 4e7e5fe69b3cad7cf7d4ec4ef1af77dd73fb3f6f Mon Sep 17 00:00:00 2001 From: Robbie Harwood Date: Thu, 26 Mar 2020 16:01:18 -0400 Subject: [PATCH] Eliminate redundant PKINIT responder invocation --- ...edundant-PKINIT-responder-invocation.patch | 93 +++++++++++++++++++ krb5.spec | 6 +- 2 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 Eliminate-redundant-PKINIT-responder-invocation.patch diff --git a/Eliminate-redundant-PKINIT-responder-invocation.patch b/Eliminate-redundant-PKINIT-responder-invocation.patch new file mode 100644 index 0000000..ea973b7 --- /dev/null +++ b/Eliminate-redundant-PKINIT-responder-invocation.patch @@ -0,0 +1,93 @@ +From b5793f8024320aaa7a85ca39cdc03bf99773bf11 Mon Sep 17 00:00:00 2001 +From: Greg Hudson +Date: Mon, 23 Mar 2020 19:10:03 -0400 +Subject: [PATCH] Eliminate redundant PKINIT responder invocation + +In pkinit_client_prep_questions(), only act if the input padata type +is KRB5_PADATA_PK_AS_REQ. Otherwise we will ask questions again when +the KDC issues a ticket. + +Commit 7621d2f9a87214327ca3b2594e34dc7cea84596b (ticket 8242) +unintentionally changed the behavior of pkinit_load_fs_cert_and_key(), +causing pkinit_client_prep_questions() to do nothing on its first +call. Restore the original behavior of returning 0 when prompting is +deferred. + +Modify the existing "FILE identity, password on key (responder)" +PKINIT test to check that the responder is only invoked once. + +ticket: 8885 +(cherry picked from commit f1286842ce7b9e507a4ce0a47f44ab361a98be63) +--- + src/plugins/preauth/pkinit/pkinit_clnt.c | 5 +++++ + src/plugins/preauth/pkinit/pkinit_crypto_openssl.c | 13 +++++++------ + src/tests/t_pkinit.py | 11 +++++++---- + 3 files changed, 19 insertions(+), 10 deletions(-) + +diff --git a/src/plugins/preauth/pkinit/pkinit_clnt.c b/src/plugins/preauth/pkinit/pkinit_clnt.c +index 2f0431991..9b991ffe0 100644 +--- a/src/plugins/preauth/pkinit/pkinit_clnt.c ++++ b/src/plugins/preauth/pkinit/pkinit_clnt.c +@@ -897,6 +897,11 @@ pkinit_client_prep_questions(krb5_context context, + k5_json_object jval = NULL; + k5_json_number jflag = NULL; + ++ /* Don't ask questions for the informational padata items or when the ++ * ticket is issued. */ ++ if (pa_data->pa_type != KRB5_PADATA_PK_AS_REQ) ++ return 0; ++ + if (!reqctx->identity_initialized) { + pkinit_client_profile(context, plgctx, reqctx, cb, rock, + &request->server->realm); +diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +index dd718c2be..dbb054378 100644 +--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c ++++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +@@ -4362,17 +4362,18 @@ pkinit_load_fs_cert_and_key(krb5_context context, + + /* Load the certificate. */ + retval = get_cert(certname, &x); +- if (retval != 0 || x == NULL) { +- retval = oerr(context, 0, _("Cannot read certificate file '%s'"), ++ if (retval) { ++ retval = oerr(context, retval, _("Cannot read certificate file '%s'"), + certname); +- goto cleanup; + } ++ if (retval || x == NULL) ++ goto cleanup; + /* Load the key. */ + retval = get_key(context, id_cryptoctx, keyname, fsname, &y, password); +- if (retval != 0 || y == NULL) { +- retval = oerr(context, 0, _("Cannot read key file '%s'"), fsname); ++ if (retval) ++ retval = oerr(context, retval, _("Cannot read key file '%s'"), fsname); ++ if (retval || y == NULL) + goto cleanup; +- } + + id_cryptoctx->creds[cindex] = malloc(sizeof(struct _pkinit_cred_info)); + if (id_cryptoctx->creds[cindex] == NULL) { +diff --git a/src/tests/t_pkinit.py b/src/tests/t_pkinit.py +index 69daf4987..ecd450e8a 100755 +--- a/src/tests/t_pkinit.py ++++ b/src/tests/t_pkinit.py +@@ -248,10 +248,13 @@ realm.run(['./adata', realm.host_princ], + # supplied by the responder. + # Supply the response in raw form. + mark('FILE identity, password on key (responder)') +-realm.run(['./responder', '-x', 'pkinit={"%s": 0}' % file_enc_identity, +- '-r', 'pkinit={"%s": "encrypted"}' % file_enc_identity, +- '-X', 'X509_user_identity=%s' % file_enc_identity, +- realm.user_princ]) ++out = realm.run(['./responder', '-x', 'pkinit={"%s": 0}' % file_enc_identity, ++ '-r', 'pkinit={"%s": "encrypted"}' % file_enc_identity, ++ '-X', 'X509_user_identity=%s' % file_enc_identity, ++ realm.user_princ]) ++# Regression test for #8885 (password question asked twice). ++if out.count('OK: ') != 1: ++ fail('Wrong number of responder calls') + # Supply the response through the convenience API. + realm.run(['./responder', '-X', 'X509_user_identity=%s' % file_enc_identity, + '-p', '%s=%s' % (file_enc_identity, 'encrypted'), realm.user_princ]) diff --git a/krb5.spec b/krb5.spec index 6e2698b..a87f08c 100644 --- a/krb5.spec +++ b/krb5.spec @@ -18,7 +18,7 @@ Summary: The Kerberos network authentication system Name: krb5 Version: 1.18 # for prerelease, should be e.g., 0.% {prerelease}.1% { ?dist } (without spaces) -Release: 8%{?dist} +Release: 9%{?dist} # rharwood has trust path to signing key and verifies on check-in Source0: https://web.mit.edu/kerberos/dist/krb5/1.18/krb5-%{version}%{prerelease}.tar.gz @@ -56,6 +56,7 @@ Patch10: Allow-deletion-of-require_auth-with-LDAP-KDB.patch Patch11: Refresh-manually-acquired-creds-from-client-keytab.patch Patch12: Document-client-keytab-usage.patch Patch13: Add-finalization-safety-check-to-com_err.patch +Patch14: Eliminate-redundant-PKINIT-responder-invocation.patch License: MIT URL: https://web.mit.edu/kerberos/www/ @@ -633,6 +634,9 @@ exit 0 %{_libdir}/libkadm5srv_mit.so.* %changelog +* Thu Mar 26 2020 Robbie Harwood - 1.18-9 +- Eliminate redundant PKINIT responder invocation + * Thu Mar 26 2020 Robbie Harwood - 1.18-8 - Add finalization safety check to com_err