Blame SOURCES/Eliminate-redundant-PKINIT-responder-invocation.patch

10fa70
From fa5d09798a56960c34f28296726ed4525e6950d9 Mon Sep 17 00:00:00 2001
10fa70
From: Greg Hudson <ghudson@mit.edu>
10fa70
Date: Mon, 23 Mar 2020 19:10:03 -0400
10fa70
Subject: [PATCH] Eliminate redundant PKINIT responder invocation
10fa70
10fa70
In pkinit_client_prep_questions(), only act if the input padata type
10fa70
is KRB5_PADATA_PK_AS_REQ.  Otherwise we will ask questions again when
10fa70
the KDC issues a ticket.
10fa70
10fa70
Commit 7621d2f9a87214327ca3b2594e34dc7cea84596b (ticket 8242)
10fa70
unintentionally changed the behavior of pkinit_load_fs_cert_and_key(),
10fa70
causing pkinit_client_prep_questions() to do nothing on its first
10fa70
call.  Restore the original behavior of returning 0 when prompting is
10fa70
deferred.
10fa70
10fa70
Modify the existing "FILE identity, password on key (responder)"
10fa70
PKINIT test to check that the responder is only invoked once.
10fa70
10fa70
ticket: 8885
10fa70
(cherry picked from commit f1286842ce7b9e507a4ce0a47f44ab361a98be63)
10fa70
(cherry picked from commit 4a05805eb39ba088c07f782fb52a6538ec3f2db6)
10fa70
---
10fa70
 src/plugins/preauth/pkinit/pkinit_clnt.c           |  5 +++++
10fa70
 src/plugins/preauth/pkinit/pkinit_crypto_openssl.c | 13 +++++++------
10fa70
 src/tests/t_pkinit.py                              | 11 +++++++----
10fa70
 3 files changed, 19 insertions(+), 10 deletions(-)
10fa70
10fa70
diff --git a/src/plugins/preauth/pkinit/pkinit_clnt.c b/src/plugins/preauth/pkinit/pkinit_clnt.c
10fa70
index 2f0431991..9b991ffe0 100644
10fa70
--- a/src/plugins/preauth/pkinit/pkinit_clnt.c
10fa70
+++ b/src/plugins/preauth/pkinit/pkinit_clnt.c
10fa70
@@ -897,6 +897,11 @@ pkinit_client_prep_questions(krb5_context context,
10fa70
     k5_json_object jval = NULL;
10fa70
     k5_json_number jflag = NULL;
10fa70
 
10fa70
+    /* Don't ask questions for the informational padata items or when the
10fa70
+     * ticket is issued. */
10fa70
+    if (pa_data->pa_type != KRB5_PADATA_PK_AS_REQ)
10fa70
+        return 0;
10fa70
+
10fa70
     if (!reqctx->identity_initialized) {
10fa70
         pkinit_client_profile(context, plgctx, reqctx, cb, rock,
10fa70
                               &request->server->realm);
10fa70
diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
10fa70
index dd718c2be..dbb054378 100644
10fa70
--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
10fa70
+++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c
10fa70
@@ -4362,17 +4362,18 @@ pkinit_load_fs_cert_and_key(krb5_context context,
10fa70
 
10fa70
     /* Load the certificate. */
10fa70
     retval = get_cert(certname, &x);
10fa70
-    if (retval != 0 || x == NULL) {
10fa70
-        retval = oerr(context, 0, _("Cannot read certificate file '%s'"),
10fa70
+    if (retval) {
10fa70
+        retval = oerr(context, retval, _("Cannot read certificate file '%s'"),
10fa70
                       certname);
10fa70
-        goto cleanup;
10fa70
     }
10fa70
+    if (retval || x == NULL)
10fa70
+        goto cleanup;
10fa70
     /* Load the key. */
10fa70
     retval = get_key(context, id_cryptoctx, keyname, fsname, &y, password);
10fa70
-    if (retval != 0 || y == NULL) {
10fa70
-        retval = oerr(context, 0, _("Cannot read key file '%s'"), fsname);
10fa70
+    if (retval)
10fa70
+        retval = oerr(context, retval, _("Cannot read key file '%s'"), fsname);
10fa70
+    if (retval || y == NULL)
10fa70
         goto cleanup;
10fa70
-    }
10fa70
 
10fa70
     id_cryptoctx->creds[cindex] = malloc(sizeof(struct _pkinit_cred_info));
10fa70
     if (id_cryptoctx->creds[cindex] == NULL) {
10fa70
diff --git a/src/tests/t_pkinit.py b/src/tests/t_pkinit.py
10fa70
index 69daf4987..ecd450e8a 100755
10fa70
--- a/src/tests/t_pkinit.py
10fa70
+++ b/src/tests/t_pkinit.py
10fa70
@@ -248,10 +248,13 @@ realm.run(['./adata', realm.host_princ],
10fa70
 # supplied by the responder.
10fa70
 # Supply the response in raw form.
10fa70
 mark('FILE identity, password on key (responder)')
10fa70
-realm.run(['./responder', '-x', 'pkinit={"%s": 0}' % file_enc_identity,
10fa70
-           '-r', 'pkinit={"%s": "encrypted"}' % file_enc_identity,
10fa70
-           '-X', 'X509_user_identity=%s' % file_enc_identity,
10fa70
-           realm.user_princ])
10fa70
+out = realm.run(['./responder', '-x', 'pkinit={"%s": 0}' % file_enc_identity,
10fa70
+                 '-r', 'pkinit={"%s": "encrypted"}' % file_enc_identity,
10fa70
+                 '-X', 'X509_user_identity=%s' % file_enc_identity,
10fa70
+                 realm.user_princ])
10fa70
+# Regression test for #8885 (password question asked twice).
10fa70
+if out.count('OK: ') != 1:
10fa70
+    fail('Wrong number of responder calls')
10fa70
 # Supply the response through the convenience API.
10fa70
 realm.run(['./responder', '-X', 'X509_user_identity=%s' % file_enc_identity,
10fa70
            '-p', '%s=%s' % (file_enc_identity, 'encrypted'), realm.user_princ])