s4:kdc: Don't modify cached user_info_dc SIDs

samba_kdc_get_pac_blobs() passes a pointer to a user_info_dc structure
obtained from samba_kdc_get_user_info_from_db() into
samba_add_asserted_identity(). The latter function modifies the SIDs of
the user_info_dc structure in order to add the Asserted Identity SID,
but samba_kdc_get_user_info_from_db() actually caches that structure
internally, meaning that subsequent calls will return the modified
structure.

We should not modify cached SIDs, so have
samba_kdc_get_user_info_from_db() return a pointer to constant data, and
copy the returned array of SIDs before adding the Asserted Identity SID.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/source4/auth/session.c b/source4/auth/session.c
index 4b9a0058dd542b5732be0875a243fd94dabe0889..2e28bc15c6d68aba8afa5520bd70e019fc763a13 100644 (file)
--- a/source4/auth/session.c
+++ b/source4/auth/session.c
@@ -52,7 +52,7 @@ _PUBLIC_ struct auth_session_info *anonymous_session(TALLOC_CTX *mem_ctx,
 _PUBLIC_ NTSTATUS auth_generate_session_info(TALLOC_CTX *mem_ctx,
                                             struct loadparm_context *lp_ctx, /* Optional, if you don't want privilages */
                                             struct ldb_context *sam_ctx, /* Optional, if you don't want local groups */
-                                            struct auth_user_info_dc *user_info_dc,
+                                            const struct auth_user_info_dc *user_info_dc,
                                             uint32_t session_info_flags,
                                             struct auth_session_info **_session_info)
 {

diff --git a/source4/auth/session.h b/source4/auth/session.h
index 97a8aba0f141c470f6239163e26be34e5b074d69..2d42396a5568bc7801e4ea93af6730d4573f0144 100644 (file)
--- a/source4/auth/session.h
+++ b/source4/auth/session.h
@@ -39,7 +39,7 @@ NTSTATUS auth_anonymous_user_info_dc(TALLOC_CTX *mem_ctx,
 NTSTATUS auth_generate_session_info(TALLOC_CTX *mem_ctx,
                                    struct loadparm_context *lp_ctx, /* Optional, if you don't want privilages */
                                    struct ldb_context *sam_ctx, /* Optional, if you don't want local groups */
-                                   struct auth_user_info_dc *interim_info,
+                                   const struct auth_user_info_dc *interim_info,
                                    uint32_t session_info_flags,
                                    struct auth_session_info **session_info);
 NTSTATUS auth_anonymous_session_info(TALLOC_CTX *parent_ctx, 

diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c
index 7a048a6a4180b95c09e11482ca0474ffec35235d..55286f04c85940363fd767b8168dbe0f712ce9fc 100644 (file)
--- a/source4/kdc/db-glue.c
+++ b/source4/kdc/db-glue.c
@@ -1394,7 +1394,7 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context,
 
        if (ent_type == SAMBA_KDC_ENT_TYPE_CLIENT && (flags & SDB_F_FOR_AS_REQ)) {
                int result;
-               struct auth_user_info_dc *user_info_dc = NULL;
+               const struct auth_user_info_dc *user_info_dc = NULL;
                /*
                 * These protections only apply to clients, so servers in the
                 * Protected Users group may still have service tickets to them
@@ -1407,7 +1407,7 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context,
                 * and computers should never be members of Protected Users, or
                 * they may fail to authenticate.
                 */
-               status = samba_kdc_get_user_info_from_db(p, msg, &user_info_dc);
+               status = samba_kdc_get_user_info_from_db(mem_ctx, p, msg, &user_info_dc);
                if (!NT_STATUS_IS_OK(status)) {
                        ret = EINVAL;
                        goto out;

diff --git a/source4/kdc/mit_samba.c b/source4/kdc/mit_samba.c
index 4d7d60cb7652f513e10168e332e75e75231ebbbb..cdd939f4105bf2a23c8e8b37a1c06a729367d25c 100644 (file)
--- a/source4/kdc/mit_samba.c
+++ b/source4/kdc/mit_samba.c
@@ -974,7 +974,7 @@ int mit_samba_kpasswd_change_password(struct mit_samba_context *ctx,
        enum samPwdChangeReason reject_reason;
        struct samr_DomInfo1 *dominfo;
        const char *error_string = NULL;
-       struct auth_user_info_dc *user_info_dc;
+       const struct auth_user_info_dc *user_info_dc = NULL;
        struct samba_kdc_entry *p =
                talloc_get_type_abort(db_entry->e_data, struct samba_kdc_entry);
        krb5_error_code code = 0;
@@ -988,7 +988,8 @@ int mit_samba_kpasswd_change_password(struct mit_samba_context *ctx,
                return ENOMEM;
        }
 
-       status = samba_kdc_get_user_info_from_db(p,
+       status = samba_kdc_get_user_info_from_db(tmp_ctx,
+                                                p,
                                                 p->msg,
                                                 &user_info_dc);
        if (!NT_STATUS_IS_OK(status)) {

diff --git a/source4/kdc/pac-glue.c b/source4/kdc/pac-glue.c
index 80e72579cc16977d9567217676107a112d45549b..fc7d657dd429010e2b5224b1ca4ae95301e81abd 100644 (file)
--- a/source4/kdc/pac-glue.c
+++ b/source4/kdc/pac-glue.c
@@ -1078,7 +1078,8 @@ int samba_krbtgt_is_in_db(struct samba_kdc_entry *p,
  */
 static NTSTATUS samba_add_asserted_identity(TALLOC_CTX *mem_ctx,
                                            enum samba_asserted_identity ai,
-                                           struct auth_user_info_dc *user_info_dc)
+                                           struct auth_SidAttr **sids,
+                                           uint32_t *num_sids)
 {
        struct dom_sid ai_sid;
        const char *sid_str = NULL;
@@ -1097,11 +1098,11 @@ static NTSTATUS samba_add_asserted_identity(TALLOC_CTX *mem_ctx,
        dom_sid_parse(sid_str, &ai_sid);
 
        return add_sid_to_array_attrs_unique(
-               user_info_dc,
+               mem_ctx,
                &ai_sid,
                SE_GROUP_DEFAULT_FLAGS,
-               &user_info_dc->sids,
-               &user_info_dc->num_sids);
+               sids,
+               num_sids);
 }
 
 /*
@@ -1109,9 +1110,10 @@ static NTSTATUS samba_add_asserted_identity(TALLOC_CTX *mem_ctx,
  * structure. If the resulting structure is not talloc_free()d, it will be
  * reused on future calls to this function.
  */
-NTSTATUS samba_kdc_get_user_info_from_db(struct samba_kdc_entry *skdc_entry,
+NTSTATUS samba_kdc_get_user_info_from_db(TALLOC_CTX *mem_ctx,
+                                         struct samba_kdc_entry *skdc_entry,
                                          const struct ldb_message *msg,
-                                         struct auth_user_info_dc **user_info_dc)
+                                         const struct auth_user_info_dc **user_info_dc)
 {
        if (skdc_entry->user_info_dc == NULL) {
                NTSTATUS nt_status;
@@ -1148,7 +1150,9 @@ NTSTATUS samba_kdc_get_pac_blobs(TALLOC_CTX *mem_ctx,
                                 DATA_BLOB **_requester_sid_blob,
                                 DATA_BLOB **_client_claims_blob)
 {
-       struct auth_user_info_dc *user_info_dc = NULL;
+       TALLOC_CTX *frame = NULL;
+       const struct auth_user_info_dc *user_info_dc_const = NULL;
+       struct auth_user_info_dc user_info_dc = {};
        DATA_BLOB *logon_blob = NULL;
        DATA_BLOB *cred_blob = NULL;
        DATA_BLOB *upn_blob = NULL;
@@ -1216,25 +1220,51 @@ NTSTATUS samba_kdc_get_pac_blobs(TALLOC_CTX *mem_ctx,
                }
        }
 
-       nt_status = samba_kdc_get_user_info_from_db(p,
+       nt_status = samba_kdc_get_user_info_from_db(mem_ctx,
+                                                   p,
                                                    p->msg,
-                                                   &user_info_dc);
+                                                   &user_info_dc_const);
        if (!NT_STATUS_IS_OK(nt_status)) {
                DEBUG(0, ("Getting user info for PAC failed: %s\n",
                          nt_errstr(nt_status)));
                return nt_status;
        }
 
-       nt_status = samba_add_asserted_identity(mem_ctx,
+       frame = talloc_stackframe();
+
+       /* Make a shallow copy of the user_info_dc structure. */
+       user_info_dc = *user_info_dc_const;
+       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.sids = talloc_memdup(frame,
+                                                 user_info_dc.sids,
+                                                 talloc_get_size(user_info_dc.sids));
+               if (user_info_dc.sids == NULL) {
+                       DBG_ERR("Failed to allocate user_info_dc SIDs: %s\n",
+                               nt_errstr(nt_status));
+                       TALLOC_FREE(frame);
+                       return NT_STATUS_NO_MEMORY;
+               }
+       }
+
+       /* Here we modify the SIDs to add the Asserted Identity SID. */
+       nt_status = samba_add_asserted_identity(frame,
                                                asserted_identity,
-                                               user_info_dc);
+                                               &user_info_dc.sids,
+                                               &user_info_dc.num_sids);
        if (!NT_STATUS_IS_OK(nt_status)) {
                DBG_ERR("Failed to add assertied identity!\n");
+               TALLOC_FREE(frame);
                return nt_status;
        }
 
        nt_status = samba_get_logon_info_pac_blob(logon_blob,
-                                                 user_info_dc,
+                                                 &user_info_dc,
                                                  NULL,
                                                  group_inclusion,
                                                  logon_blob,
@@ -1242,6 +1272,7 @@ NTSTATUS samba_kdc_get_pac_blobs(TALLOC_CTX *mem_ctx,
        if (!NT_STATUS_IS_OK(nt_status)) {
                DEBUG(0, ("Building PAC LOGON INFO failed: %s\n",
                          nt_errstr(nt_status)));
+               TALLOC_FREE(frame);
                return nt_status;
        }
 
@@ -1252,16 +1283,18 @@ NTSTATUS samba_kdc_get_pac_blobs(TALLOC_CTX *mem_ctx,
                if (!NT_STATUS_IS_OK(nt_status)) {
                        DEBUG(0, ("Building PAC CRED INFO failed: %s\n",
                                  nt_errstr(nt_status)));
+                       TALLOC_FREE(frame);
                        return nt_status;
                }
        }
 
        nt_status = samba_get_upn_info_pac_blob(upn_blob,
-                                               user_info_dc,
+                                               &user_info_dc,
                                                upn_blob);
        if (!NT_STATUS_IS_OK(nt_status)) {
                DEBUG(0, ("Building PAC UPN INFO failed: %s\n",
                          nt_errstr(nt_status)));
+               TALLOC_FREE(frame);
                return nt_status;
        }
 
@@ -1273,6 +1306,7 @@ NTSTATUS samba_kdc_get_pac_blobs(TALLOC_CTX *mem_ctx,
                if (!NT_STATUS_IS_OK(nt_status)) {
                        DEBUG(0, ("Building PAC ATTRIBUTES failed: %s\n",
                                  nt_errstr(nt_status)));
+                       TALLOC_FREE(frame);
                        return nt_status;
                }
        }
@@ -1291,6 +1325,7 @@ NTSTATUS samba_kdc_get_pac_blobs(TALLOC_CTX *mem_ctx,
        if (_client_claims_blob != NULL) {
                *_client_claims_blob = client_claims_blob;
        }
+       TALLOC_FREE(frame);
        return NT_STATUS_OK;
 }
 
@@ -1845,7 +1880,7 @@ krb5_error_code samba_kdc_update_pac(TALLOC_CTX *mem_ctx,
        }
 
        if (!is_trusted) {
-               struct auth_user_info_dc *user_info_dc = NULL;
+               const struct auth_user_info_dc *user_info_dc = NULL;
                WERROR werr;
 
                struct dom_sid *object_sids = NULL;
@@ -1887,7 +1922,8 @@ krb5_error_code samba_kdc_update_pac(TALLOC_CTX *mem_ctx,
                        goto done;
                }
 
-               nt_status = samba_kdc_get_user_info_from_db(client,
+               nt_status = samba_kdc_get_user_info_from_db(mem_ctx,
+                                                           client,
                                                            client->msg,
                                                            &user_info_dc);
                if (!NT_STATUS_IS_OK(nt_status)) {
@@ -1916,7 +1952,6 @@ krb5_error_code samba_kdc_update_pac(TALLOC_CTX *mem_ctx,
                                                          krbtgt,
                                                          client);
                TALLOC_FREE(object_sids);
-               TALLOC_FREE(user_info_dc);
                if (!W_ERROR_IS_OK(werr)) {
                        code = KRB5KDC_ERR_TGT_REVOKED;
                        if (W_ERROR_EQUAL(werr,

diff --git a/source4/kdc/pac-glue.h b/source4/kdc/pac-glue.h
index 5c83fba934c6744f90d0a4460ab0e734145922af..3c4f9676b097835dd12da38a589d7cbd88147dc8 100644 (file)
--- a/source4/kdc/pac-glue.h
+++ b/source4/kdc/pac-glue.h
@@ -66,9 +66,10 @@ int samba_krbtgt_is_in_db(struct samba_kdc_entry *skdc_entry,
                          bool *is_in_db,
                          bool *is_trusted);
 
-NTSTATUS samba_kdc_get_user_info_from_db(struct samba_kdc_entry *skdc_entry,
+NTSTATUS samba_kdc_get_user_info_from_db(TALLOC_CTX *mem_ctx,
+                                        struct samba_kdc_entry *skdc_entry,
                                         const struct ldb_message *msg,
-                                        struct auth_user_info_dc **user_info_dc);
+                                        const struct auth_user_info_dc **user_info_dc);
 
 NTSTATUS samba_kdc_get_pac_blobs(TALLOC_CTX *mem_ctx,
                                 struct samba_kdc_entry *skdc_entry,