From: Isaac Boukris Date: Wed, 15 Jan 2020 12:54:44 +0000 (+0100) Subject: Fix KDC crash in handle_signticket X-Git-Tag: krb5-1.19-beta1~131 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=96e5d384acf174e6079b0aeeec14bd8100d24840;p=thirdparty%2Fkrb5.git 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. [ghudson@mit.edu: edited commit message] ticket: 8867 (new) tags: pullup target_version: 1.18 --- diff --git a/src/kdc/kdc_authdata.c b/src/kdc/kdc_authdata.c index 9c434f3cf0..29e720507c 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 6724c46c75..384b21ad2c 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 44965dd1a2..378174a2ea 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.