krb5 commit: Fix KDC crash in handle_signticket

https://github.com/krb5/krb5/commit/96e5d384acf174e6079b0aeeec14bd8100d24840
commit 96e5d384acf174e6079b0aeeec14bd8100d24840
Author: Isaac Boukris <[email protected]>
Date:   Wed Jan 15 13:54:44 2020 +0100

    Fix KDC crash in handle_signticket
    
    Commit d47f7dba3779c9e36e1dedaac830dac1dd248fb3 changed the parameters
    passed to sign_authdata() for S4U2Proxy requests so that client is the
    entry for the impersonated client (not the impersonator), and added a
    new parameter for the impersonator entry.  It should have changed the
    call to handle_signticket() to use the impersonator entry.  Fix the
    handle_signticket() call, and change some parameter names to more
    clearly indicate the flow of subject_server from process_tgs_req() to
    handle_authdata() to its helpers.
    
    [[email protected]: edited commit message]
    
    ticket: 8867 (new)
    tags: pullup
    target_version: 1.18

 src/kdc/kdc_authdata.c  |   29 +++++++++++++++--------------
 src/kdc/kdc_util.h      |    2 +-
 src/tests/t_authdata.py |   39 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 15 deletions(-)

diff --git a/src/kdc/kdc_authdata.c b/src/kdc/kdc_authdata.c
index 9c434f3..29e7205 100644
--- a/src/kdc/kdc_authdata.c
+++ b/src/kdc/kdc_authdata.c
@@ -672,7 +672,7 @@ only_pac_p(krb5_context context, krb5_authdata **authdata)
  * element if we should. */
 static krb5_error_code
 handle_signticket(krb5_context context, unsigned int flags,
-                  krb5_db_entry *client, krb5_db_entry *server,
+                  krb5_db_entry *subject_server, krb5_db_entry *server,
                   krb5_db_entry *local_tgt, krb5_keyblock *local_tgt_key,
                   krb5_kdc_req *req, krb5_const_principal for_user_princ,
                   krb5_enc_tkt_part *enc_tkt_req,
@@ -708,8 +708,8 @@ handle_signticket(krb5_context context, unsigned int flags,
         !is_cross_tgs_principal(server->princ) &&
         !only_pac_p(context, enc_tkt_reply->authorization_data)) {
         ret = make_signedpath(context, for_user_princ,
-                              s4u2proxy ? client->princ : NULL, local_tgt_key,
-                              deleg_path, enc_tkt_reply);
+                              s4u2proxy ? subject_server->princ : NULL,
+                              local_tgt_key, deleg_path, enc_tkt_reply);
         if (ret)
             goto cleanup;
     }
@@ -807,9 +807,9 @@ cleanup:
 krb5_error_code
 handle_authdata(krb5_context context, unsigned int flags,
                 krb5_db_entry *client, krb5_db_entry *server,
-                krb5_db_entry *header_server, krb5_db_entry *local_tgt,
+                krb5_db_entry *subject_server, krb5_db_entry *local_tgt,
                 krb5_keyblock *local_tgt_key, krb5_keyblock *client_key,
-                krb5_keyblock *server_key, krb5_keyblock *header_key,
+                krb5_keyblock *server_key, krb5_keyblock *subject_key,
                 krb5_data *req_pkt, krb5_kdc_req *req,
                 krb5_const_principal altcprinc, void *ad_info,
                 krb5_enc_tkt_part *enc_tkt_req,
@@ -835,8 +835,8 @@ handle_authdata(krb5_context context, unsigned int flags,
         for (i = 0; i < n_authdata_modules; i++) {
             h = &authdata_modules[i];
             ret = h->vt.handle(context, h->data, flags, client, server,
-                               header_server, client_key, server_key,
-                               header_key, req_pkt, req, altcprinc,
+                               subject_server, client_key, server_key,
+                               subject_key, req_pkt, req, altcprinc,
                                enc_tkt_req, enc_tkt_reply);
             if (ret)
                 kdc_err(context, ret, "from authdata module %s", h->vt.name);
@@ -853,10 +853,11 @@ handle_authdata(krb5_context context, unsigned int flags,
 
     if (!isflagset(enc_tkt_reply->flags, TKT_FLG_ANONYMOUS)) {
         /* Fetch authdata from the KDB if appropriate. */
-        ret = fetch_kdb_authdata(context, flags, client, server, header_server,
-                                 local_tgt, client_key, server_key, header_key,
-                                 local_tgt_key, req, altcprinc, ad_info,
-                                 enc_tkt_req, enc_tkt_reply, auth_indicators);
+        ret = fetch_kdb_authdata(context, flags, client, server,
+                                 subject_server, local_tgt, client_key,
+                                 server_key, subject_key, local_tgt_key,
+                                 req, altcprinc, ad_info, enc_tkt_req,
+                                 enc_tkt_reply, auth_indicators);
         if (ret)
             return ret;
     }
@@ -873,9 +874,9 @@ handle_authdata(krb5_context context, unsigned int flags,
     if (!isflagset(enc_tkt_reply->flags, TKT_FLG_ANONYMOUS)) {
         /* Validate and insert AD-SIGNTICKET authdata.  This must happen last
          * since it contains a signature over the other authdata. */
-        ret = handle_signticket(context, flags, client, server, local_tgt,
-                                local_tgt_key, req, altcprinc, enc_tkt_req,
-                                enc_tkt_reply);
+        ret = handle_signticket(context, flags, subject_server, server,
+                                local_tgt, local_tgt_key, req, altcprinc,
+                                enc_tkt_req, enc_tkt_reply);
         if (ret)
             return ret;
     }
diff --git a/src/kdc/kdc_util.h b/src/kdc/kdc_util.h
index 6724c46..384b21a 100644
--- a/src/kdc/kdc_util.h
+++ b/src/kdc/kdc_util.h
@@ -226,7 +226,7 @@ handle_authdata (krb5_context context,
                  unsigned int flags,
                  krb5_db_entry *client,
                  krb5_db_entry *server,
-                 krb5_db_entry *header_server,
+                 krb5_db_entry *subject_server,
                  krb5_db_entry *local_tgt,
                  krb5_keyblock *local_tgt_key,
                  krb5_keyblock *client_key,
diff --git a/src/tests/t_authdata.py b/src/tests/t_authdata.py
index 44965dd..378174a 100644
--- a/src/tests/t_authdata.py
+++ b/src/tests/t_authdata.py
@@ -282,6 +282,45 @@ out = realm.run(['./adata', 'noauthdata'])
 if '-456: db-authdata-test' in out:
     fail('DB authdata not suppressed by +no_auth_data_required')
 
+mark('S4U2Proxy with a foreign client')
+
+a_princs = {'krbtgt/A': {'keys': 'aes128-cts'},
+            'krbtgt/B': {'keys': 'aes128-cts'},
+            'impersonator': {'keys': 'aes128-cts'},
+            'resource': {'keys': 'aes128-cts'}}
+a_kconf = {'realms': {'$realm': {'database_module': 'test'}},
+           'dbmodules': {'test': {'db_library': 'test',
+                                  'delegation': {'impersonator' : 'resource'},
+                                  'princs': a_princs}}}
+
+b_princs = {'krbtgt/B': {'keys': 'aes128-cts'},
+            'krbtgt/A': {'keys': 'aes128-cts'},
+            'user': {'keys': 'aes128-cts', 'flags': '+preauth'}}
+b_kconf = {'realms': {'$realm': {'database_module': 'test'}},
+           'dbmodules': {'test': {'db_library': 'test',
+                                  'princs': b_princs}}}
+
+ra, rb = cross_realms(2, xtgts=(),
+                          args=({'realm': 'A', 'kdc_conf': a_kconf},
+                                {'realm': 'B', 'kdc_conf': b_kconf}),
+                          create_kdb=False)
+
+ra.start_kdc()
+rb.start_kdc()
+
+ra.extract_keytab('impersonator@A', ra.keytab)
+rb.extract_keytab('user@B', rb.keytab)
+
+usercache = 'FILE:' + os.path.join(rb.testdir, 'usercache')
+rb.kinit(rb.user_princ, None, ['-k', '-f', '-c', usercache])
+rb.run([kvno, '-C', 'impersonator@A', '-c', usercache])
+
+ra.kinit('impersonator@A', None, ['-f', '-k', '-t', ra.keytab])
+ra.run(['./s4u2proxy', usercache, 'resource@A'])
+
+ra.stop()
+rb.stop()
+
 # Additional KDB module authdata behavior we don't currently test:
 # * KDB module authdata is suppressed in TGS requests if the TGT
 #   contains no authdata and the request is not cross-realm or S4U.