Skip to content

Commit

Permalink
Allow the KDB to see and modify auth indicators
Browse files Browse the repository at this point in the history
Amend the sign_authdata method signature to include a modifiable
auth_indicators array.  Bump the DAL major version and the libkdb5
soname.  Add a test case using the test KDB module.

ticket: 8823 (new)
  • Loading branch information
greghudson committed Aug 8, 2019
1 parent 2bb7156 commit 2d75f2b
Show file tree
Hide file tree
Showing 9 changed files with 65 additions and 48 deletions.
16 changes: 11 additions & 5 deletions src/include/kdb.h
Expand Up @@ -69,7 +69,7 @@

/* This version will be incremented when incompatible changes are made to the
* KDB API, and will be kept in sync with the libkdb major version. */
#define KRB5_KDB_API_VERSION 9
#define KRB5_KDB_API_VERSION 10

/* Salt types */
#define KRB5_KDB_SALTTYPE_NORMAL 0
Expand Down Expand Up @@ -672,6 +672,7 @@ krb5_error_code krb5_db_sign_authdata(krb5_context kcontext,
krb5_keyblock *session_key,
krb5_timestamp authtime,
krb5_authdata **tgt_auth_data,
krb5_data ***auth_indicators,
krb5_authdata ***signed_auth_data);

krb5_error_code krb5_db_check_transited_realms(krb5_context kcontext,
Expand Down Expand Up @@ -873,7 +874,7 @@ krb5_error_code krb5_db_register_keytab(krb5_context context);
* This number indicates the date of the last incompatible change to the DAL.
* The maj_ver field of the module's vtable structure must match this version.
*/
#define KRB5_KDB_DAL_MAJOR_VERSION 7
#define KRB5_KDB_DAL_MAJOR_VERSION 8

/*
* A krb5_context can hold one database object. Modules should use
Expand Down Expand Up @@ -1301,6 +1302,12 @@ typedef struct _kdb_vftabl {
*
* tgt_auth_data: For TGS requests, the authorization data present in the
* subject ticket. For AS requests, NULL.
*
* auth_indicators: Points to NULL or a null-terminated list of krb5_data
* pointers, each containing an authentication indicator (RFC 8129).
* The method may modify this list, or free it and replace
* *auth_indicators with NULL, to change which auth indicators will be
* included in the ticket.
*/
krb5_error_code (*sign_authdata)(krb5_context kcontext,
unsigned int flags,
Expand All @@ -1314,6 +1321,7 @@ typedef struct _kdb_vftabl {
krb5_keyblock *session_key,
krb5_timestamp authtime,
krb5_authdata **tgt_auth_data,
krb5_data ***auth_indicators,
krb5_authdata ***signed_auth_data);

/*
Expand Down Expand Up @@ -1402,8 +1410,6 @@ typedef struct _kdb_vftabl {
*/
void (*free_principal_e_data)(krb5_context kcontext, krb5_octet *e_data);

/* End of minor version 0. */

/*
* Optional: get a principal entry for S4U2Self based on X509 certificate.
*
Expand All @@ -1423,7 +1429,7 @@ typedef struct _kdb_vftabl {
unsigned int flags,
krb5_db_entry **entry_out);

/* End of minor version 1 for major version 7. */
/* End of minor version 0 for major version 8. */
} kdb_vftabl;

#endif /* !defined(_WIN32) */
Expand Down
19 changes: 5 additions & 14 deletions src/kdc/do_as_req.c
Expand Up @@ -294,20 +294,11 @@ finish_process_as_req(struct as_req_state *state, krb5_error_code errcode)
goto egress;
}

errcode = handle_authdata(kdc_context,
state->c_flags,
state->client,
state->server,
NULL,
state->local_tgt,
&state->client_keyblock,
&state->server_keyblock,
NULL,
state->req_pkt,
state->request,
NULL, /* for_user_princ */
NULL, /* enc_tkt_request */
state->auth_indicators,
errcode = handle_authdata(kdc_context, state->c_flags, state->client,
state->server, NULL, state->local_tgt,
&state->client_keyblock, &state->server_keyblock,
NULL, state->req_pkt, state->request,
NULL, NULL, &state->auth_indicators,
&state->enc_tkt_reply);
if (errcode) {
krb5_klog_syslog(LOG_INFO, _("AS_REQ : handle_authdata (%d)"),
Expand Down
9 changes: 2 additions & 7 deletions src/kdc/do_tgs_req.c
Expand Up @@ -617,15 +617,10 @@ process_tgs_req(krb5_kdc_req *request, krb5_data *pkt,
header_server, local_tgt,
subkey != NULL ? subkey :
header_ticket->enc_part2->session,
encrypting_key, /* U2U or server key */
header_key,
pkt,
request,
encrypting_key, header_key, pkt, request,
s4u_x509_user ?
s4u_x509_user->user_id.user : NULL,
subject_tkt,
auth_indicators,
&enc_tkt_reply);
subject_tkt, &auth_indicators, &enc_tkt_reply);
if (errcode) {
krb5_klog_syslog(LOG_INFO, _("TGS_REQ : handle_authdata (%d)"),
errcode);
Expand Down
27 changes: 15 additions & 12 deletions src/kdc/kdc_authdata.c
Expand Up @@ -320,7 +320,8 @@ fetch_kdb_authdata(krb5_context context, unsigned int flags,
krb5_keyblock *server_key, krb5_keyblock *header_key,
krb5_kdc_req *req, krb5_const_principal for_user_princ,
krb5_enc_tkt_part *enc_tkt_req,
krb5_enc_tkt_part *enc_tkt_reply)
krb5_enc_tkt_part *enc_tkt_reply,
krb5_data ***auth_indicators)
{
krb5_error_code ret;
krb5_authdata **tgt_authdata, **db_authdata = NULL;
Expand Down Expand Up @@ -379,7 +380,7 @@ fetch_kdb_authdata(krb5_context context, unsigned int flags,
server, krbtgt, client_key, server_key,
krbtgt_key, enc_tkt_reply->session,
enc_tkt_reply->times.authtime, tgt_authdata,
&db_authdata);
auth_indicators, &db_authdata);
if (ret)
return (ret == KRB5_PLUGIN_OP_NOTSUPP) ? 0 : ret;

Expand Down Expand Up @@ -827,8 +828,7 @@ handle_authdata(krb5_context context, unsigned int flags,
krb5_keyblock *client_key, krb5_keyblock *server_key,
krb5_keyblock *header_key, krb5_data *req_pkt,
krb5_kdc_req *req, krb5_const_principal for_user_princ,
krb5_enc_tkt_part *enc_tkt_req,
krb5_data *const *auth_indicators,
krb5_enc_tkt_part *enc_tkt_req, krb5_data ***auth_indicators,
krb5_enc_tkt_part *enc_tkt_reply)
{
kdcauthdata_handle *h;
Expand Down Expand Up @@ -866,23 +866,26 @@ handle_authdata(krb5_context context, unsigned int flags,
return ret;
}

if (!isflagset(enc_tkt_reply->flags, TKT_FLG_ANONYMOUS)) {
/* Fetch authdata from the KDB if appropriate. */
ret = fetch_kdb_authdata(context, flags, client, server, header_server,
client_key, server_key, header_key, req,
for_user_princ, enc_tkt_req, enc_tkt_reply,
auth_indicators);
if (ret)
return ret;
}

/* Add auth indicators if any were given. */
if (auth_indicators != NULL && *auth_indicators != NULL &&
!isflagset(server->attributes, KRB5_KDB_NO_AUTH_DATA_REQUIRED)) {
ret = add_auth_indicators(context, auth_indicators, server_key,
ret = add_auth_indicators(context, *auth_indicators, server_key,
local_tgt, enc_tkt_reply);
if (ret)
return ret;
}

if (!isflagset(enc_tkt_reply->flags, TKT_FLG_ANONYMOUS)) {
/* Fetch authdata from the KDB if appropriate. */
ret = fetch_kdb_authdata(context, flags, client, server, header_server,
client_key, server_key, header_key, req,
for_user_princ, enc_tkt_req, enc_tkt_reply);
if (ret)
return ret;

/* Validate and insert AD-SIGNTICKET authdata. This must happen last
* since it contains a signature over the other authdata. */
ret = handle_signticket(context, flags, client, server, local_tgt,
Expand Down
2 changes: 1 addition & 1 deletion src/kdc/kdc_util.h
Expand Up @@ -232,7 +232,7 @@ handle_authdata (krb5_context context,
krb5_kdc_req *request,
krb5_const_principal for_user_princ,
krb5_enc_tkt_part *enc_tkt_request,
krb5_data *const *auth_indicators,
krb5_data ***auth_indicators,
krb5_enc_tkt_part *enc_tkt_reply);

/* replay.c */
Expand Down
2 changes: 1 addition & 1 deletion src/lib/kdb/Makefile.in
Expand Up @@ -5,7 +5,7 @@ LOCALINCLUDES= -I.

# Keep LIBMAJOR in sync with KRB5_KDB_API_VERSION in include/kdb.h.
LIBBASE=kdb5
LIBMAJOR=9
LIBMAJOR=10
LIBMINOR=0
LIBINITFUNC=kdb_init_lock_list
LIBFINIFUNC=kdb_fini_lock_list
Expand Down
10 changes: 3 additions & 7 deletions src/lib/kdb/kdb5.c
Expand Up @@ -323,12 +323,7 @@ copy_vtable(const kdb_vftabl *in, kdb_vftabl *out)
out->refresh_config = in->refresh_config;
out->check_allowed_to_delegate = in->check_allowed_to_delegate;
out->free_principal_e_data = in->free_principal_e_data;

/* Copy fields for minor version 1 (major version 7). */
assert(KRB5_KDB_DAL_MAJOR_VERSION == 7);
out->get_s4u_x509_principal = NULL;
if (in->min_ver >= 1)
out->get_s4u_x509_principal = in->get_s4u_x509_principal;
out->get_s4u_x509_principal = in->get_s4u_x509_principal;

/* Set defaults for optional fields. */
if (out->fetch_master_key == NULL)
Expand Down Expand Up @@ -2599,6 +2594,7 @@ krb5_db_sign_authdata(krb5_context kcontext, unsigned int flags,
krb5_keyblock *client_key, krb5_keyblock *server_key,
krb5_keyblock *krbtgt_key, krb5_keyblock *session_key,
krb5_timestamp authtime, krb5_authdata **tgt_auth_data,
krb5_data ***auth_indicators,
krb5_authdata ***signed_auth_data)
{
krb5_error_code status = 0;
Expand All @@ -2613,7 +2609,7 @@ krb5_db_sign_authdata(krb5_context kcontext, unsigned int flags,
return v->sign_authdata(kcontext, flags, client_princ, client, server,
krbtgt, client_key, server_key, krbtgt_key,
session_key, authtime, tgt_auth_data,
signed_auth_data);
auth_indicators, signed_auth_data);
}

krb5_error_code
Expand Down
23 changes: 22 additions & 1 deletion src/plugins/kdb/test/kdb_test.c
Expand Up @@ -544,9 +544,12 @@ test_sign_authdata(krb5_context context, unsigned int flags,
krb5_keyblock *client_key, krb5_keyblock *server_key,
krb5_keyblock *krbtgt_key, krb5_keyblock *session_key,
krb5_timestamp authtime, krb5_authdata **tgt_auth_data,
krb5_data ***auth_indicators,
krb5_authdata ***signed_auth_data)
{
krb5_authdata **list, *ad;
krb5_data **inds, d;
int i, val;

ad = ealloc(sizeof(*ad));
ad->magic = KV5M_AUTHDATA;
Expand All @@ -557,6 +560,24 @@ test_sign_authdata(krb5_context context, unsigned int flags,
list[0] = ad;
list[1] = NULL;
*signed_auth_data = list;

/* If we see an auth indicator "dbincrX", replace the whole indicator list
* with "dbincr{X+1}". */
inds = *auth_indicators;
for (i = 0; inds != NULL && inds[i] != NULL; i++) {
if (inds[i]->length == 7 && memcmp(inds[i]->data, "dbincr", 6) == 0) {
val = inds[i]->data[6];
k5_free_data_ptr_list(inds);
inds = ealloc(2 * sizeof(*inds));
d = string2data("dbincr0");
check(krb5_copy_data(context, &d, &inds[0]));
inds[0]->data[6] = val + 1;
inds[1] = NULL;
*auth_indicators = inds;
break;
}
}

return 0;
}

Expand Down Expand Up @@ -593,7 +614,7 @@ test_check_allowed_to_delegate(krb5_context context,

kdb_vftabl PLUGIN_SYMBOL_NAME(krb5_test, kdb_function_table) = {
KRB5_KDB_DAL_MAJOR_VERSION, /* major version number */
1, /* minor version number */
0, /* minor version number */
test_init,
test_cleanup,
test_open,
Expand Down
5 changes: 5 additions & 0 deletions src/tests/t_authdata.py
Expand Up @@ -228,6 +228,11 @@
if '+97: [indcl]' not in out or '[inds1]' in out:
fail('correct auth-indicator not seen for S4U2Proxy req')

# Test alteration of auth indicators by KDB module (AS and TGS).
realm.kinit(realm.user_princ, None, ['-k', '-X', 'indicators=dummy dbincr1'])
realm.run(['./adata', realm.krbtgt_princ], expected_msg='+97: [dbincr2]')
realm.run(['./adata', 'service/1'], expected_msg='+97: [dbincr3]')

# Test that KDB module authdata is included in an AS request, by
# default or with an explicit PAC request.
mark('AS-REQ KDB module authdata')
Expand Down

0 comments on commit 2d75f2b

Please sign in to comment.