Fix AS-REQ checking of KDB-modified indicators

This commit is contained in:
Robbie Harwood 2020-02-21 13:16:49 -05:00
parent 48a220a102
commit 3b6955d99e
2 changed files with 194 additions and 1 deletions

View File

@ -0,0 +1,189 @@
From 744154b19c8000965e5a5de51d5dbef0794958be Mon Sep 17 00:00:00 2001
From: Greg Hudson <ghudson@mit.edu>
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.

View File

@ -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 <rharwood@redhat.com> - 1.18-2
- Fix AS-REQ checking of KDB-modified indicators
* Wed Feb 12 2020 Robbie Harwood <rharwood@redhat.com> - 1.18-1
- New upstream version (1.18)