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

@@ -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.

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);
     /*

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:

src/kdc/kdc_util.h

@@ -304,6 +304,11 @@ kdc_get_ticket_endtime(kdc_realm_t *kdc_active_realm,
                        krb5_db_entry *server,
                        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,

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 }
 };
 

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')