diff options
author | Joseph Sutton <josephsutton@catalyst.net.nz> | 2023-05-10 14:54:21 +1200 |
---|---|---|
committer | Andrew Bartlett <abartlet@samba.org> | 2023-05-18 01:03:37 +0000 |
commit | 633ebe1b3efee4c61e1856cad5be5723010f9bd1 (patch) | |
tree | 01bdac5545ea0118ac8bc09c0f74fbdc1469de5e /source4 | |
parent | 8cc0b76509b51bb57c2c527ea504812f8de06144 (diff) | |
download | samba-633ebe1b3efee4c61e1856cad5be5723010f9bd1.tar.gz |
s4:kdc: Make a proper shallow copy of the auth_user_info_dc structure
Just copying the structure fields is prone to lead to use-after-frees if
we access them after the original structure and its fields are freed.
Instead, call authsam_shallow_copy_user_info_dc() to make the copy. This
properly references the fields in the original structure so that they
will not be freed until we are sure we have finished with them.
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Diffstat (limited to 'source4')
-rw-r--r-- | source4/kdc/mit_samba.c | 8 | ||||
-rw-r--r-- | source4/kdc/pac-glue.c | 54 | ||||
-rw-r--r-- | source4/kdc/pac-glue.h | 2 | ||||
-rw-r--r-- | source4/kdc/wdc-samba4.c | 8 |
4 files changed, 32 insertions, 40 deletions
diff --git a/source4/kdc/mit_samba.c b/source4/kdc/mit_samba.c index 9b211b2aeda..309442ded18 100644 --- a/source4/kdc/mit_samba.c +++ b/source4/kdc/mit_samba.c @@ -464,7 +464,7 @@ int mit_samba_get_pac(struct mit_samba_context *smb_ctx, krb5_pac *pac) { TALLOC_CTX *tmp_ctx; - struct auth_user_info_dc user_info_dc = {}; + struct auth_user_info_dc *user_info_dc = NULL; DATA_BLOB *logon_info_blob = NULL; DATA_BLOB *upn_dns_info_blob = NULL; DATA_BLOB *cred_ndr = NULL; @@ -531,7 +531,7 @@ int mit_samba_get_pac(struct mit_samba_context *smb_ctx, } nt_status = samba_kdc_get_logon_info_blob(tmp_ctx, - &user_info_dc, + user_info_dc, group_inclusion, &logon_info_blob); if (!NT_STATUS_IS_OK(nt_status)) { @@ -550,7 +550,7 @@ int mit_samba_get_pac(struct mit_samba_context *smb_ctx, } nt_status = samba_kdc_get_upn_info_blob(tmp_ctx, - &user_info_dc, + user_info_dc, &upn_dns_info_blob); if (!NT_STATUS_IS_OK(nt_status)) { talloc_free(tmp_ctx); @@ -567,7 +567,7 @@ int mit_samba_get_pac(struct mit_samba_context *smb_ctx, } nt_status = samba_kdc_get_requester_sid_blob(tmp_ctx, - &user_info_dc, + user_info_dc, &requester_sid_blob); if (!NT_STATUS_IS_OK(nt_status)) { talloc_free(tmp_ctx); diff --git a/source4/kdc/pac-glue.c b/source4/kdc/pac-glue.c index 0d22e835b68..6d5883f2d17 100644 --- a/source4/kdc/pac-glue.c +++ b/source4/kdc/pac-glue.c @@ -1166,43 +1166,32 @@ NTSTATUS samba_kdc_get_user_info_dc(TALLOC_CTX *mem_ctx, enum samba_asserted_identity asserted_identity, enum samba_claims_valid claims_valid, enum samba_compounded_auth compounded_auth, - struct auth_user_info_dc *user_info_dc_out) + struct auth_user_info_dc **user_info_dc_out) { NTSTATUS nt_status; - const struct auth_user_info_dc *user_info_dc = NULL; + const struct auth_user_info_dc *user_info_dc_from_db = NULL; + struct auth_user_info_dc *user_info_dc = NULL; - nt_status = samba_kdc_get_user_info_from_db(skdc_entry, skdc_entry->msg, &user_info_dc); + nt_status = samba_kdc_get_user_info_from_db(skdc_entry, skdc_entry->msg, &user_info_dc_from_db); if (!NT_STATUS_IS_OK(nt_status)) { DBG_ERR("Getting user info for PAC failed: %s\n", nt_errstr(nt_status)); return nt_status; } - /* Make a shallow copy of the user_info_dc structure. */ - *user_info_dc_out = *user_info_dc; - if (user_info_dc->sids != NULL) { - /* - * Because we want to modify the SIDs in the user_info_dc - * structure, adding various well-known SIDs such as Asserted - * Identity or Claims Valid, make a copy of the SID array to - * guard against modification of the original. - */ - user_info_dc_out->sids = talloc_memdup(mem_ctx, - user_info_dc_out->sids, - talloc_get_size(user_info_dc_out->sids)); - if (user_info_dc_out->sids == NULL) { - DBG_ERR("Failed to allocate user_info_dc SIDs: %s\n", - nt_errstr(nt_status)); - return NT_STATUS_NO_MEMORY; - } + nt_status = authsam_shallow_copy_user_info_dc(mem_ctx, user_info_dc_from_db, &user_info_dc); + if (!NT_STATUS_IS_OK(nt_status)) { + DBG_ERR("Failed to allocate user_info_dc SIDs: %s\n", + nt_errstr(nt_status)); + return nt_status; } /* Here we modify the SIDs to add the Asserted Identity SID. */ nt_status = samba_add_asserted_identity(mem_ctx, asserted_identity, - &user_info_dc_out->sids, - &user_info_dc_out->num_sids); + &user_info_dc->sids, + &user_info_dc->num_sids); if (!NT_STATUS_IS_OK(nt_status)) { DBG_ERR("Failed to add asserted identity: %s\n", nt_errstr(nt_status)); @@ -1211,7 +1200,7 @@ NTSTATUS samba_kdc_get_user_info_dc(TALLOC_CTX *mem_ctx, nt_status = samba_add_claims_valid(mem_ctx, claims_valid, - user_info_dc_out); + user_info_dc); if (!NT_STATUS_IS_OK(nt_status)) { DBG_ERR("Failed to add Claims Valid: %s\n", nt_errstr(nt_status)); @@ -1220,13 +1209,15 @@ NTSTATUS samba_kdc_get_user_info_dc(TALLOC_CTX *mem_ctx, nt_status = samba_add_compounded_auth(mem_ctx, compounded_auth, - user_info_dc_out); + user_info_dc); if (!NT_STATUS_IS_OK(nt_status)) { DBG_ERR("Failed to add Compounded Authentication: %s\n", nt_errstr(nt_status)); return nt_status; } + *user_info_dc_out = user_info_dc; + return NT_STATUS_OK; } @@ -1952,7 +1943,7 @@ static krb5_error_code samba_kdc_get_device_info_blob(TALLOC_CTX *mem_ctx, krb5_error_code code = EINVAL; NTSTATUS nt_status; - struct auth_user_info_dc device_info_dc; + struct auth_user_info_dc *device_info_dc = NULL; struct netr_SamInfo3 *info3 = NULL; struct PAC_DOMAIN_GROUP_MEMBERSHIP *resource_groups = NULL; @@ -1978,7 +1969,7 @@ static krb5_error_code samba_kdc_get_device_info_blob(TALLOC_CTX *mem_ctx, return KRB5KDC_ERR_TGT_REVOKED; } - nt_status = auth_convert_user_info_dc_saminfo3(frame, &device_info_dc, + nt_status = auth_convert_user_info_dc_saminfo3(frame, device_info_dc, AUTH_INCLUDE_RESOURCE_GROUPS_COMPRESSED, &info3, &resource_groups); @@ -2251,6 +2242,7 @@ krb5_error_code samba_kdc_update_pac(TALLOC_CTX *mem_ctx, DATA_BLOB *device_claims_blob = NULL; DATA_BLOB *device_info_blob = NULL; int is_tgs = false; + struct auth_user_info_dc *user_info_dc = NULL; enum auth_group_inclusion group_inclusion; size_t i = 0; @@ -2357,8 +2349,6 @@ krb5_error_code samba_kdc_update_pac(TALLOC_CTX *mem_ctx, } if (!client_pac_is_trusted) { - struct auth_user_info_dc user_info_dc = {}; - /* * In this case the RWDC discards the PAC an RODC generated. * Windows adds the asserted_identity in this case too. @@ -2396,7 +2386,7 @@ krb5_error_code samba_kdc_update_pac(TALLOC_CTX *mem_ctx, } nt_status = samba_kdc_get_logon_info_blob(mem_ctx, - &user_info_dc, + user_info_dc, group_inclusion, &pac_blob); if (!NT_STATUS_IS_OK(nt_status)) { @@ -2407,7 +2397,7 @@ krb5_error_code samba_kdc_update_pac(TALLOC_CTX *mem_ctx, } nt_status = samba_kdc_get_upn_info_blob(mem_ctx, - &user_info_dc, + user_info_dc, &upn_blob); if (!NT_STATUS_IS_OK(nt_status)) { DBG_ERR("samba_kdc_get_upn_info_blob failed: %s\n", @@ -2417,7 +2407,7 @@ krb5_error_code samba_kdc_update_pac(TALLOC_CTX *mem_ctx, } nt_status = samba_kdc_get_requester_sid_blob(mem_ctx, - &user_info_dc, + user_info_dc, &requester_sid_blob); if (!NT_STATUS_IS_OK(nt_status)) { DBG_ERR("samba_kdc_get_requester_sid_blob failed: %s\n", @@ -2668,5 +2658,7 @@ done: TALLOC_FREE(pac_blob); TALLOC_FREE(upn_blob); TALLOC_FREE(deleg_blob); + /* Release our handle to user_info_dc. */ + talloc_unlink(mem_ctx, user_info_dc); return code; } diff --git a/source4/kdc/pac-glue.h b/source4/kdc/pac-glue.h index f819e2e46cf..af251984f9d 100644 --- a/source4/kdc/pac-glue.h +++ b/source4/kdc/pac-glue.h @@ -104,7 +104,7 @@ NTSTATUS samba_kdc_get_user_info_dc(TALLOC_CTX *mem_ctx, enum samba_asserted_identity asserted_identity, enum samba_claims_valid claims_valid, enum samba_compounded_auth compounded_auth, - struct auth_user_info_dc *_user_info_dc); + struct auth_user_info_dc **user_info_dc_out); NTSTATUS samba_kdc_update_delegation_info_blob(TALLOC_CTX *mem_ctx, krb5_context context, diff --git a/source4/kdc/wdc-samba4.c b/source4/kdc/wdc-samba4.c index 7f0dc7da2be..d2eb49c7cb6 100644 --- a/source4/kdc/wdc-samba4.c +++ b/source4/kdc/wdc-samba4.c @@ -125,7 +125,7 @@ static krb5_error_code samba_wdc_get_pac(void *priv, const enum samba_claims_valid claims_valid = SAMBA_CLAIMS_VALID_INCLUDE; const enum samba_compounded_auth compounded_auth = SAMBA_COMPOUNDED_AUTH_EXCLUDE; - struct auth_user_info_dc user_info_dc = {}; + struct auth_user_info_dc *user_info_dc = NULL; /* Only include resource groups in a service ticket. */ if (is_krbtgt) { @@ -157,7 +157,7 @@ static krb5_error_code samba_wdc_get_pac(void *priv, } nt_status = samba_kdc_get_logon_info_blob(mem_ctx, - &user_info_dc, + user_info_dc, group_inclusion, &logon_blob); if (!NT_STATUS_IS_OK(nt_status)) { @@ -176,7 +176,7 @@ static krb5_error_code samba_wdc_get_pac(void *priv, } nt_status = samba_kdc_get_upn_info_blob(mem_ctx, - &user_info_dc, + user_info_dc, &upn_blob); if (!NT_STATUS_IS_OK(nt_status)) { talloc_free(mem_ctx); @@ -193,7 +193,7 @@ static krb5_error_code samba_wdc_get_pac(void *priv, } nt_status = samba_kdc_get_requester_sid_blob(mem_ctx, - &user_info_dc, + user_info_dc, &requester_sid_blob); if (!NT_STATUS_IS_OK(nt_status)) { talloc_free(mem_ctx); |