From: Greg Hudson Date: Thu, 24 Aug 2017 19:58:07 +0000 (-0400) Subject: Issue trivially renewable tickets X-Git-Tag: krb5-1.16-beta1~15 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=45c19b19ea4d47ac5969a9cbdb308201b16615d8;p=thirdparty%2Fkrb5.git 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 --- diff --git a/src/kdc/kdc_util.c b/src/kdc/kdc_util.c index 5455e2a671..c3ef6af0ba 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 106c8ecd35..aa58ece2ed 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')