krb5 commit: Restrict pre-authentication fallback cases
Greg Hudson
ghudson at mit.edu
Mon Apr 9 12:09:12 EDT 2018
https://github.com/krb5/krb5/commit/7a24a088c16d326127dd2b29084d4ca085c70d10
commit 7a24a088c16d326127dd2b29084d4ca085c70d10
Author: Greg Hudson <ghudson at mit.edu>
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:',
More information about the cvs-krb5
mailing list