From 3b6955d99e3c9ab351147b8c0d14a904a7bcffb8 Mon Sep 17 00:00:00 2001 From: Robbie Harwood Date: Fri, 21 Feb 2020 13:16:49 -0500 Subject: [PATCH] Fix AS-REQ checking of KDB-modified indicators --- ...-checking-of-KDB-modified-indicators.patch | 189 ++++++++++++++++++ krb5.spec | 6 +- 2 files changed, 194 insertions(+), 1 deletion(-) create mode 100644 Fix-AS-REQ-checking-of-KDB-modified-indicators.patch diff --git a/Fix-AS-REQ-checking-of-KDB-modified-indicators.patch b/Fix-AS-REQ-checking-of-KDB-modified-indicators.patch new file mode 100644 index 0000000..1655c38 --- /dev/null +++ b/Fix-AS-REQ-checking-of-KDB-modified-indicators.patch @@ -0,0 +1,189 @@ +From 744154b19c8000965e5a5de51d5dbef0794958be Mon Sep 17 00:00:00 2001 +From: Greg Hudson +Date: Wed, 19 Feb 2020 15:36:38 -0500 +Subject: [PATCH] Fix AS-REQ checking of KDB-modified indicators + +Commit 7196c03f18f14695abeb5ae4923004469b172f0f (ticket 8823) gave the +KDB the ability to modify auth indicators, but it happens after the +asserted indicators are checked against the server principal +requirements. In finish_process_as_req(), move the call to +check_indicators() after the call to handle_authdata() so that the +final indicator list is checked. + +For the test case, add string attribute functionality to the test KDB +module, and fix a bug where test_get_principal() would return failure +if a principal has no keys. Also add a test case for AS-REQ +enforcement of normally asserted auth indicators. + +ticket: 8876 (new) +tags: pullup +target_version: 1.18-next + +(cherry picked from commit 109e30ce22c20f18b8233119f274935bdf573886) +--- + src/kdc/do_as_req.c | 14 +++++------ + src/plugins/kdb/test/kdb_test.c | 42 +++++++++++++++++++++++++++++++-- + src/tests/t_authdata.py | 11 +++++++++ + 3 files changed, 58 insertions(+), 9 deletions(-) + +diff --git a/src/kdc/do_as_req.c b/src/kdc/do_as_req.c +index 87dd7e993..9ae7b0a5e 100644 +--- a/src/kdc/do_as_req.c ++++ b/src/kdc/do_as_req.c +@@ -211,13 +211,6 @@ finish_process_as_req(struct as_req_state *state, krb5_error_code errcode) + + au_state->stage = ENCR_REP; + +- errcode = check_indicators(kdc_context, state->server, +- state->auth_indicators); +- if (errcode) { +- state->status = "HIGHER_AUTHENTICATION_REQUIRED"; +- goto egress; +- } +- + state->ticket_reply.enc_part2 = &state->enc_tkt_reply; + + errcode = check_kdcpolicy_as(kdc_context, state->request, state->client, +@@ -301,6 +294,13 @@ finish_process_as_req(struct as_req_state *state, krb5_error_code errcode) + goto egress; + } + ++ errcode = check_indicators(kdc_context, state->server, ++ state->auth_indicators); ++ if (errcode) { ++ state->status = "HIGHER_AUTHENTICATION_REQUIRED"; ++ goto egress; ++ } ++ + errcode = krb5_encrypt_tkt_part(kdc_context, &state->server_keyblock, + &state->ticket_reply); + if (errcode) +diff --git a/src/plugins/kdb/test/kdb_test.c b/src/plugins/kdb/test/kdb_test.c +index 1936cb0e4..95a6062e2 100644 +--- a/src/plugins/kdb/test/kdb_test.c ++++ b/src/plugins/kdb/test/kdb_test.c +@@ -54,6 +54,8 @@ + * # Initial number is kvno; defaults to 1. + * keys = 3 aes256-cts aes128-cts:normal + * keys = 2 rc4-hmac ++ * strings = key1:value1 ++ * strings = key2:value2 + * } + * } + * delegation = { +@@ -282,6 +284,33 @@ make_keys(char **strings, const char *princstr, const krb5_data *realm, + ent->n_key_data = nkeys; + } + ++static void ++make_strings(char **stringattrs, krb5_db_entry *ent) ++{ ++ struct k5buf buf; ++ char **p; ++ const char *str, *sep; ++ krb5_tl_data *tl; ++ ++ k5_buf_init_dynamic(&buf); ++ for (p = stringattrs; *p != NULL; p++) { ++ str = *p; ++ sep = strchr(str, ':'); ++ assert(sep != NULL); ++ k5_buf_add_len(&buf, str, sep - str); ++ k5_buf_add_len(&buf, "\0", 1); ++ k5_buf_add_len(&buf, sep + 1, strlen(sep + 1) + 1); ++ } ++ assert(buf.data != NULL); ++ ++ tl = ealloc(sizeof(*ent->tl_data)); ++ tl->tl_data_next = NULL; ++ tl->tl_data_type = KRB5_TL_STRING_ATTRS; ++ tl->tl_data_length = buf.len; ++ tl->tl_data_contents = buf.data; ++ ent->tl_data = tl; ++} ++ + static krb5_error_code + test_init() + { +@@ -339,7 +368,8 @@ test_get_principal(krb5_context context, krb5_const_principal search_for, + krb5_principal princ = NULL, tgtprinc; + krb5_principal_data empty_princ = { KV5M_PRINCIPAL }; + testhandle h = context->dal_handle->db_context; +- char *search_name = NULL, *canon = NULL, *flagstr, **names, **key_strings; ++ char *search_name = NULL, *canon = NULL, *flagstr; ++ char **names, **key_strings, **stringattrs; + const char *ename; + krb5_db_entry *ent; + +@@ -415,7 +445,7 @@ test_get_principal(krb5_context context, krb5_const_principal search_for, + ent->pw_expiration = get_time(h, "princs", ename, "pwexpiration"); + + /* Leave last_success, last_failed, fail_auth_count zeroed. */ +- /* Leave tl_data and e_data empty. */ ++ /* Leave e_data empty. */ + + set_names(h, "princs", ename, "keys"); + ret = profile_get_values(h->profile, h->names, &key_strings); +@@ -424,11 +454,19 @@ test_get_principal(krb5_context context, krb5_const_principal search_for, + profile_free_list(key_strings); + } + ++ set_names(h, "princs", ename, "strings"); ++ ret = profile_get_values(h->profile, h->names, &stringattrs); ++ if (ret != PROF_NO_RELATION) { ++ make_strings(stringattrs, ent); ++ profile_free_list(stringattrs); ++ } ++ + /* We must include mod-princ data or kadm5_get_principal() won't work and + * we can't extract keys with kadmin.local. */ + check(krb5_dbe_update_mod_princ_data(context, ent, 0, &empty_princ)); + + *entry = ent; ++ ret = 0; + + cleanup: + krb5_free_unparsed_name(context, search_name); +diff --git a/src/tests/t_authdata.py b/src/tests/t_authdata.py +index 3153ebca3..4fbdbec05 100644 +--- a/src/tests/t_authdata.py ++++ b/src/tests/t_authdata.py +@@ -158,6 +158,8 @@ realm.run(['./adata', realm.host_princ], expected_msg='+97: [indcl]') + mark('auth indicator enforcement') + realm.addprinc('restricted') + realm.run([kadminl, 'setstr', 'restricted', 'require_auth', 'superstrong']) ++realm.kinit(realm.user_princ, password('user'), ['-S', 'restricted'], ++ expected_code=1, expected_msg='KDC policy rejects request') + realm.run([kvno, 'restricted'], expected_code=1, + expected_msg='KDC policy rejects request') + realm.run([kadminl, 'setstr', 'restricted', 'require_auth', 'indcl']) +@@ -194,6 +196,8 @@ testprincs = {'krbtgt/KRBTEST.COM': {'keys': 'aes128-cts'}, + 'krbtgt/FOREIGN': {'keys': 'aes128-cts'}, + 'user': {'keys': 'aes128-cts', 'flags': '+preauth'}, + 'user2': {'keys': 'aes128-cts', 'flags': '+preauth'}, ++ 'rservice': {'keys': 'aes128-cts', ++ 'strings': 'require_auth:strong'}, + 'service/1': {'keys': 'aes128-cts', + 'flags': '+ok_to_auth_as_delegate'}, + 'service/2': {'keys': 'aes128-cts'}, +@@ -208,6 +212,7 @@ usercache = 'FILE:' + os.path.join(realm.testdir, 'usercache') + realm.extract_keytab(realm.krbtgt_princ, realm.keytab) + realm.extract_keytab('krbtgt/FOREIGN', realm.keytab) + realm.extract_keytab(realm.user_princ, realm.keytab) ++realm.extract_keytab('ruser', realm.keytab) + realm.extract_keytab('service/1', realm.keytab) + realm.extract_keytab('service/2', realm.keytab) + realm.extract_keytab('noauthdata', realm.keytab) +@@ -252,6 +257,12 @@ if ' -2: self_ad' not in out or ' -2: proxy_ad' not in out: + realm.kinit(realm.user_princ, None, ['-k', '-X', 'indicators=dummy dbincr1']) + realm.run(['./adata', realm.krbtgt_princ], expected_msg='+97: [dbincr2]') + realm.run(['./adata', 'service/1'], expected_msg='+97: [dbincr3]') ++realm.kinit(realm.user_princ, None, ++ ['-k', '-X', 'indicators=strong', '-S', 'rservice']) ++# Test enforcement of altered indicators during AS request. ++realm.kinit(realm.user_princ, None, ++ ['-k', '-X', 'indicators=strong dbincr1', '-S', 'rservice'], ++ expected_code=1) + + # Test that KDB module authdata is included in an AS request, by + # default or with an explicit PAC request. diff --git a/krb5.spec b/krb5.spec index 43bb589..1eb325d 100644 --- a/krb5.spec +++ b/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: 1%{?dist} +Release: 2%{?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 @@ -50,6 +50,7 @@ Patch4: downstream-fix-debuginfo-with-y.tab.c.patch Patch5: downstream-Remove-3des-support.patch Patch6: downstream-Use-backported-version-of-OpenSSL-3-KDF-i.patch Patch7: downstream-FIPS-with-PRNG-and-RADIUS-and-MD4.patch +Patch8: Fix-AS-REQ-checking-of-KDB-modified-indicators.patch License: MIT URL: https://web.mit.edu/kerberos/www/ @@ -623,6 +624,9 @@ exit 0 %{_libdir}/libkadm5srv_mit.so.* %changelog +* Fri Feb 21 2020 Robbie Harwood - 1.18-2 +- Fix AS-REQ checking of KDB-modified indicators + * Wed Feb 12 2020 Robbie Harwood - 1.18-1 - New upstream version (1.18)