URL: https://github.com/freeipa/freeipa/pull/5269 Author: abbra Title: #5269: [Backport][ipa-4-8] ipa-kdb: implement AS-REQ lifetime jitter Action: opened
PR body: """ This PR was opened automatically because PR #5245 was pushed to master and backport to ipa-4-8 is required. """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/5269/head:pr5269 git checkout pr5269
From 203aa2a29a8962ce0ddbbd94c1d906dd65c5f957 Mon Sep 17 00:00:00 2001 From: Robbie Harwood <rharw...@redhat.com> Date: Tue, 10 Nov 2020 16:02:30 -0500 Subject: [PATCH 1/2] Update kdcpolicy design doc for jitter implementation Signed-off-by: Robbie Harwood <rharw...@redhat.com> --- doc/designs/krb-ticket-policy.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/designs/krb-ticket-policy.md b/doc/designs/krb-ticket-policy.md index 0961044132e..b704b066597 100644 --- a/doc/designs/krb-ticket-policy.md +++ b/doc/designs/krb-ticket-policy.md @@ -91,8 +91,10 @@ where administrators can specify max renew and life for each supported auth indi ### Ticket lifetime jitter -Ticket lifetimes can be jittered so that renewals / re-issues do not overwhelm the KDC at a certain moment. -The feature is enabled automatically so that we can avoid triggering an LDAP query on every `AS_REQ` and `TGS_REQ`. +All TGT lifetimes are varied slightly to avoid overwhelming the KDC with +simultaneous renewal requests. Jitter will reduce lifetimes by up to one hour +from the configured maximum lifetime (per policy). Significantly shorter +requested lifetimes will be unaffected. ## Implementation From 21e798384e61d5e411bcaab5f37cc9590a64847f Mon Sep 17 00:00:00 2001 From: Robbie Harwood <rharw...@redhat.com> Date: Tue, 10 Nov 2020 14:07:47 -0500 Subject: [PATCH 2/2] ipa-kdb: implement AS-REQ lifetime jitter Jitter is always enabled, so there is no additional configuration. An earlier version of this patch was authored by Becky Shanley. Fixes: https://pagure.io/freeipa/issue/8010 Signed-off-by: Robbie Harwood <rharw...@redhat.com> --- daemons/ipa-kdb/ipa_kdb_kdcpolicy.c | 37 +++++++++++++- ipatests/test_integration/test_krbtpolicy.py | 53 +++++++++++++------- 2 files changed, 69 insertions(+), 21 deletions(-) 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 <errno.h> #include <syslog.h> +#include <sys/random.h> + #include <krb5/kdcpolicy_plugin.h> #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)
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/freeipa-devel@lists.fedorahosted.org
