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

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