From ed118374c6fdfbd04ee56d241ab378714077bb6d Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 12 Jul 2017 23:11:32 +1200 Subject: [PATCH 1/2] s4-smbtorture: Add test krb5.kdc to prove fix for CVE-2017-11103 Signed-off-by: Andrew Bartlett --- source4/torture/krb5/kdc-heimdal.c | 100 ++++++++++++++++++++++++++++--------- 1 file changed, 76 insertions(+), 24 deletions(-) diff --git a/source4/torture/krb5/kdc-heimdal.c b/source4/torture/krb5/kdc-heimdal.c index 7d786dc..af41c21 100644 --- a/source4/torture/krb5/kdc-heimdal.c +++ b/source4/torture/krb5/kdc-heimdal.c @@ -437,8 +437,9 @@ static bool torture_krb5_post_recv_test(struct torture_krb5_context *test_contex "Too many packets"); break; case TORTURE_KRB5_TEST_CHANGE_SERVER_IN: + case TORTURE_KRB5_TEST_CHANGE_SERVER_BOTH: { - AS_REQ mod_as_req; + AS_REP mod_as_rep; krb5_error_code k5ret; krb5_data modified_recv_buf; if (test_context->packet_count == 0) { @@ -468,31 +469,30 @@ static bool torture_krb5_post_recv_test(struct torture_krb5_context *test_contex test_context->as_rep.ticket.tkt_vno, 5, "Got wrong as_rep->ticket.tkt_vno"); torture_assert_int_equal(test_context->tctx, - test_context->as_rep.ticket.sname.len, 2, - "Got wrong as_rep->ticket.sname.len"); - free(test_context->as_rep.ticket.sname.val[0]); - free(test_context->as_rep.ticket.sname.val[1]); + test_context->as_rep.ticket.sname.name_string.len, 2, + "Got wrong as_rep->ticket.sname.name_string.len"); + free(test_context->as_rep.ticket.sname.name_string.val[0]); + free(test_context->as_rep.ticket.sname.name_string.val[1]); test_context->as_rep.ticket.sname.name_string.val[0] = strdup("bad"); - test_context->as_rep.ticket.sname.name_string.val[0] = strdup("mallory"); + test_context->as_rep.ticket.sname.name_string.val[1] = strdup("mallory"); - free_AS_REP(&test_context->as_rep); + mod_as_rep = test_context->as_rep; + + ASN1_MALLOC_ENCODE(AS_REP, modified_recv_buf.data, modified_recv_buf.length, + &mod_as_rep, &used, k5ret); + torture_assert_int_equal(test_context->tctx, + k5ret, 0, + "encode_AS_REQ failed"); + krb5_data_free(recv_buf); + + *recv_buf = modified_recv_buf; + free_AS_REQ(&test_context->as_req); } torture_assert(test_context->tctx, test_context->packet_count < 3, "too many packets"); - - ASN1_MALLOC_ENCODE(AS_REQ, modified_recv_buf->data, modified_recv_buf->length, - &mod_as_req, &used, k5ret); - torture_assert_int_equal(test_context->tctx, - k5ret, 0, - "encode_AS_REQ failed"); - krb5_data_free(*recv_buf); - - *recv_buf = modified_recv_buf; - free_AS_REQ(&test_context->as_req); - break; - case TORTURE_KRB5_TEST_CHANGE_SERVER_BOTH: break; } + } return true; @@ -593,14 +593,23 @@ static bool torture_krb5_as_req_creds(struct torture_context *tctx, krb5_creds my_creds; krb5_principal principal; struct smb_krb5_context *smb_krb5_context; + krb5_context k5_context; enum credentials_obtained obtained; const char *error_string; const char *password = cli_credentials_get_password(credentials); + const char *expected_principal_string; krb5_get_init_creds_opt *krb_options = NULL; - + const char *realm; + ok = torture_krb5_init_context(tctx, test, &smb_krb5_context); torture_assert(tctx, ok, "torture_krb5_init_context failed"); - + k5_context = smb_krb5_context->krb5_context; + + expected_principal_string + = cli_credentials_get_principal(credentials, + tctx); + + realm = strupper_talloc(tctx, cli_credentials_get_realm(credentials)); k5ret = principal_from_credentials(tctx, credentials, smb_krb5_context, &principal, &obtained, &error_string); torture_assert_int_equal(tctx, k5ret, 0, error_string); @@ -687,16 +696,55 @@ static bool torture_krb5_as_req_creds(struct torture_context *tctx, switch (test) { case TORTURE_KRB5_TEST_PLAIN: - case TORTURE_KRB5_TEST_CHANGE_SERVER_OUT: case TORTURE_KRB5_TEST_CHANGE_SERVER_IN: - case TORTURE_KRB5_TEST_CHANGE_SERVER_BOTH: case TORTURE_KRB5_TEST_PAC_REQUEST: case TORTURE_KRB5_TEST_AES: case TORTURE_KRB5_TEST_RC4: case TORTURE_KRB5_TEST_AES_RC4: + { + char *got_principal_string; + char *assertion_message; torture_assert_int_equal(tctx, k5ret, 0, "krb5_get_init_creds_password failed"); - break; + + torture_assert_int_equal(tctx, + krb5_principal_get_type(k5_context, + my_creds.client), + KRB5_NT_PRINCIPAL, + "smb_krb5_init_context gave incorrect client->name.name_type"); + + torture_assert_int_equal(tctx, + krb5_unparse_name(k5_context, + my_creds.client, + &got_principal_string), 0, + "krb5_unparse_name failed"); + + assertion_message = talloc_asprintf(tctx, + "krb5_get_init_creds_password returned a different principal %s to what was expected %s", + got_principal_string, expected_principal_string); + krb5_free_unparsed_name(k5_context, got_principal_string); + + torture_assert(tctx, krb5_principal_compare(k5_context, + my_creds.client, + principal), + assertion_message); + + + torture_assert_str_equal(tctx, + my_creds.server->name.name_string.val[0], + "krbtgt", + "Mismatch in name between AS_REP and expected response, expected krbtgt"); + torture_assert_str_equal(tctx, + my_creds.server->name.name_string.val[1], + realm, + "Mismatch in realm part of krbtgt/ in AS_REP, expected krbtgt/REALM@REALM"); + + torture_assert_str_equal(tctx, + my_creds.server->realm, + realm, + "Mismatch in server realm in AS_REP, expected krbtgt/REALM@REALM"); + break; + } case TORTURE_KRB5_TEST_BREAK_PW: torture_assert_int_equal(tctx, k5ret, KRB5KDC_ERR_PREAUTH_FAILED, "krb5_get_init_creds_password should have failed"); return true; @@ -705,6 +753,10 @@ static bool torture_krb5_as_req_creds(struct torture_context *tctx, torture_assert_int_equal(tctx, k5ret, KRB5KRB_AP_ERR_SKEW, "krb5_get_init_creds_password should have failed"); return true; + case TORTURE_KRB5_TEST_CHANGE_SERVER_OUT: + case TORTURE_KRB5_TEST_CHANGE_SERVER_BOTH: + torture_assert_int_equal(tctx, k5ret, 0, "krb5_get_init_creds_password failed"); + break; } k5ret = krb5_free_cred_contents(smb_krb5_context->krb5_context, &my_creds); -- 2.9.4 From e74f431c3fca76d98b49b9b8d5c3a8d326a98d9d Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 13 Jul 2017 00:38:29 +1200 Subject: [PATCH 2/2] s4-smbtorture: Show that the KDC provides no protection from CVE-2017-11103 The server name in the AS-REQ is unprotected, sadly. Signed-off-by: Andrew Bartlett --- source4/torture/krb5/kdc-heimdal.c | 117 ++++++++++++++++++++++++++++++++----- 1 file changed, 103 insertions(+), 14 deletions(-) diff --git a/source4/torture/krb5/kdc-heimdal.c b/source4/torture/krb5/kdc-heimdal.c index af41c21..6c0739e 100644 --- a/source4/torture/krb5/kdc-heimdal.c +++ b/source4/torture/krb5/kdc-heimdal.c @@ -57,6 +57,8 @@ struct torture_krb5_context { int packet_count; AS_REQ as_req; AS_REP as_rep; + const char *krb5_service; + const char *krb5_hostname; }; /* @@ -67,7 +69,7 @@ struct torture_krb5_context { * */ -static bool torture_krb5_pre_send_test(struct torture_krb5_context *test_context, const krb5_data *send_buf) +static bool torture_krb5_pre_send_test(struct torture_krb5_context *test_context, krb5_data *send_buf) { size_t used; switch (test_context->test) @@ -88,8 +90,41 @@ static bool torture_krb5_pre_send_test(struct torture_krb5_context *test_context break; case TORTURE_KRB5_TEST_CHANGE_SERVER_OUT: case TORTURE_KRB5_TEST_CHANGE_SERVER_BOTH: + { + AS_REQ mod_as_req; + krb5_error_code k5ret; + krb5_data modified_send_buf; + torture_assert_int_equal(test_context->tctx, + decode_AS_REQ(send_buf->data, send_buf->length, &test_context->as_req, &used), 0, + "decode_AS_REQ failed"); + torture_assert_int_equal(test_context->tctx, used, send_buf->length, "length mismatch"); + torture_assert_int_equal(test_context->tctx, test_context->as_req.pvno, 5, "Got wrong as_req->pvno"); + + /* Only change it if configured with --option=torture:krb5-hostname= */ + if (test_context->krb5_hostname[0] == '\0') { + break; + } + + mod_as_req = test_context->as_req; + + torture_assert_int_equal(test_context->tctx, + mod_as_req.req_body.sname->name_string.len, 2, + "Sending wrong mod_as_req.req_body->sname.name_string.len"); + free(mod_as_req.req_body.sname->name_string.val[0]); + free(mod_as_req.req_body.sname->name_string.val[1]); + mod_as_req.req_body.sname->name_string.val[0] = strdup(test_context->krb5_service); + mod_as_req.req_body.sname->name_string.val[1] = strdup(test_context->krb5_hostname); + + ASN1_MALLOC_ENCODE(AS_REQ, modified_send_buf.data, modified_send_buf.length, + &mod_as_req, &used, k5ret); + torture_assert_int_equal(test_context->tctx, + k5ret, 0, + "encode_AS_REQ failed"); + + *send_buf = modified_send_buf; break; } + } return true; } @@ -206,8 +241,8 @@ static bool torture_krb5_post_recv_test(struct torture_krb5_context *test_contex switch (test_context->test) { - case TORTURE_KRB5_TEST_CHANGE_SERVER_OUT: case TORTURE_KRB5_TEST_PLAIN: + case TORTURE_KRB5_TEST_CHANGE_SERVER_OUT: if (test_context->packet_count == 0) { ok = torture_check_krb5_error(test_context, recv_buf, @@ -237,14 +272,16 @@ static bool torture_krb5_post_recv_test(struct torture_krb5_context *test_contex torture_assert(test_context->tctx, test_context->as_rep.ticket.enc_part.kvno, "Did not get a KVNO in test_context->as_rep.ticket.enc_part.kvno"); - if (torture_setting_bool(test_context->tctx, "expect_cached_at_rodc", false)) { - torture_assert_int_not_equal(test_context->tctx, - *test_context->as_rep.ticket.enc_part.kvno & 0xFFFF0000, - 0, "Did not get a RODC number in the KVNO"); - } else { - torture_assert_int_equal(test_context->tctx, - *test_context->as_rep.ticket.enc_part.kvno & 0xFFFF0000, + if (test_context->test == TORTURE_KRB5_TEST_PLAIN) { + if (torture_setting_bool(test_context->tctx, "expect_cached_at_rodc", false)) { + torture_assert_int_not_equal(test_context->tctx, + *test_context->as_rep.ticket.enc_part.kvno & 0xFFFF0000, + 0, "Did not get a RODC number in the KVNO"); + } else { + torture_assert_int_equal(test_context->tctx, + *test_context->as_rep.ticket.enc_part.kvno & 0xFFFF0000, 0, "Unexpecedly got a RODC number in the KVNO"); + } } free_AS_REP(&test_context->as_rep); } @@ -473,7 +510,7 @@ static bool torture_krb5_post_recv_test(struct torture_krb5_context *test_contex "Got wrong as_rep->ticket.sname.name_string.len"); free(test_context->as_rep.ticket.sname.name_string.val[0]); free(test_context->as_rep.ticket.sname.name_string.val[1]); - test_context->as_rep.ticket.sname.name_string.val[0] = strdup("bad"); + test_context->as_rep.ticket.sname.name_string.val[0] = strdup("good"); test_context->as_rep.ticket.sname.name_string.val[1] = strdup("mallory"); mod_as_rep = test_context->as_rep; @@ -522,17 +559,18 @@ static krb5_error_code smb_krb5_send_and_recv_func_override(krb5_context context { krb5_error_code k5ret; bool ok; - + krb5_data modified_send_buf = *send_buf; + struct torture_krb5_context *test_context = talloc_get_type_abort(data, struct torture_krb5_context); - ok = torture_krb5_pre_send_test(test_context, send_buf); + ok = torture_krb5_pre_send_test(test_context, &modified_send_buf); if (ok == false) { return EINVAL; } k5ret = smb_krb5_send_and_recv_func_forced(context, test_context->server, - hi, timeout, send_buf, recv_buf); + hi, timeout, &modified_send_buf, recv_buf); if (k5ret != 0) { return k5ret; } @@ -567,6 +605,9 @@ static bool torture_krb5_init_context(struct torture_context *tctx, test_context->test = test; test_context->tctx = tctx; + test_context->krb5_service = torture_setting_string(tctx, "krb5-service", "host"); + test_context->krb5_hostname = torture_setting_string(tctx, "krb5-hostname", ""); + k5ret = smb_krb5_init_context(tctx, tctx->lp_ctx, smb_krb5_context); torture_assert_int_equal(tctx, k5ret, 0, "smb_krb5_init_context failed"); @@ -600,7 +641,10 @@ static bool torture_krb5_as_req_creds(struct torture_context *tctx, const char *expected_principal_string; krb5_get_init_creds_opt *krb_options = NULL; const char *realm; + const char *krb5_service = torture_setting_string(tctx, "krb5-service", "host"); + const char *krb5_hostname = torture_setting_string(tctx, "krb5-hostname", ""); + ok = torture_krb5_init_context(tctx, test, &smb_krb5_context); torture_assert(tctx, ok, "torture_krb5_init_context failed"); k5_context = smb_krb5_context->krb5_context; @@ -755,10 +799,55 @@ static bool torture_krb5_as_req_creds(struct torture_context *tctx, case TORTURE_KRB5_TEST_CHANGE_SERVER_OUT: case TORTURE_KRB5_TEST_CHANGE_SERVER_BOTH: + { + char *got_principal_string; + char *assertion_message; torture_assert_int_equal(tctx, k5ret, 0, "krb5_get_init_creds_password failed"); + + torture_assert_int_equal(tctx, + krb5_principal_get_type(k5_context, + my_creds.client), + KRB5_NT_PRINCIPAL, + "smb_krb5_init_context gave incorrect client->name.name_type"); + + torture_assert_int_equal(tctx, + krb5_unparse_name(k5_context, + my_creds.client, + &got_principal_string), 0, + "krb5_unparse_name failed"); + + assertion_message = talloc_asprintf(tctx, + "krb5_get_init_creds_password returned a different principal %s to what was expected %s", + got_principal_string, expected_principal_string); + krb5_free_unparsed_name(k5_context, got_principal_string); + + torture_assert(tctx, krb5_principal_compare(k5_context, + my_creds.client, + principal), + assertion_message); + + if (krb5_hostname[0] == '\0') { + break; + } + + torture_assert_str_equal(tctx, + my_creds.server->name.name_string.val[0], + krb5_service, + "Mismatch in name[0] between AS_REP and expected response"); + torture_assert_str_equal(tctx, + my_creds.server->name.name_string.val[1], + krb5_hostname, + "Mismatch in name[1] between AS_REP and expected response"); + + torture_assert_str_equal(tctx, + my_creds.server->realm, + realm, + "Mismatch in server realm in AS_REP, expected krbtgt/REALM@REALM"); + break; } - + } + k5ret = krb5_free_cred_contents(smb_krb5_context->krb5_context, &my_creds); torture_assert_int_equal(tctx, k5ret, 0, "krb5_free_creds failed"); -- 2.9.4