krb5 commit: Clear forwardable flag instead of denying request
Greg Hudson
ghudson at mit.edu
Wed Nov 21 00:10:37 EST 2018
https://github.com/krb5/krb5/commit/08e948cce2c79a3604066fcf7a64fc527456f83d
commit 08e948cce2c79a3604066fcf7a64fc527456f83d
Author: Greg Hudson <ghudson at mit.edu>
Date: Thu Nov 15 13:40:43 2018 -0500
Clear forwardable flag instead of denying request
If the client requests a forwardable or proxiable ticket and the
option cannot be honored by policy, issue a non-forwardable or
non-proxiable ticket rather than denying the request.
Add a test script for testing KDC request options and populate it with
tests for the forwardable and proxiable flags.
ticket: 7871
src/kdc/do_as_req.c | 19 ++------
src/kdc/do_tgs_req.c | 58 +++++---------------------
src/kdc/kdc_util.c | 82 +++++++++++++++++++++----------------
src/kdc/kdc_util.h | 9 ++--
src/kdc/tgs_policy.c | 8 +---
src/tests/Makefile.in | 1 +
src/tests/gcred.c | 28 +++++++++----
src/tests/t_kdcoptions.py | 100 +++++++++++++++++++++++++++++++++++++++++++++
8 files changed, 190 insertions(+), 115 deletions(-)
diff --git a/src/kdc/do_as_req.c b/src/kdc/do_as_req.c
index 588c137..8a96c12 100644
--- a/src/kdc/do_as_req.c
+++ b/src/kdc/do_as_req.c
@@ -192,13 +192,6 @@ finish_process_as_req(struct as_req_state *state, krb5_error_code errcode)
au_state->stage = ENCR_REP;
- if ((errcode = validate_forwardable(state->request, *state->client,
- *state->server, state->kdc_time,
- &state->status))) {
- errcode += ERROR_TABLE_BASE_krb5;
- goto egress;
- }
-
errcode = check_indicators(kdc_context, state->server,
state->auth_indicators);
if (errcode) {
@@ -708,12 +701,11 @@ process_as_req(krb5_kdc_req *request, krb5_data *req_pkt,
}
/* Copy options that request the corresponding ticket flags. */
- state->enc_tkt_reply.flags = OPTS2FLAGS(state->request->kdc_options);
+ state->enc_tkt_reply.flags = get_ticket_flags(state->request->kdc_options,
+ state->client, state->server,
+ NULL);
state->enc_tkt_reply.times.authtime = state->authtime;
- setflag(state->enc_tkt_reply.flags, TKT_FLG_INITIAL);
- setflag(state->enc_tkt_reply.flags, TKT_FLG_ENC_PA_REP);
-
/*
* It should be noted that local policy may affect the
* processing of any of these flags. For example, some
@@ -732,10 +724,9 @@ process_as_req(krb5_kdc_req *request, krb5_data *req_pkt,
state->enc_tkt_reply.transited.tr_type = KRB5_DOMAIN_X500_COMPRESS;
state->enc_tkt_reply.transited.tr_contents = empty_string;
- if (isflagset(state->request->kdc_options, KDC_OPT_POSTDATED)) {
- setflag(state->enc_tkt_reply.flags, TKT_FLG_INVALID);
+ if (isflagset(state->request->kdc_options, KDC_OPT_POSTDATED))
state->enc_tkt_reply.times.starttime = state->request->from;
- } else
+ else
state->enc_tkt_reply.times.starttime = state->kdc_time;
kdc_get_ticket_endtime(kdc_active_realm,
diff --git a/src/kdc/do_tgs_req.c b/src/kdc/do_tgs_req.c
index 587342a..1da0993 100644
--- a/src/kdc/do_tgs_req.c
+++ b/src/kdc/do_tgs_req.c
@@ -378,15 +378,16 @@ process_tgs_req(krb5_kdc_req *request, krb5_data *pkt,
else
ticket_reply.server = request->server; /* XXX careful for realm... */
- enc_tkt_reply.flags = OPTS2FLAGS(request->kdc_options);
- enc_tkt_reply.flags |= COPY_TKT_FLAGS(header_enc_tkt->flags);
+ enc_tkt_reply.flags = get_ticket_flags(request->kdc_options, client,
+ server, header_enc_tkt);
enc_tkt_reply.times.starttime = 0;
- if (isflagset(server->attributes, KRB5_KDB_OK_AS_DELEGATE))
- setflag(enc_tkt_reply.flags, TKT_FLG_OK_AS_DELEGATE);
-
- /* Indicate support for encrypted padata (RFC 6806). */
- setflag(enc_tkt_reply.flags, TKT_FLG_ENC_PA_REP);
+ /* OK_TO_AUTH_AS_DELEGATE must be set on the service requesting S4U2Self
+ * for forwardable tickets to be issued. */
+ if (isflagset(c_flags, KRB5_KDB_FLAG_PROTOCOL_TRANSITION) &&
+ !is_referral &&
+ !isflagset(server->attributes, KRB5_KDB_OK_TO_AUTH_AS_DELEGATE))
+ clear(enc_tkt_reply.flags, TKT_FLG_FORWARDABLE);
/* don't use new addresses unless forwarded, see below */
@@ -401,37 +402,6 @@ process_tgs_req(krb5_kdc_req *request, krb5_data *pkt,
* realms may refuse to issue renewable tickets
*/
- if (isflagset(request->kdc_options, KDC_OPT_FORWARDABLE)) {
-
- if (isflagset(c_flags, KRB5_KDB_FLAG_PROTOCOL_TRANSITION)) {
- /*
- * If S4U2Self principal is not forwardable, then mark ticket as
- * unforwardable. This behaviour matches Windows, but it is
- * different to the MIT AS-REQ path, which returns an error
- * (KDC_ERR_POLICY) if forwardable tickets cannot be issued.
- *
- * Consider this block the S4U2Self equivalent to
- * validate_forwardable().
- */
- if (client != NULL &&
- isflagset(client->attributes, KRB5_KDB_DISALLOW_FORWARDABLE))
- clear(enc_tkt_reply.flags, TKT_FLG_FORWARDABLE);
- /*
- * Forwardable flag is propagated along referral path.
- */
- else if (!isflagset(header_enc_tkt->flags, TKT_FLG_FORWARDABLE))
- clear(enc_tkt_reply.flags, TKT_FLG_FORWARDABLE);
- /*
- * OK_TO_AUTH_AS_DELEGATE must be set on the service requesting
- * S4U2Self in order for forwardable tickets to be returned.
- */
- else if (!is_referral &&
- !isflagset(server->attributes,
- KRB5_KDB_OK_TO_AUTH_AS_DELEGATE))
- clear(enc_tkt_reply.flags, TKT_FLG_FORWARDABLE);
- }
- }
-
if (isflagset(request->kdc_options, KDC_OPT_FORWARDED) ||
isflagset(request->kdc_options, KDC_OPT_PROXY)) {
@@ -440,16 +410,10 @@ process_tgs_req(krb5_kdc_req *request, krb5_data *pkt,
enc_tkt_reply.caddrs = request->addresses;
reply_encpart.caddrs = request->addresses;
}
- /* We don't currently handle issuing anonymous tickets based on
- * non-anonymous ones, so just ignore the option. */
- if (isflagset(request->kdc_options, KDC_OPT_REQUEST_ANONYMOUS) &&
- !isflagset(header_enc_tkt->flags, TKT_FLG_ANONYMOUS))
- clear(enc_tkt_reply.flags, TKT_FLG_ANONYMOUS);
-
- if (isflagset(request->kdc_options, KDC_OPT_POSTDATED)) {
- setflag(enc_tkt_reply.flags, TKT_FLG_INVALID);
+
+ if (isflagset(request->kdc_options, KDC_OPT_POSTDATED))
enc_tkt_reply.times.starttime = request->from;
- } else
+ else
enc_tkt_reply.times.starttime = kdc_time;
if (isflagset(request->kdc_options, KDC_OPT_VALIDATE)) {
diff --git a/src/kdc/kdc_util.c b/src/kdc/kdc_util.c
index dfeaf7e..6d53173 100644
--- a/src/kdc/kdc_util.c
+++ b/src/kdc/kdc_util.c
@@ -697,29 +697,6 @@ validate_as_request(kdc_realm_t *kdc_active_realm,
return(KDC_ERR_CANNOT_POSTDATE);
}
- /*
- * A Windows KDC will return KDC_ERR_PREAUTH_REQUIRED instead of
- * KDC_ERR_POLICY in the following case:
- *
- * - KDC_OPT_FORWARDABLE is set in KDCOptions but local
- * policy has KRB5_KDB_DISALLOW_FORWARDABLE set for the
- * client, and;
- * - KRB5_KDB_REQUIRES_PRE_AUTH is set for the client but
- * preauthentication data is absent in the request.
- *
- * Hence, this check most be done after the check for preauth
- * data, and is now performed by validate_forwardable() (the
- * contents of which were previously below).
- */
-
- /* Client and server must allow proxiable tickets */
- if (isflagset(request->kdc_options, KDC_OPT_PROXIABLE) &&
- (isflagset(client.attributes, KRB5_KDB_DISALLOW_PROXIABLE) ||
- isflagset(server.attributes, KRB5_KDB_DISALLOW_PROXIABLE))) {
- *status = "PROXIABLE NOT ALLOWED";
- return(KDC_ERR_POLICY);
- }
-
/* Check to see if client is locked out */
if (isflagset(client.attributes, KRB5_KDB_DISALLOW_ALL_TIX)) {
*status = "CLIENT LOCKED OUT";
@@ -752,19 +729,54 @@ validate_as_request(kdc_realm_t *kdc_active_realm,
return 0;
}
-int
-validate_forwardable(krb5_kdc_req *request, krb5_db_entry client,
- krb5_db_entry server, krb5_timestamp kdc_time,
- const char **status)
+/*
+ * Compute ticket flags based on the request, the client and server DB entry
+ * (which may prohibit forwardable or proxiable tickets), and the header
+ * ticket. client may be NULL for a TGS request (although it may be set, such
+ * as for an S4U2Self request). header_enc may be NULL for an AS request.
+ */
+krb5_flags
+get_ticket_flags(krb5_flags reqflags, krb5_db_entry *client,
+ krb5_db_entry *server, krb5_enc_tkt_part *header_enc)
{
- *status = NULL;
- if (isflagset(request->kdc_options, KDC_OPT_FORWARDABLE) &&
- (isflagset(client.attributes, KRB5_KDB_DISALLOW_FORWARDABLE) ||
- isflagset(server.attributes, KRB5_KDB_DISALLOW_FORWARDABLE))) {
- *status = "FORWARDABLE NOT ALLOWED";
- return(KDC_ERR_POLICY);
- } else
- return 0;
+ krb5_flags flags;
+
+ /* Indicate support for encrypted padata (RFC 6806), and set flags based on
+ * request options and the header ticket. */
+ flags = OPTS2FLAGS(reqflags) | TKT_FLG_ENC_PA_REP;
+ if (reqflags & KDC_OPT_POSTDATED)
+ flags |= TKT_FLG_INVALID;
+ if (header_enc != NULL)
+ flags |= COPY_TKT_FLAGS(header_enc->flags);
+ if (header_enc == NULL)
+ flags |= TKT_FLG_INITIAL;
+
+ /* For TGS requests, indicate if the service is marked ok-as-delegate. */
+ if (header_enc != NULL && (server->attributes & KRB5_KDB_OK_AS_DELEGATE))
+ flags |= TKT_FLG_OK_AS_DELEGATE;
+
+ /* Unset PROXIABLE if it is disallowed. */
+ if (client != NULL && (client->attributes & KRB5_KDB_DISALLOW_PROXIABLE))
+ flags &= ~TKT_FLG_PROXIABLE;
+ if (server->attributes & KRB5_KDB_DISALLOW_PROXIABLE)
+ flags &= ~TKT_FLG_PROXIABLE;
+ if (header_enc != NULL && !(header_enc->flags & TKT_FLG_PROXIABLE))
+ flags &= ~TKT_FLG_PROXIABLE;
+
+ /* Unset FORWARDABLE if it is disallowed. */
+ if (client != NULL && (client->attributes & KRB5_KDB_DISALLOW_FORWARDABLE))
+ flags &= ~TKT_FLG_FORWARDABLE;
+ if (server->attributes & KRB5_KDB_DISALLOW_FORWARDABLE)
+ flags &= ~TKT_FLG_FORWARDABLE;
+ if (header_enc != NULL && !(header_enc->flags & TKT_FLG_FORWARDABLE))
+ flags &= ~TKT_FLG_FORWARDABLE;
+
+ /* We don't currently handle issuing anonymous tickets based on
+ * non-anonymous ones. */
+ if (header_enc != NULL && !(header_enc->flags & TKT_FLG_ANONYMOUS))
+ flags &= ~TKT_FLG_ANONYMOUS;
+
+ return flags;
}
/* Return KRB5KDC_ERR_POLICY if indicators does not contain the required auth
diff --git a/src/kdc/kdc_util.h b/src/kdc/kdc_util.h
index 6ec645f..b89a3de 100644
--- a/src/kdc/kdc_util.h
+++ b/src/kdc/kdc_util.h
@@ -86,15 +86,14 @@ validate_as_request (kdc_realm_t *, krb5_kdc_req *, krb5_db_entry,
const char **, krb5_pa_data ***);
int
-validate_forwardable(krb5_kdc_req *, krb5_db_entry,
- krb5_db_entry, krb5_timestamp,
- const char **);
-
-int
validate_tgs_request (kdc_realm_t *, krb5_kdc_req *, krb5_db_entry,
krb5_ticket *, krb5_timestamp,
const char **, krb5_pa_data ***);
+krb5_flags
+get_ticket_flags(krb5_flags reqflags, krb5_db_entry *client,
+ krb5_db_entry *server, krb5_enc_tkt_part *header_enc);
+
krb5_error_code
check_indicators(krb5_context context, krb5_db_entry *server,
krb5_data *const *indicators);
diff --git a/src/kdc/tgs_policy.c b/src/kdc/tgs_policy.c
index 907fcd3..554345b 100644
--- a/src/kdc/tgs_policy.c
+++ b/src/kdc/tgs_policy.c
@@ -63,9 +63,9 @@ static check_tgs_svc_pol_fn * const svc_pol_fns[] = {
};
static const struct tgsflagrule tgsflagrules[] = {
- { (KDC_OPT_FORWARDED | KDC_OPT_FORWARDABLE), TKT_FLG_FORWARDABLE,
+ { KDC_OPT_FORWARDED, TKT_FLG_FORWARDABLE,
"TGT NOT FORWARDABLE", KDC_ERR_BADOPTION },
- { (KDC_OPT_PROXY | KDC_OPT_PROXIABLE), TKT_FLG_PROXIABLE,
+ { KDC_OPT_PROXY, TKT_FLG_PROXIABLE,
"TGT NOT PROXIABLE", KDC_ERR_BADOPTION },
{ (KDC_OPT_ALLOW_POSTDATE | KDC_OPT_POSTDATED), TKT_FLG_MAY_POSTDATE,
"TGT NOT POSTDATABLE", KDC_ERR_BADOPTION },
@@ -98,12 +98,8 @@ check_tgs_opts(krb5_kdc_req *req, krb5_ticket *tkt, const char **status)
}
static const struct tgsflagrule svcdenyrules[] = {
- { KDC_OPT_FORWARDABLE, KRB5_KDB_DISALLOW_FORWARDABLE,
- "NON-FORWARDABLE TICKET", KDC_ERR_POLICY },
{ KDC_OPT_RENEWABLE, KRB5_KDB_DISALLOW_RENEWABLE,
"NON-RENEWABLE TICKET", KDC_ERR_POLICY },
- { KDC_OPT_PROXIABLE, KRB5_KDB_DISALLOW_PROXIABLE,
- "NON-PROXIABLE TICKET", KDC_ERR_POLICY },
{ KDC_OPT_ALLOW_POSTDATE, KRB5_KDB_DISALLOW_POSTDATED,
"NON-POSTDATABLE TICKET", KDC_ERR_CANNOT_POSTDATE },
{ KDC_OPT_ENC_TKT_IN_SKEY, KRB5_KDB_DISALLOW_DUP_SKEY,
diff --git a/src/tests/Makefile.in b/src/tests/Makefile.in
index e27617e..0eec2b6 100644
--- a/src/tests/Makefile.in
+++ b/src/tests/Makefile.in
@@ -177,6 +177,7 @@ check-pytests: unlockiter
$(RUNPYTEST) $(srcdir)/t_y2038.py $(PYTESTFLAGS)
$(RUNPYTEST) $(srcdir)/t_kdcpolicy.py $(PYTESTFLAGS)
$(RUNPYTEST) $(srcdir)/t_u2u.py $(PYTESTFLAGS)
+ $(RUNPYTEST) $(srcdir)/t_kdcoptions.py $(PYTESTFLAGS)
clean:
$(RM) adata etinfo forward gcred hist hooks hrealm icinterleave icred
diff --git a/src/tests/gcred.c b/src/tests/gcred.c
index cb0ae6a..b14e4fc 100644
--- a/src/tests/gcred.c
+++ b/src/tests/gcred.c
@@ -66,20 +66,32 @@ main(int argc, char **argv)
krb5_principal client, server;
krb5_ccache ccache;
krb5_creds in_creds, *creds;
+ krb5_flags options = 0;
char *name;
+ int c;
check(krb5_init_context(&ctx));
- /* Parse arguments. */
- assert(argc == 3);
- check(krb5_parse_name(ctx, argv[2], &server));
- if (strcmp(argv[1], "unknown") == 0)
+ while ((c = getopt(argc, argv, "f")) != -1) {
+ switch (c) {
+ case 'f':
+ options |= KRB5_GC_FORWARDABLE;
+ break;
+ default:
+ abort();
+ }
+ }
+ argc -= optind;
+ argv += optind;
+ assert(argc == 2);
+ check(krb5_parse_name(ctx, argv[1], &server));
+ if (strcmp(argv[0], "unknown") == 0)
server->type = KRB5_NT_UNKNOWN;
- else if (strcmp(argv[1], "principal") == 0)
+ else if (strcmp(argv[0], "principal") == 0)
server->type = KRB5_NT_PRINCIPAL;
- else if (strcmp(argv[1], "srv-inst") == 0)
+ else if (strcmp(argv[0], "srv-inst") == 0)
server->type = KRB5_NT_SRV_INST;
- else if (strcmp(argv[1], "srv-hst") == 0)
+ else if (strcmp(argv[0], "srv-hst") == 0)
server->type = KRB5_NT_SRV_HST;
else
abort();
@@ -89,7 +101,7 @@ main(int argc, char **argv)
memset(&in_creds, 0, sizeof(in_creds));
in_creds.client = client;
in_creds.server = server;
- check(krb5_get_credentials(ctx, 0, ccache, &in_creds, &creds));
+ check(krb5_get_credentials(ctx, options, ccache, &in_creds, &creds));
check(krb5_unparse_name(ctx, creds->server, &name));
printf("%s\n", name);
diff --git a/src/tests/t_kdcoptions.py b/src/tests/t_kdcoptions.py
new file mode 100644
index 0000000..7ec5750
--- /dev/null
+++ b/src/tests/t_kdcoptions.py
@@ -0,0 +1,100 @@
+from k5test import *
+import re
+
+# KDC option test coverage notes:
+#
+# FORWARDABLE here
+# FORWARDED no test
+# PROXIABLE here
+# PROXY no test
+# ALLOW_POSTDATE no test
+# POSTDATED no test
+# RENEWABLE t_renew.py
+# CNAME_IN_ADDL_TKT gssapi/t_s4u.py
+# CANONICALIZE t_kdb.py and various other tests
+# REQUEST_ANONYMOUS t_pkinit.py
+# DISABLE_TRANSITED_CHECK no test
+# RENEWABLE_OK t_renew.py
+# ENC_TKT_IN_SKEY t_u2u.py
+# RENEW t_renew.py
+# VALIDATE no test
+
+# Run klist -f and return the flags on the ticket for svcprinc.
+def get_flags(realm, svcprinc):
+ grab_flags = False
+ for line in realm.run([klist, '-f']).splitlines():
+ if grab_flags:
+ return re.findall(r'Flags: ([a-zA-Z]*)', line)[0]
+ grab_flags = line.endswith(svcprinc)
+
+
+# Get the flags on the ticket for svcprinc, and check for an expected
+# element and an expected-absent element, either of which can be None.
+def check_flags(realm, svcprinc, expected_flag, expected_noflag):
+ flags = get_flags(realm, svcprinc)
+ if expected_flag is not None and not expected_flag in flags:
+ fail('expected flag ' + expected_flag)
+ if expected_noflag is not None and expected_noflag in flags:
+ fail('did not expect flag ' + expected_noflag)
+
+
+# Run kinit with the given flags, and check the flags on the resulting
+# TGT.
+def kinit_check_flags(realm, flags, expected_flag, expected_noflag):
+ realm.kinit(realm.user_princ, password('user'), flags)
+ check_flags(realm, realm.krbtgt_princ, expected_flag, expected_noflag)
+
+
+# Run kinit with kflags. Then get credentials for the host principal
+# with gflags, and check the flags on the resulting ticket.
+def gcred_check_flags(realm, kflags, gflags, expected_flag, expected_noflag):
+ realm.kinit(realm.user_princ, password('user'), kflags)
+ realm.run(['./gcred'] + gflags + ['unknown', realm.host_princ])
+ check_flags(realm, realm.host_princ, expected_flag, expected_noflag)
+
+
+realm = K5Realm()
+
+mark('proxiable (AS)')
+kinit_check_flags(realm, [], None, 'P')
+kinit_check_flags(realm, ['-p'], 'P', None)
+realm.run([kadminl, 'modprinc', '-allow_proxiable', realm.user_princ])
+kinit_check_flags(realm, ['-p'], None, 'P')
+realm.run([kadminl, 'modprinc', '+allow_proxiable', realm.user_princ])
+realm.run([kadminl, 'modprinc', '-allow_proxiable', realm.krbtgt_princ])
+kinit_check_flags(realm, ['-p'], None, 'P')
+realm.run([kadminl, 'modprinc', '+allow_proxiable', realm.krbtgt_princ])
+
+mark('proxiable (TGS)')
+gcred_check_flags(realm, [], [], None, 'P')
+gcred_check_flags(realm, ['-p'], [], 'P', None)
+
+# Not tested: PROXIABLE option set with a non-proxiable TGT (because
+# there is no krb5_get_credentials() flag to request this; would
+# expect a non-proxiable ticket).
+
+# Not tested: proxiable TGT but PROXIABLE flag not set (because we
+# internally set the PROXIABLE option when using a proxiable TGT;
+# would expect a non-proxiable ticket).
+
+mark('forwardable (AS)')
+kinit_check_flags(realm, [], None, 'F')
+kinit_check_flags(realm, ['-f'], 'F', None)
+realm.run([kadminl, 'modprinc', '-allow_forwardable', realm.user_princ])
+kinit_check_flags(realm, ['-f'], None, 'F')
+realm.run([kadminl, 'modprinc', '+allow_forwardable', realm.user_princ])
+realm.run([kadminl, 'modprinc', '-allow_forwardable', realm.krbtgt_princ])
+kinit_check_flags(realm, ['-f'], None, 'F')
+realm.run([kadminl, 'modprinc', '+allow_forwardable', realm.krbtgt_princ])
+
+mark('forwardable (TGS)')
+realm.kinit(realm.user_princ, password('user'))
+gcred_check_flags(realm, [], [], None, 'F')
+gcred_check_flags(realm, [], ['-f'], None, 'F')
+gcred_check_flags(realm, ['-f'], [], 'F', None)
+
+# Not tested: forwardable TGT but FORWARDABLE flag not set (because we
+# internally set the FORWARDABLE option when using a forwardable TGT;
+# would expect a non-proxiable ticket).
+
+success('KDC option tests')
More information about the cvs-krb5
mailing list