From: Greg Hudson Date: Mon, 14 Apr 2025 06:16:10 +0000 (-0400) Subject: Limit -keepold for self-service key changes X-Git-Tag: krb5-1.22-beta1~9 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=b43ac6b2b02b0d81370aab337d31159aba219ed6;p=thirdparty%2Fkrb5.git 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 7beee28df8..17a3456822 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 b2371e67ac..7fd980794b 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 4af1ea2256..3a0c389a5a 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 9764900935..4bcfed3e4c 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 8f381882d1..a850b133a9 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 8b012e19ef..ee32922f1c 100644 --- a/src/lib/kdb/kdb_cpw.c +++ b/src/lib/kdb/kdb_cpw.c @@ -54,8 +54,6 @@ #include #include -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 e9840dfae8..036c0c3c6f 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