Limit -keepold for self-service key changes

In libkadm5, change the type of the keepold parameters from
krb5_boolean to unsigned int (which is the underlying type of
krb5_boolean, so this is not an API or ABI change).  In libkadm5srv,
interpret a keepold value greater than 2 to be a limit on the number
of resulting key versions including the new one.

In kadmind, when a principal changes its own keys, limit the number of
resulting key versions to 5.

ticket: 9173 (new)

diff --git a/src/include/kdb.h b/src/include/kdb.h
index 7beee28df8146e86ca64fc9bc070decfa1851728..17a3456822d0cd389045dec1c708f17c9c2130ef 100644 (file)
--- a/src/include/kdb.h
+++ b/src/include/kdb.h
@@ -636,7 +636,7 @@ krb5_dbe_cpw( krb5_context        kcontext,
               int                         ks_tuple_count,
               char              * passwd,
               int                         new_kvno,
-              krb5_boolean        keepold,
+              unsigned int        keepold,
               krb5_db_entry     * db_entry);
 
 
@@ -652,7 +652,7 @@ krb5_dbe_crk( krb5_context        context,
               krb5_keyblock       * master_key,
               krb5_key_salt_tuple       * ks_tuple,
               int                         ks_tuple_count,
-              krb5_boolean        keepold,
+              unsigned int        keepold,
               krb5_db_entry     * db_entry);
 
 krb5_error_code
@@ -774,7 +774,7 @@ krb5_dbe_def_cpw( krb5_context    context,
                   int                     ks_tuple_count,
                   char          * passwd,
                   int                     new_kvno,
-                  krb5_boolean    keepold,
+                  unsigned int    keepold,
                   krb5_db_entry * db_entry);
 
 krb5_error_code
@@ -1250,7 +1250,7 @@ typedef struct _kdb_vftabl {
                                   krb5_keyblock *master_key,
                                   krb5_key_salt_tuple *ks_tuple,
                                   int ks_tuple_count, char *passwd,
-                                  int new_kvno, krb5_boolean keepold,
+                                  int new_kvno, unsigned int keepold,
                                   krb5_db_entry *db_entry);
 
     /*

diff --git a/src/kadmin/server/server_stubs.c b/src/kadmin/server/server_stubs.c
index b2371e67ace6a0ab517ee134d59739ff40b33702..7fd980794b12afd239bc1f946f45abfa4b0009da 100644 (file)
--- a/src/kadmin/server/server_stubs.c
+++ b/src/kadmin/server/server_stubs.c
@@ -17,6 +17,10 @@
 #include "misc.h"
 #include "auth.h"
 
+/* A principal changing its own keys may retain at most this many key
+ * versions. */
+#define MAX_SELF_KEEPOLD 5
+
 extern gss_name_t                       gss_changepw_name;
 extern gss_name_t                       gss_oldchangepw_name;
 extern void *                           global_server_handle;
@@ -377,6 +381,24 @@ check_self_keychange(kadm5_server_handle_t handle, struct svc_req *rqstp,
     return check_min_life(handle, princ, NULL, 0);
 }
 
+/*
+ * Return the appropriate libkadm5 keepold value for a key change request,
+ * given the boolean keepold input from the client.  0 means discard all old
+ * keys, 1 means retain all old keys, and a greater value means to retain that
+ * many key versions including the new one.  A principal modifying its own keys
+ * is allowed to retain only a limited number of key versions.
+ */
+static unsigned int
+clamp_self_keepold(kadm5_server_handle_t handle, krb5_principal princ,
+                   krb5_boolean keepold)
+{
+    if (!keepold)
+        return 0;
+    if (krb5_principal_compare(handle->context, handle->current_caller, princ))
+        return MAX_SELF_KEEPOLD;
+    return 1;
+}
+
 static int
 log_unauth(
     char *op,
@@ -875,6 +897,7 @@ chpass_principal3_2_svc(chpass3_arg *arg, generic_ret *ret,
     kadm5_principal_ent_rec         rec = { 0 };
     gss_buffer_desc                 client_name = GSS_C_EMPTY_BUFFER;
     gss_buffer_desc                 service_name = GSS_C_EMPTY_BUFFER;
+    unsigned int                    keepold;
     kadm5_server_handle_t           handle;
     const char                      *errmsg = NULL;
 
@@ -899,8 +922,9 @@ chpass_principal3_2_svc(chpass3_arg *arg, generic_ret *ret,
     } else  {
         ret->code = check_self_keychange(handle, rqstp, rec.principal);
         if (!ret->code) {
+            keepold = clamp_self_keepold(handle, rec.principal, arg->keepold);
             ret->code = kadm5_chpass_principal_3(handle, rec.principal,
-                                                 arg->keepold, arg->n_ks_tuple,
+                                                 keepold, arg->n_ks_tuple,
                                                  arg->ks_tuple, arg->pass);
         }
     }
@@ -981,6 +1005,7 @@ setkey_principal3_2_svc(setkey3_arg *arg, generic_ret *ret,
     kadm5_principal_ent_rec         rec = { 0 };
     gss_buffer_desc                 client_name = GSS_C_EMPTY_BUFFER;
     gss_buffer_desc                 service_name = GSS_C_EMPTY_BUFFER;
+    unsigned int                    keepold;
     kadm5_server_handle_t           handle;
     const char                      *errmsg = NULL;
 
@@ -999,10 +1024,10 @@ setkey_principal3_2_svc(setkey3_arg *arg, generic_ret *ret,
         }
     } else if (!(CHANGEPW_SERVICE(rqstp)) &&
                stub_auth(handle, OP_SETKEY, rec.principal, NULL, NULL, NULL)) {
-        ret->code = kadm5_setkey_principal_3(handle, rec.principal,
-                                             arg->keepold, arg->n_ks_tuple,
-                                             arg->ks_tuple, arg->keyblocks,
-                                             arg->n_keys);
+        keepold = clamp_self_keepold(handle, rec.principal, arg->keepold);
+        ret->code = kadm5_setkey_principal_3(handle, rec.principal, keepold,
+                                             arg->n_ks_tuple, arg->ks_tuple,
+                                             arg->keyblocks, arg->n_keys);
     } else {
         log_unauth("kadm5_setkey_principal", prime_arg,
                    &client_name, &service_name, rqstp);
@@ -1034,6 +1059,7 @@ setkey_principal4_2_svc(setkey4_arg *arg, generic_ret *ret,
     kadm5_principal_ent_rec         rec = { 0 };
     gss_buffer_desc                 client_name = GSS_C_EMPTY_BUFFER;
     gss_buffer_desc                 service_name = GSS_C_EMPTY_BUFFER;
+    unsigned int                    keepold;
     kadm5_server_handle_t           handle;
     const char                      *errmsg = NULL;
 
@@ -1052,9 +1078,9 @@ setkey_principal4_2_svc(setkey4_arg *arg, generic_ret *ret,
         }
     } else if (!(CHANGEPW_SERVICE(rqstp)) &&
                stub_auth(handle, OP_SETKEY, rec.principal, NULL, NULL, NULL)) {
-        ret->code = kadm5_setkey_principal_4(handle, rec.principal,
-                                             arg->keepold, arg->key_data,
-                                             arg->n_key_data);
+        keepold = clamp_self_keepold(handle, rec.principal, arg->keepold);
+        ret->code = kadm5_setkey_principal_4(handle, rec.principal, keepold,
+                                             arg->key_data, arg->n_key_data);
     } else {
         log_unauth("kadm5_setkey_principal", prime_arg, &client_name,
                    &service_name, rqstp);
@@ -1167,6 +1193,7 @@ chrand_principal3_2_svc(chrand3_arg *arg, chrand_ret *ret,
     gss_buffer_desc             service_name = GSS_C_EMPTY_BUFFER;
     krb5_keyblock               *k;
     int                         nkeys;
+    unsigned int                keepold;
     kadm5_server_handle_t       handle;
     const char                  *errmsg = NULL;
 
@@ -1186,9 +1213,9 @@ chrand_principal3_2_svc(chrand3_arg *arg, chrand_ret *ret,
     } else {
         ret->code = check_self_keychange(handle, rqstp, rec.principal);
         if (!ret->code) {
+            keepold = clamp_self_keepold(handle, rec.principal, arg->keepold);
             ret->code = kadm5_randkey_principal_3(handle, rec.principal,
-                                                  arg->keepold,
-                                                  arg->n_ks_tuple,
+                                                  keepold, arg->n_ks_tuple,
                                                   arg->ks_tuple, &k, &nkeys);
         }
     }

diff --git a/src/lib/kadm5/admin.h b/src/lib/kadm5/admin.h
index 4af1ea225607f5fec44f5aefc5c8ad8cc73dc71b..3a0c389a5a0a283f723c8a1785dcfc566c9433b6 100644 (file)
--- a/src/lib/kadm5/admin.h
+++ b/src/lib/kadm5/admin.h
@@ -381,7 +381,7 @@ kadm5_ret_t    kadm5_chpass_principal(void *server_handle,
                                       char *pass);
 kadm5_ret_t    kadm5_chpass_principal_3(void *server_handle,
                                         krb5_principal principal,
-                                        krb5_boolean keepold,
+                                        unsigned int keepold,
                                         int n_ks_tuple,
                                         krb5_key_salt_tuple *ks_tuple,
                                         char *pass);
@@ -391,7 +391,7 @@ kadm5_ret_t    kadm5_randkey_principal(void *server_handle,
                                        int *n_keys);
 kadm5_ret_t    kadm5_randkey_principal_3(void *server_handle,
                                          krb5_principal principal,
-                                         krb5_boolean keepold,
+                                         unsigned int keepold,
                                          int n_ks_tuple,
                                          krb5_key_salt_tuple *ks_tuple,
                                          krb5_keyblock **keyblocks,
@@ -404,7 +404,7 @@ kadm5_ret_t    kadm5_setkey_principal(void *server_handle,
 
 kadm5_ret_t    kadm5_setkey_principal_3(void *server_handle,
                                         krb5_principal principal,
-                                        krb5_boolean keepold,
+                                        unsigned int keepold,
                                         int n_ks_tuple,
                                         krb5_key_salt_tuple *ks_tuple,
                                         krb5_keyblock *keyblocks,
@@ -412,7 +412,7 @@ kadm5_ret_t    kadm5_setkey_principal_3(void *server_handle,
 
 kadm5_ret_t    kadm5_setkey_principal_4(void *server_handle,
                                         krb5_principal principal,
-                                        krb5_boolean keepold,
+                                        unsigned int keepold,
                                         kadm5_key_data *key_data,
                                         int n_key_data);
 

diff --git a/src/lib/kadm5/clnt/client_principal.c b/src/lib/kadm5/clnt/client_principal.c
index 97649009352680b44045b90b733d2b0e8b1a1269..4bcfed3e4c9d8f02028ef9524ea5c66d35d8d387 100644 (file)
--- a/src/lib/kadm5/clnt/client_principal.c
+++ b/src/lib/kadm5/clnt/client_principal.c
@@ -249,7 +249,7 @@ kadm5_chpass_principal(void *server_handle,
 
 kadm5_ret_t
 kadm5_chpass_principal_3(void *server_handle,
-                         krb5_principal princ, krb5_boolean keepold,
+                         krb5_principal princ, unsigned int keepold,
                          int n_ks_tuple, krb5_key_salt_tuple *ks_tuple,
                          char *password)
 {
@@ -300,7 +300,7 @@ kadm5_setkey_principal(void *server_handle,
 kadm5_ret_t
 kadm5_setkey_principal_3(void *server_handle,
                          krb5_principal princ,
-                         krb5_boolean keepold, int n_ks_tuple,
+                         unsigned int keepold, int n_ks_tuple,
                          krb5_key_salt_tuple *ks_tuple,
                          krb5_keyblock *keyblocks,
                          int n_keys)
@@ -329,7 +329,7 @@ kadm5_setkey_principal_3(void *server_handle,
 kadm5_ret_t
 kadm5_setkey_principal_4(void *server_handle,
                          krb5_principal princ,
-                         krb5_boolean keepold,
+                         unsigned int keepold,
                          kadm5_key_data *key_data,
                          int n_key_data)
 {
@@ -355,7 +355,7 @@ kadm5_setkey_principal_4(void *server_handle,
 kadm5_ret_t
 kadm5_randkey_principal_3(void *server_handle,
                           krb5_principal princ,
-                          krb5_boolean keepold, int n_ks_tuple,
+                          unsigned int keepold, int n_ks_tuple,
                           krb5_key_salt_tuple *ks_tuple,
                           krb5_keyblock **key, int *n_keys)
 {

diff --git a/src/lib/kadm5/srv/svr_principal.c b/src/lib/kadm5/srv/svr_principal.c
index 8f381882d16550fb144e837bd039511a340df7e6..a850b133a9ae84cbb89f9d596ac83548075c84d3 100644 (file)
--- a/src/lib/kadm5/srv/svr_principal.c
+++ b/src/lib/kadm5/srv/svr_principal.c
@@ -1218,7 +1218,7 @@ kadm5_chpass_principal(void *server_handle,
 
 kadm5_ret_t
 kadm5_chpass_principal_3(void *server_handle,
-                         krb5_principal principal, krb5_boolean keepold,
+                         krb5_principal principal, unsigned int keepold,
                          int n_ks_tuple, krb5_key_salt_tuple *ks_tuple,
                          char *password)
 {
@@ -1387,7 +1387,7 @@ kadm5_randkey_principal(void *server_handle,
 kadm5_ret_t
 kadm5_randkey_principal_3(void *server_handle,
                           krb5_principal principal,
-                          krb5_boolean keepold,
+                          unsigned int keepold,
                           int n_ks_tuple, krb5_key_salt_tuple *ks_tuple,
                           krb5_keyblock **keyblocks,
                           int *n_keys)
@@ -1518,7 +1518,7 @@ kadm5_setkey_principal(void *server_handle,
 kadm5_ret_t
 kadm5_setkey_principal_3(void *server_handle,
                          krb5_principal principal,
-                         krb5_boolean keepold,
+                         unsigned int keepold,
                          int n_ks_tuple, krb5_key_salt_tuple *ks_tuple,
                          krb5_keyblock *keyblocks,
                          int n_keys)
@@ -1579,7 +1579,7 @@ make_ks_from_key_data(krb5_context context, kadm5_key_data *key_data,
 
 kadm5_ret_t
 kadm5_setkey_principal_4(void *server_handle, krb5_principal principal,
-                         krb5_boolean keepold, kadm5_key_data *key_data,
+                         unsigned int keepold, kadm5_key_data *key_data,
                          int n_key_data)
 {
     krb5_db_entry *kdb;

diff --git a/src/lib/kdb/kdb_cpw.c b/src/lib/kdb/kdb_cpw.c
index 8b012e19efaf54367e4191944e0386816896e5de..ee32922f1c861f1e799ef1319da4d10e15fccfdd 100644 (file)
--- a/src/lib/kdb/kdb_cpw.c
+++ b/src/lib/kdb/kdb_cpw.c
@@ -54,8 +54,6 @@
 #include <stdio.h>
 #include <errno.h>
 
-enum save { DISCARD_ALL, KEEP_LAST_KVNO, KEEP_ALL };
-
 int
 krb5_db_get_key_data_kvno(krb5_context context, int count, krb5_key_data *data)
 {
@@ -117,20 +115,28 @@ preserve_one_old_key(krb5_context context, krb5_keyblock *mkey,
 
 /*
  * Add key_data to dbent, making sure that each entry is encrypted in mkey.  If
- * kvno is non-zero, preserve only keys of that kvno.  May steal some elements
- * from key_data and zero them out.
+ * keepold is greater than 1, preserve only the first (keepold-1) key versions,
+ * so that the total number of key versions including the new key set is
+ * keepold.  May steal some elements from key_data and zero them out.
  */
 static krb5_error_code
 preserve_old_keys(krb5_context context, krb5_keyblock *mkey,
-                  krb5_db_entry *dbent, int kvno, int n_key_data,
+                  krb5_db_entry *dbent, unsigned int keepold, int n_key_data,
                   krb5_key_data *key_data)
 {
     krb5_error_code ret;
+    krb5_kvno last_kvno = 0, kvno_changes = 0;
     int i;
 
     for (i = 0; i < n_key_data; i++) {
-        if (kvno != 0 && key_data[i].key_data_kvno != kvno)
-            continue;
+        if (keepold > 1) {
+            if (i > 0 && key_data[i].key_data_kvno != last_kvno)
+                kvno_changes++;
+            if (kvno_changes >= keepold - 1)
+                break;
+            last_kvno = key_data[i].key_data_kvno;
+        }
+
         ret = krb5_dbe_create_key_data(context, dbent);
         if (ret)
             return ret;
@@ -334,11 +340,11 @@ add_key_pwd(krb5_context context, krb5_keyblock *master_key,
 static krb5_error_code
 rekey(krb5_context context, krb5_keyblock *mkey, krb5_key_salt_tuple *ks_tuple,
       int ks_tuple_count, const char *password, int new_kvno,
-      enum save savekeys, krb5_db_entry *db_entry)
+      unsigned int keepold, krb5_db_entry *db_entry)
 {
     krb5_error_code ret;
     krb5_key_data *key_data;
-    int n_key_data, old_kvno, save_kvno;
+    int n_key_data, old_kvno;
 
     /* Save aside the old key data. */
     n_key_data = db_entry->n_key_data;
@@ -372,9 +378,8 @@ rekey(krb5_context context, krb5_keyblock *mkey, krb5_key_salt_tuple *ks_tuple,
 
     /* Possibly add some or all of the old keys to the back of the list.  May
      * steal from and zero out some of the old key data entries. */
-    if (savekeys != DISCARD_ALL) {
-        save_kvno = (savekeys == KEEP_LAST_KVNO) ? old_kvno : 0;
-        ret = preserve_old_keys(context, mkey, db_entry, save_kvno, n_key_data,
+    if (keepold > 0) {
+        ret = preserve_old_keys(context, mkey, db_entry, keepold, n_key_data,
                                 key_data);
     }
 
@@ -392,10 +397,10 @@ rekey(krb5_context context, krb5_keyblock *mkey, krb5_key_salt_tuple *ks_tuple,
 krb5_error_code
 krb5_dbe_crk(krb5_context context, krb5_keyblock *mkey,
              krb5_key_salt_tuple *ks_tuple, int ks_tuple_count,
-             krb5_boolean keepold, krb5_db_entry *dbent)
+             unsigned int keepold, krb5_db_entry *dbent)
 {
-    return rekey(context, mkey, ks_tuple, ks_tuple_count, NULL, 0,
-                 keepold ? KEEP_ALL : DISCARD_ALL, dbent);
+    return rekey(context, mkey, ks_tuple, ks_tuple_count, NULL, 0, keepold,
+                 dbent);
 }
 
 /*
@@ -409,8 +414,7 @@ krb5_dbe_ark(krb5_context context, krb5_keyblock *mkey,
              krb5_key_salt_tuple *ks_tuple, int ks_tuple_count,
              krb5_db_entry *dbent)
 {
-    return rekey(context, mkey, ks_tuple, ks_tuple_count, NULL, 0,
-                 KEEP_LAST_KVNO, dbent);
+    return rekey(context, mkey, ks_tuple, ks_tuple_count, NULL, 0, 2, dbent);
 }
 
 /*
@@ -422,11 +426,11 @@ krb5_dbe_ark(krb5_context context, krb5_keyblock *mkey,
 krb5_error_code
 krb5_dbe_def_cpw(krb5_context context, krb5_keyblock *mkey,
                  krb5_key_salt_tuple *ks_tuple, int ks_tuple_count,
-                 char *password, int new_kvno, krb5_boolean keepold,
+                 char *password, int new_kvno, unsigned int keepold,
                  krb5_db_entry *dbent)
 {
     return rekey(context, mkey, ks_tuple, ks_tuple_count, password, new_kvno,
-                 keepold ? KEEP_ALL : DISCARD_ALL, dbent);
+                 keepold, dbent);
 }
 
 /*
@@ -440,6 +444,6 @@ krb5_dbe_apw(krb5_context context, krb5_keyblock *mkey,
              krb5_key_salt_tuple *ks_tuple, int ks_tuple_count, char *password,
              krb5_db_entry *dbent)
 {
-    return rekey(context, mkey, ks_tuple, ks_tuple_count, password, 0,
-                 KEEP_LAST_KVNO, dbent);
+    return rekey(context, mkey, ks_tuple, ks_tuple_count, password, 0, 2,
+                 dbent);
 }

diff --git a/src/tests/t_keyrollover.py b/src/tests/t_keyrollover.py
index e9840dfae83dfc83b0edff5534b7b24872d5cbd8..036c0c3c6f42215aa2da7be88c911cfde3975933 100755 (executable)
--- a/src/tests/t_keyrollover.py
+++ b/src/tests/t_keyrollover.py
@@ -1,4 +1,5 @@
 from k5test import *
+import re
 
 rollover_krb5_conf = {'libdefaults': {'allow_weak_crypto': 'true'}}
 
@@ -55,6 +56,30 @@ if 'vno 1, aes256-cts' not in out or \
 # Now present the DES3 ticket to the KDC and make sure it's rejected.
 realm.run([kvno, realm.host_princ], expected_code=1)
 
+# Test -keepold limit for self-service requests through kadmind.
+def count_kvnos(princ, expected_count):
+    out = realm.run_kadmin(['getprinc', princ])
+    vnos = re.findall(r' vno \d+,', out)
+    if len(set(vnos)) != expected_count:
+        fail('expected %d key versions' % expected_count)
+realm.start_kadmind()
+realm.prep_kadmin(realm.user_princ, password('user'))
+realm.run_kadmin(['cpw', '-randkey', '-keepold', realm.user_princ])
+realm.run_kadmin(['cpw', '-randkey', '-keepold', realm.user_princ])
+realm.run_kadmin(['cpw', '-randkey', '-keepold', realm.user_princ])
+realm.run_kadmin(['cpw', '-randkey', '-keepold', realm.user_princ])
+count_kvnos(realm.user_princ, 5)
+realm.run_kadmin(['cpw', '-randkey', '-keepold', realm.user_princ])
+count_kvnos(realm.user_princ, 5)
+realm.run_kadmin(['cpw', '-pw', 'pw', '-keepold', realm.user_princ])
+count_kvnos(realm.user_princ, 5)
+# Test that the limit doesn't apply when modifying another principal.
+realm.prep_kadmin()
+realm.run_kadmin(['cpw', '-randkey', '-keepold', realm.user_princ])
+count_kvnos(realm.user_princ, 6)
+realm.run_kadmin(['cpw', '-pw', 'pw', '-keepold', realm.user_princ])
+count_kvnos(realm.user_princ, 7)
+
 realm.stop()
 
 # Test a cross-realm TGT key rollover scenario where realm 1 mimics