krb5 commit: Limit -keepold for self-service key changes
ghudson at mit.edu
ghudson at mit.edu
Tue Apr 22 16:04:20 EDT 2025
https://github.com/krb5/krb5/commit/b43ac6b2b02b0d81370aab337d31159aba219ed6
commit b43ac6b2b02b0d81370aab337d31159aba219ed6
Author: Greg Hudson <ghudson at mit.edu>
Date: Mon Apr 14 02:16:10 2025 -0400
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)
src/include/kdb.h | 8 +++---
src/kadmin/server/server_stubs.c | 47 +++++++++++++++++++++++++++--------
src/lib/kadm5/admin.h | 8 +++---
src/lib/kadm5/clnt/client_principal.c | 8 +++---
src/lib/kadm5/srv/svr_principal.c | 8 +++---
src/lib/kdb/kdb_cpw.c | 46 ++++++++++++++++++----------------
src/tests/t_keyrollover.py | 25 +++++++++++++++++++
7 files changed, 103 insertions(+), 47 deletions(-)
diff --git a/src/include/kdb.h b/src/include/kdb.h
index 7beee28df..17a345682 100644
--- 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 b2371e67a..7fd980794 100644
--- 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 4af1ea225..3a0c389a5 100644
--- 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 976490093..4bcfed3e4 100644
--- 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 8f381882d..a850b133a 100644
--- 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 8b012e19e..ee32922f1 100644
--- 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 e9840dfae..036c0c3c6 100755
--- 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
More information about the cvs-krb5
mailing list