Restrict pre-authentication fallback cases

This commit is contained in:
Robbie Harwood 2018-04-09 12:12:08 -04:00
parent 9f52d3d29f
commit 8ed07abedf
4 changed files with 850 additions and 1 deletions

View File

@ -0,0 +1,491 @@
From 4a13b97ffba771de4b45b1ed309934cc840569d1 Mon Sep 17 00:00:00 2001
From: Greg Hudson <ghudson@mit.edu>
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:',

View File

@ -0,0 +1,204 @@
From 06d48c8d04a5efb098b026a1ec1c1609a5491ab0 Mon Sep 17 00:00:00 2001
From: Greg Hudson <ghudson@mit.edu>
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 <dlfcn.h>
@@ -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

View File

@ -0,0 +1,148 @@
From 8924d4bbbf82a29f1d6bf524a416d6e44b694734 Mon Sep 17 00:00:00 2001
From: Greg Hudson <ghudson@mit.edu>
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

View File

@ -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 <rharwood@redhat.com> - 1.16-21
- Restrict pre-authentication fallback cases
* Tue Apr 03 2018 Robbie Harwood <rharwood@redhat.com> - 1.16-20
- Be more careful asking for AS key in SPAKE client