diff options
author | Karolin Seeger <kseeger@samba.org> | 2019-05-14 08:25:01 +0200 |
---|---|---|
committer | Karolin Seeger <kseeger@samba.org> | 2019-05-14 08:25:01 +0200 |
commit | 1a248d16ab941491edcbf2ac8ba0f88cd7c65c29 (patch) | |
tree | 1a63efc21f43ce8a8daf0e202b3e1c8f097d15fc | |
parent | d7fef72c6a373e10289675ef180d49d739cd6a5b (diff) | |
parent | a72d4598bf4a2186769f25050663f4779ea581e0 (diff) | |
download | samba-1a248d16ab941491edcbf2ac8ba0f88cd7c65c29.tar.gz |
Merge tag 'samba-4.8.12' into v4-8-test
samba: tag release samba-4.8.12
-rw-r--r-- | WHATSNEW.txt | 58 | ||||
-rw-r--r-- | source4/heimdal/kdc/krb5tgs.c | 7 | ||||
-rw-r--r-- | source4/torture/krb5/kdc-canon-heimdal.c | 115 |
3 files changed, 174 insertions, 6 deletions
diff --git a/WHATSNEW.txt b/WHATSNEW.txt index b51ba11f813..3b8f058af8e 100644 --- a/WHATSNEW.txt +++ b/WHATSNEW.txt @@ -1,4 +1,58 @@ ============================== + Release Notes for Samba 4.8.12 + May 14, 2019 + ============================== + + +This is a security release in order to address the following defect: + +o CVE-2018-16860 (Samba AD DC S4U2Self/S4U2Proxy unkeyed checksum) + + +======= +Details +======= + +o CVE-2018-16860: + The checksum validation in the S4U2Self handler in the embedded Heimdal KDC + did not first confirm that the checksum was keyed, allowing replacement of + the requested target (client) principal. + +For more details and workarounds, please refer to the security advisory. + + +Changes since 4.8.11: +--------------------- + +o Isaac Boukris <iboukris@gmail.com> + * BUG 13685: CVE-2018-16860: Heimdal KDC: Reject PA-S4U2Self with unkeyed + checksum. + + +####################################### +Reporting bugs & Development Discussion +####################################### + +Please discuss this release on the samba-technical mailing list or by +joining the #samba-technical IRC channel on irc.freenode.net. + +If you do report problems then please try to send high quality +feedback. If you don't provide vital information to help us track down +the problem then you will probably be ignored. All bug reports should +be filed under the "Samba 4.1 and newer" product in the project's Bugzilla +database (https://bugzilla.samba.org/). + + +====================================================================== +== Our Code, Our Bugs, Our Responsibility. +== The Samba Team +====================================================================== + + +Release notes for older releases follow: +---------------------------------------- + + ============================== Release Notes for Samba 4.8.11 April 8, 2019 ============================== @@ -49,8 +103,8 @@ database (https://bugzilla.samba.org/). ====================================================================== -Release notes for older releases follow: ----------------------------------------- +---------------------------------------------------------------------- + ============================== Release Notes for Samba 4.8.10 diff --git a/source4/heimdal/kdc/krb5tgs.c b/source4/heimdal/kdc/krb5tgs.c index a888788bb6f..ff7d93138c0 100644 --- a/source4/heimdal/kdc/krb5tgs.c +++ b/source4/heimdal/kdc/krb5tgs.c @@ -1925,6 +1925,13 @@ server_lookup: goto out; } + if (!krb5_checksum_is_keyed(context, self.cksum.cksumtype)) { + free_PA_S4U2Self(&self); + kdc_log(context, config, 0, "Reject PA-S4U2Self with unkeyed checksum"); + ret = KRB5KRB_AP_ERR_INAPP_CKSUM; + goto out; + } + ret = _krb5_s4u2self_to_checksumdata(context, &self, &datack); if (ret) goto out; diff --git a/source4/torture/krb5/kdc-canon-heimdal.c b/source4/torture/krb5/kdc-canon-heimdal.c index 7f806e73e66..1b2af2d002d 100644 --- a/source4/torture/krb5/kdc-canon-heimdal.c +++ b/source4/torture/krb5/kdc-canon-heimdal.c @@ -43,7 +43,9 @@ #define TEST_UPN 0x0000040 #define TEST_S4U2SELF 0x0000080 #define TEST_REMOVEDOLLAR 0x0000100 -#define TEST_ALL 0x00001FF +#define TEST_AS_REQ_SPN 0x0000200 /* not used */ +#define TEST_MITM_S4U2SELF 0x0000400 +#define TEST_ALL 0x00007FF struct test_data { const char *test_name; @@ -61,6 +63,7 @@ struct test_data { bool upn; bool other_upn_suffix; bool s4u2self; + bool mitm_s4u2self; bool removedollar; const char *krb5_service; const char *krb5_hostname; @@ -209,6 +212,67 @@ static bool test_accept_ticket(struct torture_context *tctx, return true; } +krb5_error_code +_krb5_s4u2self_to_checksumdata(krb5_context context, + const PA_S4U2Self *self, + krb5_data *data); + +/* Helper function to modify the principal in PA_FOR_USER padata */ +static bool change_for_user_principal(struct torture_krb5_context *test_context, + krb5_data *modified_send_buf) +{ + PA_DATA *for_user; + int i = 0; + size_t used; + krb5_error_code ret; + PA_S4U2Self self, mod_self; + krb5_data cksum_data; + krb5_principal admin; + heim_octet_string orig_padata_value; + krb5_context k5_ctx = test_context->smb_krb5_context->krb5_context; + + for_user = krb5_find_padata(test_context->tgs_req.padata->val, + test_context->tgs_req.padata->len, KRB5_PADATA_FOR_USER, &i); + torture_assert(test_context->tctx, for_user != NULL, "No PA_FOR_USER in s4u2self request"); + orig_padata_value = for_user->padata_value; + + torture_assert_int_equal(test_context->tctx, + krb5_make_principal(k5_ctx, &admin, test_context->test_data->realm, + "Administrator", NULL), + 0, "krb5_make_principal() failed"); + torture_assert_int_equal(test_context->tctx, + decode_PA_S4U2Self(for_user->padata_value.data, + for_user->padata_value.length, &self, NULL), + 0, "decode_PA_S4U2Self() failed"); + mod_self = self; + mod_self.name = admin->name; + + torture_assert_int_equal(test_context->tctx, + _krb5_s4u2self_to_checksumdata(k5_ctx, &mod_self, &cksum_data), + 0, "_krb5_s4u2self_to_checksumdata() failed"); + torture_assert_int_equal(test_context->tctx, + krb5_create_checksum(k5_ctx, NULL, KRB5_KU_OTHER_CKSUM, + CKSUMTYPE_CRC32, cksum_data.data, + cksum_data.length, &mod_self.cksum), + 0, "krb5_create_checksum() failed"); + + ASN1_MALLOC_ENCODE(PA_S4U2Self, for_user->padata_value.data, for_user->padata_value.length, + &mod_self, &used, ret); + torture_assert(test_context->tctx, ret == 0, "Failed to encode PA_S4U2Self ASN1 struct"); + ASN1_MALLOC_ENCODE(TGS_REQ, modified_send_buf->data, modified_send_buf->length, + &test_context->tgs_req, &used, ret); + torture_assert(test_context->tctx, ret == 0, "Failed to encode TGS_REQ ASN1 struct"); + + free(for_user->padata_value.data); + for_user->padata_value = orig_padata_value; + + free_PA_S4U2Self(&self); + krb5_data_free(&cksum_data); + free_Checksum(&mod_self.cksum); + + return true; +} + /* * TEST_AS_REQ and TEST_AS_REQ_SELF - SEND * @@ -598,7 +662,12 @@ static bool torture_krb5_pre_send_tgs_req_canon_test(struct torture_krb5_context } - *modified_send_buf = *send_buf; + if (test_context->test_data->mitm_s4u2self) { + torture_assert(test_context->tctx, change_for_user_principal(test_context, modified_send_buf), + "Failed to modify PA_FOR_USER principal name"); + } else { + *modified_send_buf = *send_buf; + } return true; } @@ -617,6 +686,7 @@ static bool torture_krb5_post_recv_tgs_req_canon_test(struct torture_krb5_contex { KRB_ERROR error; size_t used; + krb5_error_code expected_error; /* * If this account did not have a servicePrincipalName, then @@ -627,9 +697,13 @@ static bool torture_krb5_post_recv_tgs_req_canon_test(struct torture_krb5_contex torture_assert_int_equal(test_context->tctx, error.pvno, 5, "Got wrong error.pvno"); + expected_error = KRB5KDC_ERR_S_PRINCIPAL_UNKNOWN - KRB5KDC_ERR_NONE; + if (error.error_code != expected_error && test_context->test_data->mitm_s4u2self) { + expected_error = KRB5KRB_AP_ERR_INAPP_CKSUM - KRB5KDC_ERR_NONE; + } torture_assert_int_equal(test_context->tctx, error.error_code, - KRB5KDC_ERR_S_PRINCIPAL_UNKNOWN - KRB5KDC_ERR_NONE, + expected_error, "Got wrong error.error_code"); } else { torture_assert_int_equal(test_context->tctx, @@ -672,6 +746,8 @@ static bool torture_krb5_post_recv_tgs_req_canon_test(struct torture_krb5_contex torture_assert_int_equal(test_context->tctx, *test_context->tgs_rep.ticket.enc_part.kvno & 0xFFFF0000, 0, "Unexpecedly got a RODC number in the KVNO, should just be principal KVNO"); + torture_assert(test_context->tctx, test_context->test_data->mitm_s4u2self == false, + "KDC accepted PA_S4U2Self with unkeyed checksum!"); free_TGS_REP(&test_context->tgs_rep); } torture_assert(test_context->tctx, test_context->packet_count == 0, "too many packets"); @@ -1845,7 +1921,23 @@ static bool torture_krb5_as_req_canon(struct torture_context *tctx, const void * */ if (torture_setting_bool(tctx, "expect_machine_account", false) && (test_data->enterprise || test_data->upn == false)) { + + if (test_data->mitm_s4u2self) { + torture_assert_int_equal(tctx, k5ret, KRB5KRB_AP_ERR_INAPP_CKSUM, + assertion_message); + /* Done testing mitm-s4u2self */ + return true; + } + torture_assert_int_equal(tctx, k5ret, 0, assertion_message); + + /* Check that the impersonate principal is not being canonicalized by the KDC. */ + if (test_data->s4u2self) { + torture_assert(tctx, krb5_principal_compare(k5_context, server_creds->client, + principal), + "TGS-REP cname does not match requested client principal"); + } + torture_assert_int_equal(tctx, krb5_cc_store_cred(k5_context, ccache, server_creds), 0, "krb5_cc_store_cred failed"); @@ -2222,11 +2314,25 @@ struct torture_suite *torture_krb5_canon(TALLOC_CTX *mem_ctx) (i & TEST_NETBIOS_REALM) ? "netbios-realm" : "krb5-realm", (i & TEST_WIN2K) ? "win2k" : "no-win2k", (i & TEST_UPN) ? "upn" : "no-upn", - (i & TEST_S4U2SELF) ? "s4u2self" : "normal", + (i & TEST_S4U2SELF) ? (i & TEST_MITM_S4U2SELF) ? "mitm-s4u2self" : "s4u2self" : "normal", (i & TEST_REMOVEDOLLAR) ? "removedollar" : "keepdollar"); struct test_data *test_data = talloc_zero(suite, struct test_data); + if (i & TEST_MITM_S4U2SELF) { + if (!(i & TEST_S4U2SELF)) { + continue; + } + } + + /* + * Due to backport: this flag is not used until Samba + * 4.10 + */ + if (i & TEST_AS_REQ_SPN) { + continue; + } + test_data->test_name = name; test_data->real_realm = strupper_talloc(test_data, @@ -2246,6 +2352,7 @@ struct torture_suite *torture_krb5_canon(TALLOC_CTX *mem_ctx) test_data->win2k = (i & TEST_WIN2K) != 0; test_data->upn = (i & TEST_UPN) != 0; test_data->s4u2self = (i & TEST_S4U2SELF) != 0; + test_data->mitm_s4u2self = (i & TEST_MITM_S4U2SELF) != 0; test_data->removedollar = (i & TEST_REMOVEDOLLAR) != 0; torture_suite_add_simple_tcase_const(suite, name, torture_krb5_as_req_canon, test_data); |