krb5 commit: Refactor KDC renewable ticket handling
Greg Hudson
ghudson at MIT.EDU
Fri Jun 7 00:46:59 EDT 2013
https://github.com/krb5/krb5/commit/4f551a7ec126c52ee1f8fea4c3954015b70987bd
commit 4f551a7ec126c52ee1f8fea4c3954015b70987bd
Author: Greg Hudson <ghudson at mit.edu>
Date: Thu Jun 6 14:44:30 2013 -0400
Refactor KDC renewable ticket handling
Create a new helper to compute the renewable lifetime for AS and TGS
requests. This has some minor behavior differences:
* We only issue a renewable ticket if the renewable lifetime is greater
than the normal ticket lifetime.
* We give RENEWABLE precedence over RENEWABLE-OK in determining the
requested renewable lifetime, instead of sometimes doing the
reverse.
* We use the client's maximum renewable life for TGS requests if we
have looked up its DB entry.
* Instead of rejecting requests for renewable tickets (if the client
or server principal doesn't allow it, or a TGS request's TGT isn't
renewable), issue non-renewable tickets.
ticket: 7661 (new)
src/kdc/do_as_req.c | 29 ++-------------------
src/kdc/do_tgs_req.c | 28 +++-----------------
src/kdc/kdc_util.c | 56 +++++++++++++++++++++++++++++++++++------
src/kdc/kdc_util.h | 5 +++
src/kdc/tgs_policy.c | 2 +-
src/tests/t_renew.py | 68 ++++++++++++++++++++++++++++++++++++++++++++++---
6 files changed, 124 insertions(+), 64 deletions(-)
diff --git a/src/kdc/do_as_req.c b/src/kdc/do_as_req.c
index def7075..51ac4aa 100644
--- a/src/kdc/do_as_req.c
+++ b/src/kdc/do_as_req.c
@@ -450,7 +450,6 @@ process_as_req(krb5_kdc_req *request, krb5_data *req_pkt,
verto_ctx *vctx, loop_respond_fn respond, void *arg)
{
krb5_error_code errcode;
- krb5_timestamp rtime;
unsigned int s_flags = 0;
krb5_data encoded_req_body;
krb5_enctype useenctype;
@@ -684,31 +683,9 @@ process_as_req(krb5_kdc_req *request, krb5_data *req_pkt,
kdc_infinity, state->request->till, state->client,
state->server, &state->enc_tkt_reply.times.endtime);
- if (isflagset(state->request->kdc_options, KDC_OPT_RENEWABLE_OK) &&
- !isflagset(state->client->attributes, KRB5_KDB_DISALLOW_RENEWABLE) &&
- (state->enc_tkt_reply.times.endtime < state->request->till)) {
-
- /* we set the RENEWABLE option for later processing */
-
- setflag(state->request->kdc_options, KDC_OPT_RENEWABLE);
- state->request->rtime = state->request->till;
- }
- rtime = (state->request->rtime == 0) ? kdc_infinity :
- state->request->rtime;
-
- if (isflagset(state->request->kdc_options, KDC_OPT_RENEWABLE)) {
- /*
- * XXX Should we squelch the output renew_till to be no
- * earlier than the endtime of the ticket?
- */
- setflag(state->enc_tkt_reply.flags, TKT_FLG_RENEWABLE);
- state->enc_tkt_reply.times.renew_till =
- min(rtime, state->enc_tkt_reply.times.starttime +
- min(state->client->max_renewable_life,
- min(state->server->max_renewable_life,
- kdc_active_realm->realm_maxrlife)));
- } else
- state->enc_tkt_reply.times.renew_till = 0; /* XXX */
+ kdc_get_ticket_renewtime(kdc_active_realm, state->request, NULL,
+ state->client, state->server,
+ &state->enc_tkt_reply);
/*
* starttime is optional, and treated as authtime if not present.
diff --git a/src/kdc/do_tgs_req.c b/src/kdc/do_tgs_req.c
index d2b89e2..7ddb84a 100644
--- a/src/kdc/do_tgs_req.c
+++ b/src/kdc/do_tgs_req.c
@@ -116,7 +116,6 @@ process_tgs_req(struct server_handle *handle, krb5_data *pkt,
krb5_keyblock encrypting_key;
krb5_timestamp kdc_time, authtime = 0;
krb5_keyblock session_key;
- krb5_timestamp rtime;
krb5_keyblock *reply_key = NULL;
krb5_key_data *server_key;
krb5_principal cprinc = NULL, sprinc = NULL, altcprinc = NULL;
@@ -442,30 +441,11 @@ process_tgs_req(struct server_handle *handle, krb5_data *pkt,
kdc_get_ticket_endtime(kdc_active_realm, enc_tkt_reply.times.starttime,
header_enc_tkt->times.endtime, request->till,
client, server, &enc_tkt_reply.times.endtime);
-
- if (isflagset(request->kdc_options, KDC_OPT_RENEWABLE_OK) &&
- (enc_tkt_reply.times.endtime < request->till) &&
- isflagset(header_enc_tkt->flags, TKT_FLG_RENEWABLE)) {
- setflag(request->kdc_options, KDC_OPT_RENEWABLE);
- request->rtime =
- min(request->till, header_enc_tkt->times.renew_till);
- }
- }
- rtime = (request->rtime == 0) ? kdc_infinity : request->rtime;
-
- if (isflagset(request->kdc_options, KDC_OPT_RENEWABLE)) {
- /* already checked above in policy check to reject request for a
- renewable ticket using a non-renewable ticket */
- setflag(enc_tkt_reply.flags, TKT_FLG_RENEWABLE);
- enc_tkt_reply.times.renew_till =
- min(rtime,
- min(header_enc_tkt->times.renew_till,
- enc_tkt_reply.times.starttime +
- min(server->max_renewable_life,
- kdc_active_realm->realm_maxrlife)));
- } else {
- enc_tkt_reply.times.renew_till = 0;
}
+
+ kdc_get_ticket_renewtime(kdc_active_realm, request, header_enc_tkt, client,
+ server, &enc_tkt_reply);
+
if (isflagset(header_enc_tkt->flags, TKT_FLG_ANONYMOUS))
setflag(enc_tkt_reply.flags, TKT_FLG_ANONYMOUS);
/*
diff --git a/src/kdc/kdc_util.c b/src/kdc/kdc_util.c
index 9948e1b..e61a867 100644
--- a/src/kdc/kdc_util.c
+++ b/src/kdc/kdc_util.c
@@ -662,14 +662,6 @@ validate_as_request(kdc_realm_t *kdc_active_realm,
* contents of which were previously below).
*/
- /* Client and server must allow renewable tickets */
- if (isflagset(request->kdc_options, KDC_OPT_RENEWABLE) &&
- (isflagset(client.attributes, KRB5_KDB_DISALLOW_RENEWABLE) ||
- isflagset(server.attributes, KRB5_KDB_DISALLOW_RENEWABLE))) {
- *status = "RENEWABLE NOT ALLOWED";
- return(KDC_ERR_POLICY);
- }
-
/* Client and server must allow proxiable tickets */
if (isflagset(request->kdc_options, KDC_OPT_PROXIABLE) &&
(isflagset(client.attributes, KRB5_KDB_DISALLOW_PROXIABLE) ||
@@ -1898,6 +1890,54 @@ kdc_get_ticket_endtime(kdc_realm_t *kdc_active_realm,
*out_endtime = starttime + life;
}
+/*
+ * Set tkt->renew_till to the requested renewable lifetime as modified by
+ * policy. Set the TKT_FLG_RENEWABLE flag if we set a nonzero renew_till.
+ * client and tgt may be NULL.
+ */
+void
+kdc_get_ticket_renewtime(kdc_realm_t *realm, krb5_kdc_req *request,
+ krb5_enc_tkt_part *tgt, krb5_db_entry *client,
+ krb5_db_entry *server, krb5_enc_tkt_part *tkt)
+{
+ krb5_timestamp rtime, max_rlife;
+
+ tkt->times.renew_till = 0;
+
+ /* Don't issue renewable tickets if the client or server don't allow it,
+ * or if this is a TGS request and the TGT isn't renewable. */
+ if (server->attributes & KRB5_KDB_DISALLOW_RENEWABLE)
+ return;
+ if (client != NULL && (client->attributes & KRB5_KDB_DISALLOW_RENEWABLE))
+ return;
+ if (tgt != NULL && !(tgt->flags & TKT_FLG_RENEWABLE))
+ return;
+
+ /* Determine the requested renewable time. */
+ if (isflagset(request->kdc_options, KDC_OPT_RENEWABLE))
+ rtime = request->rtime ? request->rtime : kdc_infinity;
+ else if (isflagset(request->kdc_options, KDC_OPT_RENEWABLE_OK) &&
+ tkt->times.endtime < request->till)
+ rtime = request->till;
+ else
+ return;
+
+ /* Truncate it to the allowable renewable time. */
+ if (tgt != NULL)
+ rtime = min(rtime, tgt->times.renew_till);
+ max_rlife = min(server->max_renewable_life, realm->realm_maxrlife);
+ if (client != NULL)
+ max_rlife = min(max_rlife, client->max_renewable_life);
+ rtime = min(rtime, tkt->times.starttime + max_rlife);
+
+ /* Make the ticket renewable if the truncated requested time is larger than
+ * the ticket end time. */
+ if (rtime > tkt->times.endtime) {
+ setflag(tkt->flags, TKT_FLG_RENEWABLE);
+ tkt->times.renew_till = rtime;
+ }
+}
+
/**
* Handle protected negotiation of FAST using enc_padata
* - If ENCPADATA_REQ_ENC_PA_REP is present, then:
diff --git a/src/kdc/kdc_util.h b/src/kdc/kdc_util.h
index 8fff99c..8e8d102 100644
--- a/src/kdc/kdc_util.h
+++ b/src/kdc/kdc_util.h
@@ -305,6 +305,11 @@ kdc_get_ticket_endtime(kdc_realm_t *kdc_active_realm,
krb5_timestamp *out_endtime);
void
+kdc_get_ticket_renewtime(kdc_realm_t *realm, krb5_kdc_req *request,
+ krb5_enc_tkt_part *tgt, krb5_db_entry *client,
+ krb5_db_entry *server, krb5_enc_tkt_part *tkt);
+
+void
log_as_req(krb5_context context, const krb5_fulladdr *from,
krb5_kdc_req *request, krb5_kdc_rep *reply,
krb5_db_entry *client, const char *cname,
diff --git a/src/kdc/tgs_policy.c b/src/kdc/tgs_policy.c
index 0650c23..894b6d4 100644
--- a/src/kdc/tgs_policy.c
+++ b/src/kdc/tgs_policy.c
@@ -71,7 +71,7 @@ static const struct tgsflagrule tgsflagrules[] = {
"TGT NOT POSTDATABLE", KDC_ERR_BADOPTION },
{ KDC_OPT_VALIDATE, TKT_FLG_INVALID,
"VALIDATE VALID TICKET", KDC_ERR_BADOPTION },
- { (KDC_OPT_RENEW | KDC_OPT_RENEWABLE), TKT_FLG_RENEWABLE,
+ { KDC_OPT_RENEW, TKT_FLG_RENEWABLE,
"TICKET NOT RENEWABLE", KDC_ERR_BADOPTION }
};
diff --git a/src/tests/t_renew.py b/src/tests/t_renew.py
index ce36a5b..a5fd012 100644
--- a/src/tests/t_renew.py
+++ b/src/tests/t_renew.py
@@ -1,16 +1,74 @@
#!/usr/bin/python
from k5test import *
-realm = K5Realm(create_host=False, get_creds=False)
+conf = {'realms': {'$realm': {'max_life': '20h', 'max_renewable_life': '20h'}}}
+realm = K5Realm(create_host=False, get_creds=False, kdc_conf=conf)
-# Configure the realm to allow renewable tickets and acquire some.
-realm.run_kadminl('modprinc -maxrenewlife "2 days" user')
-realm.run_kadminl('modprinc -maxrenewlife "2 days" %s' % realm.krbtgt_princ)
-realm.kinit(realm.user_princ, password('user'), flags=['-r', '2d'])
+def test(testname, life, rlife, expect_renewable, env=None):
+ global realm
+ flags = ['-l', life]
+ if rlife is not None:
+ flags += ['-r', rlife]
+ realm.kinit(realm.user_princ, password('user'), flags=flags, env=env)
+ out = realm.run([klist])
+ if ('Default principal: %s\n' % realm.user_princ) not in out:
+ fail('%s: did not get tickets' % testname)
+ renewable = 'renew until' in out
+ if renewable and not expect_renewable:
+ fail('%s: tickets unexpectedly renewable' % testname)
+ elif not renewable and expect_renewable:
+ fail('%s: tickets unexpectedly non-renewable' % testname)
+
+# Get renewable tickets.
+test('simple', '1h', '2h', True)
# Renew twice, to test that renewed tickets are renewable.
realm.kinit(realm.user_princ, flags=['-R'])
realm.kinit(realm.user_princ, flags=['-R'])
realm.klist(realm.user_princ)
+# Make sure we can't renew non-renewable tickets.
+test('non-renewable', '1h', '1h', False)
+out = realm.kinit(realm.user_princ, flags=['-R'], expected_code=1)
+if "KDC can't fulfill requested option" not in out:
+ fail('expected error not seen renewing non-renewable ticket')
+
+# Test that -allow_reneable on the client principal works.
+realm.run_kadminl('modprinc -allow_renewable user')
+test('disallowed client', '1h', '2h', False)
+realm.run_kadminl('modprinc +allow_renewable user')
+
+# Test that -allow_reneable on the server principal works.
+realm.run_kadminl('modprinc -allow_renewable %s' % realm.krbtgt_princ)
+test('disallowed server', '1h', '2h', False)
+realm.run_kadminl('modprinc +allow_renewable %s' % realm.krbtgt_princ)
+
+# Test that non-renewable tickets are issued if renew_till < till.
+test('short', '2h', '1h', False)
+
+# Test that renewable tickets are issued if till > max life by
+# default, but not if we configure away the RENEWABLE-OK option.
+no_opts_conf = {'libdefaults': {'kdc_default_options': '0'}}
+no_opts = realm.special_env('no_opts', False, krb5_conf=no_opts_conf)
+realm.run_kadminl('modprinc -maxlife "10 hours" user')
+test('long', '15h', None, True)
+test('long noopts', '15h', None, False, env=no_opts)
+realm.run_kadminl('modprinc -maxlife "20 hours" user')
+
+# Test maximum renewable life on the client principal.
+realm.run_kadminl('modprinc -maxrenewlife "5 hours" user')
+test('maxrenewlife client yes', '4h', '5h', True)
+test('maxrenewlife client no', '5h', '10h', False)
+
+# Test maximum renewable life on the server principal.
+realm.run_kadminl('modprinc -maxrenewlife "4 hours" %s' % realm.krbtgt_princ)
+test('maxrenewlife server yes', '3h', '4h', True)
+test('maxrenewlife server no', '4h', '8h', False)
+
+# Test realm maximum life.
+realm.run_kadminl('modprinc -maxrenewlife "40 hours" user')
+realm.run_kadminl('modprinc -maxrenewlife "40 hours" %s' % realm.krbtgt_princ)
+test('maxrenewlife realm yes', '10h', '20h', True)
+test('maxrenewlife realm no', '20h', '40h', False)
+
success('Renewing credentials')
More information about the cvs-krb5
mailing list