Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Improve gss_store_cred() behavior 
Select an output credential cache using similar logic to kinit.  Do
not require the target cache to be initialized.

Try to use the per-thread cache set by gss_krb5_ccache_name() if no
output cache was specified via a cred store.

When the destination is a collection, honor the default_cred flag by
switching the primary cache to the selected output cache.  When the
destination is not a collection, ignore the default_cred flag.
(Previously the default_cred flag was mandatory for gss_store_cred()
even though it is an advisory flag, and ignored for
gss_store_cred_into() even if no ccache was specified in the cred
store.)

Honor the overwrite_cred flag by refusing to replace an initialized
cache if it is not set.  Stop using gss_acquire_cred() for this
purpose as it could go out and fetch credentials from a client keytab.

Perform atomic replacement of the target cache when possible, using
krb5_cc_move().

Add a test harness for calling gss_store_cred() or
gss_store_cred_into() and a suite of tests.  Fix a broken trace log
message for krb5_cc_move() and update the expected trace logs for an
existing t_credstore.py test.

ticket: 8010
  • Loading branch information
@greghudson
greghudson committed Sep 14, 2021
1 parent 789a4d3 commit 3f5a348
Show file tree
Hide file tree
Showing 8 changed files with 285 additions and 90 deletions.
1 change: 1 addition & 0 deletions .gitignore
View file
Expand Up @@ -470,6 +470,7 @@ local.properties
/src/tests/gssapi/t_saslname
/src/tests/gssapi/t_spnego
/src/tests/gssapi/t_srcattrs
/src/tests/gssapi/t_store_cred
/src/tests/gssapi/t_inq_ctx

/src/tests/hammer/kdc5_hammer
Expand Down
15 changes: 11 additions & 4 deletions doc/appdev/gssapi.rst
View file
Expand Up @@ -252,10 +252,8 @@ The following options are supported by the krb5 mechanism:

* **ccache**: For acquiring initiator credentials, the name of the
:ref:`credential cache <ccache_definition>` to which the handle will
refer. For storing credentials, the name of the cache where the
credentials should be stored. If a collection name is given, the
primary cache of the collection will be used; this behavior may
change in future releases to select a cache from the collection.
refer. For storing credentials, the name of the cache or collection
where the credentials will be stored (see below).

* **client_keytab**: For acquiring initiator credentials, the name of
the :ref:`keytab <keytab_definition>` which will be used, if
Expand Down Expand Up @@ -285,6 +283,15 @@ The following options are supported by the krb5 mechanism:
the empty string. If the empty string is given, any ``host``
service principal in the keytab may be used. (New in release 1.19.)

In release 1.20 or later, if a collection name is specified for
**cache** in a call to gss_store_cred_into(), an existing cache for
the client principal within the collection will be selected, or a new
cache will be created within the collection. If *overwrite_cred* is
false and the selected credential cache already exists, a
**GSS_S_DUPLICATE_ELEMENT** error will be returned. If *default_cred*
is true, the primary cache of the collection will be switched to the
selected cache.


Importing and exporting credentials
-----------------------------------
Expand Down
2 changes: 1 addition & 1 deletion src/include/k5-trace.h
View file
Expand Up @@ -119,7 +119,7 @@ void krb5int_trace(krb5_context context, const char *fmt, ...);
TRACE(c, "Initializing {ccache} with default princ {princ}", \
cache, princ)
#define TRACE_CC_MOVE(c, src, dst) \
TRACE(c, "Moving contents of ccache {src} to {dst}", src, dst)
TRACE(c, "Moving ccache {ccache} to {ccache}", src, dst)
#define TRACE_CC_NEW_UNIQUE(c, type) \
TRACE(c, "Resolving unique ccache of type {str}", type)
#define TRACE_CC_REMOVE(c, cache, creds) \
Expand Down
145 changes: 67 additions & 78 deletions src/lib/gssapi/krb5/store_cred.c
View file
Expand Up @@ -27,36 +27,6 @@
#include "k5-int.h"
#include "gssapiP_krb5.h"

static int
has_unexpired_creds(krb5_gss_cred_id_t kcred,
const gss_OID desired_mech,
int default_cred,
gss_const_key_value_set_t cred_store)
{
OM_uint32 major_status, minor;
gss_name_t cred_name;
gss_OID_set_desc desired_mechs;
gss_cred_id_t tmp_cred = GSS_C_NO_CREDENTIAL;
OM_uint32 time_rec;

desired_mechs.count = 1;
desired_mechs.elements = (gss_OID)desired_mech;

if (default_cred)
cred_name = GSS_C_NO_NAME;
else
cred_name = (gss_name_t)kcred->name;

major_status = krb5_gss_acquire_cred_from(&minor, cred_name, 0,
&desired_mechs, GSS_C_INITIATE,
cred_store, &tmp_cred, NULL,
&time_rec);

krb5_gss_release_cred(&minor, &tmp_cred);

return (GSS_ERROR(major_status) || time_rec);
}

static OM_uint32
copy_initiator_creds(OM_uint32 *minor_status,
gss_cred_id_t input_cred_handle,
Expand All @@ -66,26 +36,19 @@ copy_initiator_creds(OM_uint32 *minor_status,
gss_const_key_value_set_t cred_store)
{
OM_uint32 major_status;
krb5_error_code code;
krb5_error_code ret;
krb5_gss_cred_id_t kcred = NULL;
krb5_context context = NULL;
krb5_ccache ccache = NULL;
const char *ccache_name;
krb5_ccache cache = NULL, defcache = NULL, mcc = NULL;
krb5_principal princ = NULL;
krb5_boolean switch_to_cache = FALSE;
const char *ccache_name, *deftype;

*minor_status = 0;

if (!default_cred && cred_store == GSS_C_NO_CRED_STORE) {
*minor_status = G_STORE_NON_DEFAULT_CRED_NOSUPP;
major_status = GSS_S_FAILURE;
goto cleanup;
}

code = krb5_gss_init_context(&context);
if (code != 0) {
*minor_status = code;
major_status = GSS_S_FAILURE;
goto cleanup;
}
ret = krb5_gss_init_context(&context);
if (ret)
goto kerr_cleanup;

major_status = krb5_gss_validate_cred_1(minor_status,
input_cred_handle,
Expand All @@ -101,52 +64,69 @@ copy_initiator_creds(OM_uint32 *minor_status,
goto cleanup;
}

if (!overwrite_cred &&
has_unexpired_creds(kcred, desired_mech, default_cred, cred_store)) {
major_status = GSS_S_DUPLICATE_ELEMENT;
goto cleanup;
}

major_status = kg_value_from_cred_store(cred_store,
KRB5_CS_CCACHE_URN, &ccache_name);
if (GSS_ERROR(major_status))
goto cleanup;

if (ccache_name != NULL) {
code = krb5_cc_resolve(context, ccache_name, &ccache);
if (code != 0) {
*minor_status = code;
major_status = GSS_S_FAILURE;
ret = krb5_cc_set_default_name(context, ccache_name);
if (ret)
goto kerr_cleanup;
} else {
major_status = kg_sync_ccache_name(context, minor_status);
if (major_status != GSS_S_COMPLETE)
goto cleanup;
}
code = krb5_cc_initialize(context, ccache,
kcred->name->princ);
if (code != 0) {
*minor_status = code;
major_status = GSS_S_FAILURE;
goto cleanup;
}
}

if (ccache == NULL) {
if (!default_cred) {
*minor_status = G_STORE_NON_DEFAULT_CRED_NOSUPP;
major_status = GSS_S_FAILURE;
/* Resolve the default ccache and get its type. */
ret = krb5_cc_default(context, &defcache);
if (ret)
goto kerr_cleanup;
deftype = krb5_cc_get_type(context, defcache);

if (krb5_cc_support_switch(context, deftype)) {
/* Use an existing or new cache within the collection. */
ret = krb5_cc_cache_match(context, kcred->name->princ, &cache);
if (!ret && !overwrite_cred) {
major_status = GSS_S_DUPLICATE_ELEMENT;
goto cleanup;
}
code = krb5int_cc_default(context, &ccache);
if (code != 0) {
*minor_status = code;
major_status = GSS_S_FAILURE;
if (ret == KRB5_CC_NOTFOUND)
ret = krb5_cc_new_unique(context, deftype, NULL, &cache);
if (ret)
goto kerr_cleanup;
switch_to_cache = default_cred;
} else {
/* Use the default cache. */
cache = defcache;
defcache = NULL;
ret = krb5_cc_get_principal(context, cache, &princ);
krb5_free_principal(context, princ);
if (!ret && !overwrite_cred) {
major_status = GSS_S_DUPLICATE_ELEMENT;
goto cleanup;
}
}

code = krb5_cc_copy_creds(context, kcred->ccache, ccache);
if (code != 0) {
*minor_status = code;
major_status = GSS_S_FAILURE;
goto cleanup;
ret = krb5_cc_new_unique(context, "MEMORY", NULL, &mcc);
if (ret)
goto kerr_cleanup;
ret = krb5_cc_initialize(context, mcc, kcred->name->princ);
if (ret)
goto kerr_cleanup;
ret = krb5_cc_copy_creds(context, kcred->ccache, mcc);
if (ret)
goto kerr_cleanup;
ret = krb5_cc_move(context, mcc, cache);
if (ret)
goto kerr_cleanup;
mcc = NULL;

if (switch_to_cache) {
ret = krb5_cc_switch(context, cache);
if (ret)
goto kerr_cleanup;
}

*minor_status = 0;
Expand All @@ -155,11 +135,20 @@ copy_initiator_creds(OM_uint32 *minor_status,
cleanup:
if (kcred != NULL)
k5_mutex_unlock(&kcred->lock);
if (ccache != NULL)
krb5_cc_close(context, ccache);
if (defcache != NULL)
krb5_cc_close(context, defcache);
if (cache != NULL)
krb5_cc_close(context, cache);
if (mcc != NULL)
krb5_cc_destroy(context, mcc);
krb5_free_context(context);

return major_status;

kerr_cleanup:
*minor_status = ret;
major_status = GSS_S_FAILURE;
goto cleanup;
}

OM_uint32 KRB5_CALLCONV
Expand Down
14 changes: 9 additions & 5 deletions src/tests/gssapi/Makefile.in
View file
Expand Up @@ -19,15 +19,15 @@ SRCS= $(srcdir)/ccinit.c $(srcdir)/ccrefresh.c $(srcdir)/common.c \
$(srcdir)/t_lifetime.c $(srcdir)/t_namingexts.c $(srcdir)/t_oid.c \
$(srcdir)/t_pcontok.c $(srcdir)/t_prf.c $(srcdir)/t_s4u.c \
$(srcdir)/t_s4u2proxy_krb5.c $(srcdir)/t_saslname.c \
$(srcdir)/t_spnego.c $(srcdir)/t_srcattrs.c
$(srcdir)/t_spnego.c $(srcdir)/t_srcattrs.c $(srcdir)/t_store_cred.c

OBJS= ccinit.o ccrefresh.o common.o reload.o t_accname.o t_add_cred.o \
t_bindings.o t_ccselect.o t_ciflags.o t_context.o t_credstore.o \
t_enctypes.o t_err.o t_export_cred.o t_export_name.o t_gssexts.o \
t_imp_cred.o t_imp_name.o t_invalid.o t_inq_cred.o t_inq_ctx.o \
t_inq_mechs_name.o t_iov.o t_lifetime.o t_namingexts.o t_oid.o \
t_pcontok.o t_prf.o t_s4u.o t_s4u2proxy_krb5.o t_saslname.o \
t_spnego.o t_srcattrs.o
t_spnego.o t_srcattrs.o t_store_cred.o

COMMON_DEPS= common.o $(GSS_DEPLIBS) $(KRB5_BASE_DEPLIBS)
COMMON_LIBS= common.o $(GSS_LIBS) $(KRB5_BASE_LIBS)
Expand All @@ -36,7 +36,7 @@ all: ccinit ccrefresh t_accname t_add_cred t_bindings t_ccselect t_ciflags \
t_context t_credstore t_enctypes t_err t_export_cred t_export_name \
t_gssexts t_imp_cred t_imp_name t_invalid t_inq_cred t_inq_ctx \
t_inq_mechs_name t_iov t_lifetime t_namingexts t_oid t_pcontok t_prf \
t_s4u t_s4u2proxy_krb5 t_saslname t_spnego t_srcattrs
t_s4u t_s4u2proxy_krb5 t_saslname t_spnego t_srcattrs t_store_cred

check-unix: t_oid reload
$(RUN_TEST) ./t_invalid
Expand All @@ -48,8 +48,10 @@ check-unix: t_oid reload
check-pytests: ccinit ccrefresh t_accname t_add_cred t_bindings t_ccselect \
t_ciflags t_context t_credstore t_enctypes t_err t_export_cred \
t_export_name t_imp_cred t_inq_cred t_inq_ctx t_inq_mechs_name t_iov \
t_lifetime t_pcontok t_s4u t_s4u2proxy_krb5 t_spnego t_srcattrs
t_lifetime t_pcontok t_s4u t_s4u2proxy_krb5 t_spnego t_srcattrs \
t_store_cred
$(RUNPYTEST) $(srcdir)/t_gssapi.py $(PYTESTFLAGS)
$(RUNPYTEST) $(srcdir)/t_store_cred.py $(PYTESTFLAGS)
$(RUNPYTEST) $(srcdir)/t_credstore.py $(PYTESTFLAGS)
$(RUNPYTEST) $(srcdir)/t_bindings.py $(PYTESTFLAGS)
$(RUNPYTEST) $(srcdir)/t_ccselect.py $(PYTESTFLAGS)
Expand Down Expand Up @@ -124,11 +126,13 @@ t_spnego: t_spnego.o $(COMMON_DEPS)
$(CC_LINK) -o $@ t_spnego.o $(COMMON_LIBS)
t_srcattrs: t_srcattrs.o $(COMMON_DEPS)
$(CC_LINK) -o $@ t_srcattrs.o $(COMMON_LIBS)
t_store_cred: t_store_cred.o $(COMMON_DEPS)
$(CC_LINK) -o $@ t_store_cred.o $(COMMON_LIBS)

clean:
$(RM) ccinit ccrefresh reload t_accname t_add_cred t_bindings
$(RM) t_ccselect t_ciflags t_context t_credstore t_enctypes t_err
$(RM) t_export_cred t_export_name t_gssexts t_imp_cred t_imp_name
$(RM) t_invalid t_inq_cred t_inq_ctx t_inq_mechs_name t_iov t_lifetime
$(RM) t_namingexts t_oid t_pcontok t_prf t_s4u t_s4u2proxy_krb5
$(RM) t_saslname t_spnego t_srcattrs
$(RM) t_saslname t_spnego t_srcattrs t_store_cred
4 changes: 2 additions & 2 deletions src/tests/gssapi/t_credstore.py
View file
Expand Up @@ -9,8 +9,8 @@
realm.addprinc(service_cs)
realm.extract_keytab(service_cs, servicekeytab)
realm.kinit(service_cs, None, ['-k', '-t', servicekeytab])
msgs = ('Storing %s -> %s in %s' % (service_cs, realm.krbtgt_princ,
storagecache),
msgs = ('Storing %s -> %s in MEMORY:' % (service_cs, realm.krbtgt_princ),
'Moving ccache MEMORY:',
'Retrieving %s from FILE:%s' % (service_cs, servicekeytab))
realm.run(['./t_credstore', '-s', 'p:' + service_cs, 'ccache', storagecache,
'keytab', servicekeytab], expected_trace=msgs)
Expand Down

0 comments on commit 3f5a348

Please sign in to comment.