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 <rharwood@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>

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) {

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)