krb5 commit: Restrict pre-authentication fallback cases
Greg Hudson <[email protected]>
| Newsgroups | gmane.comp.encryption.kerberos.cvs |
|---|---|
| Message-ID | <[email protected]> |
https://github.com/krb5/krb5/commit/7a24a088c16d326127dd2b29084d4ca085c70d10 commit 7a24a088c16d326127dd2b29084d4ca085c70d10 Author: Greg Hudson <[email protected]> Date: Thu Apr 5 16:23:34 2018 -0400 Restrict pre-authentication fallback cases Add a new callback disable_fallback() and call it from each clpreauth module when it generates a client message using credentials to authenticate. (For SPAKE, this is the message responding to a challenge; for all other current mechanisms, it is the first and only client message.) If disable_fallback() is called, do not try another mechanism after a KDC error. Remove k5_reset_preauth_types_tried() and its call sites, so that preauth mechanisms which are tried optimistically will no longer be retried after a failure. ticket: 8654 src/include/krb5/clpreauth_plugin.h | 14 +++++ src/lib/krb5/krb/get_in_tkt.c | 21 +++----- src/lib/krb5/krb/init_creds_ctx.h | 1 + src/lib/krb5/krb/int-proto.h | 3 - src/lib/krb5/krb/preauth2.c | 23 +++----- src/lib/krb5/krb/preauth_ec.c | 1 + src/lib/krb5/krb/preauth_encts.c | 2 + src/lib/krb5/krb/preauth_otp.c | 4 ++ src/lib/krb5/krb/preauth_sam2.c | 1 + src/plugins/preauth/pkinit/pkinit_clnt.c | 1 + src/plugins/preauth/spake/spake_client.c | 4 ++ src/plugins/preauth/test/cltest.c | 11 ++++ src/tests/t_preauth.py | 88 ++++++++++++++++++++++++++---- src/tests/t_spake.py | 9 +--- 14 files changed, 134 insertions(+), 49 deletions(-) diff --git a/src/include/krb5/clpreauth_plugin.h b/src/include/krb5/clpreauth_plugin.h index e47607c..22a5e9b 100644 --- a/src/include/krb5/clpreauth_plugin.h +++ b/src/include/krb5/clpreauth_plugin.h @@ -159,7 +159,21 @@ typedef struct krb5_clpreauth_callbacks_st { krb5_error_code (*set_cc_config)(krb5_context context, krb5_clpreauth_rock rock, const char *key, const char *data); + /* End of version 2 clpreauth callbacks (added in 1.11). */ + + /* + * Prevent further fallbacks to other preauth mechanisms if the KDC replies + * with an error. (The module itself can still respond to errors with its + * tryagain method, or continue after KDC_ERR_MORE_PREAUTH_DATA_REQUIRED + * errors with its process method.) A module should invoke this callback + * from the process method when it generates an authenticated request using + * credentials; often this will be the first or only client message + * generated by the mechanism. + */ + void (*disable_fallback)(krb5_context context, krb5_clpreauth_rock rock); + + /* End of version 3 clpreauth callbacks (added in 1.17). */ } *krb5_clpreauth_callbacks; /* diff --git a/src/lib/krb5/krb/get_in_tkt.c b/src/lib/krb5/krb/get_in_tkt.c index 1d96ff1..c026bbc 100644 --- a/src/lib/krb5/krb/get_in_tkt.c +++ b/src/lib/krb5/krb/get_in_tkt.c @@ -1331,9 +1331,7 @@ init_creds_step_request(krb5_context context, krb5_free_pa_data(context, ctx->optimistic_padata); ctx->optimistic_padata = NULL; if (code) { - /* Make an unauthenticated request, and possibly try again using - * the same mechanisms as we tried optimistically. */ - k5_reset_preauth_types_tried(ctx); + /* Make an unauthenticated request. */ krb5_clear_error_message(context); code = 0; } @@ -1361,6 +1359,9 @@ init_creds_step_request(krb5_context context, /* Don't continue after a keyboard interrupt. */ if (code == KRB5_LIBOS_PWDINTR) goto cleanup; + /* Don't continue if fallback is disabled. */ + if (code && ctx->fallback_disabled) + goto cleanup; if (code) { /* See if we can try a different preauth mech before giving up. */ k5_save_ctx_error(context, code, &save); @@ -1549,16 +1550,10 @@ init_creds_step_reply(krb5_context context, } else if (reply_code == KDC_ERR_PREAUTH_FAILED && retry) { note_req_timestamp(context, ctx, ctx->err_reply->stime, ctx->err_reply->susec); - if (ctx->method_padata == NULL) { - /* Optimistic preauth failed on the KDC. Allow all mechanisms - * to be tried again using method data. */ - k5_reset_preauth_types_tried(ctx); - } else { - /* Don't try again with the mechanism that failed. */ - code = k5_preauth_note_failed(ctx, ctx->selected_preauth_type); - if (code) - goto cleanup; - } + /* Don't try again with the mechanism that failed. */ + code = k5_preauth_note_failed(ctx, ctx->selected_preauth_type); + if (code) + goto cleanup; ctx->selected_preauth_type = KRB5_PADATA_NONE; /* Accept or update method data if the KDC sent it. */ if (ctx->err_padata != NULL) diff --git a/src/lib/krb5/krb/init_creds_ctx.h b/src/lib/krb5/krb/init_creds_ctx.h index b19410a..7ba61e1 100644 --- a/src/lib/krb5/krb/init_creds_ctx.h +++ b/src/lib/krb5/krb/init_creds_ctx.h @@ -60,6 +60,7 @@ struct _krb5_init_creds_context { krb5_enctype etype; krb5_boolean info_pa_permitted; krb5_boolean restarted; + krb5_boolean fallback_disabled; struct krb5_responder_context_st rctx; krb5_preauthtype selected_preauth_type; krb5_preauthtype allowed_preauth_type; diff --git a/src/lib/krb5/krb/int-proto.h b/src/lib/krb5/krb/int-proto.h index cda9010..d201338 100644 --- a/src/lib/krb5/krb/int-proto.h +++ b/src/lib/krb5/krb/int-proto.h @@ -197,9 +197,6 @@ k5_init_preauth_context(krb5_context context); void k5_free_preauth_context(krb5_context context); -void -k5_reset_preauth_types_tried(krb5_init_creds_context ctx); - krb5_error_code k5_preauth_note_failed(krb5_init_creds_context ctx, krb5_preauthtype pa_type); diff --git a/src/lib/krb5/krb/preauth2.c b/src/lib/krb5/krb/preauth2.c index d6cdeb8..c578548 100644 --- a/src/lib/krb5/krb/preauth2.c +++ b/src/lib/krb5/krb/preauth2.c @@ -203,18 +203,6 @@ cleanup: free_handles(context, list); } -/* Reset the memory of which preauth types we have already tried. */ -void -k5_reset_preauth_types_tried(krb5_init_creds_context ctx) -{ - krb5_preauth_req_context reqctx = ctx->preauth_reqctx; - - if (reqctx == NULL) - return; - free(reqctx->failed); - reqctx->failed = NULL; -} - /* Add pa_type to the list of types which has previously failed. */ krb5_error_code k5_preauth_note_failed(krb5_init_creds_context ctx, krb5_preauthtype pa_type) @@ -557,8 +545,14 @@ set_cc_config(krb5_context context, krb5_clpreauth_rock rock, return ret; } +static void +disable_fallback(krb5_context context, krb5_clpreauth_rock rock) +{ + ((krb5_init_creds_context)rock)->fallback_disabled = TRUE; +} + static struct krb5_clpreauth_callbacks_st callbacks = { - 2, + 3, get_etype, fast_armor, get_as_key, @@ -568,7 +562,8 @@ static struct krb5_clpreauth_callbacks_st callbacks = { responder_get_answer, need_as_key, get_cc_config, - set_cc_config + set_cc_config, + disable_fallback }; /* Tweak the request body, for now adding any enctypes which the module claims diff --git a/src/lib/krb5/krb/preauth_ec.c b/src/lib/krb5/krb/preauth_ec.c index c1aa909..75aab77 100644 --- a/src/lib/krb5/krb/preauth_ec.c +++ b/src/lib/krb5/krb/preauth_ec.c @@ -138,6 +138,7 @@ ec_process(krb5_context context, krb5_clpreauth_moddata moddata, encoded_ts->data = NULL; *out_padata = pa; pa = NULL; + cb->disable_fallback(context, rock); } free(pa); krb5_free_data(context, encoded_ts); diff --git a/src/lib/krb5/krb/preauth_encts.c b/src/lib/krb5/krb/preauth_encts.c index cec3842..45bf9da 100644 --- a/src/lib/krb5/krb/preauth_encts.c +++ b/src/lib/krb5/krb/preauth_encts.c @@ -109,6 +109,8 @@ encts_process(krb5_context context, krb5_clpreauth_moddata moddata, *out_padata = pa; pa = NULL; + cb->disable_fallback(context, rock); + cleanup: krb5_free_data(context, ts); krb5_free_data(context, enc_ts); diff --git a/src/lib/krb5/krb/preauth_otp.c b/src/lib/krb5/krb/preauth_otp.c index 48fcbb5..13e5846 100644 --- a/src/lib/krb5/krb/preauth_otp.c +++ b/src/lib/krb5/krb/preauth_otp.c @@ -1123,6 +1123,10 @@ otp_client_process(krb5_context context, krb5_clpreauth_moddata moddata, /* Encode the request into the pa_data output. */ retval = set_pa_data(req, pa_data_out); + if (retval != 0) + goto error; + cb->disable_fallback(context, rock); + error: krb5_free_data_contents(context, &value); krb5_free_data_contents(context, &pin); diff --git a/src/lib/krb5/krb/preauth_sam2.c b/src/lib/krb5/krb/preauth_sam2.c index c8a3306..4c70021 100644 --- a/src/lib/krb5/krb/preauth_sam2.c +++ b/src/lib/krb5/krb/preauth_sam2.c @@ -410,6 +410,7 @@ sam2_process(krb5_context context, krb5_clpreauth_moddata moddata, sam_padata[1] = NULL; *out_padata = sam_padata; + cb->disable_fallback(context, rock); return(0); } diff --git a/src/plugins/preauth/pkinit/pkinit_clnt.c b/src/plugins/preauth/pkinit/pkinit_clnt.c index 613f391..e721aa7 100644 --- a/src/plugins/preauth/pkinit/pkinit_clnt.c +++ b/src/plugins/preauth/pkinit/pkinit_clnt.c @@ -179,6 +179,7 @@ pa_pkinit_gen_req(krb5_context context, *out_padata = return_pa_data; return_pa_data = NULL; + cb->disable_fallback(context, rock); cleanup: krb5_free_data(context, der_req); diff --git a/src/plugins/preauth/spake/spake_client.c b/src/plugins/preauth/spake/spake_client.c index 47a6ba2..00734a1 100644 --- a/src/plugins/preauth/spake/spake_client.c +++ b/src/plugins/preauth/spake/spake_client.c @@ -278,6 +278,10 @@ process_challenge(krb5_context context, groupstate *gstate, reqstate *st, goto cleanup; TRACE_SPAKE_SEND_RESPONSE(context); ret = convert_to_padata(response, pa_out); + if (ret) + goto cleanup; + + cb->disable_fallback(context, rock); cleanup: krb5_free_keyblock(context, k0); diff --git a/src/plugins/preauth/test/cltest.c b/src/plugins/preauth/test/cltest.c index f5f7c5a..51b8484 100644 --- a/src/plugins/preauth/test/cltest.c +++ b/src/plugins/preauth/test/cltest.c @@ -53,6 +53,9 @@ * - If the "fail_optimistic", "fail_2rt", or "fail_tryagain" gic options are * set, it fails with a recognizable error string at the requested point in * processing. + * + * - If the "disable_fallback" gic option is set, fallback is disabled when a + * client message is generated. */ #include "k5-int.h" @@ -66,6 +69,7 @@ struct client_state { krb5_boolean fail_optimistic; krb5_boolean fail_2rt; krb5_boolean fail_tryagain; + krb5_boolean disable_fallback; }; struct client_request_state { @@ -81,6 +85,7 @@ test_init(krb5_context context, krb5_clpreauth_moddata *moddata_out) assert(st != NULL); st->indicators = NULL; st->fail_optimistic = st->fail_2rt = st->fail_tryagain = FALSE; + st->disable_fallback = FALSE; *moddata_out = (krb5_clpreauth_moddata)st; return 0; } @@ -138,6 +143,8 @@ test_process(krb5_context context, krb5_clpreauth_moddata moddata, return KRB5_PREAUTH_FAILED; } *out_pa_data = make_pa_list("optimistic", 10); + if (st->disable_fallback) + cb->disable_fallback(context, rock); return 0; } else if (reqst->second_round_trip) { printf("2rt: %.*s\n", pa_data->length, pa_data->contents); @@ -166,6 +173,8 @@ test_process(krb5_context context, krb5_clpreauth_moddata moddata, indstr = (st->indicators != NULL) ? st->indicators : ""; *out_pa_data = make_pa_list(indstr, strlen(indstr)); + if (st->disable_fallback) + cb->disable_fallback(context, rock); return 0; } @@ -212,6 +221,8 @@ test_gic_opt(krb5_context kcontext, krb5_clpreauth_moddata moddata, st->fail_2rt = TRUE; } else if (strcmp(attr, "fail_tryagain") == 0) { st->fail_tryagain = TRUE; + } else if (strcmp(attr, "disable_fallback") == 0) { + st->disable_fallback = TRUE; } return 0; } diff --git a/src/tests/t_preauth.py b/src/tests/t_preauth.py index efb3ea2..32e35b0 100644 --- a/src/tests/t_preauth.py +++ b/src/tests/t_preauth.py @@ -37,8 +37,8 @@ expected_trace = ('Attempting optimistic preauth', realm.run(['./icred', '-o', '-123', realm.user_princ, password('user')], expected_trace=expected_trace) -# Test optimistic preauth failing on client, followed by successful -# preauth using the same module. +# Test optimistic preauth failing on client, falling back to encrypted +# timestamp. msgs = ('Attempting optimistic preauth', 'Processing preauth types: -123', '/induced optimistic fail', @@ -46,15 +46,15 @@ msgs = ('Attempting optimistic preauth', '/Additional pre-authentication required', 'Preauthenticating using KDC method data', 'Processing preauth types:', - 'Preauth module test (-123) (real) returned: 0/Success', - 'Produced preauth for next request: PA-FX-COOKIE (133), -123', + 'Encrypted timestamp (for ', + 'module encrypted_timestamp (2) (real) returned: 0/Success', + 'preauth for next request: PA-FX-COOKIE (133), PA-ENC-TIMESTAMP (2)', 'Decrypted AS reply') realm.run(['./icred', '-o', '-123', '-X', 'fail_optimistic', realm.user_princ, - password('user')], expected_msg='testval', - expected_trace=msgs) + password('user')], expected_trace=msgs) -# Test optimistic preauth failing on KDC, followed by successful preauth -# using the same module. +# Test optimistic preauth failing on KDC, falling back to encrypted +# timestamp. realm.run([kadminl, 'setstr', realm.user_princ, 'failopt', 'yes']) msgs = ('Attempting optimistic preauth', 'Processing preauth types: -123', @@ -63,11 +63,24 @@ msgs = ('Attempting optimistic preauth', '/Preauthentication failed', 'Preauthenticating using KDC method data', 'Processing preauth types:', - 'Preauth module test (-123) (real) returned: 0/Success', - 'Produced preauth for next request: PA-FX-COOKIE (133), -123', + 'Encrypted timestamp (for ', + 'module encrypted_timestamp (2) (real) returned: 0/Success', + 'preauth for next request: PA-FX-COOKIE (133), PA-ENC-TIMESTAMP (2)', 'Decrypted AS reply') realm.run(['./icred', '-o', '-123', realm.user_princ, password('user')], - expected_msg='testval', expected_trace=msgs) + expected_trace=msgs) +# Leave failopt set for the next test. + +# Test optimistic preauth failing on KDC, stopping because the test +# module disabled fallback. +msgs = ('Attempting optimistic preauth', + 'Processing preauth types: -123', + 'Preauth module test (-123) (real) returned: 0/Success', + 'Produced preauth for next request: -123', + '/Preauthentication failed') +realm.run(['./icred', '-X', 'disable_fallback', '-o', '-123', realm.user_princ, + password('user')], expected_code=1, + expected_msg='Preauthentication failed', expected_trace=msgs) realm.run([kadminl, 'delstr', realm.user_princ, 'failopt']) # Test KDC_ERR_MORE_PREAUTH_DATA_REQUIRED and secure cookies. @@ -107,6 +120,23 @@ msgs = ('Sending unauthenticated request', realm.run(['./icred', '-X', 'fail_2rt', realm.user_princ, password('user')], expected_msg='2rt: secondtrip', expected_trace=msgs) +# Test client-side failure after KDC_ERR_MORE_PREAUTH_DATA_REQUIRED, +# stopping because the test module disabled fallback. +msgs = ('Sending unauthenticated request', + '/Additional pre-authentication required', + 'Preauthenticating using KDC method data', + 'Processing preauth types:', + 'Preauth module test (-123) (real) returned: 0/Success', + 'Produced preauth for next request: PA-FX-COOKIE (133), -123', + '/More preauthentication data is required', + 'Continuing preauth mech -123', + 'Processing preauth types: -123, PA-FX-COOKIE (133)', + '/induced 2rt fail') +realm.run(['./icred', '-X', 'fail_2rt', '-X', 'disable_fallback', + realm.user_princ, password('user')], expected_code=1, + expected_msg='Pre-authentication failed: induced 2rt fail', + expected_trace=msgs) + # Test KDC-side failure after KDC_ERR_MORE_PREAUTH_DATA_REQUIRED, # falling back to encrypted timestamp. realm.run([kadminl, 'setstr', realm.user_princ, 'fail2rt', 'yes']) @@ -130,6 +160,25 @@ msgs = ('Sending unauthenticated request', 'Decrypted AS reply') realm.run(['./icred', realm.user_princ, password('user')], expected_msg='2rt: secondtrip', expected_trace=msgs) +# Leave fail2rt set for the next test. + +# Test KDC-side failure after KDC_ERR_MORE_PREAUTH_DATA_REQUIRED, +# stopping because the test module disabled fallback. +msgs = ('Sending unauthenticated request', + '/Additional pre-authentication required', + 'Preauthenticating using KDC method data', + 'Processing preauth types:', + 'Preauth module test (-123) (real) returned: 0/Success', + 'Produced preauth for next request: PA-FX-COOKIE (133), -123', + '/More preauthentication data is required', + 'Continuing preauth mech -123', + 'Processing preauth types: -123, PA-FX-COOKIE (133)', + 'Preauth module test (-123) (real) returned: 0/Success', + 'Produced preauth for next request: PA-FX-COOKIE (133), -123', + '/Preauthentication failed') +realm.run(['./icred', '-X', 'disable_fallback', + realm.user_princ, password('user')], expected_code=1, + expected_msg='Preauthentication failed', expected_trace=msgs) realm.run([kadminl, 'delstr', realm.user_princ, 'fail2rt']) # Test tryagain flow by inducing a KDC_ERR_ENCTYPE_NOSUPP error on the KDC. @@ -170,6 +219,23 @@ msgs = ('Sending unauthenticated request', realm.run(['./icred', '-X', 'fail_tryagain', realm.user_princ, password('user')], expected_trace=msgs) +# Test a client-side tryagain failure, stopping because the test +# module disabled fallback. +msgs = ('Sending unauthenticated request', + '/Additional pre-authentication required', + 'Preauthenticating using KDC method data', + 'Processing preauth types:', + 'Preauth module test (-123) (real) returned: 0/Success', + 'Produced preauth for next request: PA-FX-COOKIE (133), -123', + '/KDC has no support for encryption type', + 'Recovering from KDC error 14 using preauth mech -123', + 'Preauth tryagain input types (-123): -123, PA-FX-COOKIE (133)', + '/induced tryagain fail') +realm.run(['./icred', '-X', 'fail_tryagain', '-X', 'disable_fallback', + realm.user_princ, password('user')], expected_code=1, + expected_msg='KDC has no support for encryption type', + expected_trace=msgs) + # Test that multiple stepwise initial creds operations can be # performed with the same krb5_context, with proper tracking of # clpreauth module request handles. diff --git a/src/tests/t_spake.py b/src/tests/t_spake.py index a81a238..5b47e62 100644 --- a/src/tests/t_spake.py +++ b/src/tests/t_spake.py @@ -31,9 +31,7 @@ for gnum, gname in groups: 'Decrypted AS reply') realm.kinit('user', 'pw', expected_trace=msgs) - # Test an unsuccessful authentication. (The client will try - # again with encrypted timestamp, which isn't really desired, - # but check for that as long as it is expected.) + # Test an unsuccessful authentication. msgs = ('/Additional pre-authentication required', 'Selected etype info:', 'Sending SPAKE support message', @@ -42,9 +40,6 @@ for gnum, gname in groups: 'Continuing preauth mech PA-SPAKE (151)', 'SPAKE challenge received with group ' + str(gnum), 'Sending SPAKE response', - '/Preauthentication failed', - 'Encrypted timestamp ', - 'for next request: PA-FX-COOKIE (133), PA-ENC-TIMESTAMP (2)', '/Preauthentication failed') realm.kinit('user', 'wrongpw', expected_code=1, expected_trace=msgs) @@ -114,8 +109,6 @@ msgs = ('Attempting optimistic preauth', 'for next request: PA-SPAKE (151)', '/Preauthentication failed', 'Selected etype info:', - 'SPAKE challenge with group 1 rejected', - 'spake (151) (real) returned: -1765328360/Preauthentication failed', 'Encrypted timestamp ', 'for next request: PA-FX-COOKIE (133), PA-ENC-TIMESTAMP (2)', 'AS key determined by preauth:',