krb5 commit: Issue trivially renewable tickets

https://github.com/krb5/krb5/commit/45c19b19ea4d47ac5969a9cbdb308201b16615d8
commit 45c19b19ea4d47ac5969a9cbdb308201b16615d8
Author: Greg Hudson <[email protected]>
Date:   Thu Aug 24 15:58:07 2017 -0400

    Issue trivially renewable tickets
    
    If the client specifically asks for renewable tickets but the
    renewable end time (either requested or after restrictions) doesn't
    exceed the ticket end time, issue a renewable ticket anyway.  Issuing
    a non-renewable ticket (as we started doing in release 1.12, due to
    the refactoring in commit 4f551a7ec126c52ee1f8fea4c3954015b70987bd)
    can be unfriendly to scripts.
    
    Also make sure never to issue a ticket with the renewable flag set but
    no renew-till field, by clearing the renewable flag at the start of
    kdc_get_ticket_renewtime().  The flag could have been previously set
    by the assignment "enc_tkt_reply = *(header_ticket->enc_part2)" in
    process_tgs_req() when processing a renewal request.
    
    Modify t_renew.py to expect renewable tickets in some tests where it
    previously did not, to check for specific lifetimes, and to check the
    renewable flag as well as the renewable lifetime.
    
    ticket: 8609

 src/kdc/kdc_util.c   |   15 ++++++----
 src/tests/t_renew.py |   71 +++++++++++++++++++++++++++++++++++---------------
 2 files changed, 59 insertions(+), 27 deletions(-)

diff --git a/src/kdc/kdc_util.c b/src/kdc/kdc_util.c
index 5455e2a..c3ef6af 100644
--- a/src/kdc/kdc_util.c
+++ b/src/kdc/kdc_util.c
@@ -1790,6 +1790,7 @@ kdc_get_ticket_renewtime(kdc_realm_t *realm, krb5_kdc_req *request,
 {
     krb5_timestamp rtime, max_rlife;
 
+    clear(tkt->flags, TKT_FLG_RENEWABLE);
     tkt->times.renew_till = 0;
 
     /* Don't issue renewable tickets if the client or server don't allow it,
@@ -1818,12 +1819,14 @@ kdc_get_ticket_renewtime(kdc_realm_t *realm, krb5_kdc_req *request,
         max_rlife = min(max_rlife, client->max_renewable_life);
     rtime = ts_min(rtime, ts_incr(tkt->times.starttime, max_rlife));
 
-    /* Make the ticket renewable if the truncated requested time is larger than
-     * the ticket end time. */
-    if (ts_after(rtime, tkt->times.endtime)) {
-        setflag(tkt->flags, TKT_FLG_RENEWABLE);
-        tkt->times.renew_till = rtime;
-    }
+    /* If the client only specified renewable-ok, don't issue a renewable
+     * ticket unless the truncated renew time exceeds the ticket end time. */
+    if (!isflagset(request->kdc_options, KDC_OPT_RENEWABLE) &&
+        !ts_after(rtime, tkt->times.endtime))
+        return;
+
+    setflag(tkt->flags, TKT_FLG_RENEWABLE);
+    tkt->times.renew_till = rtime;
 }
 
 /**
diff --git a/src/tests/t_renew.py b/src/tests/t_renew.py
index 106c8ec..aa58ece 100755
--- a/src/tests/t_renew.py
+++ b/src/tests/t_renew.py
@@ -1,26 +1,53 @@
 #!/usr/bin/python
 from k5test import *
+from datetime import datetime
+import re
 
 conf = {'realms': {'$realm': {'max_life': '20h', 'max_renewable_life': '20h'}}}
 realm = K5Realm(create_host=False, get_creds=False, kdc_conf=conf)
 
-def test(testname, life, rlife, expect_renewable, env=None):
+def test(testname, life, rlife, exp_life, exp_rlife, 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])
+    out = realm.run([klist, '-f'])
+
     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)
+
+    # Extract flags and check the renewable flag against expectations.
+    flags = re.findall(r'Flags: ([a-zA-Z]*)', out)[0]
+    if exp_rlife is None and 'R' in flags:
+        fail('%s: ticket unexpectedly renewable' % testname)
+    if exp_rlife is not None and 'R' not in flags:
+        fail('%s: ticket unexpectedly non-renewable' % testname)
+
+    # Extract the start time, end time, and renewable end time if present.
+    times = re.findall(r'\d\d/\d\d/\d\d \d\d:\d\d:\d\d', out)
+    times = [datetime.strptime(t, '%m/%d/%y %H:%M:%S') for t in times]
+    starttime = times[0]
+    endtime = times[1]
+    rtime = times[2] if len(times) >= 3 else None
+
+    # Check the ticket lifetime against expectations.
+    life = (endtime - starttime).seconds
+    if life != exp_life:
+        fail('%s: expected life %d, got %d' % (testname, exp_life, life))
+
+    # Check the ticket renewable lifetime against expectations.
+    if exp_rlife is None and rtime is not None:
+        fail('%s: ticket has unexpected renew_till' % testname)
+    if exp_rlife is not None and rtime is None:
+        fail('%s: ticket is renewable but has no renew_till' % testname)
+    if rtime is not None:
+        rlife = (rtime - starttime).seconds
+        if rlife != exp_rlife:
+            fail('%s: expected rlife %d, got %d' (testname, exp_rlife, rlife))
 
 # Get renewable tickets.
-test('simple', '1h', '2h', True)
+test('simple', '1h', '2h', 3600, 7200)
 
 # Renew twice, to test that renewed tickets are renewable.
 realm.kinit(realm.user_princ, flags=['-R'])
@@ -31,48 +58,50 @@ realm.klist(realm.user_princ)
 realm.run([kvno, realm.user_princ])
 
 # Make sure we can't renew non-renewable tickets.
-test('non-renewable', '1h', '1h', False)
+test('non-renewable', '1h', None, 3600, None)
 realm.kinit(realm.user_princ, flags=['-R'], expected_code=1,
             expected_msg="KDC can't fulfill requested option")
 
 # Test that -allow_renewable on the client principal works.
 realm.run([kadminl, 'modprinc', '-allow_renewable', 'user'])
-test('disallowed client', '1h', '2h', False)
+test('disallowed client', '1h', '2h', 3600, None)
 realm.run([kadminl, 'modprinc', '+allow_renewable', 'user'])
 
 # Test that -allow_renewable on the server principal works.
 realm.run([kadminl, 'modprinc', '-allow_renewable',  realm.krbtgt_princ])
-test('disallowed server', '1h', '2h', False)
+test('disallowed server', '1h', '2h', 3600, None)
 realm.run([kadminl, 'modprinc', '+allow_renewable', realm.krbtgt_princ])
 
-# Test that non-renewable tickets are issued if renew_till < till.
-test('short', '2h', '1h', False)
+# Test that trivially renewable tickets are issued if renew_till <=
+# till.  (Our client code bumps up the requested renewable life to the
+# requested life.)
+test('short', '2h', '1h', 7200, 7200)
 
 # 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)
+test('long', '15h', None, 10 * 3600, 15 * 3600)
+test('long noopts', '15h', None, 10 * 3600, None, 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', '6h', '10h', False)
+test('maxrenewlife client 1', '4h', '5h', 4 * 3600, 5 * 3600)
+test('maxrenewlife client 2', '6h', '10h', 6 * 3600, 5 * 3600)
 
 # Test maximum renewable life on the server principal.
 realm.run([kadminl, 'modprinc', '-maxrenewlife', '3 hours',
            realm.krbtgt_princ])
-test('maxrenewlife server yes', '2h', '3h', True)
-test('maxrenewlife server no', '4h', '8h', False)
+test('maxrenewlife server 1', '2h', '3h', 2 * 3600, 3 * 3600)
+test('maxrenewlife server 2', '4h', '8h', 4 * 3600, 3 * 3600)
 
 # Test realm maximum life.
 realm.run([kadminl, 'modprinc', '-maxrenewlife', '40 hours', 'user'])
 realm.run([kadminl, 'modprinc', '-maxrenewlife', '40 hours',
            realm.krbtgt_princ])
-test('maxrenewlife realm yes', '10h', '20h', True)
-test('maxrenewlife realm no', '21h', '40h', False)
+test('maxrenewlife realm 1', '10h', '20h', 10 * 3600, 20 * 3600)
+test('maxrenewlife realm 2', '21h', '40h', 20 * 3600, 20 * 3600)
 
 success('Renewing credentials')