From 8ed07abedfe4ae7d853bbd2440007af1bca86fd5 Mon Sep 17 00:00:00 2001 From: Robbie Harwood Date: Mon, 9 Apr 2018 12:12:08 -0400 Subject: [PATCH] Restrict pre-authentication fallback cases --- ...ct-pre-authentication-fallback-cases.patch | 491 ++++++++++++++++++ Return-UPN-SANs-as-strings.patch | 204 ++++++++ ...ately-and-unparse-them-with-NO_REALM.patch | 148 ++++++ krb5.spec | 8 +- 4 files changed, 850 insertions(+), 1 deletion(-) create mode 100644 Restrict-pre-authentication-fallback-cases.patch create mode 100644 Return-UPN-SANs-as-strings.patch create mode 100644 Save-SANs-separately-and-unparse-them-with-NO_REALM.patch diff --git a/Restrict-pre-authentication-fallback-cases.patch b/Restrict-pre-authentication-fallback-cases.patch new file mode 100644 index 0000000..0d42ba8 --- /dev/null +++ b/Restrict-pre-authentication-fallback-cases.patch @@ -0,0 +1,491 @@ +From 4a13b97ffba771de4b45b1ed309934cc840569d1 Mon Sep 17 00:00:00 2001 +From: Greg Hudson +Date: Thu, 5 Apr 2018 16:23:34 -0400 +Subject: [PATCH] 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 +(cherry picked from commit 7a24a088c16d326127dd2b29084d4ca085c70d10) +--- + 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 0106734ad..5317669b7 100644 +--- a/src/include/krb5/clpreauth_plugin.h ++++ b/src/include/krb5/clpreauth_plugin.h +@@ -160,7 +160,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 1d96ff163..c026bbc6d 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 b19410a13..7ba61e17c 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 cda9010e3..d20133885 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 451e0b7a8..1f17ec2b0 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) +@@ -553,8 +541,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, +@@ -564,7 +558,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 c1aa9090f..75aab770e 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 cec384227..45bf9da92 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 48fcbb5d5..13e584657 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 c8a330655..4c70021a9 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 9483d69e5..77e9e5308 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 47a6ba26c..00734a13b 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 f5f7c5aba..51b848481 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 efb3ea20d..32e35b08b 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 a81a238b4..5b47e62d3 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:', diff --git a/Return-UPN-SANs-as-strings.patch b/Return-UPN-SANs-as-strings.patch new file mode 100644 index 0000000..d8aab07 --- /dev/null +++ b/Return-UPN-SANs-as-strings.patch @@ -0,0 +1,204 @@ +From 06d48c8d04a5efb098b026a1ec1c1609a5491ab0 Mon Sep 17 00:00:00 2001 +From: Greg Hudson +Date: Thu, 22 Mar 2018 20:07:17 -0400 +Subject: [PATCH] Return UPN SANs as strings + +(cherry picked from commit fd3c824e3be56a1fa77d140fd7e93934bfd6e565) +--- + src/plugins/preauth/pkinit/pkinit_crypto.h | 4 ++-- + src/plugins/preauth/pkinit/pkinit_crypto_openssl.c | 28 ++++++++-------------- + src/plugins/preauth/pkinit/pkinit_matching.c | 16 +++---------- + src/plugins/preauth/pkinit/pkinit_srv.c | 21 ++++++++++------ + 4 files changed, 29 insertions(+), 40 deletions(-) + +diff --git a/src/plugins/preauth/pkinit/pkinit_crypto.h b/src/plugins/preauth/pkinit/pkinit_crypto.h +index c7ff29fb2..4e4752ff7 100644 +--- a/src/plugins/preauth/pkinit/pkinit_crypto.h ++++ b/src/plugins/preauth/pkinit/pkinit_crypto.h +@@ -98,7 +98,7 @@ typedef struct _pkinit_cert_matching_data { + unsigned int ku_bits; /* key usage information */ + unsigned int eku_bits; /* extended key usage information */ + krb5_principal *sans; /* Null-terminated array of PKINIT SANs */ +- krb5_principal *upns; /* Null-terimnated array of UPN SANs */ ++ char **upns; /* Null-terimnated array of UPN SANs */ + } pkinit_cert_matching_data; + + /* +@@ -250,7 +250,7 @@ krb5_error_code crypto_retrieve_cert_sans + if non-NULL, a null-terminated array of + id-pkinit-san values found in the certificate + are returned */ +- krb5_principal **upn_sans, /* OUT ++ char ***upn_sans, /* OUT + if non-NULL, a null-terminated array of + id-ms-upn-san values found in the certificate + are returned */ +diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +index bc6de7ae8..b5a549c2c 100644 +--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c ++++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +@@ -29,6 +29,7 @@ + * SUCH DAMAGES. + */ + ++#include "k5-int.h" + #include "pkinit_crypto_openssl.h" + #include "k5-buf.h" + #include +@@ -2083,15 +2084,14 @@ crypto_retrieve_X509_sans(krb5_context context, + pkinit_plg_crypto_context plgctx, + pkinit_req_crypto_context reqctx, + X509 *cert, +- krb5_principal **princs_ret, +- krb5_principal **upn_ret, ++ krb5_principal **princs_ret, char ***upn_ret, + unsigned char ***dns_ret) + { + krb5_error_code retval = EINVAL; + char buf[DN_BUF_LEN]; + int p = 0, u = 0, d = 0, ret = 0, l; + krb5_principal *princs = NULL; +- krb5_principal *upns = NULL; ++ char **upns = NULL; + unsigned char **dnss = NULL; + unsigned int i, num_found = 0, num_sans = 0; + X509_EXTENSION *ext = NULL; +@@ -2141,7 +2141,7 @@ crypto_retrieve_X509_sans(krb5_context context, + } + } + if (upn_ret != NULL) { +- upns = calloc(num_sans + 1, sizeof(krb5_principal)); ++ upns = calloc(num_sans + 1, sizeof(*upns)); + if (upns == NULL) { + retval = ENOMEM; + goto cleanup; +@@ -2184,16 +2184,9 @@ crypto_retrieve_X509_sans(krb5_context context, + /* Prevent abuse of embedded null characters. */ + if (memchr(name.data, '\0', name.length)) + break; +- ret = krb5_parse_name_flags(context, name.data, +- KRB5_PRINCIPAL_PARSE_ENTERPRISE, +- &upns[u]); +- if (ret) { +- pkiDebug("%s: failed parsing ms-upn san value\n", +- __FUNCTION__); +- } else { +- u++; +- num_found++; +- } ++ upns[u] = k5memdup0(name.data, name.length, &ret); ++ if (upns[u] == NULL) ++ goto cleanup; + } else { + pkiDebug("%s: unrecognized othername oid in SAN\n", + __FUNCTION__); +@@ -2245,7 +2238,7 @@ cleanup: + krb5_free_principal(context, princs[i]); + free(princs); + for (i = 0; upns != NULL && upns[i] != NULL; i++) +- krb5_free_principal(context, upns[i]); ++ free(upns[i]); + free(upns); + for (i = 0; dnss != NULL && dnss[i] != NULL; i++) + free(dnss[i]); +@@ -2269,8 +2262,7 @@ crypto_retrieve_cert_sans(krb5_context context, + pkinit_plg_crypto_context plgctx, + pkinit_req_crypto_context reqctx, + pkinit_identity_crypto_context idctx, +- krb5_principal **princs_ret, +- krb5_principal **upn_ret, ++ krb5_principal **princs_ret, char ***upn_ret, + unsigned char ***dns_ret) + { + krb5_error_code retval = EINVAL; +@@ -5094,7 +5086,7 @@ crypto_cert_free_matching_data(krb5_context context, + krb5_free_principal(context, md->sans[i]); + free(md->sans); + for (i = 0; md->upns != NULL && md->upns[i] != NULL; i++) +- krb5_free_principal(context, md->upns[i]); ++ free(md->upns[i]); + free(md->upns); + free(md); + } +diff --git a/src/plugins/preauth/pkinit/pkinit_matching.c b/src/plugins/preauth/pkinit/pkinit_matching.c +index 37bd0251a..c2a4c084d 100644 +--- a/src/plugins/preauth/pkinit/pkinit_matching.c ++++ b/src/plugins/preauth/pkinit/pkinit_matching.c +@@ -490,11 +490,7 @@ component_match(krb5_context context, + break; + } + for (i = 0; md->upns != NULL && md->upns[i] != NULL; i++) { +- krb5_unparse_name_flags(context, md->upns[i], +- KRB5_PRINCIPAL_UNPARSE_NO_REALM, +- &princ_string); +- match = regexp_match(context, rc, princ_string); +- krb5_free_unparsed_name(context, princ_string); ++ match = regexp_match(context, rc, md->upns[i]); + if (match) + break; + } +@@ -584,14 +580,8 @@ check_all_certs(krb5_context context, + pkiDebug("%s: PKINIT san: '%s'\n", __FUNCTION__, san_string); + krb5_free_unparsed_name(context, san_string); + } +- for (j = 0; md->upns != NULL && md->upns[j] != NULL; j++) { +- char *san_string; +- krb5_unparse_name_flags(context, md->upns[j], +- KRB5_PRINCIPAL_UNPARSE_NO_REALM, +- &san_string); +- pkiDebug("%s: UPN san: '%s'\n", __FUNCTION__, san_string); +- krb5_free_unparsed_name(context, san_string); +- } ++ for (j = 0; md->upns != NULL && md->upns[j] != NULL; j++) ++ pkiDebug("%s: UPN san: '%s'\n", __FUNCTION__, md->upns[j]); + #endif + certs_checked++; + for (rc = rs->crs; rc != NULL; rc = rc->next) { +diff --git a/src/plugins/preauth/pkinit/pkinit_srv.c b/src/plugins/preauth/pkinit/pkinit_srv.c +index bbfde34b2..3cc573813 100644 +--- a/src/plugins/preauth/pkinit/pkinit_srv.c ++++ b/src/plugins/preauth/pkinit/pkinit_srv.c +@@ -178,8 +178,9 @@ verify_client_san(krb5_context context, + int *valid_san) + { + krb5_error_code retval; +- krb5_principal *princs = NULL; +- krb5_principal *upns = NULL; ++ krb5_principal *princs = NULL, upn; ++ krb5_boolean match; ++ char **upns = NULL; + int i; + #ifdef DEBUG_SAN_INFO + char *client_string = NULL, *san_string; +@@ -255,12 +256,18 @@ verify_client_san(krb5_context context, + pkiDebug("%s: Checking upn sans\n", __FUNCTION__); + for (i = 0; upns[i] != NULL; i++) { + #ifdef DEBUG_SAN_INFO +- krb5_unparse_name(context, upns[i], &san_string); + pkiDebug("%s: Comparing client '%s' to upn san value '%s'\n", +- __FUNCTION__, client_string, san_string); +- krb5_free_unparsed_name(context, san_string); ++ __FUNCTION__, client_string, upns[i]); + #endif +- if (cb->match_client(context, rock, upns[i])) { ++ retval = krb5_parse_name_flags(context, upns[i], ++ KRB5_PRINCIPAL_PARSE_ENTERPRISE, &upn); ++ if (retval) { ++ /* XXX trace */ ++ continue; ++ } ++ match = cb->match_client(context, rock, upn); ++ krb5_free_principal(context, upn); ++ if (match) { + TRACE_PKINIT_SERVER_MATCHING_UPN_FOUND(context); + *valid_san = 1; + retval = 0; +@@ -286,7 +293,7 @@ out: + } + if (upns != NULL) { + for (i = 0; upns[i] != NULL; i++) +- krb5_free_principal(context, upns[i]); ++ free(upns[i]); + free(upns); + } + #ifdef DEBUG_SAN_INFO diff --git a/Save-SANs-separately-and-unparse-them-with-NO_REALM.patch b/Save-SANs-separately-and-unparse-them-with-NO_REALM.patch new file mode 100644 index 0000000..689ed40 --- /dev/null +++ b/Save-SANs-separately-and-unparse-them-with-NO_REALM.patch @@ -0,0 +1,148 @@ +From 8924d4bbbf82a29f1d6bf524a416d6e44b694734 Mon Sep 17 00:00:00 2001 +From: Greg Hudson +Date: Thu, 22 Mar 2018 19:46:22 -0400 +Subject: [PATCH] Save SANs separately and unparse them with NO_REALM + +(cherry picked from commit 23ea8d6a9617d17ae5a529c23174d77adac39055) +--- + src/plugins/preauth/pkinit/pkinit_crypto.h | 4 +-- + src/plugins/preauth/pkinit/pkinit_crypto_openssl.c | 37 +++------------------- + src/plugins/preauth/pkinit/pkinit_matching.c | 30 +++++++++++++----- + 3 files changed, 28 insertions(+), 43 deletions(-) + +diff --git a/src/plugins/preauth/pkinit/pkinit_crypto.h b/src/plugins/preauth/pkinit/pkinit_crypto.h +index 2d3733bbc..c7ff29fb2 100644 +--- a/src/plugins/preauth/pkinit/pkinit_crypto.h ++++ b/src/plugins/preauth/pkinit/pkinit_crypto.h +@@ -97,8 +97,8 @@ typedef struct _pkinit_cert_matching_data { + char *issuer_dn; /* rfc2253-style issuer name string */ + unsigned int ku_bits; /* key usage information */ + unsigned int eku_bits; /* extended key usage information */ +- krb5_principal *sans; /* Null-terminated array of subject alternative +- name info (pkinit and ms-upn) */ ++ krb5_principal *sans; /* Null-terminated array of PKINIT SANs */ ++ krb5_principal *upns; /* Null-terimnated array of UPN SANs */ + } pkinit_cert_matching_data; + + /* +diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +index 4f21f90d2..bc6de7ae8 100644 +--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c ++++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +@@ -5093,6 +5093,9 @@ crypto_cert_free_matching_data(krb5_context context, + for (i = 0; md->sans != NULL && md->sans[i] != NULL; i++) + krb5_free_principal(context, md->sans[i]); + free(md->sans); ++ for (i = 0; md->upns != NULL && md->upns[i] != NULL; i++) ++ krb5_free_principal(context, md->upns[i]); ++ free(md->upns); + free(md); + } + +@@ -5121,8 +5124,6 @@ get_matching_data(krb5_context context, + { + krb5_error_code ret = ENOMEM; + pkinit_cert_matching_data *md = NULL; +- krb5_principal *pkinit_sans = NULL, *upn_sans = NULL; +- size_t i, j; + + *md_out = NULL; + +@@ -5139,40 +5140,10 @@ get_matching_data(krb5_context context, + + /* Get the SAN data. */ + ret = crypto_retrieve_X509_sans(context, plg_cryptoctx, req_cryptoctx, +- cert, &pkinit_sans, &upn_sans, NULL); ++ cert, &md->sans, &md->upns, NULL); + if (ret) + goto cleanup; + +- j = 0; +- if (pkinit_sans != NULL) { +- for (i = 0; pkinit_sans[i] != NULL; i++) +- j++; +- } +- if (upn_sans != NULL) { +- for (i = 0; upn_sans[i] != NULL; i++) +- j++; +- } +- if (j != 0) { +- md->sans = calloc((size_t)j+1, sizeof(*md->sans)); +- if (md->sans == NULL) { +- ret = ENOMEM; +- goto cleanup; +- } +- j = 0; +- if (pkinit_sans != NULL) { +- for (i = 0; pkinit_sans[i] != NULL; i++) +- md->sans[j++] = pkinit_sans[i]; +- free(pkinit_sans); +- } +- if (upn_sans != NULL) { +- for (i = 0; upn_sans[i] != NULL; i++) +- md->sans[j++] = upn_sans[i]; +- free(upn_sans); +- } +- md->sans[j] = NULL; +- } else +- md->sans = NULL; +- + /* Get the KU and EKU data. */ + ret = crypto_retrieve_X509_key_usage(context, plg_cryptoctx, + req_cryptoctx, cert, &md->ku_bits, +diff --git a/src/plugins/preauth/pkinit/pkinit_matching.c b/src/plugins/preauth/pkinit/pkinit_matching.c +index c1ce84b82..37bd0251a 100644 +--- a/src/plugins/preauth/pkinit/pkinit_matching.c ++++ b/src/plugins/preauth/pkinit/pkinit_matching.c +@@ -470,7 +470,6 @@ component_match(krb5_context context, + { + int match = 0; + int i; +- krb5_principal p; + char *princ_string; + + switch (rc->kwval_type) { +@@ -483,10 +482,17 @@ component_match(krb5_context context, + match = regexp_match(context, rc, md->issuer_dn); + break; + case kw_san: +- if (md->sans == NULL) +- break; +- for (i = 0, p = md->sans[i]; p != NULL; p = md->sans[++i]) { +- krb5_unparse_name(context, p, &princ_string); ++ for (i = 0; md->sans != NULL && md->sans[i] != NULL; i++) { ++ krb5_unparse_name(context, md->sans[i], &princ_string); ++ match = regexp_match(context, rc, princ_string); ++ krb5_free_unparsed_name(context, princ_string); ++ if (match) ++ break; ++ } ++ for (i = 0; md->upns != NULL && md->upns[i] != NULL; i++) { ++ krb5_unparse_name_flags(context, md->upns[i], ++ KRB5_PRINCIPAL_UNPARSE_NO_REALM, ++ &princ_string); + match = regexp_match(context, rc, princ_string); + krb5_free_unparsed_name(context, princ_string); + if (match) +@@ -572,10 +578,18 @@ check_all_certs(krb5_context context, + pkiDebug("%s: subject: '%s'\n", __FUNCTION__, md->subject_dn); + #if 0 + pkiDebug("%s: issuer: '%s'\n", __FUNCTION__, md->subject_dn); +- for (j = 0, p = md->sans[j]; p != NULL; p = md->sans[++j]) { ++ for (j = 0; md->sans != NULL && md->sans[j] != NULL; j++) { + char *san_string; +- krb5_unparse_name(context, p, &san_string); +- pkiDebug("%s: san: '%s'\n", __FUNCTION__, san_string); ++ krb5_unparse_name(context, md->sans[j], &san_string); ++ pkiDebug("%s: PKINIT san: '%s'\n", __FUNCTION__, san_string); ++ krb5_free_unparsed_name(context, san_string); ++ } ++ for (j = 0; md->upns != NULL && md->upns[j] != NULL; j++) { ++ char *san_string; ++ krb5_unparse_name_flags(context, md->upns[j], ++ KRB5_PRINCIPAL_UNPARSE_NO_REALM, ++ &san_string); ++ pkiDebug("%s: UPN san: '%s'\n", __FUNCTION__, san_string); + krb5_free_unparsed_name(context, san_string); + } + #endif diff --git a/krb5.spec b/krb5.spec index 8b22173..abe7092 100644 --- a/krb5.spec +++ b/krb5.spec @@ -18,7 +18,7 @@ Summary: The Kerberos network authentication system Name: krb5 Version: 1.16 # for prerelease, should be e.g., 0.% {prerelease}.1% { ?dist } (without spaces) -Release: 20%{?dist} +Release: 21%{?dist} # lookaside-cached sources; two downloads and a build artifact Source0: https://web.mit.edu/kerberos/dist/krb5/1.16/krb5-%{version}%{prerelease}.tar.gz @@ -89,6 +89,9 @@ Patch62: Fix-SPAKE-memory-leak.patch Patch63: Continue-after-KRB5_CC_END-in-KCM-cache-iteration.patch Patch64: Zap-data-when-freeing-krb5_spake_factor.patch Patch65: Be-more-careful-asking-for-AS-key-in-SPAKE-client.patch +Patch66: Save-SANs-separately-and-unparse-them-with-NO_REALM.patch +Patch67: Return-UPN-SANs-as-strings.patch +Patch68: Restrict-pre-authentication-fallback-cases.patch License: MIT URL: http://web.mit.edu/kerberos/www/ @@ -739,6 +742,9 @@ exit 0 %{_libdir}/libkadm5srv_mit.so.* %changelog +* Mon Apr 09 2018 Robbie Harwood - 1.16-21 +- Restrict pre-authentication fallback cases + * Tue Apr 03 2018 Robbie Harwood - 1.16-20 - Be more careful asking for AS key in SPAKE client