s3:auth: add add_builtin_guests() handling to finalize_local_nt_token()

We should add Builtin_Guests depending on the current token
not based on 'is_guest'. Even authenticated users can be member
a guest related group and therefore get Builtin_Guests.

Sadly we still need to use 'is_guest' within create_local_nt_token()
as we only have S-1-22-* SIDs there and still need to
add Builtin_Guests.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13328

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

diff --git a/source3/auth/token_util.c b/source3/auth/token_util.c
index f3d24cdac2f1be50f22b5a6fa8467ab80efa7b5c..30f2f8d346bf3aa14b23a09013a730c0db1ed147 100644 (file)
--- a/source3/auth/token_util.c
+++ b/source3/auth/token_util.c
@@ -211,6 +211,74 @@ static NTSTATUS add_builtin_administrators(struct security_token *token,
        return NT_STATUS_OK;
 }
 
+static NTSTATUS add_builtin_guests(struct security_token *token,
+                                  const struct dom_sid *dom_sid)
+{
+       struct dom_sid tmp_sid;
+       NTSTATUS status;
+
+       /*
+        * First check the local GUEST account.
+        */
+       sid_copy(&tmp_sid, get_global_sam_sid());
+       sid_append_rid(&tmp_sid, DOMAIN_RID_GUEST);
+
+       if (nt_token_check_sid(&tmp_sid, token)) {
+               status = add_sid_to_array_unique(token,
+                                       &global_sid_Builtin_Guests,
+                                       &token->sids, &token->num_sids);
+               if (!NT_STATUS_IS_OK(status)) {
+                       return status;
+               }
+
+               return NT_STATUS_OK;
+       }
+
+       /*
+        * First check the local GUESTS group.
+        */
+       sid_copy(&tmp_sid, get_global_sam_sid());
+       sid_append_rid(&tmp_sid, DOMAIN_RID_GUESTS);
+
+       if (nt_token_check_sid(&tmp_sid, token)) {
+               status = add_sid_to_array_unique(token,
+                                       &global_sid_Builtin_Guests,
+                                       &token->sids, &token->num_sids);
+               if (!NT_STATUS_IS_OK(status)) {
+                       return status;
+               }
+
+               return NT_STATUS_OK;
+       }
+
+       if (lp_server_role() != ROLE_DOMAIN_MEMBER) {
+               return NT_STATUS_OK;
+       }
+
+       if (dom_sid == NULL) {
+               return NT_STATUS_INVALID_PARAMETER_MIX;
+       }
+
+       /*
+        * First check the domain GUESTS group.
+        */
+       sid_copy(&tmp_sid, dom_sid);
+       sid_append_rid(&tmp_sid, DOMAIN_RID_GUESTS);
+
+       if (nt_token_check_sid(&tmp_sid, token)) {
+               status = add_sid_to_array_unique(token,
+                                       &global_sid_Builtin_Guests,
+                                       &token->sids, &token->num_sids);
+               if (!NT_STATUS_IS_OK(status)) {
+                       return status;
+               }
+
+               return NT_STATUS_OK;
+       }
+
+       return NT_STATUS_OK;
+}
+
 static NTSTATUS add_local_groups(struct security_token *result,
                                 bool is_guest);
 static NTSTATUS finalize_local_nt_token(struct security_token *result,
@@ -416,6 +484,29 @@ struct security_token *create_local_nt_token(TALLOC_CTX *mem_ctx,
                return NULL;
        }
 
+       if (is_guest) {
+               /*
+                * It's ugly, but for now it's
+                * needed to add Builtin_Guests
+                * here, the "local" token only
+                * consist of S-1-22-* SIDs
+                * and finalize_local_nt_token()
+                * doesn't have the chance to
+                * to detect it need to
+                * add Builtin_Guests via
+                * add_builtin_guests().
+                */
+               status = add_sid_to_array_unique(result,
+                                                &global_sid_Builtin_Guests,
+                                                &result->sids,
+                                                &result->num_sids);
+               if (!NT_STATUS_IS_OK(status)) {
+                       DEBUG(3, ("Failed to add SID to nt token\n"));
+                       TALLOC_FREE(result);
+                       return NULL;
+               }
+       }
+
        return result;
 }
 
@@ -535,14 +626,7 @@ static NTSTATUS finalize_local_nt_token(struct security_token *result,
                return status;
        }
 
-       if (is_guest) {
-               status = add_sid_to_array(result, &global_sid_Builtin_Guests,
-                                         &result->sids,
-                                         &result->num_sids);
-               if (!NT_STATUS_IS_OK(status)) {
-                       return status;
-               }
-       } else {
+       if (!is_guest) {
                status = add_sid_to_array(result,
                                          &global_sid_Authenticated_Users,
                                          &result->sids,
@@ -613,6 +697,28 @@ static NTSTATUS finalize_local_nt_token(struct security_token *result,
                }
        }
 
+       /*
+        * Add BUILTIN\Guests directly to token.
+        * But only if the token already indicates
+        * real guest access by:
+        * - local GUEST account
+        * - local GUESTS group
+        * - domain GUESTS group
+        *
+        * Even if a user was authenticated, it
+        * can be member of a guest related group.
+        */
+       status = add_builtin_guests(result, domain_sid);
+       if (!NT_STATUS_IS_OK(status)) {
+               DEBUG(3, ("Failed to check for local "
+                         "Guests membership (%s)\n",
+                         nt_errstr(status)));
+               /*
+                * This is a hard error.
+                */
+               return status;
+       }
+
        TALLOC_FREE(info);
 
        /* Deal with local groups */