Eliminate redundant PKINIT responder invocation

Eliminate-redundant-PKINIT-responder-invocation.patch

@@ -0,0 +1,93 @@
+From b5793f8024320aaa7a85ca39cdc03bf99773bf11 Mon Sep 17 00:00:00 2001
+From: Greg Hudson <ghudson@mit.edu>
+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])

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 <rharwood@redhat.com> - 1.18-9
+- Eliminate redundant PKINIT responder invocation
+
 * Thu Mar 26 2020 Robbie Harwood <rharwood@redhat.com> - 1.18-8
 - Add finalization safety check to com_err