krb5 commit: Improve kadmin parsing of time intervals

Greg Hudson ghudson at mit.edu
Mon Apr 25 21:10:14 EDT 2016


https://github.com/krb5/krb5/commit/0e668054974b07ec7ffbe2d9d474062d590c7e69
commit 0e668054974b07ec7ffbe2d9d474062d590c7e69
Author: Greg Hudson <ghudson at mit.edu>
Date:   Tue Apr 19 11:33:37 2016 -0400

    Improve kadmin parsing of time intervals
    
    When parsing time intervals in kadmin commands, try
    krb5_string_to_deltat() first, then fall back to subtracting the
    current time from get_date().  If we do fall back, treat "never" as a
    zero interval, and error out rather than yield a huge interval if
    get_date() returns a time in the past.
    
    Notable behavior differences: bare numbers of seconds and suffixed
    numbers (e.g. "5m" or "6h") are now supported for all intervals;
    HH:MM:SS and HH:MM are now treated as intervals rather than absolute
    times with the current time subtracted.
    
    ticket: 8393

 doc/admin/admin_commands/kadmin_local.rst |   33 +++++----
 src/kadmin/cli/kadmin.c                   |  117 +++++++++++++++++------------
 2 files changed, 86 insertions(+), 64 deletions(-)

diff --git a/doc/admin/admin_commands/kadmin_local.rst b/doc/admin/admin_commands/kadmin_local.rst
index 7ae2a3f..a91aa06 100644
--- a/doc/admin/admin_commands/kadmin_local.rst
+++ b/doc/admin/admin_commands/kadmin_local.rst
@@ -260,11 +260,12 @@ Options:
     (:ref:`getdate` string) The password expiration date.
 
 **-maxlife** *maxlife*
-    (:ref:`getdate` string) The maximum ticket life for the principal.
+    (:ref:`duration` or :ref:`getdate` string) The maximum ticket life
+    for the principal.
 
 **-maxrenewlife** *maxrenewlife*
-    (:ref:`getdate` string) The maximum renewable life of tickets for
-    the principal.
+    (:ref:`duration` or :ref:`getdate` string) The maximum renewable
+    life of tickets for the principal.
 
 **-kvno** *kvno*
     The initial key version number.
@@ -702,10 +703,12 @@ Alias: **addpol**
 The following options are available:
 
 **-maxlife** *time*
-    (:ref:`getdate` string) Sets the maximum lifetime of a password.
+    (:ref:`duration` or :ref:`getdate` string) Sets the maximum
+    lifetime of a password.
 
 **-minlife** *time*
-    (:ref:`getdate` string) Sets the minimum lifetime of a password.
+    (:ref:`duration` or :ref:`getdate` string) Sets the minimum
+    lifetime of a password.
 
 **-minlength** *length*
     Sets the minimum length of a password.
@@ -731,21 +734,21 @@ The following options are available:
 .. _policy_failurecountinterval:
 
 **-failurecountinterval** *failuretime*
-    (:ref:`getdate` string) Sets the allowable time between
-    authentication failures.  If an authentication failure happens
-    after *failuretime* has elapsed since the previous failure,
-    the number of authentication failures is reset to 1.  A
+    (:ref:`duration` or :ref:`getdate` string) Sets the allowable time
+    between authentication failures.  If an authentication failure
+    happens after *failuretime* has elapsed since the previous
+    failure, the number of authentication failures is reset to 1.  A
     *failuretime* value of 0 (the default) means forever.
 
 .. _policy_lockoutduration:
 
 **-lockoutduration** *lockouttime*
-    (:ref:`getdate` string) Sets the duration for which the principal
-    is locked from authenticating if too many authentication failures
-    occur without the specified failure count interval elapsing.
-    A duration of 0 (the default) means the principal remains locked
-    out until it is administratively unlocked with ``modprinc
-    -unlock``.
+    (:ref:`duration` or :ref:`getdate` string) Sets the duration for
+    which the principal is locked from authenticating if too many
+    authentication failures occur without the specified failure count
+    interval elapsing.  A duration of 0 (the default) means the
+    principal remains locked out until it is administratively unlocked
+    with ``modprinc -unlock``.
 
 **-allowedkeysalts**
     Specifies the key/salt tuples supported for long-term keys when
diff --git a/src/kadmin/cli/kadmin.c b/src/kadmin/cli/kadmin.c
index 41f172e..d791cee 100644
--- a/src/kadmin/cli/kadmin.c
+++ b/src/kadmin/cli/kadmin.c
@@ -151,6 +151,50 @@ strdate(krb5_timestamp when)
     return out;
 }
 
+/* Parse a date string using getdate.y.  On failure, output an error message
+ * and return (time_t)-1. */
+static time_t
+parse_date(char *str)
+{
+    time_t date;
+
+    date = get_date(str);
+    if (date == (time_t)-1)
+        error(_("Invalid date specification \"%s\".\n"), str);
+    return date;
+}
+
+/*
+ * Parse a time interval.  Use krb5_string_to_deltat() if it works; otherwise
+ * use getdate.y and subtract now, with sanity checks.  On failure, output an
+ * error message and return (time_t)-1.
+ */
+static time_t
+parse_interval(char *str, time_t now)
+{
+    time_t date;
+    krb5_deltat delta;
+
+    if (krb5_string_to_deltat(str, &delta) == 0)
+        return delta;
+
+    date = parse_date(str);
+    if (date == (time_t)-1)
+        return date;
+
+    /* Interpret an absolute time of 0 (e.g. "never") as an interval of 0. */
+    if (date == 0)
+        return 0;
+
+    /* Don't return a negative interval if the date is in the past. */
+    if (date < now) {
+        error(_("Interval specification \"%s\" is in the past.\n"), str);
+        return (time_t)-1;
+    }
+
+    return date - now;
+}
+
 /* this is a wrapper to go around krb5_parse_principal so we can set
    the default realm up properly */
 static krb5_error_code
@@ -952,8 +996,7 @@ kadmin_parse_princ_args(int argc, char *argv[], kadm5_principal_ent_t oprinc,
                         int *n_ks_tuple, char *caller)
 {
     int i;
-    time_t date;
-    time_t now;
+    time_t now, date, interval;
     krb5_error_code retval;
 
     *mask = 0;
@@ -977,11 +1020,9 @@ kadmin_parse_princ_args(int argc, char *argv[], kadm5_principal_ent_t oprinc,
         if (!strcmp("-expire", argv[i])) {
             if (++i > argc - 2)
                 return -1;
-            date = get_date(argv[i]);
-            if (date == (time_t)-1) {
-                error(_("Invalid date specification \"%s\".\n"), argv[i]);
+            date = parse_date(argv[i]);
+            if (date == (time_t)-1)
                 return -1;
-            }
             oprinc->princ_expire_time = date;
             *mask |= KADM5_PRINC_EXPIRE_TIME;
             continue;
@@ -989,11 +1030,9 @@ kadmin_parse_princ_args(int argc, char *argv[], kadm5_principal_ent_t oprinc,
         if (!strcmp("-pwexpire", argv[i])) {
             if (++i > argc - 2)
                 return -1;
-            date = get_date(argv[i]);
-            if (date == (time_t)-1) {
-                error(_("Invalid date specification \"%s\".\n"), argv[i]);
+            date = parse_date(argv[i]);
+            if (date == (time_t)-1)
                 return -1;
-            }
             oprinc->pw_expiration = date;
             *mask |= KADM5_PW_EXPIRATION;
             continue;
@@ -1001,24 +1040,20 @@ kadmin_parse_princ_args(int argc, char *argv[], kadm5_principal_ent_t oprinc,
         if (!strcmp("-maxlife", argv[i])) {
             if (++i > argc - 2)
                 return -1;
-            date = get_date(argv[i]);
-            if (date == (time_t)-1) {
-                error(_("Invalid date specification \"%s\".\n"), argv[i]);
+            interval = parse_interval(argv[i], now);
+            if (interval == (time_t)-1)
                 return -1;
-            }
-            oprinc->max_life = date - now;
+            oprinc->max_life = interval;
             *mask |= KADM5_MAX_LIFE;
             continue;
         }
         if (!strcmp("-maxrenewlife", argv[i])) {
             if (++i > argc - 2)
                 return -1;
-            date = get_date(argv[i]);
-            if (date == (time_t)-1) {
-                error(_("Invalid date specification \"%s\".\n"), argv[i]);
+            interval = parse_interval(argv[i], now);
+            if (interval == (time_t)-1)
                 return -1;
-            }
-            oprinc->max_renewable_life = date - now;
+            oprinc->max_renewable_life = interval;
             *mask |= KADM5_MAX_RLIFE;
             continue;
         }
@@ -1502,7 +1537,7 @@ kadmin_parse_policy_args(int argc, char *argv[], kadm5_policy_ent_t policy,
 {
     krb5_error_code retval;
     int i;
-    time_t now, date;
+    time_t now, interval;
 
     time(&now);
     *mask = 0;
@@ -1510,23 +1545,19 @@ kadmin_parse_policy_args(int argc, char *argv[], kadm5_policy_ent_t policy,
         if (!strcmp(argv[i], "-maxlife")) {
             if (++i > argc -2)
                 return -1;
-            date = get_date(argv[i]);
-            if (date == (time_t)-1) {
-                error(_("Invalid date specification \"%s\".\n"), argv[i]);
+            interval = parse_interval(argv[i], now);
+            if (interval == (time_t)-1)
                 return -1;
-            }
-            policy->pw_max_life = date - now;
+            policy->pw_max_life = interval;
             *mask |= KADM5_PW_MAX_LIFE;
             continue;
         } else if (!strcmp(argv[i], "-minlife")) {
             if (++i > argc - 2)
                 return -1;
-            date = get_date(argv[i]);
-            if (date == (time_t)-1) {
-                error(_("Invalid date specification \"%s\".\n"), argv[i]);
+            interval = parse_interval(argv[i], now);
+            if (interval == (time_t)-1)
                 return -1;
-            }
-            policy->pw_min_life = date - now;
+            policy->pw_min_life = interval;
             *mask |= KADM5_PW_MIN_LIFE;
             continue;
         } else if (!strcmp(argv[i], "-minlength")) {
@@ -1558,32 +1589,20 @@ kadmin_parse_policy_args(int argc, char *argv[], kadm5_policy_ent_t policy,
                    !strcmp(argv[i], "-failurecountinterval")) {
             if (++i > argc - 2)
                 return -1;
-            /* Allow bare numbers for compatibility with 1.8-1.9. */
-            date = get_date(argv[i]);
-            if (date != (time_t)-1)
-                policy->pw_failcnt_interval = date - now;
-            else if (isdigit(*argv[i]))
-                policy->pw_failcnt_interval = atoi(argv[i]);
-            else {
-                error(_("Invalid date specification \"%s\".\n"), argv[i]);
+            interval = parse_interval(argv[i], now);
+            if (interval == (time_t)-1)
                 return -1;
-            }
+            policy->pw_failcnt_interval = interval;
             *mask |= KADM5_PW_FAILURE_COUNT_INTERVAL;
             continue;
         } else if (strlen(argv[i]) == 16 &&
                    !strcmp(argv[i], "-lockoutduration")) {
             if (++i > argc - 2)
                 return -1;
-            /* Allow bare numbers for compatibility with 1.8-1.9. */
-            date = get_date(argv[i]);
-            if (date != (time_t)-1)
-                policy->pw_lockout_duration = date - now;
-            else if (isdigit(*argv[i]))
-                policy->pw_lockout_duration = atoi(argv[i]);
-            else {
-                error(_("Invalid date specification \"%s\".\n"), argv[i]);
+            interval = parse_interval(argv[i], now);
+            if (interval == (time_t)-1)
                 return -1;
-            }
+            policy->pw_lockout_duration = interval;
             *mask |= KADM5_PW_LOCKOUT_DURATION;
             continue;
         } else if (!strcmp(argv[i], "-allowedkeysalts")) {


More information about the cvs-krb5 mailing list