diff --git a/daemons/ipa-kdb/ipa_kdb_kdcpolicy.c b/daemons/ipa-kdb/ipa_kdb_kdcpolicy.c index 8d2ad66f715..7f03f2f03fc 100644 --- a/daemons/ipa-kdb/ipa_kdb_kdcpolicy.c +++ b/daemons/ipa-kdb/ipa_kdb_kdcpolicy.c @@ -1,14 +1,44 @@ /* - * Copyright (C) 2018 FreeIPA Contributors see COPYING for license + * Copyright (C) 2018,2020 FreeIPA Contributors see COPYING for license */ #include #include +#include + #include #include "ipa_krb5.h" #include "ipa_kdb.h" +#define ONE_DAY_SECONDS (24 * 60 * 60) +#define JITTER_WINDOW_SECONDS (1 * 60 * 60) + +static void +jitter(krb5_deltat baseline, krb5_deltat *lifetime_out) +{ + krb5_deltat offset; + ssize_t ret; + + if (baseline < JITTER_WINDOW_SECONDS) { + /* A negative value here would correspond to a never-valid ticket, + * which isn't the goal. */ + *lifetime_out = baseline; + return; + } + + do { + ret = getrandom(&offset, sizeof(offset), 0); + } while (ret == -1 && errno == EINTR); + if (ret < 0) { + krb5_klog_syslog(LOG_INFO, "IPA kdcpolicy: getrandom failed (errno %d); skipping jitter...", + errno); + return; + } + + *lifetime_out = baseline - offset % JITTER_WINDOW_SECONDS; +} + static krb5_error_code ipa_kdcpolicy_check_as(krb5_context context, krb5_kdcpolicy_moddata moddata, const krb5_kdc_req *request, @@ -56,6 +86,7 @@ ipa_kdcpolicy_check_as(krb5_context context, krb5_kdcpolicy_moddata moddata, /* If no mechanisms are set, allow every auth method */ if (ua == IPADB_USER_AUTH_NONE) { + jitter(ONE_DAY_SECONDS, lifetime_out); return 0; } @@ -108,7 +139,9 @@ ipa_kdcpolicy_check_as(krb5_context context, krb5_kdcpolicy_moddata moddata, * apply them */ if (pol_limits != NULL) { if (pol_limits->max_life != 0) { - *lifetime_out = pol_limits->max_life; + jitter(pol_limits->max_life, lifetime_out); + } else { + jitter(ONE_DAY_SECONDS, lifetime_out); } if (pol_limits->max_renewable_life != 0) { diff --git a/ipatests/test_integration/test_krbtpolicy.py b/ipatests/test_integration/test_krbtpolicy.py index 98aa1210b5f..e43c8d122b1 100644 --- a/ipatests/test_integration/test_krbtpolicy.py +++ b/ipatests/test_integration/test_krbtpolicy.py @@ -1,5 +1,5 @@ # -# Copyright (C) 2019 FreeIPA Contributors see COPYING for license +# Copyright (C) 2019,2020 FreeIPA Contributors see COPYING for license # """ @@ -8,6 +8,7 @@ from __future__ import absolute_import +import pytest import time from datetime import datetime @@ -44,10 +45,17 @@ def reset_to_default_policy(host, user): """Reset default user authentication and user authentication type""" tasks.kinit_admin(host) host.run_command(['ipa', 'user-mod', user, '--user-auth-type=']) - host.run_command(['ipa', 'krbtpolicy-reset']) host.run_command(['ipa', 'krbtpolicy-reset', user]) +def kinit_check_life(master, user): + """Acquire a TGT and check if it's within the lifetime window""" + master.run_command(["kinit", user], stdin_text=f"{PASSWORD}\n") + result = master.run_command("klist | grep krbtgt") + assert maxlife_within_policy(result.stdout_text, MAXLIFE, + slush=60) is True + + class TestPWPolicy(IntegrationTest): """Tests password custom and default password policies. """ @@ -59,11 +67,15 @@ def install(cls, mh): tasks.create_active_user(cls.master, USER1, PASSWORD) tasks.create_active_user(cls.master, USER2, PASSWORD) + @pytest.fixture(autouse=True, scope="class") + def with_admin(self): + tasks.kinit_admin(self.master) + yield + tasks.kdestroy_all(self.master) + def test_krbtpolicy_default(self): """Test the default kerberos ticket policy 24-hr tickets""" master = self.master - - tasks.kinit_admin(master) master.run_command(['ipa', 'krbtpolicy-mod', USER1, '--maxlife', str(MAXLIFE)]) tasks.kdestroy_all(master) @@ -73,13 +85,9 @@ def test_krbtpolicy_default(self): result = master.run_command('klist | grep krbtgt') assert maxlife_within_policy(result.stdout_text, MAXLIFE) is True - tasks.kdestroy_all(master) - def test_krbtpolicy_hardended(self): """Test a hardened kerberos ticket policy with 10 min tickets""" master = self.master - - tasks.kinit_admin(master) master.run_command(['ipa', 'user-mod', USER1, '--user-auth-type', 'password', '--user-auth-type', 'hardened']) @@ -104,13 +112,9 @@ def test_krbtpolicy_hardended(self): result = master.run_command('klist | grep krbtgt') assert maxlife_within_policy(result.stdout_text, MAXLIFE) is True - tasks.kdestroy_all(master) - def test_krbtpolicy_password(self): """Test the kerberos ticket policy which issues 20 min tickets""" master = self.master - - tasks.kinit_admin(master) master.run_command(['ipa', 'krbtpolicy-mod', USER2, '--maxlife', '1200']) @@ -121,25 +125,18 @@ def test_krbtpolicy_password(self): result = master.run_command('klist | grep krbtgt') assert maxlife_within_policy(result.stdout_text, 1200) is True - tasks.kdestroy_all(master) - def test_krbtpolicy_reset(self): """Test a hardened kerberos ticket policy reset""" master = self.master - - tasks.kinit_admin(master) master.run_command(['ipa', 'krbtpolicy-reset', USER2]) master.run_command(['kinit', USER2], stdin_text=PASSWORD + '\n') result = master.run_command('klist | grep krbtgt') assert maxlife_within_policy(result.stdout_text, MAXLIFE) is True - tasks.kdestroy_all(master) - def test_krbtpolicy_otp(self): """Test otp ticket policy""" master = self.master - tasks.kinit_admin(self.master) master.run_command(['ipa', 'user-mod', USER1, '--user-auth-type', 'otp']) master.run_command(['ipa', 'config-mod', @@ -177,3 +174,21 @@ def test_krbtpolicy_otp(self): reset_to_default_policy(master, USER1) self.master.run_command(['rm', '-f', armor]) master.run_command(['ipa', 'config-mod', '--user-auth-type=']) + + def test_krbtpolicy_jitter(self): + """Test jitter lifetime with no auth indicators""" + kinit_check_life(self.master, USER1) + + def test_krbtpolicy_jitter_otp(self): + """Test jitter lifetime with OTP""" + self.master.run_command(["ipa", "user-mod", USER1, + "--user-auth-type", "otp"]) + kinit_check_life(self.master, USER1) + reset_to_default_policy(self.master, USER1) + + def test_krbtpolicy_jitter_pkinit(self): + """Test jitter lifetime with PKINIT""" + self.master.run_command(["ipa", "user-mod", USER2, + "--user-auth-type", "pkinit"]) + kinit_check_life(self.master, USER2) + reset_to_default_policy(self.master, USER2)