krb5 commit: Perform atomic ccache refreshes when possible
Greg Hudson <[email protected]>
| Newsgroups | gmane.comp.encryption.kerberos.cvs |
|---|---|
| Message-ID | <[email protected]> |
https://github.com/krb5/krb5/commit/371f09d4bf4ca0c7ba15c5ef909bc35307ed9cc3 commit 371f09d4bf4ca0c7ba15c5ef909bc35307ed9cc3 Author: Greg Hudson <[email protected]> Date: Tue Aug 17 11:26:59 2021 -0400 Perform atomic ccache refreshes when possible Allow ccache types to implement atomic replacement via a new replace method (replacing the unused "move" vtable slot). Make krb5_cc_move() use this method when possible, falling back to non-atomic replacement. Implement atomic replacement for FILE, DIR, MEMORY, and KCM (using a new opcode, falling back when it is not implemented). Use krb5_cc_move() in get_in_tkt.c when an output ccache is specified, in kinit for ticket validation and renewal, and in kvno --out-cache. Add a test program to exercise concurrent krb5_get_credentials() and cache refresh. This commit does not implement atomic replacement for KEYRING or for gss_store_creds(). ticket: 7707 src/clients/kinit/kinit.c | 22 ++++- src/clients/kvno/kvno.c | 24 ++++- src/include/kcm.h | 2 + src/lib/krb5/ccache/cc-int.h | 8 +- src/lib/krb5/ccache/cc_dir.c | 11 ++- src/lib/krb5/ccache/cc_file.c | 110 ++++++++++++++++++----- src/lib/krb5/ccache/cc_kcm.c | 39 ++++++++- src/lib/krb5/ccache/cc_memory.c | 100 +++++++++++++------- src/lib/krb5/ccache/ccbase.c | 119 +++++++++++++++++------- src/lib/krb5/krb/get_in_tkt.c | 83 ++++++++++++----- src/lib/krb5/krb/t_vfy_increds.c | 15 ++- src/tests/Makefile.in | 23 +++-- src/tests/conccache.c | 190 ++++++++++++++++++++++++++++++++++++++ src/tests/kcmserver.py | 74 ++++++++++----- src/tests/t_ccache.py | 5 + 15 files changed, 661 insertions(+), 164 deletions(-) diff --git a/src/clients/kinit/kinit.c b/src/clients/kinit/kinit.c index 79775c2..f4c7b2b 100644 --- a/src/clients/kinit/kinit.c +++ b/src/clients/kinit/kinit.c @@ -645,6 +645,8 @@ k5_kinit(struct k_opts *opts, struct k5_data *k5) krb5_get_init_creds_opt *options = NULL; krb5_boolean pwprompt = FALSE; krb5_address **addresses = NULL; + krb5_principal cprinc; + krb5_ccache mcc = NULL; int i; memset(&my_creds, 0, sizeof(my_creds)); @@ -793,21 +795,29 @@ k5_kinit(struct k_opts *opts, struct k5_data *k5) } if (opts->action != INIT_PW && opts->action != INIT_KT) { - ret = krb5_cc_initialize(k5->ctx, k5->out_cc, opts->canonicalize ? - my_creds.client : k5->me); + cprinc = opts->canonicalize ? my_creds.client : k5->me; + ret = krb5_cc_new_unique(k5->ctx, "MEMORY", NULL, &mcc); + if (!ret) + ret = krb5_cc_initialize(k5->ctx, mcc, cprinc); if (ret) { - com_err(progname, ret, _("when initializing cache %s"), - opts->k5_out_cache_name ? opts->k5_out_cache_name : ""); + com_err(progname, ret, _("when creating temporary cache")); goto cleanup; } if (opts->verbose) fprintf(stderr, _("Initialized cache\n")); - ret = k5_cc_store_primary_cred(k5->ctx, k5->out_cc, &my_creds); + ret = k5_cc_store_primary_cred(k5->ctx, mcc, &my_creds); if (ret) { com_err(progname, ret, _("while storing credentials")); goto cleanup; } + ret = krb5_cc_move(k5->ctx, mcc, k5->out_cc); + if (ret) { + com_err(progname, ret, _("while saving to cache %s"), + opts->k5_out_cache_name ? opts->k5_out_cache_name : ""); + goto cleanup; + } + mcc = NULL; if (opts->verbose) fprintf(stderr, _("Stored credentials\n")); } @@ -824,6 +834,8 @@ cleanup: #ifndef _WIN32 kinit_kdb_fini(); #endif + if (mcc != NULL) + krb5_cc_destroy(k5->ctx, mcc); if (options) krb5_get_init_creds_opt_free(k5->ctx, options); if (my_creds.client == k5->me) diff --git a/src/clients/kvno/kvno.c b/src/clients/kvno/kvno.c index 89b5ce9..7f8667c 100644 --- a/src/clients/kvno/kvno.c +++ b/src/clients/kvno/kvno.c @@ -454,7 +454,7 @@ do_v5_kvno(int count, char *names[], char * ccachestr, char *etypestr, krb5_error_code ret; int i, errors, flags, initialized = 0; krb5_enctype etype; - krb5_ccache ccache, out_ccache = NULL; + krb5_ccache ccache, mcc, out_ccache = NULL; krb5_principal me; krb5_keytab keytab = NULL; krb5_principal for_user_princ = NULL; @@ -545,6 +545,14 @@ do_v5_kvno(int count, char *names[], char * ccachestr, char *etypestr, exit(1); } + if (out_ccache != NULL) { + ret = krb5_cc_new_unique(context, "MEMORY", NULL, &mcc); + if (ret) { + com_err(prog, ret, _("while creating temporary output ccache")); + exit(1); + } + } + errors = 0; for (i = 0; i < count; i++) { if (kvno(names[i], ccache, me, etype, keytab, sname, options, unknown, @@ -552,7 +560,7 @@ do_v5_kvno(int count, char *names[], char * ccachestr, char *etypestr, errors++; } else if (out_ccache != NULL) { if (!initialized) { - ret = krb5_cc_initialize(context, out_ccache, creds->client); + ret = krb5_cc_initialize(context, mcc, creds->client); if (ret) { com_err(prog, ret, _("while initializing output ccache")); exit(1); @@ -560,9 +568,9 @@ do_v5_kvno(int count, char *names[], char * ccachestr, char *etypestr, initialized = 1; } if (count == 1) - ret = k5_cc_store_primary_cred(context, out_ccache, creds); + ret = k5_cc_store_primary_cred(context, mcc, creds); else - ret = krb5_cc_store_cred(context, out_ccache, creds); + ret = krb5_cc_store_cred(context, mcc, creds); if (ret) { com_err(prog, ret, _("while storing creds in output ccache")); exit(1); @@ -572,6 +580,14 @@ do_v5_kvno(int count, char *names[], char * ccachestr, char *etypestr, krb5_free_creds(context, creds); } + if (!errors && out_ccache != NULL) { + ret = krb5_cc_move(context, mcc, out_ccache); + if (ret) { + com_err(prog, ret, _("while writing output ccache")); + exit(1); + } + } + if (keytab != NULL) krb5_kt_close(context, keytab); krb5_free_principal(context, me); diff --git a/src/include/kcm.h b/src/include/kcm.h index 85c20d3..d5d74aa 100644 --- a/src/include/kcm.h +++ b/src/include/kcm.h @@ -113,6 +113,8 @@ typedef enum kcm_opcode { /* MIT extensions */ KCM_OP_MIT_EXTENSION_BASE = 13000, KCM_OP_GET_CRED_LIST, /* (name) -> (count, count*{len, cred}) */ + KCM_OP_REPLACE, /* (name, offset, princ, + * count, count*{len, cred}) -> () */ } kcm_opcode; #endif /* KCM_H */ diff --git a/src/lib/krb5/ccache/cc-int.h b/src/lib/krb5/ccache/cc-int.h index 9fe6e1b..70f2827 100644 --- a/src/lib/krb5/ccache/cc-int.h +++ b/src/lib/krb5/ccache/cc-int.h @@ -51,6 +51,10 @@ krb5int_cc_initialize(void); void krb5int_cc_finalize(void); +krb5_error_code +k5_nonatomic_replace(krb5_context context, krb5_ccache ccache, + krb5_principal princ, krb5_creds **creds); + /* * Cursor for iterating over ccache types */ @@ -210,8 +214,8 @@ struct _krb5_cc_ops { krb5_ccache *); krb5_error_code (KRB5_CALLCONV *ptcursor_free)(krb5_context, krb5_cc_ptcursor *); - krb5_error_code (KRB5_CALLCONV *move)(krb5_context, krb5_ccache, - krb5_ccache); + krb5_error_code (KRB5_CALLCONV *replace)(krb5_context, krb5_ccache, + krb5_principal, krb5_creds **); krb5_error_code (KRB5_CALLCONV *wasdefault)(krb5_context, krb5_ccache, krb5_timestamp *); krb5_error_code (KRB5_CALLCONV *lock)(krb5_context, krb5_ccache); diff --git a/src/lib/krb5/ccache/cc_dir.c b/src/lib/krb5/ccache/cc_dir.c index 7b100a0..1da40b5 100644 --- a/src/lib/krb5/ccache/cc_dir.c +++ b/src/lib/krb5/ccache/cc_dir.c @@ -692,6 +692,15 @@ dcc_ptcursor_free(krb5_context context, krb5_cc_ptcursor *cursor) } static krb5_error_code KRB5_CALLCONV +dcc_replace(krb5_context context, krb5_ccache cache, krb5_principal princ, + krb5_creds **creds) +{ + dcc_data *data = cache->data; + + return krb5_fcc_ops.replace(context, data->fcc, princ, creds); +} + +static krb5_error_code KRB5_CALLCONV dcc_lock(krb5_context context, krb5_ccache cache) { dcc_data *data = cache->data; @@ -752,7 +761,7 @@ const krb5_cc_ops krb5_dcc_ops = { dcc_ptcursor_new, dcc_ptcursor_next, dcc_ptcursor_free, - NULL, /* move */ + dcc_replace, NULL, /* wasdefault */ dcc_lock, dcc_unlock, diff --git a/src/lib/krb5/ccache/cc_file.c b/src/lib/krb5/ccache/cc_file.c index 9a9b45a..c70a282 100644 --- a/src/lib/krb5/ccache/cc_file.c +++ b/src/lib/krb5/ccache/cc_file.c @@ -439,16 +439,40 @@ read_header(krb5_context context, FILE *fp, int *version_out) return 0; } +static void +marshal_header(krb5_context context, struct k5buf *buf, krb5_principal princ) +{ + krb5_os_context os_ctx = &context->os_context; + int version = context->fcc_default_format - FVNO_BASE; + uint16_t fields_len; + + version = context->fcc_default_format - FVNO_BASE; + k5_buf_add_uint16_be(buf, FVNO_BASE + version); + if (version >= 4) { + /* Add tagged header fields. */ + fields_len = 0; + if (os_ctx->os_flags & KRB5_OS_TOFFSET_VALID) + fields_len += 12; + k5_buf_add_uint16_be(buf, fields_len); + if (os_ctx->os_flags & KRB5_OS_TOFFSET_VALID) { + /* Add time offset tag. */ + k5_buf_add_uint16_be(buf, FCC_TAG_DELTATIME); + k5_buf_add_uint16_be(buf, 8); + k5_buf_add_uint32_be(buf, os_ctx->time_offset); + k5_buf_add_uint32_be(buf, os_ctx->usec_offset); + } + } + k5_marshal_princ(buf, version, princ); +} + /* Create or overwrite the cache file with a header and default principal. */ static krb5_error_code KRB5_CALLCONV fcc_initialize(krb5_context context, krb5_ccache id, krb5_principal princ) { krb5_error_code ret; - krb5_os_context os_ctx = &context->os_context; fcc_data *data = id->data; - uint16_t fields_len; ssize_t nwritten; - int st, flags, version, fd = -1; + int st, flags, fd = -1; struct k5buf buf = EMPTY_K5BUF; krb5_boolean file_locked = FALSE; @@ -482,23 +506,7 @@ fcc_initialize(krb5_context context, krb5_ccache id, krb5_principal princ) /* Prepare the header and principal in buf. */ k5_buf_init_dynamic(&buf); - version = context->fcc_default_format - FVNO_BASE; - k5_buf_add_uint16_be(&buf, FVNO_BASE + version); - if (version >= 4) { - /* Add tagged header fields. */ - fields_len = 0; - if (os_ctx->os_flags & KRB5_OS_TOFFSET_VALID) - fields_len += 12; - k5_buf_add_uint16_be(&buf, fields_len); - if (os_ctx->os_flags & KRB5_OS_TOFFSET_VALID) { - /* Add time offset tag. */ - k5_buf_add_uint16_be(&buf, FCC_TAG_DELTATIME); - k5_buf_add_uint16_be(&buf, 8); - k5_buf_add_uint32_be(&buf, os_ctx->time_offset); - k5_buf_add_uint32_be(&buf, os_ctx->usec_offset); - } - } - k5_marshal_princ(&buf, version, princ); + marshal_header(context, &buf, princ); ret = k5_buf_status(&buf); if (ret) goto cleanup; @@ -1260,6 +1268,64 @@ fcc_unlock(krb5_context context, krb5_ccache id) return 0; } +static krb5_error_code KRB5_CALLCONV +fcc_replace(krb5_context context, krb5_ccache id, krb5_principal princ, + krb5_creds **creds) +{ + krb5_error_code ret; + fcc_data *data = id->data; + char *tmpname = NULL; + int i, st, fd = -1, version = context->fcc_default_format - FVNO_BASE; + ssize_t nwritten; + struct k5buf buf = EMPTY_K5BUF; + krb5_boolean tmpfile_exists = FALSE; + + if (asprintf(&tmpname, "%s.XXXXXX", data->filename) < 0) + return ENOMEM; + fd = mkstemp(tmpname); + if (fd < 0) + goto errno_cleanup; + tmpfile_exists = TRUE; + + k5_buf_init_dynamic_zap(&buf); + marshal_header(context, &buf, princ); + for (i = 0; creds[i] != NULL; i++) + k5_marshal_cred(&buf, version, creds[i]); + ret = k5_buf_status(&buf); + if (ret) + goto cleanup; + + nwritten = write(fd, buf.data, buf.len); + if (nwritten == -1) + goto errno_cleanup; + if ((size_t)nwritten != buf.len) { + ret = KRB5_CC_IO; + goto cleanup; + } + st = close(fd); + fd = -1; + if (st != 0) + goto errno_cleanup; + + st = rename(tmpname, data->filename); + if (st != 0) + goto errno_cleanup; + tmpfile_exists = FALSE; + +cleanup: + k5_buf_free(&buf); + if (fd != -1) + close(fd); + if (tmpfile_exists) + unlink(tmpname); + free(tmpname); + return ret; + +errno_cleanup: + ret = interpret_errno(context, errno); + goto cleanup; +} + /* Translate a system errno value to a Kerberos com_err code. */ static krb5_error_code interpret_errno(krb5_context context, int errnum) @@ -1335,7 +1401,7 @@ const krb5_cc_ops krb5_fcc_ops = { fcc_ptcursor_new, fcc_ptcursor_next, fcc_ptcursor_free, - NULL, /* move */ + fcc_replace, NULL, /* wasdefault */ fcc_lock, fcc_unlock, @@ -1405,7 +1471,7 @@ const krb5_cc_ops krb5_cc_file_ops = { fcc_ptcursor_new, fcc_ptcursor_next, fcc_ptcursor_free, - NULL, /* move */ + fcc_replace, NULL, /* wasdefault */ fcc_lock, fcc_unlock, diff --git a/src/lib/krb5/ccache/cc_kcm.c b/src/lib/krb5/ccache/cc_kcm.c index 18505cd..204454d 100644 --- a/src/lib/krb5/ccache/cc_kcm.c +++ b/src/lib/krb5/ccache/cc_kcm.c @@ -1235,6 +1235,43 @@ kcm_ptcursor_free(krb5_context context, krb5_cc_ptcursor *cursor) } static krb5_error_code KRB5_CALLCONV +kcm_replace(krb5_context context, krb5_ccache cache, krb5_principal princ, + krb5_creds **creds) +{ + krb5_error_code ret; + struct kcmreq req = EMPTY_KCMREQ; + size_t pos; + uint8_t *lenptr; + int ncreds, i; + krb5_os_context octx = &context->os_context; + int32_t offset; + + kcmreq_init(&req, KCM_OP_REPLACE, cache); + offset = (octx->os_flags & KRB5_OS_TOFFSET_VALID) ? octx->time_offset : 0; + k5_buf_add_uint32_be(&req.reqbuf, offset); + k5_marshal_princ(&req.reqbuf, 4, princ); + for (ncreds = 0; creds[ncreds] != NULL; ncreds++); + k5_buf_add_uint32_be(&req.reqbuf, ncreds); + for (i = 0; creds[i] != NULL; i++) { + /* Store a dummy length, then fix it up after marshalling the cred. */ + pos = req.reqbuf.len; + k5_buf_add_uint32_be(&req.reqbuf, 0); + k5_marshal_cred(&req.reqbuf, 4, creds[i]); + if (k5_buf_status(&req.reqbuf) == 0) { + lenptr = (uint8_t *)req.reqbuf.data + pos; + store_32_be(req.reqbuf.len - (pos + 4), lenptr); + } + } + ret = cache_call(context, cache, &req); + kcmreq_free(&req); + + if (unsupported_op_error(ret)) + return k5_nonatomic_replace(context, cache, princ, creds); + + return ret; +} + +static krb5_error_code KRB5_CALLCONV kcm_lock(krb5_context context, krb5_ccache cache) { k5_cc_mutex_lock(context, &((struct kcm_cache_data *)cache->data)->lock); @@ -1281,7 +1318,7 @@ const krb5_cc_ops krb5_kcm_ops = { kcm_ptcursor_new, kcm_ptcursor_next, kcm_ptcursor_free, - NULL, /* move */ + kcm_replace, NULL, /* wasdefault */ kcm_lock, kcm_unlock, diff --git a/src/lib/krb5/ccache/cc_memory.c b/src/lib/krb5/ccache/cc_memory.c index 0897d6b..2df76ed 100644 --- a/src/lib/krb5/ccache/cc_memory.c +++ b/src/lib/krb5/ccache/cc_memory.c @@ -166,6 +166,45 @@ empty_mcc_cache(krb5_context context, krb5_mcc_data *d) d->prin = NULL; } +/* Remove all creds from d and initialize it with princ as the default client + * principal. The caller is responsible for locking. */ +static krb5_error_code +init_mcc_cache(krb5_context context, krb5_mcc_data *d, krb5_principal princ) +{ + krb5_os_context os_ctx = &context->os_context; + + empty_mcc_cache(context, d); + if (os_ctx->os_flags & KRB5_OS_TOFFSET_VALID) { + /* Store client time offsets in the cache. */ + d->time_offset = os_ctx->time_offset; + d->usec_offset = os_ctx->usec_offset; + } + return krb5_copy_principal(context, princ, &d->prin); +} + +/* Add cred to d. The caller is responsible for locking. */ +static krb5_error_code +store_cred(krb5_context context, krb5_mcc_data *d, krb5_creds *cred) +{ + krb5_error_code ret; + krb5_mcc_link *new_node; + + new_node = malloc(sizeof(*new_node)); + if (new_node == NULL) + return ENOMEM; + new_node->next = NULL; + ret = krb5_copy_creds(context, cred, &new_node->creds); + if (ret) { + free(new_node); + return ret; + } + + /* Place the new node at the tail of the list. */ + *d->tail = new_node; + d->tail = &new_node->next; + return 0; +} + /* * Modifies: * id @@ -180,21 +219,11 @@ empty_mcc_cache(krb5_context context, krb5_mcc_data *d) krb5_error_code KRB5_CALLCONV krb5_mcc_initialize(krb5_context context, krb5_ccache id, krb5_principal princ) { - krb5_os_context os_ctx = &context->os_context; krb5_error_code ret; krb5_mcc_data *d = id->data; k5_cc_mutex_lock(context, &d->lock); - empty_mcc_cache(context, d); - - ret = krb5_copy_principal(context, princ, &d->prin); - - if (os_ctx->os_flags & KRB5_OS_TOFFSET_VALID) { - /* Store client time offsets in the cache */ - d->time_offset = os_ctx->time_offset; - d->usec_offset = os_ctx->usec_offset; - } - + ret = init_mcc_cache(context, d, princ); k5_cc_mutex_unlock(context, &d->lock); if (ret == KRB5_OK) krb5_change_cache(); @@ -660,34 +689,19 @@ krb5_mcc_get_flags(krb5_context context, krb5_ccache id, krb5_flags *flags) * Save away creds in the ccache. * * Errors: - * system errors (mutex locking) * ENOMEM */ krb5_error_code KRB5_CALLCONV -krb5_mcc_store(krb5_context ctx, krb5_ccache id, krb5_creds *creds) +krb5_mcc_store(krb5_context context, krb5_ccache id, krb5_creds *creds) { - krb5_error_code err; - krb5_mcc_link *new_node; - krb5_mcc_data *mptr = (krb5_mcc_data *)id->data; - - new_node = malloc(sizeof(krb5_mcc_link)); - if (new_node == NULL) - return ENOMEM; - new_node->next = NULL; - err = krb5_copy_creds(ctx, creds, &new_node->creds); - if (err) - goto cleanup; + krb5_error_code ret; + krb5_mcc_data *d = id->data; /* Place the new node at the tail of the list. */ - k5_cc_mutex_lock(ctx, &mptr->lock); - *mptr->tail = new_node; - mptr->tail = &new_node->next; - k5_cc_mutex_unlock(ctx, &mptr->lock); - - return 0; -cleanup: - free(new_node); - return err; + k5_cc_mutex_lock(context, &d->lock); + ret = store_cred(context, d, creds); + k5_cc_mutex_unlock(context, &d->lock); + return ret; } static krb5_error_code KRB5_CALLCONV @@ -752,6 +766,24 @@ krb5_mcc_ptcursor_free( } static krb5_error_code KRB5_CALLCONV +krb5_mcc_replace(krb5_context context, krb5_ccache id, krb5_principal princ, + krb5_creds **creds) +{ + krb5_error_code ret; + krb5_mcc_data *d = id->data; + int i; + + k5_cc_mutex_lock(context, &d->lock); + ret = init_mcc_cache(context, d, princ); + for (i = 0; !ret && creds[i] != NULL; i++) + ret = store_cred(context, d, creds[i]); + k5_cc_mutex_unlock(context, &d->lock); + if (!ret) + krb5_change_cache(); + return ret; +} + +static krb5_error_code KRB5_CALLCONV krb5_mcc_lock(krb5_context context, krb5_ccache id) { krb5_mcc_data *data = (krb5_mcc_data *) id->data; @@ -790,7 +822,7 @@ const krb5_cc_ops krb5_mcc_ops = { krb5_mcc_ptcursor_new, krb5_mcc_ptcursor_next, krb5_mcc_ptcursor_free, - NULL, /* move */ + krb5_mcc_replace, NULL, /* wasdefault */ krb5_mcc_lock, krb5_mcc_unlock, diff --git a/src/lib/krb5/ccache/ccbase.c b/src/lib/krb5/ccache/ccbase.c index f53ba50..8d5bcef 100644 --- a/src/lib/krb5/ccache/ccbase.c +++ b/src/lib/krb5/ccache/ccbase.c @@ -348,50 +348,103 @@ krb5int_cc_typecursor_free(krb5_context context, krb5_cc_typecursor *t) return 0; } +krb5_error_code +k5_nonatomic_replace(krb5_context context, krb5_ccache ccache, + krb5_principal princ, krb5_creds **creds) +{ + krb5_error_code ret; + int i; + + ret = krb5_cc_initialize(context, ccache, princ); + for (i = 0; !ret && creds[i] != NULL; creds++) + ret = krb5_cc_store_cred(context, ccache, creds[i]); + return ret; +} + +static krb5_error_code +read_creds(krb5_context context, krb5_ccache ccache, krb5_creds ***creds_out) +{ + krb5_error_code ret; + krb5_cc_cursor cur = NULL; + krb5_creds **list = NULL, *cred = NULL, **newptr; + int i; + + *creds_out = NULL; + + ret = krb5_cc_start_seq_get(context, ccache, &cur); + if (ret) + goto cleanup; + + /* Allocate one extra entry so that list remains valid for freeing after + * we add the next entry and before we reallocate it. */ + list = k5calloc(2, sizeof(*list), &ret); + if (list == NULL) + goto cleanup; + + i = 0; + for (;;) { + cred = k5alloc(sizeof(*cred), &ret); + if (cred == NULL) + goto cleanup; + ret = krb5_cc_next_cred(context, ccache, &cur, cred); + if (ret == KRB5_CC_END) + break; + if (ret) + goto cleanup; + list[i++] = cred; + list[i] = NULL; + cred = NULL; + + newptr = realloc(list, (i + 2) * sizeof(*list)); + if (newptr == NULL) { + ret = ENOMEM; + goto cleanup; + } + list = newptr; + list[i + 1] = NULL; + } + ret = 0; + + *creds_out = list; + list = NULL; + +cleanup: + if (cur != NULL) + (void)krb5_cc_end_seq_get(context, ccache, &cur); + krb5_free_tgt_creds(context, list); + free(cred); + return ret; +} + krb5_error_code KRB5_CALLCONV krb5_cc_move(krb5_context context, krb5_ccache src, krb5_ccache dst) { - krb5_error_code ret = 0; + krb5_error_code ret; krb5_principal princ = NULL; + krb5_creds **creds = NULL; TRACE_CC_MOVE(context, src, dst); - ret = k5_cccol_lock(context); - if (ret) { - return ret; - } - - ret = k5_cc_lock(context, src); - if (ret) { - k5_cccol_unlock(context); - return ret; - } ret = krb5_cc_get_principal(context, src, &princ); - if (!ret) { - ret = krb5_cc_initialize(context, dst, princ); - } - if (ret) { - k5_cc_unlock(context, src); - k5_cccol_unlock(context); - return ret; - } + if (ret) + goto cleanup; - ret = k5_cc_lock(context, dst); - if (!ret) { - ret = krb5_cc_copy_creds(context, src, dst); - k5_cc_unlock(context, dst); - } + ret = read_creds(context, src, &creds); + if (ret) + goto cleanup; - k5_cc_unlock(context, src); - if (!ret) { - ret = krb5_cc_destroy(context, src); - } - k5_cccol_unlock(context); - if (princ) { - krb5_free_principal(context, princ); - princ = NULL; - } + if (dst->ops->replace == NULL) + ret = k5_nonatomic_replace(context, dst, princ, creds); + else + ret = dst->ops->replace(context, dst, princ, creds); + if (ret) + goto cleanup; + + ret = krb5_cc_destroy(context, src); +cleanup: + krb5_free_principal(context, princ); + krb5_free_tgt_creds(context, creds); return ret; } diff --git a/src/lib/krb5/krb/get_in_tkt.c b/src/lib/krb5/krb/get_in_tkt.c index b28b38a..4195a55 100644 --- a/src/lib/krb5/krb/get_in_tkt.c +++ b/src/lib/krb5/krb/get_in_tkt.c @@ -1604,6 +1604,61 @@ warn_des3(krb5_context context, krb5_init_creds_context ctx, (*ctx->prompter)(context, ctx->prompter_data, NULL, banner, 0, NULL); } +/* + * If ctx specifies an output ccache, create or refresh it (atomically, if + * possible) with the obtained credential and any appropriate ccache + * configuration. + */ +static krb5_error_code +write_out_ccache(krb5_context context, krb5_init_creds_context ctx, + krb5_boolean fast_avail) +{ + krb5_error_code ret; + krb5_ccache out_ccache = k5_gic_opt_get_out_ccache(ctx->opt); + krb5_ccache mcc = NULL; + krb5_data yes = string2data("yes"); + + if (out_ccache == NULL) + return 0; + + ret = krb5_cc_new_unique(context, "MEMORY", NULL, &mcc); + if (ret) + goto cleanup; + + ret = krb5_cc_initialize(context, mcc, ctx->cred.client); + if (ret) + goto cleanup; + + if (fast_avail) { + ret = krb5_cc_set_config(context, mcc, ctx->cred.server, + KRB5_CC_CONF_FAST_AVAIL, &yes); + if (ret) + goto cleanup; + } + + ret = save_selected_preauth_type(context, mcc, ctx); + if (ret) + goto cleanup; + + ret = save_cc_config_out_data(context, mcc, ctx); + if (ret) + goto cleanup; + + ret = k5_cc_store_primary_cred(context, mcc, &ctx->cred); + if (ret) + goto cleanup; + + ret = krb5_cc_move(context, mcc, out_ccache); + if (ret) + goto cleanup; + mcc = NULL; + +cleanup: + if (mcc != NULL) + krb5_cc_destroy(context, mcc); + return ret; +} + static krb5_error_code init_creds_step_reply(krb5_context context, krb5_init_creds_context ctx, @@ -1618,7 +1673,6 @@ init_creds_step_reply(krb5_context context, krb5_keyblock *strengthen_key = NULL; krb5_keyblock encrypting_key; krb5_boolean fast_avail; - krb5_ccache out_ccache = k5_gic_opt_get_out_ccache(ctx->opt); encrypting_key.length = 0; encrypting_key.contents = NULL; @@ -1786,30 +1840,9 @@ init_creds_step_reply(krb5_context context, code = stash_as_reply(context, ctx->reply, &ctx->cred, NULL); if (code != 0) goto cleanup; - if (out_ccache != NULL) { - krb5_data config_data; - code = krb5_cc_initialize(context, out_ccache, ctx->cred.client); - if (code != 0) - goto cc_cleanup; - code = k5_cc_store_primary_cred(context, out_ccache, &ctx->cred); - if (code != 0) - goto cc_cleanup; - if (fast_avail) { - config_data.data = "yes"; - config_data.length = strlen(config_data.data); - code = krb5_cc_set_config(context, out_ccache, ctx->cred.server, - KRB5_CC_CONF_FAST_AVAIL, &config_data); - if (code != 0) - goto cc_cleanup; - } - code = save_selected_preauth_type(context, out_ccache, ctx); - if (code != 0) - goto cc_cleanup; - code = save_cc_config_out_data(context, out_ccache, ctx); - cc_cleanup: - if (code != 0) - k5_prependmsg(context, code, _("Failed to store credentials")); - } + code = write_out_ccache(context, ctx, fast_avail); + if (code) + k5_prependmsg(context, code, _("Failed to store credentials")); k5_preauth_request_context_fini(context, ctx); diff --git a/src/lib/krb5/krb/t_vfy_increds.c b/src/lib/krb5/krb/t_vfy_increds.c index 693eb37..00a6b03 100644 --- a/src/lib/krb5/krb/t_vfy_increds.c +++ b/src/lib/krb5/krb/t_vfy_increds.c @@ -26,9 +26,9 @@ /* * This program is intended to be run from t_vfy_increds.py. It retrieves the - * first credential from the default ccache and verifies it against the default - * keytab, exiting with status 0 on successful verification and 1 on - * unsuccessful verification. + * first non-config credential from the default ccache and verifies it against + * the default keytab, exiting with status 0 on successful verification and 1 + * on unsuccessful verification. */ #include "k5-int.h" @@ -64,10 +64,15 @@ main(int argc, char **argv) if (*argv != NULL) check(krb5_parse_name(context, *argv, &princ)); - /* Fetch the first credential from the default ccache. */ + /* Fetch the first non-config credential from the default ccache. */ check(krb5_cc_default(context, &ccache)); check(krb5_cc_start_seq_get(context, ccache, &cursor)); - check(krb5_cc_next_cred(context, ccache, &cursor, &creds)); + for (;;) { + check(krb5_cc_next_cred(context, ccache, &cursor, &creds)); + if (!krb5_is_config_principal(context, creds.server)) + break; + krb5_free_cred_contents(context, &creds); + } check(krb5_cc_end_seq_get(context, ccache, &cursor)); check(krb5_cc_close(context, ccache)); diff --git a/src/tests/Makefile.in b/src/tests/Makefile.in index 1a19383..e7cf64e 100644 --- a/src/tests/Makefile.in +++ b/src/tests/Makefile.in @@ -6,13 +6,13 @@ SUBDIRS = asn.1 create hammer verify gssapi shlib gss-threads misc threads \ RUN_DB_TEST = $(RUN_SETUP) KRB5_KDC_PROFILE=kdc.conf KRB5_CONFIG=krb5.conf \ GSS_MECH_CONFIG=mech.conf LC_ALL=C $(VALGRIND) -OBJS= adata.o etinfo.o forward.o gcred.o hist.o hooks.o hrealm.o \ +OBJS= adata.o conccache.o etinfo.o forward.o gcred.o hist.o hooks.o hrealm.o \ icinterleave.o icred.o kdbtest.o localauth.o plugorder.o rdreq.o \ replay.o responder.o s2p.o s4u2self.o s4u2proxy.o t_inetd.o \ unlockiter.o -EXTRADEPSRCS= adata.c etinfo.c forward.c gcred.c hist.c hooks.c hrealm.c \ - icinterleave.c icred.c kdbtest.c localauth.c plugorder.c rdreq.c \ - replay.c responder.c s2p.c s4u2self.c s4u2proxy.c t_inetd.c \ +EXTRADEPSRCS= adata.c conccache.c etinfo.c forward.c gcred.c hist.c hooks.c \ + hrealm.c icinterleave.c icred.c kdbtest.c localauth.c plugorder.c \ + rdreq.c replay.c responder.c s2p.c s4u2self.c s4u2proxy.c t_inetd.c \ unlockiter.c TEST_DB = ./testdb @@ -28,6 +28,9 @@ KTEST_OPTS= $(KADMIN_OPTS) -p $(TEST_PREFIX) -n $(TEST_NUM) -D $(TEST_DEPTH) adata: adata.o $(KRB5_BASE_DEPLIBS) $(CC_LINK) -o $@ adata.o $(KRB5_BASE_LIBS) +conccache: conccache.o $(KRB5_BASE_DEPLIBS) + $(CC_LINK) -o $@ conccache.o $(KRB5_BASE_LIBS) + etinfo: etinfo.o $(KRB5_BASE_DEPLIBS) $(CC_LINK) -o $@ etinfo.o $(KRB5_BASE_LIBS) @@ -125,9 +128,9 @@ kdb_check: kdc.conf krb5.conf $(RUN_DB_TEST) ../kadmin/dbutil/kdb5_util $(KADMIN_OPTS) destroy -f $(RM) $(TEST_DB)* stash_file -check-pytests: adata etinfo forward gcred hist hooks hrealm icinterleave icred -check-pytests: kdbtest localauth plugorder rdreq replay responder s2p s4u2proxy -check-pytests: unlockiter s4u2self +check-pytests: adata conccache etinfo forward gcred hist hooks hrealm +check-pytests: icinterleave icred kdbtest localauth plugorder rdreq replay +check-pytests: responder s2p s4u2proxy unlockiter s4u2self $(RUNPYTEST) $(srcdir)/t_general.py $(PYTESTFLAGS) $(RUNPYTEST) $(srcdir)/t_hooks.py $(PYTESTFLAGS) $(RUNPYTEST) $(srcdir)/t_dump.py $(PYTESTFLAGS) @@ -190,9 +193,9 @@ check-pytests: unlockiter s4u2self $(RUNPYTEST) $(srcdir)/t_replay.py $(PYTESTFLAGS) clean: - $(RM) adata etinfo forward gcred hist hooks hrealm icinterleave icred - $(RM) kdbtest localauth plugorder rdreq replay responder s2p s4u2proxy - $(RM) s4u2self t_inetd unlockiter + $(RM) adata conccache etinfo forward gcred hist hooks hrealm + $(RM) icinterleave icred kdbtest localauth plugorder rdreq replay + $(RM) responder s2p s4u2proxy s4u2self t_inetd unlockiter $(RM) krb5.conf kdc.conf $(RM) -rf kdc_realm/sandbox ldap $(RM) au.log diff --git a/src/tests/conccache.c b/src/tests/conccache.c new file mode 100644 index 0000000..7b0ca63 --- /dev/null +++ b/src/tests/conccache.c @@ -0,0 +1,190 @@ +/* -*- mode: c; c-basic-offset: 4; indent-tabs-mode: nil -*- */ +/* tests/conccache.c - ccache concurrent get_creds/refresh test program */ +/* + * Copyright (C) 2021 by the Massachusetts Institute of Technology. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE + * COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED + * OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +/* + * Usage: conccache ccname clientprinc serverprinc + * + * This program spawns two subprocesses. One repeatedly runs + * krb5_get_credentials() on ccname, and the other repeatedly refreshes ccname + * from the default keytab. If either subprocess fails, the program exits with + * status 1. The goal is to expose time windows where cache refreshes cause + * get_cred operations to fail. + */ + +#include "k5-platform.h" +#include <sys/types.h> +#include <sys/wait.h> +#include <pthread.h> +#include <krb5.h> + +/* Run this many iterations of each operation. */ +static const int iterations = 200; + +/* Saved command-line arguments. */ +static const char *ccname, *server_name, *client_name; + +static void +check(krb5_error_code code) +{ + if (code) + abort(); +} + +static krb5_boolean +get_cred(krb5_context context) +{ + krb5_error_code ret; + krb5_ccache cc; + krb5_principal client, server; + krb5_creds mcred, *cred; + + check(krb5_cc_resolve(context, ccname, &cc)); + check(krb5_parse_name(context, client_name, &client)); + check(krb5_parse_name(context, server_name, &server)); + + memset(&mcred, 0, sizeof(mcred)); + mcred.client = client; + mcred.server = server; + ret = krb5_get_credentials(context, 0, cc, &mcred, &cred); + + krb5_free_creds(context, cred); + krb5_free_principal(context, client); + krb5_free_principal(context, server); + krb5_cc_close(context, cc); + + return ret == 0; +} + +static krb5_boolean +refresh_cache(krb5_context context) +{ + krb5_error_code ret; + krb5_ccache cc; + krb5_principal client; + krb5_get_init_creds_opt *opt; + krb5_creds cred; + + check(krb5_cc_resolve(context, ccname, &cc)); + check(krb5_parse_name(context, client_name, &client)); + + check(krb5_get_init_creds_opt_alloc(context, &opt)); + check(krb5_get_init_creds_opt_set_out_ccache(context, opt, cc)); + ret = krb5_get_init_creds_keytab(context, &cred, client, NULL, 0, NULL, + opt); + + krb5_get_init_creds_opt_free(context, opt); + krb5_free_cred_contents(context, &cred); + krb5_free_principal(context, client); + krb5_cc_close(context, cc); + + return ret == 0; +} + +static pid_t +spawn_cred_subprocess() +{ + krb5_context context; + pid_t pid; + int i; + + pid = fork(); + assert(pid >= 0); + if (pid > 0) + return pid; + + check(krb5_init_context(&context)); + for (i = 0; i < iterations; i++) { + if (!get_cred(context)) { + fprintf(stderr, "cred worker failed after %d successes\n", i); + exit(1); + } + } + krb5_free_context(context); + exit(0); +} + +static pid_t +spawn_refresh_subprocess() +{ + krb5_context context; + pid_t pid; + int i; + + pid = fork(); + assert(pid >= 0); + if (pid > 0) + return pid; + + check(krb5_init_context(&context)); + for (i = 0; i < iterations; i++) { + if (!refresh_cache(context)) { + fprintf(stderr, "refresh worker failed after %d successes\n", i); + exit(1); + } + } + krb5_free_context(context); + exit(0); +} + +int +main(int argc, char *argv[]) +{ + krb5_context context; + pid_t cred_pid, refresh_pid, pid; + int cstatus, rstatus; + + assert(argc == 4); + ccname = argv[1]; + client_name = argv[2]; + server_name = argv[3]; + + /* Begin with an initialized cache. */ + check(krb5_init_context(&context)); + refresh_cache(context); + krb5_free_context(context); + + cred_pid = spawn_cred_subprocess(); + refresh_pid = spawn_refresh_subprocess(); + + pid = waitpid(cred_pid, &cstatus, 0); + if (pid == -1) + abort(); + pid = waitpid(refresh_pid, &rstatus, 0); + if (pid == -1) + abort(); + + if (!WIFEXITED(cstatus) || WEXITSTATUS(cstatus) != 0) + return 1; + if (!WIFEXITED(rstatus) || WEXITSTATUS(rstatus) != 0) + return 1; + return 0; +} diff --git a/src/tests/kcmserver.py b/src/tests/kcmserver.py index 25e6f2b..f6dfcb7 100644 --- a/src/tests/kcmserver.py +++ b/src/tests/kcmserver.py @@ -52,6 +52,7 @@ class KCMOpcodes(object): GET_KDC_OFFSET = 22 SET_KDC_OFFSET = 23 GET_CRED_LIST = 13001 + REPLACE = 13002 class KRB5Errors(object): @@ -88,27 +89,28 @@ def get_cache(name): return cache -def unpack_data(argbytes): - dlen, = struct.unpack('>L', argbytes[:4]) - return argbytes[4:dlen+4], argbytes[dlen+4:] - - def unmarshal_name(argbytes): offset = argbytes.find(b'\0') return argbytes[0:offset], argbytes[offset+1:] -def unmarshal_princ(argbytes): - # Ignore the type at argbytes[0:4]. - ncomps, = struct.unpack('>L', argbytes[4:8]) - realm, rest = unpack_data(argbytes[8:]) - comps = [] +# Find the bounds of a marshalled principal, returning it and the +# remainder of argbytes. +def extract_princ(argbytes): + ncomps, rlen = struct.unpack('>LL', argbytes[4:12]) + pos = 12 + rlen for i in range(ncomps): - comp, rest = unpack_data(rest) - comps.append(comp) - # Asssume no quoting is needed. - princ = b'/'.join(comps) + b'@' + realm - return princ, rest + clen, = struct.unpack('>L', argbytes[pos:pos+4]) + pos += 4 + clen + return argbytes[0:pos], argbytes[pos:] + + +# Return true if the marshalled principals p1 and p2 name the same +# principal. +def princ_eq(p1, p2): + # Ignore the name-types at bytes 0..3. The remaining bytes should + # be identical if the principals are the same. + return p1[4:] == p2[4:] def op_gen_new(argbytes): @@ -151,13 +153,13 @@ def op_retrieve(argbytes): # Ignore the flags at rest[0:4] and the header at rest[4:8]. # Assume there are client and server creds in the tag and match # only against them. - cprinc, rest = unmarshal_princ(rest[8:]) - sprinc, rest = unmarshal_princ(rest) + cprinc, rest = extract_princ(rest[8:]) + sprinc, rest = extract_princ(rest) cache = get_cache(name) for cred in (cache.creds[u] for u in cache.cred_uuids): - cred_cprinc, rest = unmarshal_princ(cred) - cred_sprinc, rest = unmarshal_princ(rest) - if cred_cprinc == cprinc and cred_sprinc == sprinc: + cred_cprinc, rest = extract_princ(cred) + cred_sprinc, rest = extract_princ(rest) + if princ_eq(cred_cprinc, cprinc) and princ_eq(cred_sprinc, sprinc): return 0, cred return KRB5Errors.KRB5_CC_NOTFOUND, b'' @@ -230,6 +232,31 @@ def op_get_cred_list(argbytes): b''.join(struct.pack('>L', len(c)) + c for c in creds)) +def op_replace(argbytes): + name, rest = unmarshal_name(argbytes) + offset, = struct.unpack('>L', rest[0:4]) + princ, rest = extract_princ(rest[4:]) + ncreds, = struct.unpack('>L', rest[0:4]) + rest = rest[4:] + creds = [] + for i in range(ncreds): + len, = struct.unpack('>L', rest[0:4]) + creds.append(rest[4:4+len]) + rest = rest[4+len:] + + cache = get_cache(name) + cache.princ = princ + cache.cred_uuids = [] + cache.creds = {} + cache.time_offset = offset + for i in range(ncreds): + uuid = make_uuid() + cache.creds[uuid] = creds[i] + cache.cred_uuids.append(uuid) + + return 0, b'' + + ophandlers = { KCMOpcodes.GEN_NEW : op_gen_new, KCMOpcodes.INITIALIZE : op_initialize, @@ -246,7 +273,8 @@ ophandlers = { KCMOpcodes.SET_DEFAULT_CACHE : op_set_default_cache, KCMOpcodes.GET_KDC_OFFSET : op_get_kdc_offset, KCMOpcodes.SET_KDC_OFFSET : op_set_kdc_offset, - KCMOpcodes.GET_CRED_LIST : op_get_cred_list + KCMOpcodes.GET_CRED_LIST : op_get_cred_list, + KCMOpcodes.REPLACE : op_replace } # Read and respond to a request from the socket s. @@ -281,11 +309,13 @@ def service_request(s): parser = optparse.OptionParser() parser.add_option('-f', '--fallback', action='store_true', dest='fallback', - default=False, help='Do not support RETRIEVE/GET_CRED_LIST') + default=False, + help='Do not support RETRIEVE/GET_CRED_LIST/REPLACE') (options, args) = parser.parse_args() if options.fallback: del ophandlers[KCMOpcodes.RETRIEVE] del ophandlers[KCMOpcodes.GET_CRED_LIST] + del ophandlers[KCMOpcodes.REPLACE] server = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) server.bind(args[0]) diff --git a/src/tests/t_ccache.py b/src/tests/t_ccache.py index 9e005ec..9371a0c 100755 --- a/src/tests/t_ccache.py +++ b/src/tests/t_ccache.py @@ -29,6 +29,11 @@ conf = {'libdefaults': {'kcm_socket': kcm_socket_path, 'kcm_mach_service': '-'}} realm = K5Realm(krb5_conf=conf) +realm.addprinc('contest') +realm.extract_keytab('contest', realm.keytab) +realm.run(['./conccache', realm.ccache + '.contest', 'contest', + realm.host_princ]) + keyctl = which('keyctl') out = realm.run([klist, '-c', 'KEYRING:process:abcd'], expected_code=1) test_keyring = (keyctl is not None and