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)

5 files changed, 61 insertions, 59 deletions

diff --git a/src/kdc/do_as_req.c b/src/kdc/do_as_req.c
index def7075d7b..51ac4aae39 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 d2b89e25ec..7ddb84a420 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 9948e1bbe3..e61a867d60 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 8fff99c9cd..8e8d102745 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 0650c23f02..894b6d4fd9 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 }
 };