From bbf27f0cae6d793e8d8d53c18e5018bf9d00d078 Mon Sep 17 00:00:00 2001 From: Bob Owen Date: Tue, 11 Jul 2017 09:44:20 +0100 Subject: [PATCH] Bug 1377555 Part 2: Add option to Windows chromium sandbox policy to not use restricting SIDs. r=jimm --- .../sandbox/win/src/restricted_token_utils.cc | 50 ++++++++++++------- .../sandbox/win/src/restricted_token_utils.h | 1 + .../chromium/sandbox/win/src/sandbox_policy.h | 5 ++ .../sandbox/win/src/sandbox_policy_base.cc | 10 +++- .../sandbox/win/src/sandbox_policy_base.h | 2 + 5 files changed, 47 insertions(+), 21 deletions(-) diff --git a/security/sandbox/chromium/sandbox/win/src/restricted_token_utils.cc b/security/sandbox/chromium/sandbox/win/src/restricted_token_utils.cc index 31e36cf69e47..42c447002778 100644 --- a/security/sandbox/chromium/sandbox/win/src/restricted_token_utils.cc +++ b/security/sandbox/chromium/sandbox/win/src/restricted_token_utils.cc @@ -22,6 +22,7 @@ DWORD CreateRestrictedToken(TokenLevel security_level, IntegrityLevel integrity_level, TokenType token_type, bool lockdown_default_dacl, + bool use_restricting_sids, base::win::ScopedHandle* token) { RestrictedToken restricted_token; restricted_token.Init(NULL); // Initialized with the current process token @@ -45,9 +46,12 @@ DWORD CreateRestrictedToken(TokenLevel security_level, deny_sids = false; remove_privileges = false; - unsigned err_code = restricted_token.AddRestrictingSidAllSids(); - if (ERROR_SUCCESS != err_code) - return err_code; + if (use_restricting_sids) { + unsigned err_code = restricted_token.AddRestrictingSidAllSids(); + if (ERROR_SUCCESS != err_code) { + return err_code; + } + } break; } @@ -71,11 +75,13 @@ DWORD CreateRestrictedToken(TokenLevel security_level, sid_exceptions.push_back(WinInteractiveSid); sid_exceptions.push_back(WinAuthenticatedUserSid); privilege_exceptions.push_back(SE_CHANGE_NOTIFY_NAME); - restricted_token.AddRestrictingSid(WinBuiltinUsersSid); - restricted_token.AddRestrictingSid(WinWorldSid); - restricted_token.AddRestrictingSid(WinRestrictedCodeSid); - restricted_token.AddRestrictingSidCurrentUser(); - restricted_token.AddRestrictingSidLogonSession(); + if (use_restricting_sids) { + restricted_token.AddRestrictingSid(WinBuiltinUsersSid); + restricted_token.AddRestrictingSid(WinWorldSid); + restricted_token.AddRestrictingSid(WinRestrictedCodeSid); + restricted_token.AddRestrictingSidCurrentUser(); + restricted_token.AddRestrictingSidLogonSession(); + } break; } case USER_LIMITED: { @@ -83,27 +89,33 @@ DWORD CreateRestrictedToken(TokenLevel security_level, sid_exceptions.push_back(WinWorldSid); sid_exceptions.push_back(WinInteractiveSid); privilege_exceptions.push_back(SE_CHANGE_NOTIFY_NAME); - restricted_token.AddRestrictingSid(WinBuiltinUsersSid); - restricted_token.AddRestrictingSid(WinWorldSid); - restricted_token.AddRestrictingSid(WinRestrictedCodeSid); + if (use_restricting_sids) { + restricted_token.AddRestrictingSid(WinBuiltinUsersSid); + restricted_token.AddRestrictingSid(WinWorldSid); + restricted_token.AddRestrictingSid(WinRestrictedCodeSid); - // This token has to be able to create objects in BNO. - // Unfortunately, on Vista+, it needs the current logon sid - // in the token to achieve this. You should also set the process to be - // low integrity level so it can't access object created by other - // processes. - restricted_token.AddRestrictingSidLogonSession(); + // This token has to be able to create objects in BNO. + // Unfortunately, on Vista+, it needs the current logon sid + // in the token to achieve this. You should also set the process to be + // low integrity level so it can't access object created by other + // processes. + restricted_token.AddRestrictingSidLogonSession(); + } break; } case USER_RESTRICTED: { privilege_exceptions.push_back(SE_CHANGE_NOTIFY_NAME); restricted_token.AddUserSidForDenyOnly(); - restricted_token.AddRestrictingSid(WinRestrictedCodeSid); + if (use_restricting_sids) { + restricted_token.AddRestrictingSid(WinRestrictedCodeSid); + } break; } case USER_LOCKDOWN: { restricted_token.AddUserSidForDenyOnly(); - restricted_token.AddRestrictingSid(WinNullSid); + if (use_restricting_sids) { + restricted_token.AddRestrictingSid(WinNullSid); + } break; } default: { diff --git a/security/sandbox/chromium/sandbox/win/src/restricted_token_utils.h b/security/sandbox/chromium/sandbox/win/src/restricted_token_utils.h index 1e312909bea4..910def615c07 100644 --- a/security/sandbox/chromium/sandbox/win/src/restricted_token_utils.h +++ b/security/sandbox/chromium/sandbox/win/src/restricted_token_utils.h @@ -40,6 +40,7 @@ DWORD CreateRestrictedToken(TokenLevel security_level, IntegrityLevel integrity_level, TokenType token_type, bool lockdown_default_dacl, + bool use_restricting_sids, base::win::ScopedHandle* token); // Sets the integrity label on a object handle. diff --git a/security/sandbox/chromium/sandbox/win/src/sandbox_policy.h b/security/sandbox/chromium/sandbox/win/src/sandbox_policy.h index 72a8f6100b87..33a7929b8277 100644 --- a/security/sandbox/chromium/sandbox/win/src/sandbox_policy.h +++ b/security/sandbox/chromium/sandbox/win/src/sandbox_policy.h @@ -100,6 +100,11 @@ class TargetPolicy { // Returns the lockdown token level. virtual TokenLevel GetLockdownTokenLevel() const = 0; + // Sets that we should not use restricting SIDs in the access tokens. We need + // to do this in some circumstances even though it weakens the sandbox. + // The default is to use them. + virtual void SetDoNotUseRestrictingSIDs() = 0; + // Sets the security level of the Job Object to which the target process will // belong. This setting is permanent and cannot be changed once the target // process is spawned. The job controls the global security settings which diff --git a/security/sandbox/chromium/sandbox/win/src/sandbox_policy_base.cc b/security/sandbox/chromium/sandbox/win/src/sandbox_policy_base.cc index ce99b111783b..c887bb75bb59 100644 --- a/security/sandbox/chromium/sandbox/win/src/sandbox_policy_base.cc +++ b/security/sandbox/chromium/sandbox/win/src/sandbox_policy_base.cc @@ -183,6 +183,10 @@ TokenLevel PolicyBase::GetLockdownTokenLevel() const { return lockdown_level_; } +void PolicyBase::SetDoNotUseRestrictingSIDs() { + use_restricting_sids_ = false; +} + ResultCode PolicyBase::SetJobLevel(JobLevel job_level, uint32_t ui_exceptions) { if (memory_limit_ && job_level == JOB_NONE) { return SBOX_ERROR_BAD_PARAMS; @@ -437,7 +441,8 @@ ResultCode PolicyBase::MakeTokens(base::win::ScopedHandle* initial, // with the process and therefore with any thread that is not impersonating. DWORD result = CreateRestrictedToken(lockdown_level_, integrity_level_, PRIMARY, - lockdown_default_dacl_, lockdown); + lockdown_default_dacl_, use_restricting_sids_, + lockdown); if (ERROR_SUCCESS != result) return SBOX_ERROR_GENERIC; @@ -493,7 +498,8 @@ ResultCode PolicyBase::MakeTokens(base::win::ScopedHandle* initial, // what we need (before reaching main( )) result = CreateRestrictedToken(initial_level_, integrity_level_, IMPERSONATION, - lockdown_default_dacl_, initial); + lockdown_default_dacl_, use_restricting_sids_, + initial); if (ERROR_SUCCESS != result) return SBOX_ERROR_GENERIC; diff --git a/security/sandbox/chromium/sandbox/win/src/sandbox_policy_base.h b/security/sandbox/chromium/sandbox/win/src/sandbox_policy_base.h index 5fc3f50c1455..d79bb99a7f89 100644 --- a/security/sandbox/chromium/sandbox/win/src/sandbox_policy_base.h +++ b/security/sandbox/chromium/sandbox/win/src/sandbox_policy_base.h @@ -42,6 +42,7 @@ class PolicyBase final : public TargetPolicy { ResultCode SetTokenLevel(TokenLevel initial, TokenLevel lockdown) override; TokenLevel GetInitialTokenLevel() const override; TokenLevel GetLockdownTokenLevel() const override; + void SetDoNotUseRestrictingSIDs() final; ResultCode SetJobLevel(JobLevel job_level, uint32_t ui_exceptions) override; JobLevel GetJobLevel() const override; ResultCode SetJobMemoryLimit(size_t memory_limit) override; @@ -127,6 +128,7 @@ class PolicyBase final : public TargetPolicy { // The user-defined global policy settings. TokenLevel lockdown_level_; TokenLevel initial_level_; + bool use_restricting_sids_ = true; JobLevel job_level_; uint32_t ui_exceptions_; size_t memory_limit_;