From e98d94d2bc328a01051b88cda4793fd851f29f89 Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Tue, 28 May 2013 17:47:00 -0400 Subject: [PATCH] Add proposed fix for handling AS client clock skew In addition to basing the contents of an encrypted-timestamp preauth data item on the server's idea of the current time, go ahead and do the same for the times in the request. --- krb5-1.11.2-skew1.patch | 115 ++++++++++++++++++++++++++++++++ krb5-1.11.2-skew2.patch | 144 ++++++++++++++++++++++++++++++++++++++++ krb5.spec | 8 +++ 3 files changed, 267 insertions(+) create mode 100644 krb5-1.11.2-skew1.patch create mode 100644 krb5-1.11.2-skew2.patch diff --git a/krb5-1.11.2-skew1.patch b/krb5-1.11.2-skew1.patch new file mode 100644 index 0000000..c7dc919 --- /dev/null +++ b/krb5-1.11.2-skew1.patch @@ -0,0 +1,115 @@ +>From 8f6d12bae1a0f1d274593c4a06dfa5948aa61418 Mon Sep 17 00:00:00 2001 +From: Stef Walter +Date: Thu, 23 May 2013 08:38:20 +0200 +Subject: [PATCH 1/2] krb5: Refator duplicate code for setting the AS REQ nonce + +--- + src/lib/krb5/krb/get_in_tkt.c | 64 +++++++++++++++++++++++-------------------- + 1 file changed, 35 insertions(+), 29 deletions(-) + +diff --git a/src/lib/krb5/krb/get_in_tkt.c b/src/lib/krb5/krb/get_in_tkt.c +index 828b0fb..1058112 100644 +--- a/src/lib/krb5/krb/get_in_tkt.c ++++ b/src/lib/krb5/krb/get_in_tkt.c +@@ -650,6 +650,34 @@ cleanup: + return code; + } + ++static krb5_error_code ++update_req_before_encoding(krb5_context context, krb5_init_creds_context ctx) ++{ ++ krb5_error_code code = 0; ++ unsigned char random_buf[4]; ++ krb5_data random_data; ++ ++ /* ++ * RFC 6113 requires a new nonce for the inner request on each try. It's ++ * permitted to change the nonce even for non-FAST as well. ++ */ ++ random_data.length = 4; ++ random_data.data = (char *)random_buf; ++ code = krb5_c_random_make_octets(context, &random_data); ++ if (code != 0) ++ goto cleanup; ++ ++ /* ++ * See RT ticket 3196 at MIT. If we set the high bit, we may have ++ * compatibility problems with Heimdal, because we (incorrectly) encode ++ * this value as signed. ++ */ ++ ctx->request->nonce = 0x7fffffff & load_32_n(random_buf); ++ ++cleanup: ++ return code; ++} ++ + /** + * Throw away any state related to specific realm either at the beginning of a + * request, or when a realm changes, or when we start to use FAST after +@@ -664,8 +692,6 @@ restart_init_creds_loop(krb5_context context, krb5_init_creds_context ctx, + krb5_pa_data **padata) + { + krb5_error_code code = 0; +- unsigned char random_buf[4]; +- krb5_data random_data; + krb5_timestamp from; + + if (ctx->preauth_to_use) { +@@ -693,18 +719,10 @@ restart_init_creds_loop(krb5_context context, krb5_init_creds_context ctx, + goto cleanup; + } + +- /* Set the request nonce. */ +- random_data.length = 4; +- random_data.data = (char *)random_buf; +- code = krb5_c_random_make_octets(context, &random_data); +- if (code !=0) ++ code = update_req_before_encoding(context, ctx); ++ if (code != 0) + goto cleanup; +- /* +- * See RT ticket 3196 at MIT. If we set the high bit, we may have +- * compatibility problems with Heimdal, because we (incorrectly) encode +- * this value as signed. +- */ +- ctx->request->nonce = 0x7fffffff & load_32_n(random_buf); ++ + krb5_free_principal(context, ctx->request->server); + ctx->request->server = NULL; + +@@ -1188,28 +1206,16 @@ init_creds_step_request(krb5_context context, + { + krb5_error_code code; + krb5_boolean got_real; +- char random_buf[4]; +- krb5_data random_data; + + if (ctx->loopcount >= MAX_IN_TKT_LOOPS) { + code = KRB5_GET_IN_TKT_LOOP; + goto cleanup; + } +- /* +- * RFC 6113 requires a new nonce for the inner request on each try. It's +- * permitted to change the nonce even for non-FAST so we do here. +- */ +- random_data.length = 4; +- random_data.data = (char *)random_buf; +- code = krb5_c_random_make_octets(context, &random_data); +- if (code !=0) ++ ++ code = update_req_before_encoding(context, ctx); ++ if (code != 0) + goto cleanup; +- /* +- * See RT ticket 3196 at MIT. If we set the high bit, we may have +- * compatibility problems with Heimdal, because we (incorrectly) encode +- * this value as signed. +- */ +- ctx->request->nonce = 0x7fffffff & load_32_n(random_buf); ++ + krb5_free_data(context, ctx->inner_request_body); + ctx->inner_request_body = NULL; + code = encode_krb5_kdc_req_body(ctx->request, &ctx->inner_request_body); +-- +1.8.1.4 + diff --git a/krb5-1.11.2-skew2.patch b/krb5-1.11.2-skew2.patch new file mode 100644 index 0000000..d5063bd --- /dev/null +++ b/krb5-1.11.2-skew2.patch @@ -0,0 +1,144 @@ +>From 51ab359d7cc6643cfd4fac28def2e1c756553201 Mon Sep 17 00:00:00 2001 +From: Stef Walter +Date: Thu, 23 May 2013 08:44:43 +0200 +Subject: [PATCH 2/2] krb5: Fix ticket start and end time to respect skew + +Since the kerberos protocol uses timestamp rather than duration deltas +for its starttime, endtime, and renewtime KDC AS REQ fields, we have +to calculate these with respect to the offsets we know about received +from the server. + +Leverage the unauthenticated server time we received during preauth when +calculating these these timestamps from the duration deltas we use +in our krb5 api and tools. + +In order to do this we have to update certain fields of the AS REQ +each time we encode it for sending to the KDC. +--- + src/lib/krb5/krb/get_in_tkt.c | 44 +++++++++++++++++++++++-------------------- + src/lib/krb5/krb/int-proto.h | 5 +++++ + src/lib/krb5/krb/preauth2.c | 8 ++++++++ + 3 files changed, 37 insertions(+), 20 deletions(-) + +diff --git a/src/lib/krb5/krb/get_in_tkt.c b/src/lib/krb5/krb/get_in_tkt.c +index 1058112..694c9b0b 100644 +--- a/src/lib/krb5/krb/get_in_tkt.c ++++ b/src/lib/krb5/krb/get_in_tkt.c +@@ -656,6 +656,8 @@ update_req_before_encoding(krb5_context context, krb5_init_creds_context ctx) + krb5_error_code code = 0; + unsigned char random_buf[4]; + krb5_data random_data; ++ krb5_timestamp from; ++ krb5_int32 unused; + + /* + * RFC 6113 requires a new nonce for the inner request on each try. It's +@@ -674,6 +676,28 @@ update_req_before_encoding(krb5_context context, krb5_init_creds_context ctx) + */ + ctx->request->nonce = 0x7fffffff & load_32_n(random_buf); + ++ code = k5_preauth_get_time(context, &ctx->preauth_rock, TRUE, &ctx->request_time, &unused); ++ if (code != 0) ++ goto cleanup; ++ ++ /* Omit request start time in the common case. MIT and Heimdal KDCs will ++ * ignore it for non-postdated tickets anyway. */ ++ from = krb5int_addint32(ctx->request_time, ctx->start_time); ++ if (ctx->start_time != 0) ++ ctx->request->from = from; ++ ctx->request->till = krb5int_addint32(from, ctx->tkt_life); ++ ++ if (ctx->renew_life > 0) { ++ ctx->request->rtime = ++ krb5int_addint32(from, ctx->renew_life); ++ if (ctx->request->rtime < ctx->request->till) { ++ /* don't ask for a smaller renewable time than the lifetime */ ++ ctx->request->rtime = ctx->request->till; ++ } ++ ctx->request->kdc_options &= ~(KDC_OPT_RENEWABLE_OK); ++ } else ++ ctx->request->rtime = 0; ++ + cleanup: + return code; + } +@@ -692,7 +716,6 @@ restart_init_creds_loop(krb5_context context, krb5_init_creds_context ctx, + krb5_pa_data **padata) + { + krb5_error_code code = 0; +- krb5_timestamp from; + + if (ctx->preauth_to_use) { + krb5_free_pa_data(context, ctx->preauth_to_use); +@@ -732,8 +755,6 @@ restart_init_creds_loop(krb5_context context, krb5_init_creds_context ctx, + if (code != 0) + goto cleanup; + +- ctx->request_time = time(NULL); +- + code = krb5int_fast_as_armor(context, ctx->fast_state, + ctx->opte, ctx->request); + if (code != 0) +@@ -747,23 +768,6 @@ restart_init_creds_loop(krb5_context context, krb5_init_creds_context ctx, + /* give the preauth plugins a chance to prep the request body */ + krb5_preauth_prepare_request(context, ctx->opte, ctx->request); + +- /* Omit request start time in the common case. MIT and Heimdal KDCs will +- * ignore it for non-postdated tickets anyway. */ +- from = krb5int_addint32(ctx->request_time, ctx->start_time); +- if (ctx->start_time != 0) +- ctx->request->from = from; +- ctx->request->till = krb5int_addint32(from, ctx->tkt_life); +- +- if (ctx->renew_life > 0) { +- ctx->request->rtime = +- krb5int_addint32(from, ctx->renew_life); +- if (ctx->request->rtime < ctx->request->till) { +- /* don't ask for a smaller renewable time than the lifetime */ +- ctx->request->rtime = ctx->request->till; +- } +- ctx->request->kdc_options &= ~(KDC_OPT_RENEWABLE_OK); +- } else +- ctx->request->rtime = 0; + code = krb5int_fast_prep_req_body(context, ctx->fast_state, + ctx->request, + &ctx->outer_request_body); +diff --git a/src/lib/krb5/krb/int-proto.h b/src/lib/krb5/krb/int-proto.h +index 3326154..83a47c0 100644 +--- a/src/lib/krb5/krb/int-proto.h ++++ b/src/lib/krb5/krb/int-proto.h +@@ -142,6 +142,11 @@ krb5_preauth_supply_preauth_data(krb5_context context, + const char *value); + + krb5_error_code ++k5_preauth_get_time(krb5_context context, krb5_clpreauth_rock rock, ++ krb5_boolean allow_unauth_time, krb5_timestamp *time_out, ++ krb5_int32 *usec_out); ++ ++krb5_error_code + clpreauth_encrypted_challenge_initvt(krb5_context context, int maj_ver, + int min_ver, krb5_plugin_vtable vtable); + +diff --git a/src/lib/krb5/krb/preauth2.c b/src/lib/krb5/krb/preauth2.c +index 747611e..167f611 100644 +--- a/src/lib/krb5/krb/preauth2.c ++++ b/src/lib/krb5/krb/preauth2.c +@@ -397,6 +397,15 @@ get_preauth_time(krb5_context context, krb5_clpreauth_rock rock, + krb5_boolean allow_unauth_time, krb5_timestamp *time_out, + krb5_int32 *usec_out) + { ++ return k5_preauth_get_time(context, rock, allow_unauth_time, ++ time_out, usec_out); ++} ++ ++krb5_error_code ++k5_preauth_get_time(krb5_context context, krb5_clpreauth_rock rock, ++ krb5_boolean allow_unauth_time, krb5_timestamp *time_out, ++ krb5_int32 *usec_out) ++{ + if (rock->pa_offset_state != NO_OFFSET && + (allow_unauth_time || rock->pa_offset_state == AUTH_OFFSET) && + (context->library_options & KRB5_LIBOPT_SYNC_KDCTIME)) { +-- +1.8.1.4 + diff --git a/krb5.spec b/krb5.spec index a56d346..49a7253 100644 --- a/krb5.spec +++ b/krb5.spec @@ -82,6 +82,8 @@ Patch121: krb5-cccol-primary.patch Patch122: krb5-1.11.2-gss_transited.patch Patch123: krb5-1.11.2-empty_passwords.patch Patch124: krb5-1.11.2-arcfour_short.patch +Patch125: krb5-1.11.2-skew1.patch +Patch126: krb5-1.11.2-skew2.patch # Patches for otp plugin backport Patch201: krb5-1.11.2-keycheck.patch @@ -306,6 +308,8 @@ ln -s NOTICE LICENSE %patch122 -p1 -b .gss_transited %patch123 -p1 -b .empty_passwords %patch124 -p1 -b .arcfour_short +%patch125 -p1 -b .skew1 +%patch126 -p1 -b .skew2 %patch201 -p1 -b .keycheck %patch202 -p1 -b .otp @@ -836,6 +840,10 @@ exit 0 in GSS acceptors (RT#7639, #959685) - backport fix for not being able to pass an empty password to the get-init-creds APIs and have them actually use it (RT#7642, #960001) +- add backported proposed fix to use the unauthenticated server time + as the basis for computing the requested credential expiration times, + rather than the client's idea of the current time, which could be + significantly incorrect (#961221) * Tue May 21 2013 Nalin Dahyabhai 1.11.2-6 - pull in upstream fix to start treating a KRB5CCNAME value that begins