From 1566a68a3dc210a8119e5def2e38fef969a9c91f Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 1 Oct 2021 16:14:37 +1300 Subject: [PATCH] CVE-2020-25718 kdc: Confirm the RODC was allowed to issue a particular ticket BUG: https://bugzilla.samba.org/show_bug.cgi?id=14558 Signed-off-by: Andrew Bartlett Reviewed-by: Joseph Sutton --- selftest/knownfail_heimdal_kdc | 12 --- source4/auth/sam.c | 5 +- source4/dsdb/common/rodc_helper.c | 47 +++++++----- source4/kdc/mit_samba.c | 6 +- source4/kdc/pac-glue.c | 106 +++++++++++++++++++++++++- source4/kdc/pac-glue.h | 13 +++- source4/kdc/wdc-samba4.c | 40 ++++++++-- source4/rpc_server/drsuapi/getncchanges.c | 11 +-- source4/rpc_server/netlogon/dcerpc_netlogon.c | 1 + 9 files changed, 187 insertions(+), 54 deletions(-) diff --git a/selftest/knownfail_heimdal_kdc b/selftest/knownfail_heimdal_kdc index c315446386d..7d0597fa279 100644 --- a/selftest/knownfail_heimdal_kdc +++ b/selftest/knownfail_heimdal_kdc @@ -265,25 +265,16 @@ # KDC TGT tests # ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_renew_authdata_no_pac -^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_renew_rodc_allowed_denied -^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_renew_rodc_denied ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_renew_rodc_no_krbtgt_link ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_renew_rodc_no_partial_secrets -^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_renew_rodc_not_allowed ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_renew_rodc_not_revealed ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_s4u2self_authdata_no_pac -^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_s4u2self_rodc_allowed_denied -^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_s4u2self_rodc_denied ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_s4u2self_rodc_no_krbtgt_link ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_s4u2self_rodc_no_partial_secrets -^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_s4u2self_rodc_not_allowed ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_s4u2self_rodc_not_revealed ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_tgs_authdata_no_pac -^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_tgs_rodc_allowed_denied -^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_tgs_rodc_denied ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_tgs_rodc_no_krbtgt_link ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_tgs_rodc_no_partial_secrets -^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_tgs_rodc_not_allowed ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_tgs_rodc_not_revealed ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_upn_dns_info_ex_mac ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_upn_dns_info_ex_upn_mac @@ -316,11 +307,8 @@ ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_user2user_wrong_sname_krbtgt ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_user2user_wrong_srealm ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_validate_authdata_no_pac -^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_validate_rodc_allowed_denied -^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_validate_rodc_denied ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_validate_rodc_no_krbtgt_link ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_validate_rodc_no_partial_secrets -^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_validate_rodc_not_allowed ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_validate_rodc_not_revealed # # PAC request tests diff --git a/source4/auth/sam.c b/source4/auth/sam.c index 39e48c26b52..93b41be3b21 100644 --- a/source4/auth/sam.c +++ b/source4/auth/sam.c @@ -57,7 +57,10 @@ \ "pwdLastSet", \ "msDS-UserPasswordExpiryTimeComputed", \ - "accountExpires" + "accountExpires", \ + \ + /* Needed for RODC rule processing */ \ + "msDS-KrbTgtLinkBL" const char *krbtgt_attrs[] = { KRBTGT_ATTRS, NULL diff --git a/source4/dsdb/common/rodc_helper.c b/source4/dsdb/common/rodc_helper.c index 09aa3f5e710..1cd644c6def 100644 --- a/source4/dsdb/common/rodc_helper.c +++ b/source4/dsdb/common/rodc_helper.c @@ -47,19 +47,18 @@ bool sid_list_match(uint32_t num_sids1, /* * Return an array of SIDs from a ldb_message given an attribute name assumes - * the SIDs are in NDR form (with additional sids applied on the end). + * the SIDs are in NDR form (with primary_sid applied on the start). */ -WERROR samdb_result_sid_array_ndr(struct ldb_context *sam_ctx, - struct ldb_message *msg, - TALLOC_CTX *mem_ctx, - const char *attr, - uint32_t *num_sids, - struct dom_sid **sids, - const struct dom_sid *additional_sids, - unsigned int num_additional) +static WERROR samdb_result_sid_array_ndr(struct ldb_context *sam_ctx, + struct ldb_message *msg, + TALLOC_CTX *mem_ctx, + const char *attr, + uint32_t *num_sids, + struct dom_sid **sids, + const struct dom_sid *primary_sid) { struct ldb_message_element *el; - unsigned int i, j; + unsigned int i; el = ldb_msg_find_element(msg, attr); if (!el) { @@ -69,24 +68,25 @@ WERROR samdb_result_sid_array_ndr(struct ldb_context *sam_ctx, /* Make array long enough for NULL and additional SID */ (*sids) = talloc_array(mem_ctx, struct dom_sid, - el->num_values + num_additional); + el->num_values + 1); W_ERROR_HAVE_NO_MEMORY(*sids); - for (i=0; inum_values; i++) { + (*sids)[0] = *primary_sid; + + for (i = 0; inum_values; i++) { enum ndr_err_code ndr_err; + struct dom_sid sid = { 0, }; - ndr_err = ndr_pull_struct_blob_all_noalloc(&el->values[i], &(*sids)[i], + ndr_err = ndr_pull_struct_blob_all_noalloc(&el->values[i], &sid, (ndr_pull_flags_fn_t)ndr_pull_dom_sid); if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { return WERR_INTERNAL_DB_CORRUPTION; } + /* Primary SID is already in position zero. */ + (*sids)[i+1] = sid; } - for (j = 0; j < num_additional; j++) { - (*sids)[i++] = additional_sids[j]; - } - - *num_sids = i; + *num_sids = i+1; return WERR_OK; } @@ -131,6 +131,7 @@ WERROR samdb_result_sid_array_dn(struct ldb_context *sam_ctx, } WERROR samdb_confirm_rodc_allowed_to_repl_to_sid_list(struct ldb_context *sam_ctx, + const struct dom_sid *rodc_machine_account_sid, struct ldb_message *rodc_msg, struct ldb_message *obj_msg, uint32_t num_token_sids, @@ -202,6 +203,12 @@ WERROR samdb_confirm_rodc_allowed_to_repl_to_sid_list(struct ldb_context *sam_ct return WERR_DS_DRA_SECRETS_DENIED; } + /* The RODC can replicate and print tickets for itself. */ + if (dom_sid_equal(&token_sids[0], rodc_machine_account_sid)) { + TALLOC_FREE(frame); + return WERR_OK; + } + if (never_reveal_sids && sid_list_match(num_token_sids, token_sids, @@ -230,6 +237,7 @@ WERROR samdb_confirm_rodc_allowed_to_repl_to_sid_list(struct ldb_context *sam_ct * rather than relying on the caller providing those */ WERROR samdb_confirm_rodc_allowed_to_repl_to(struct ldb_context *sam_ctx, + struct dom_sid *rodc_machine_account_sid, struct ldb_message *rodc_msg, struct ldb_message *obj_msg) { @@ -256,7 +264,7 @@ WERROR samdb_confirm_rodc_allowed_to_repl_to(struct ldb_context *sam_ctx, frame, "tokenGroups", &num_token_sids, &token_sids, - object_sid, 1); + object_sid); if (!W_ERROR_IS_OK(werr) || token_sids==NULL) { DBG_ERR("Failed to get tokenGroups on %s to confirm access via RODC %s: %s\n", ldb_dn_get_linearized(obj_msg->dn), @@ -266,6 +274,7 @@ WERROR samdb_confirm_rodc_allowed_to_repl_to(struct ldb_context *sam_ctx, } werr = samdb_confirm_rodc_allowed_to_repl_to_sid_list(sam_ctx, + rodc_machine_account_sid, rodc_msg, obj_msg, num_token_sids, diff --git a/source4/kdc/mit_samba.c b/source4/kdc/mit_samba.c index 69cdbfba929..99a5214a896 100644 --- a/source4/kdc/mit_samba.c +++ b/source4/kdc/mit_samba.c @@ -435,7 +435,8 @@ int mit_samba_get_pac(struct mit_samba_context *smb_ctx, &logon_info_blob, cred_ndr_ptr, &upn_dns_info_blob, - NULL, NULL); + NULL, NULL, + NULL); if (!NT_STATUS_IS_OK(nt_status)) { talloc_free(tmp_ctx); if (NT_STATUS_EQUAL(nt_status, @@ -567,7 +568,8 @@ krb5_error_code mit_samba_reget_pac(struct mit_samba_context *ctx, &pac_blob, NULL, &upn_blob, - NULL, NULL); + NULL, NULL, + NULL); if (!NT_STATUS_IS_OK(nt_status)) { code = EINVAL; goto done; diff --git a/source4/kdc/pac-glue.c b/source4/kdc/pac-glue.c index 7d45391dba4..c94e4f08e76 100644 --- a/source4/kdc/pac-glue.c +++ b/source4/kdc/pac-glue.c @@ -35,6 +35,7 @@ #include "libcli/security/security.h" #include "dsdb/samdb/samdb.h" #include "auth/kerberos/pac_utils.h" +#include "source4/dsdb/common/util.h" static NTSTATUS samba_get_logon_info_pac_blob(TALLOC_CTX *mem_ctx, @@ -744,13 +745,19 @@ int samba_krbtgt_is_in_db(struct samba_kdc_entry *p, return 0; } +/* + * We return not just the blobs, but also the user_info_dc because we + * will need, in the RODC case, to confirm that the returned user is + * permitted to be replicated to the KDC + */ NTSTATUS samba_kdc_get_pac_blobs(TALLOC_CTX *mem_ctx, struct samba_kdc_entry *p, DATA_BLOB **_logon_info_blob, DATA_BLOB **_cred_ndr_blob, DATA_BLOB **_upn_info_blob, DATA_BLOB **_pac_attrs_blob, - const krb5_boolean *pac_request) + const krb5_boolean *pac_request, + struct auth_user_info_dc **_user_info_dc) { struct auth_user_info_dc *user_info_dc; DATA_BLOB *logon_blob = NULL; @@ -848,7 +855,15 @@ NTSTATUS samba_kdc_get_pac_blobs(TALLOC_CTX *mem_ctx, } } - TALLOC_FREE(user_info_dc); + /* + * Return to the caller to allow a check on the allowed/denied + * RODC replication groups + */ + if (_user_info_dc == NULL) { + TALLOC_FREE(user_info_dc); + } else { + *_user_info_dc = user_info_dc; + } *_logon_info_blob = logon_blob; if (_cred_ndr_blob != NULL) { *_cred_ndr_blob = cred_blob; @@ -1094,3 +1109,90 @@ out: TALLOC_FREE(frame); return code; } + + +/* + * In the RODC case, to confirm that the returned user is permitted to + * be replicated to the KDC (krbgtgt_xxx user) represented by *rodc + */ +WERROR samba_rodc_confirm_user_is_allowed(uint32_t num_object_sids, + struct dom_sid *object_sids, + struct samba_kdc_entry *rodc, + struct samba_kdc_entry *object) +{ + int ret; + WERROR werr; + TALLOC_CTX *frame = talloc_stackframe(); + const char *rodc_attrs[] = { "msDS-KrbTgtLink", + "msDS-NeverRevealGroup", + "msDS-RevealOnDemandGroup", + "userAccountControl", + "objectSid", + NULL }; + struct ldb_result *rodc_machine_account = NULL; + struct ldb_dn *rodc_machine_account_dn = samdb_result_dn(rodc->kdc_db_ctx->samdb, + frame, + rodc->msg, + "msDS-KrbTgtLinkBL", + NULL); + const struct dom_sid *rodc_machine_account_sid = NULL; + + if (rodc_machine_account_dn == NULL) { + DBG_ERR("krbtgt account %s has no msDS-KrbTgtLinkBL to find RODC machine account for allow/deny list\n", + ldb_dn_get_linearized(rodc->msg->dn)); + TALLOC_FREE(frame); + return WERR_DS_DRA_BAD_DN; + } + + /* + * Follow the link and get the RODC account (the krbtgt + * account is the krbtgt_XXX account, but the + * msDS-NeverRevealGroup and msDS-RevealOnDemandGroup is on + * the RODC$ account) + * + * We need DSDB_SEARCH_SHOW_EXTENDED_DN as we get a SID lists + * out of the extended DNs + */ + + ret = dsdb_search_dn(rodc->kdc_db_ctx->samdb, + frame, + &rodc_machine_account, + rodc_machine_account_dn, + rodc_attrs, + DSDB_SEARCH_SHOW_EXTENDED_DN); + if (ret != LDB_SUCCESS) { + DBG_ERR("Failed to fetch RODC machine account %s pointed to by %s to check allow/deny list: %s\n", + ldb_dn_get_linearized(rodc_machine_account_dn), + ldb_dn_get_linearized(rodc->msg->dn), + ldb_errstring(rodc->kdc_db_ctx->samdb)); + TALLOC_FREE(frame); + return WERR_DS_DRA_BAD_DN; + } + + if (rodc_machine_account->count != 1) { + DBG_ERR("Failed to fetch RODC machine account %s pointed to by %s to check allow/deny list: (%d)\n", + ldb_dn_get_linearized(rodc_machine_account_dn), + ldb_dn_get_linearized(rodc->msg->dn), + rodc_machine_account->count); + TALLOC_FREE(frame); + return WERR_DS_DRA_BAD_DN; + } + + /* if the object SID is equal to the user_sid, allow */ + rodc_machine_account_sid = samdb_result_dom_sid(frame, + rodc_machine_account->msgs[0], + "objectSid"); + if (rodc_machine_account_sid == NULL) { + return WERR_DS_DRA_BAD_DN; + } + + werr = samdb_confirm_rodc_allowed_to_repl_to_sid_list(rodc->kdc_db_ctx->samdb, + rodc_machine_account_sid, + rodc_machine_account->msgs[0], + object->msg, + num_object_sids, + object_sids); + + TALLOC_FREE(frame); + return werr; +} diff --git a/source4/kdc/pac-glue.h b/source4/kdc/pac-glue.h index 2a7cb68f274..89aa8da63c3 100644 --- a/source4/kdc/pac-glue.h +++ b/source4/kdc/pac-glue.h @@ -52,8 +52,8 @@ NTSTATUS samba_kdc_get_pac_blobs(TALLOC_CTX *mem_ctx, DATA_BLOB **_cred_ndr_blob, DATA_BLOB **_upn_info_blob, DATA_BLOB **_pac_attrs_blob, - const krb5_boolean *pac_request); - + const krb5_boolean *pac_request, + struct auth_user_info_dc **_user_info_dc); NTSTATUS samba_kdc_update_pac_blob(TALLOC_CTX *mem_ctx, krb5_context context, struct ldb_context *samdb, @@ -79,3 +79,12 @@ krb5_error_code samba_kdc_validate_pac_blob( krb5_context context, struct samba_kdc_entry *client_skdc_entry, const krb5_pac pac); + +/* + * In the RODC case, to confirm that the returned user is permitted to + * be replicated to the KDC (krbgtgt_xxx user) represented by *rodc + */ +WERROR samba_rodc_confirm_user_is_allowed(uint32_t num_sids, + struct dom_sid *sids, + struct samba_kdc_entry *rodc, + struct samba_kdc_entry *object); diff --git a/source4/kdc/wdc-samba4.c b/source4/kdc/wdc-samba4.c index 11d9ff84f04..71507018120 100644 --- a/source4/kdc/wdc-samba4.c +++ b/source4/kdc/wdc-samba4.c @@ -27,6 +27,7 @@ #include "kdc/pac-glue.h" #include "sdb.h" #include "sdb_hdb.h" +#include "librpc/gen_ndr/auth.h" /* * Given the right private pointer from hdb_samba4, @@ -68,7 +69,8 @@ static krb5_error_code samba_wdc_get_pac(void *priv, krb5_context context, cred_ndr_ptr, &upn_blob, &pac_attrs_blob, - pac_request); + pac_request, + NULL); if (!NT_STATUS_IS_OK(nt_status)) { talloc_free(mem_ctx); return EINVAL; @@ -161,9 +163,15 @@ static krb5_error_code samba_wdc_reget_pac2(krb5_context context, } } - /* If the krbtgt was generated by an RODC, and we are not that + /* + * If the krbtgt was generated by an RODC, and we are not that * RODC, then we need to regenerate the PAC - we can't trust - * it */ + * it, and confirm that the RODC was permitted to print this ticket + * + * Becasue of the samba_kdc_validate_pac_blob() step we can be + * sure that the record in 'client' matches the SID in the + * original PAC. + */ ret = samba_krbtgt_is_in_db(krbtgt_skdc_entry, &is_in_db, &is_untrusted); if (ret != 0) { talloc_free(mem_ctx); @@ -237,6 +245,8 @@ static krb5_error_code samba_wdc_reget_pac2(krb5_context context, if (is_untrusted) { struct samba_kdc_entry *client_skdc_entry = NULL; + struct auth_user_info_dc *user_info_dc = NULL; + WERROR werr; if (client == NULL) { return KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN; @@ -247,12 +257,30 @@ static krb5_error_code samba_wdc_reget_pac2(krb5_context context, nt_status = samba_kdc_get_pac_blobs(mem_ctx, client_skdc_entry, &pac_blob, NULL, &upn_blob, - NULL, NULL); + NULL, NULL, + &user_info_dc); if (!NT_STATUS_IS_OK(nt_status)) { talloc_free(mem_ctx); - return EINVAL; + return KRB5KDC_ERR_TGT_REVOKED; + } + + /* + * Now check if the SID list in the user_info_dc + * intersects correctly with the RODC allow/deny + * lists + */ + + werr = samba_rodc_confirm_user_is_allowed(user_info_dc->num_sids, + user_info_dc->sids, + krbtgt_skdc_entry, + client_skdc_entry); + if (!W_ERROR_IS_OK(werr)) { + talloc_free(mem_ctx); + return KRB5KDC_ERR_TGT_REVOKED; } - } else { + } + + if (!is_untrusted) { pac_blob = talloc_zero(mem_ctx, DATA_BLOB); if (!pac_blob) { talloc_free(mem_ctx); diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index 28223104c94..c3330a622af 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -1174,7 +1174,6 @@ static WERROR getncchanges_repl_secret(struct drsuapi_bind_state *b_state, NULL }; const char *obj_attrs[] = { "tokenGroups", "objectSid", "UserAccountControl", "msDS-KrbTgtLinkBL", NULL }; struct ldb_result *rodc_res = NULL, *obj_res = NULL; - const struct dom_sid *object_sid = NULL; WERROR werr; DEBUG(3,(__location__ ": DRSUAPI_EXOP_REPL_SECRET extended op on %s\n", @@ -1262,15 +1261,6 @@ static WERROR getncchanges_repl_secret(struct drsuapi_bind_state *b_state, ret = dsdb_search_dn(b_state->sam_ctx_system, mem_ctx, &obj_res, obj_dn, obj_attrs, 0); if (ret != LDB_SUCCESS || obj_res->count != 1) goto failed; - /* if the object SID is equal to the user_sid, allow */ - object_sid = samdb_result_dom_sid(mem_ctx, obj_res->msgs[0], "objectSid"); - if (object_sid == NULL) { - goto failed; - } - if (dom_sid_equal(user_sid, object_sid)) { - goto allowed; - } - /* * Must be an RODC account at this point, verify machine DN matches the * SID account @@ -1288,6 +1278,7 @@ static WERROR getncchanges_repl_secret(struct drsuapi_bind_state *b_state, } werr = samdb_confirm_rodc_allowed_to_repl_to(b_state->sam_ctx_system, + user_sid, rodc_res->msgs[0], obj_res->msgs[0]); diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c index a38e78a37e7..09d0252c0c2 100644 --- a/source4/rpc_server/netlogon/dcerpc_netlogon.c +++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c @@ -2871,6 +2871,7 @@ static bool sam_rodc_access_check(struct ldb_context *sam_ctx, if (ret != LDB_SUCCESS || obj_res->count != 1) goto denied; werr = samdb_confirm_rodc_allowed_to_repl_to(sam_ctx, + user_sid, rodc_res->msgs[0], obj_res->msgs[0]); -- 2.11.4.GIT