From 25ea5ee6ddd2e7260048700ca75e0d8a374d5624 Mon Sep 17 00:00:00 2001 From: "G.Reijn" <26114636+Gijsreyn@users.noreply.github.com> Date: Fri, 16 Oct 2026 13:16:36 +0200 Subject: [PATCH 1/3] (GH-1755) Add new DSC resource to manage SecurityPolicy --- .../code-review/security.instructions.md | 2 +- .../code-review/windows.instructions.md | 2 +- CHANGELOG.md | 14 + Cargo.lock | 11 + Cargo.toml | 6 +- data.build.json | 2 + .../configure-user-rights-assignment.md | 264 +++++++++++++ .../examples/get-user-rights-assignment.md | 132 +++++++ .../userrightsassignment.config.dsc.yaml | 23 ++ .../Windows/UserRightsAssignment/index.md | 373 ++++++++++++++++++ docs/reference/resources/overview.md | 2 + .../.project.data.json | 14 + resources/windows_security_policy/Cargo.toml | 18 + resources/windows_security_policy/build.rs | 7 + .../locales/en-us.toml | 36 ++ resources/windows_security_policy/src/lsa.rs | 234 +++++++++++ resources/windows_security_policy/src/main.rs | 217 ++++++++++ .../windows_security_policy/src/rights.rs | 106 +++++ resources/windows_security_policy/src/sid.rs | 338 ++++++++++++++++ .../windows_security_policy/src/types.rs | 150 +++++++ .../src/user_rights.rs | 295 ++++++++++++++ ...ecurity_policy_user_right_export.tests.ps1 | 63 +++ ...s_security_policy_user_right_get.tests.ps1 | 117 ++++++ ...s_security_policy_user_right_set.tests.ps1 | 170 ++++++++ ..._security_policy_user_right_test.tests.ps1 | 100 +++++ ...ecurity_policy_user_right_whatif.tests.ps1 | 96 +++++ ...windows_security_policy.dsc.manifests.json | 182 +++++++++ 27 files changed, 2971 insertions(+), 3 deletions(-) create mode 100644 docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/configure-user-rights-assignment.md create mode 100644 docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/get-user-rights-assignment.md create mode 100644 docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/userrightsassignment.config.dsc.yaml create mode 100644 docs/reference/resources/Microsoft/Windows/UserRightsAssignment/index.md create mode 100644 resources/windows_security_policy/.project.data.json create mode 100644 resources/windows_security_policy/Cargo.toml create mode 100644 resources/windows_security_policy/build.rs create mode 100644 resources/windows_security_policy/locales/en-us.toml create mode 100644 resources/windows_security_policy/src/lsa.rs create mode 100644 resources/windows_security_policy/src/main.rs create mode 100644 resources/windows_security_policy/src/rights.rs create mode 100644 resources/windows_security_policy/src/sid.rs create mode 100644 resources/windows_security_policy/src/types.rs create mode 100644 resources/windows_security_policy/src/user_rights.rs create mode 100644 resources/windows_security_policy/tests/windows_security_policy_user_right_export.tests.ps1 create mode 100644 resources/windows_security_policy/tests/windows_security_policy_user_right_get.tests.ps1 create mode 100644 resources/windows_security_policy/tests/windows_security_policy_user_right_set.tests.ps1 create mode 100644 resources/windows_security_policy/tests/windows_security_policy_user_right_test.tests.ps1 create mode 100644 resources/windows_security_policy/tests/windows_security_policy_user_right_whatif.tests.ps1 create mode 100644 resources/windows_security_policy/windows_security_policy.dsc.manifests.json diff --git a/.github/skills/code-review/security.instructions.md b/.github/skills/code-review/security.instructions.md index 42de16d5c..9cc49439c 100644 --- a/.github/skills/code-review/security.instructions.md +++ b/.github/skills/code-review/security.instructions.md @@ -1,5 +1,5 @@ --- -applyTo: 'lib/dsc-lib/src/util.rs,lib/dsc-lib-security_context/**,lib/dsc-lib-registry/**,resources/registry/**,resources/windows_firewall/**,resources/windows_service/**,resources/dism_dsc/**' +applyTo: 'lib/dsc-lib/src/util.rs,lib/dsc-lib-security_context/**,lib/dsc-lib-registry/**,resources/registry/**,resources/windows_firewall/**,resources/windows_service/**,resources/windows_security_policy/**,resources/dism_dsc/**' description: 'Code review guidance for security-sensitive code (ACLs, policy, credentials, elevated resources)' --- diff --git a/.github/skills/code-review/windows.instructions.md b/.github/skills/code-review/windows.instructions.md index 2a6d5e545..ce4aa708a 100644 --- a/.github/skills/code-review/windows.instructions.md +++ b/.github/skills/code-review/windows.instructions.md @@ -1,5 +1,5 @@ --- -applyTo: 'resources/windows_firewall/**,resources/windows_service/**,resources/WindowsUpdate/**,resources/windows_personalization/**,resources/dism_dsc/**,resources/registry/**,lib/dsc-lib-registry/**,lib/dsc-lib-pal/**' +applyTo: 'resources/windows_firewall/**,resources/windows_service/**,resources/windows_security_policy/**,resources/WindowsUpdate/**,resources/windows_personalization/**,resources/dism_dsc/**,resources/registry/**,lib/dsc-lib-registry/**,lib/dsc-lib-pal/**' description: 'Code review guidance for Windows-specific code (FFI, COM, registry, DISM, services)' --- diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ae6e6f41..f6fd2e0e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,19 @@ changes since the last release, see the [diff on GitHub][unreleased]. ### Added +- Added the `Microsoft.Windows/UserRightsAssignment` built-in resource for managing which accounts + hold a Windows user right or privilege (Local Security Policy > User Rights Assignment) natively + through the LSA account-rights APIs, as the first part of a replacement for the + `SecurityPolicyDsc` PowerShell module. Identities can be given as names or SIDs, the + `setAction` property chooses between replacing, adding or removing holders, and the resource + supports `get`, `set`, `test`, `export`, and `--what-if`. + +
Related work items + + - Issues: [#1755][#1755] + +
+ - Added what-if support to the MCP server tools. The `invoke_dsc_config` tool now accepts a `what_if` option for the `set` operation, and the `invoke_dsc_resource` tool accepts `what_if` for the `set` and `delete` operations. This mirrors the `--what-if` flag on the `dsc config set`, @@ -1982,4 +1995,5 @@ Version `3.0.0` is the first generally available release of DSC. [#1557]: https://github.com/PowerShell/DSC/issues/1557 [#1558]: https://github.com/PowerShell/DSC/issues/1558 [#1562]: https://github.com/PowerShell/DSC/issues/1562 +[#1755]: https://github.com/PowerShell/DSC/issues/1755 diff --git a/Cargo.lock b/Cargo.lock index 31ec37535..0e23ad34a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4513,6 +4513,17 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66" +[[package]] +name = "windows_security_policy" +version = "0.1.0" +dependencies = [ + "dsc-lib-security_context", + "rust-i18n", + "serde", + "serde_json", + "windows 0.62.2", +] + [[package]] name = "windows_service" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index d371a5b4d..9feb64e38 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,7 @@ members = [ "resources/windows_service", "resources/WindowsUpdate", "resources/environment_variable", + "resources/windows_security_policy", "adapters/group_policy_template", "tools/dsctest", "tools/test_group_resource", @@ -62,6 +63,7 @@ default-members = [ "resources/windows_service", "resources/WindowsUpdate", "resources/environment_variable", + "resources/windows_security_policy", "adapters/group_policy_template", "tools/dsctest", "tools/test_group_resource", @@ -98,6 +100,7 @@ Windows = [ "resources/windows_service", "resources/WindowsUpdate", "resources/environment_variable", + "resources/windows_security_policy", "adapters/group_policy_template", "tools/dsctest", "tools/test_group_resource", @@ -290,11 +293,12 @@ urlencoding = { version = "2.1" } which = { version = "8.0.6" } # dsc-lib ipnetwork = { version = "0.21" } -# WindowsUpdate, windows_service, dsc-lib (ACL checks), dsc-lib-registry +# WindowsUpdate, windows_service, windows_security_policy, dsc-lib (ACL checks), dsc-lib-registry windows = { version = "0.62", features = [ "Win32_Foundation", "Win32_NetworkManagement_WindowsFirewall", "Win32_Security", + "Win32_Security_Authentication_Identity", "Win32_Security_Authorization", "Win32_System_Com", "Win32_System_Memory", diff --git a/data.build.json b/data.build.json index a91dc0240..7fd4e6ed1 100644 --- a/data.build.json +++ b/data.build.json @@ -117,6 +117,8 @@ "windows_firewall.exe", "windows_service.exe", "windows_service.dsc.resource.json", + "windows_security_policy.exe", + "windows_security_policy.dsc.manifests.json", "windows_feature.dsc.resource.json", "wmi.dsc.resource.json", "wmi.resource.ps1", diff --git a/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/configure-user-rights-assignment.md b/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/configure-user-rights-assignment.md new file mode 100644 index 000000000..5464f2c3a --- /dev/null +++ b/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/configure-user-rights-assignment.md @@ -0,0 +1,264 @@ +--- +description: > + Example showing how to use the Microsoft.Windows/UserRightsAssignment resource in a DSC + configuration document to enforce user rights assignments. +ms.date: 10/16/2026 +ms.topic: reference +title: Configure user rights assignments +--- + +# Configure user rights assignments + +This example shows how you can use the `Microsoft.Windows/UserRightsAssignment` resource in a DSC +configuration document to enforce user rights the way a hardening baseline requires, using each of +the three `setAction` modes. + +> [!IMPORTANT] +> All operations of this resource require an elevated (administrator) process context. Run your +> terminal or PowerShell session as Administrator before using `dsc config set`. + +## Definition + +The configuration document for this example defines three instances of the +`UserRightsAssignment` resource. + +The first instance makes `BUILTIN\Guests` the only account that is denied batch logon; because +`setAction` isn't specified, it defaults to `replace`. The second instance ensures that +`NT SERVICE\ALL SERVICES` can log on as a service without touching the other holders. The third +instance ensures that `BUILTIN\Guests` can't log on locally, whoever else holds that right. + +:::code language="yaml" source="userrightsassignment.config.dsc.yaml"::: + +Copy the configuration document and save it as `userrightsassignment.config.dsc.yaml`. + +## Setup + +The output in this example assumes that nobody is denied batch logon, that `ALL SERVICES` already +holds `SeServiceLogonRight`, and that `Guests` holds `SeInteractiveLogonRight`. You can set the +system to this starting state with a security template: + +```powershell +@' +[Unicode] +Unicode=yes +[Privilege Rights] +SeDenyBatchLogonRight = +SeInteractiveLogonRight = *S-1-5-32-544,*S-1-5-32-545,*S-1-5-32-546,*S-1-5-32-551 +[Version] +signature="$CHICAGO$" +Revision=1 +'@ | Set-Content -Path "$env:TEMP\setup.inf" -Encoding unicode +secedit /configure /db "$env:TEMP\setup.sdb" /cfg "$env:TEMP\setup.inf" /areas USER_RIGHTS /quiet +``` + +## Test the configuration + +To see whether the system is already in the desired state, use the [dsc config test][01] command. + +```powershell +dsc config test --file ./userrightsassignment.config.dsc.yaml +``` + +```yaml +executionInformation: + # Elided for brevity +metadata: + # Elided for brevity +results: +- executionInformation: + duration: PT0.0532118S + metadata: + Microsoft.DSC: + duration: PT0.0532118S + name: Deny batch logon to guests only + type: Microsoft.Windows/UserRightsAssignment + result: + desiredState: + right: SeDenyBatchLogonRight + identity: + - BUILTIN\Guests + actualState: + right: SeDenyBatchLogonRight + identity: [] + _inDesiredState: false + inDesiredState: false + differingProperties: + - identity +- executionInformation: + duration: PT0.0498742S + metadata: + Microsoft.DSC: + duration: PT0.0498742S + name: Services may log on as a service + type: Microsoft.Windows/UserRightsAssignment + result: + desiredState: + right: SeServiceLogonRight + identity: + - NT SERVICE\ALL SERVICES + setAction: add + actualState: + right: SeServiceLogonRight + identity: + - NT SERVICE\ALL SERVICES + - NT VIRTUAL MACHINE\Virtual Machines + _inDesiredState: true + inDesiredState: true + differingProperties: + - identity +- executionInformation: + duration: PT0.0511306S + metadata: + Microsoft.DSC: + duration: PT0.0511306S + name: Guests may not log on locally + type: Microsoft.Windows/UserRightsAssignment + result: + desiredState: + right: SeInteractiveLogonRight + identity: + - BUILTIN\Guests + setAction: remove + actualState: + right: SeInteractiveLogonRight + identity: + - BUILTIN\Administrators + - BUILTIN\Backup Operators + - BUILTIN\Guests + - BUILTIN\Users + _inDesiredState: false + inDesiredState: false + differingProperties: + - identity +messages: [] +hadErrors: false +``` + +The second instance is in the desired state because `ALL SERVICES` already holds the right. The +resource decides `inDesiredState` from `setAction`; DSC still lists `identity` under +`differingProperties` because the arrays differ. + +## Set the configuration + +To enforce the desired state, use the [dsc config set][02] command. + +```powershell +dsc config set --file ./userrightsassignment.config.dsc.yaml +``` + +```yaml +executionInformation: + # Elided for brevity +metadata: + # Elided for brevity +results: +- executionInformation: + duration: PT0.0987421S + metadata: + Microsoft.DSC: + duration: PT0.0987421S + name: Deny batch logon to guests only + type: Microsoft.Windows/UserRightsAssignment + result: + beforeState: + right: SeDenyBatchLogonRight + identity: [] + _inDesiredState: false + afterState: + right: SeDenyBatchLogonRight + identity: + - BUILTIN\Guests + changedProperties: + - identity + - _inDesiredState +- executionInformation: + duration: PT0.0509873S + metadata: + Microsoft.DSC: + duration: PT0.0509873S + name: Services may log on as a service + type: Microsoft.Windows/UserRightsAssignment + result: + beforeState: + right: SeServiceLogonRight + identity: + - NT SERVICE\ALL SERVICES + - NT VIRTUAL MACHINE\Virtual Machines + _inDesiredState: true + afterState: + right: SeServiceLogonRight + identity: + - NT SERVICE\ALL SERVICES + - NT VIRTUAL MACHINE\Virtual Machines + _inDesiredState: true + changedProperties: null +- executionInformation: + duration: PT0.0975219S + metadata: + Microsoft.DSC: + duration: PT0.0975219S + name: Guests may not log on locally + type: Microsoft.Windows/UserRightsAssignment + result: + beforeState: + right: SeInteractiveLogonRight + identity: + - BUILTIN\Administrators + - BUILTIN\Backup Operators + - BUILTIN\Guests + - BUILTIN\Users + _inDesiredState: false + afterState: + right: SeInteractiveLogonRight + identity: + - BUILTIN\Administrators + - BUILTIN\Backup Operators + - BUILTIN\Users + changedProperties: + - identity + - _inDesiredState +messages: [] +hadErrors: false +``` + +DSC skipped the second instance because its test reported it in the desired state. The third +instance only revoked the right from `Guests` and kept the other holders, because it uses +`setAction: remove`. + +## Preview the changes + +To see which accounts would be granted or revoked without applying the change, add the +`--what-if` option. The resource reports the projected holders and describes each change under +`_metadata.whatIf`. + +```powershell +dsc config set --file ./userrightsassignment.config.dsc.yaml --what-if +``` + +```yaml +# Elided for brevity +results: +- name: Deny batch logon to guests only + type: Microsoft.Windows/UserRightsAssignment + result: + beforeState: + right: SeDenyBatchLogonRight + identity: [] + _inDesiredState: false + afterState: + right: SeDenyBatchLogonRight + identity: + - BUILTIN\Guests + _metadata: + whatIf: + - Would grant 'SeDenyBatchLogonRight' to 'BUILTIN\Guests' + changedProperties: + - identity + - _inDesiredState + - _metadata +# Elided for brevity +``` + + +[01]: ../../../../../../cli/config/test.md +[02]: ../../../../../../cli/config/set.md diff --git a/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/get-user-rights-assignment.md b/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/get-user-rights-assignment.md new file mode 100644 index 000000000..e824a6b1d --- /dev/null +++ b/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/get-user-rights-assignment.md @@ -0,0 +1,132 @@ +--- +description: > + Example showing how to use the Microsoft.Windows/UserRightsAssignment resource with the dsc + resource commands to retrieve, test and export user rights assignments. +ms.date: 10/16/2026 +ms.topic: reference +title: Get user rights assignments +--- + +# Get user rights assignments + +This example shows how you can use the `Microsoft.Windows/UserRightsAssignment` resource to +retrieve the accounts that hold a user right, test whether a right is assigned as expected, and +export every right on the system. + +> [!IMPORTANT] +> All operations of this resource require an elevated (administrator) process context. Run your +> terminal or PowerShell session as Administrator before using the `dsc resource` commands. + +## Get the holders of a right + +To retrieve the accounts that hold a right, use the [dsc resource get][01] command. The only +required property is `right`. + +```powershell +dsc resource get --resource Microsoft.Windows/UserRightsAssignment --input '{"right":"SeServiceLogonRight"}' +``` + +```yaml +actualState: + right: SeServiceLogonRight + identity: + - NT SERVICE\ALL SERVICES + - NT VIRTUAL MACHINE\Virtual Machines +``` + +The resource returns every holder as `DOMAIN\Name`. Accounts without a domain, such as `Everyone`, +are returned by name only, and a SID that no longer resolves to an account is returned as the SID +string. + +## Test the holders of a right + +To check whether a right is assigned the way you expect, use the [dsc resource test][02] command. +The resource compares accounts by SID, so you can write the identities in any accepted form. The +following command checks that `Guests` is the only account denied batch logon: + +```powershell +dsc resource test --resource Microsoft.Windows/UserRightsAssignment --input '{"right":"SeDenyBatchLogonRight","identity":["S-1-5-32-546"]}' +``` + +```yaml +desiredState: + right: SeDenyBatchLogonRight + identity: + - S-1-5-32-546 +actualState: + right: SeDenyBatchLogonRight + identity: [] + _inDesiredState: false +inDesiredState: false +differingProperties: +- identity +``` + +To check that an account holds a right regardless of the other holders, add `setAction: add`: + +```powershell +dsc resource test --resource Microsoft.Windows/UserRightsAssignment --input '{"right":"SeChangeNotifyPrivilege","identity":["Everyone"],"setAction":"add"}' +``` + +```yaml +desiredState: + right: SeChangeNotifyPrivilege + identity: + - Everyone + setAction: add +actualState: + right: SeChangeNotifyPrivilege + identity: + - BUILTIN\Administrators + - BUILTIN\Backup Operators + - BUILTIN\Users + - Everyone + - NT AUTHORITY\LOCAL SERVICE + - NT AUTHORITY\NETWORK SERVICE + _inDesiredState: true +inDesiredState: true +differingProperties: +- identity +``` + +The resource reports `inDesiredState: true` because `Everyone` holds the right. DSC still lists +`identity` under `differingProperties` because the arrays differ; rely on `inDesiredState` when +you use `add` or `remove`. + +## Export every right + +To export the holders of every user right, use the [dsc resource export][03] command. The output +is a configuration document with one instance per right, which you can use as the starting point +for a baseline. + +```powershell +dsc resource export --resource Microsoft.Windows/UserRightsAssignment +``` + +```yaml +$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json +resources: +- name: Microsoft.Windows/UserRightsAssignment-0 + type: Microsoft.Windows/UserRightsAssignment + properties: + right: SeTrustedCredManAccessPrivilege + identity: [] +- name: Microsoft.Windows/UserRightsAssignment-1 + type: Microsoft.Windows/UserRightsAssignment + properties: + right: SeNetworkLogonRight + identity: + - BUILTIN\Administrators + - BUILTIN\Backup Operators + - BUILTIN\Users + - Everyone +# Elided for brevity +``` + +Applying an exported document with `dsc config set` makes every right match the exported holders, +because `setAction` defaults to `replace`. + + +[01]: ../../../../../../cli/resource/get.md +[02]: ../../../../../../cli/resource/test.md +[03]: ../../../../../../cli/resource/export.md diff --git a/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/userrightsassignment.config.dsc.yaml b/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/userrightsassignment.config.dsc.yaml new file mode 100644 index 000000000..d3f0a3210 --- /dev/null +++ b/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/userrightsassignment.config.dsc.yaml @@ -0,0 +1,23 @@ +# yaml-language-server: $schema=https://aka.ms/dsc/schemas/v3/bundled/config/document.vscode.json +$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json +resources: + - name: Deny batch logon to guests only + type: Microsoft.Windows/UserRightsAssignment + properties: + right: SeDenyBatchLogonRight + identity: + - BUILTIN\Guests + - name: Services may log on as a service + type: Microsoft.Windows/UserRightsAssignment + properties: + right: SeServiceLogonRight + identity: + - NT SERVICE\ALL SERVICES + setAction: add + - name: Guests may not log on locally + type: Microsoft.Windows/UserRightsAssignment + properties: + right: SeInteractiveLogonRight + identity: + - BUILTIN\Guests + setAction: remove diff --git a/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/index.md b/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/index.md new file mode 100644 index 000000000..98812a7bf --- /dev/null +++ b/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/index.md @@ -0,0 +1,373 @@ +--- +description: Microsoft.Windows/UserRightsAssignment resource reference documentation +ms.date: 10/16/2026 +ms.topic: reference +title: Microsoft.Windows/UserRightsAssignment +--- + +# Microsoft.Windows/UserRightsAssignment + +## Synopsis + +Manage which accounts hold a Windows user right or privilege. + +## Metadata + +```yaml +Version : 0.1.0 +Kind : resource +Tags : [Windows, Security, UserRights] +Author : Microsoft +``` + +## Instance definition syntax + +```yaml +resources: + - name: + type: Microsoft.Windows/UserRightsAssignment + properties: + # Key properties + right: string + # Instance properties + identity: + - string + setAction: replace | add | remove +``` + +## Description + +The `Microsoft.Windows/UserRightsAssignment` resource enables you to idempotently manage the +accounts that hold a user right or privilege on a Windows system. These are the settings under +**Local Policies > User Rights Assignment** in the Local Security Policy console, which +`secedit` exports as the `[Privilege Rights]` section of a security template. Each instance of the +resource represents one right, such as `SeServiceLogonRight` (Log on as a service), and lists the +accounts that hold it. + +The resource can: + +- Retrieve the accounts that hold a right. +- Make the listed accounts the only holders of a right, grant a right to additional accounts, or + revoke a right from specific accounts. +- Test whether a right is held by exactly, at least, or none of the listed accounts. +- Export the holders of every right on the system. + +The resource reads and writes the policy through the Local Security Authority (LSA) account +rights APIs, not through `secedit`, so it needs no template files and works on any system +language. Accounts are compared by security identifier (SID), so `Administrators`, +`BUILTIN\Administrators` and `S-1-5-32-544` all refer to the same holder. + +Windows defines the following user rights: + +| Constant | Local Security Policy name | +|:--------------------------------------------|:--------------------------------------------------------------------| +| `SeTrustedCredManAccessPrivilege` | Access Credential Manager as a trusted caller | +| `SeNetworkLogonRight` | Access this computer from the network | +| `SeTcbPrivilege` | Act as part of the operating system | +| `SeMachineAccountPrivilege` | Add workstations to domain | +| `SeIncreaseQuotaPrivilege` | Adjust memory quotas for a process | +| `SeInteractiveLogonRight` | Allow log on locally | +| `SeRemoteInteractiveLogonRight` | Allow log on through Remote Desktop Services | +| `SeBackupPrivilege` | Back up files and directories | +| `SeChangeNotifyPrivilege` | Bypass traverse checking | +| `SeSystemtimePrivilege` | Change the system time | +| `SeTimeZonePrivilege` | Change the time zone | +| `SeCreatePagefilePrivilege` | Create a pagefile | +| `SeCreateTokenPrivilege` | Create a token object | +| `SeCreateGlobalPrivilege` | Create global objects | +| `SeCreatePermanentPrivilege` | Create permanent shared objects | +| `SeCreateSymbolicLinkPrivilege` | Create symbolic links | +| `SeDebugPrivilege` | Debug programs | +| `SeDenyNetworkLogonRight` | Deny access to this computer from the network | +| `SeDenyBatchLogonRight` | Deny log on as a batch job | +| `SeDenyServiceLogonRight` | Deny log on as a service | +| `SeDenyInteractiveLogonRight` | Deny log on locally | +| `SeDenyRemoteInteractiveLogonRight` | Deny log on through Remote Desktop Services | +| `SeEnableDelegationPrivilege` | Enable computer and user accounts to be trusted for delegation | +| `SeRemoteShutdownPrivilege` | Force shutdown from a remote system | +| `SeAuditPrivilege` | Generate security audits | +| `SeImpersonatePrivilege` | Impersonate a client after authentication | +| `SeIncreaseWorkingSetPrivilege` | Increase a process working set | +| `SeIncreaseBasePriorityPrivilege` | Increase scheduling priority | +| `SeLoadDriverPrivilege` | Load and unload device drivers | +| `SeLockMemoryPrivilege` | Lock pages in memory | +| `SeBatchLogonRight` | Log on as a batch job | +| `SeServiceLogonRight` | Log on as a service | +| `SeSecurityPrivilege` | Manage auditing and security log | +| `SeRelabelPrivilege` | Modify an object label | +| `SeSystemEnvironmentPrivilege` | Modify firmware environment values | +| `SeDelegateSessionUserImpersonatePrivilege` | Obtain an impersonation token for another user in the same session | +| `SeManageVolumePrivilege` | Perform volume maintenance tasks | +| `SeProfileSingleProcessPrivilege` | Profile single process | +| `SeSystemProfilePrivilege` | Profile system performance | +| `SeUndockPrivilege` | Remove computer from docking station | +| `SeAssignPrimaryTokenPrivilege` | Replace a process level token | +| `SeRestorePrivilege` | Restore files and directories | +| `SeShutdownPrivilege` | Shut down the system | +| `SeSyncAgentPrivilege` | Synchronize directory service data | +| `SeTakeOwnershipPrivilege` | Take ownership of files or other objects | + +> [!NOTE] +> This resource is installed with DSC itself on Windows systems. +> +> You can update this resource by updating DSC. When you update DSC, the updated version of this +> resource is automatically available. + +## Requirements + +- The resource is only usable on a Windows system. +- All operations, including **Get**, **Test** and **Export**, require an elevated (administrator) + process context because the LSA only lets administrators enumerate account rights. DSC refuses + to invoke the resource from a non-elevated process. +- When user rights are managed by Group Policy, the domain or local Group Policy setting + overwrites the local assignment at the next policy refresh. Manage the right through Group + Policy instead, or exclude it from Group Policy. + +## Capabilities + +The resource has the following capabilities: + +- `get` - You can use the resource to retrieve the accounts that hold a right. +- `set` - You can use the resource to enforce the holders of a right. +- `test` - The resource implements its own test. It compares the current and desired holders by + SID and honours [setAction](#setaction), so any accepted spelling of an account matches. +- `export` - You can use the resource to export the holders of every right on the system. +- `whatIf` - You can use the resource with the `--what-if` option of the `dsc config set` and + `dsc resource set` commands to see which accounts would be granted or revoked without applying + the change. + +For more information about resource capabilities, see [DSC resource capabilities][01]. + +## Examples + +1. [Get user rights assignments][02] - Shows how to retrieve, test and export user rights with the + `dsc resource` commands. +1. [Configure user rights assignments][03] - Shows how to enforce user rights using a DSC + configuration document. + +## Properties + +The following list describes the properties for the resource. + +- **Key properties:** The following properties uniquely identify an + instance. If two instances of a resource have the same values for their key properties, the + instances are conflicting. For more information about key properties, see the "Key resource + properties" section in [DSC resource properties][04]. + + - [right](#right) - The user right or privilege to manage. + +- **Instance properties:** The following properties are optional. + They define the desired state for an instance of the resource. + + - [identity](#identity) - The accounts that hold the right. + - [setAction](#setaction) - How the `identity` list is applied. + +- **Read-only properties:** The resource returns the following + properties, but they aren't configurable. For more information about read-only properties, see + the "Read-only resource properties" section in [DSC resource properties][05]. + + - [_inDesiredState](#_indesiredstate) - Whether the current holders satisfy the desired state. + +### right + +
Expand for right property metadata + +```yaml +Type : string +IsRequired : true +IsKey : true +IsReadOnly : false +IsWriteOnly : false +Enum : [SeTrustedCredManAccessPrivilege, SeNetworkLogonRight, SeTcbPrivilege, SeMachineAccountPrivilege, SeIncreaseQuotaPrivilege, SeInteractiveLogonRight, SeRemoteInteractiveLogonRight, SeBackupPrivilege, SeChangeNotifyPrivilege, SeSystemtimePrivilege, SeTimeZonePrivilege, SeCreatePagefilePrivilege, SeCreateTokenPrivilege, SeCreateGlobalPrivilege, SeCreatePermanentPrivilege, SeCreateSymbolicLinkPrivilege, SeDebugPrivilege, SeDenyNetworkLogonRight, SeDenyBatchLogonRight, SeDenyServiceLogonRight, SeDenyInteractiveLogonRight, SeDenyRemoteInteractiveLogonRight, SeEnableDelegationPrivilege, SeRemoteShutdownPrivilege, SeAuditPrivilege, SeImpersonatePrivilege, SeIncreaseWorkingSetPrivilege, SeIncreaseBasePriorityPrivilege, SeLoadDriverPrivilege, SeLockMemoryPrivilege, SeBatchLogonRight, SeServiceLogonRight, SeSecurityPrivilege, SeRelabelPrivilege, SeSystemEnvironmentPrivilege, SeDelegateSessionUserImpersonatePrivilege, SeManageVolumePrivilege, SeProfileSingleProcessPrivilege, SeSystemProfilePrivilege, SeUndockPrivilege, SeAssignPrimaryTokenPrivilege, SeRestorePrivilege, SeShutdownPrivilege, SeSyncAgentPrivilege, SeTakeOwnershipPrivilege] +``` + +
+ +The constant of the user right or privilege to manage, such as `SeServiceLogonRight`. These are +the names that the LSA, `secedit` templates, Group Policy and security benchmarks use; the table +in the [Description](#description) maps them to the names shown in the Local Security Policy +console. The resource returns the value exactly as you specified it, and **Export** returns the +canonical casing. + +### identity + +
Expand for identity property metadata + +```yaml +Type : array +ItemsType : string +ItemsMustBeUnique : true +IsRequired : false +IsKey : false +IsReadOnly : false +IsWriteOnly : false +``` + +
+ +The accounts that hold the right. This property is required for the **Set** and **Test** +operations. Each entry can be any of the following: + +- An account name with its domain, such as `CONTOSO\svc-sql`, `BUILTIN\Administrators` or + `NT SERVICE\MSSQLSERVER`. +- A bare account name that the local system can resolve, such as `Administrators`. +- A well-known name, such as `Everyone` or `NT AUTHORITY\LOCAL SERVICE`. +- A SID string, such as `S-1-5-32-544`, or the form that `secedit` exports, `*S-1-5-32-544`. + +The resource resolves every entry to a SID. An account name that doesn't exist is an error. A SID +that doesn't resolve to an account is accepted, because Windows keeps rights assigned to deleted +accounts. Entries that resolve to the same SID count once. + +On **Get** and **Export**, the resource returns each holder as `DOMAIN\Name`, as a bare name when +the account has no domain (for example `Everyone`), or as the SID string when the SID no longer +resolves to an account. An empty array means that no one holds the right. + +### setAction + +
Expand for setAction property metadata + +```yaml +Type : string +IsRequired : false +IsKey : false +IsReadOnly : false +IsWriteOnly : true +DefaultValue : replace +Enum : [replace, add, remove] +``` + +
+ +Defines how the **Set** operation applies the `identity` list and how the **Test** operation +judges it. The following values are valid: + +| Value | Set | Test is in the desired state when | +|:----------|:-------------------------------------------------------------------------|:----------------------------------------------------| +| `replace` | The listed accounts become the only holders; other holders are revoked. | The holders are exactly the listed accounts. | +| `add` | The listed accounts are granted the right; other holders are kept. | Every listed account holds the right. | +| `remove` | The listed accounts lose the right; other holders are kept. | None of the listed accounts hold the right. | + +With `replace`, an empty `identity` list revokes the right from everyone. With `add` or `remove`, +an empty list changes nothing. + +Because this property is write-only, DSC doesn't compare it and it never appears in the actual +state. When you use `add` or `remove`, the `differingProperties` list that DSC reports may still +name `identity` even though the resource reports the instance as in the desired state; rely on +`inDesiredState` for the verdict. + +### _inDesiredState + +
Expand for _inDesiredState property metadata + +```yaml +Type : boolean +IsRequired : false +IsKey : false +IsReadOnly : true +IsWriteOnly : false +``` + +
+ +Returned by the **Test** operation only. Indicates whether the current holders of the right +satisfy `identity` under the chosen `setAction`. + +## Instance validating schema + +The following snippet contains the JSON Schema that validates an instance of the resource. The +`enum` for `right` is abbreviated; it contains the 45 constants listed in the +[Description](#description). + +```json +{ + "type": "object", + "required": ["right"], + "additionalProperties": false, + "properties": { + "right": { + "type": "string", + "enum": ["SeTrustedCredManAccessPrivilege", "SeNetworkLogonRight", "..."] + }, + "identity": { + "type": "array", + "items": { + "type": "string", + "minLength": 1 + }, + "uniqueItems": true + }, + "setAction": { + "type": "string", + "writeOnly": true, + "default": "replace", + "enum": ["replace", "add", "remove"] + }, + "_inDesiredState": { + "type": "boolean", + "readOnly": true + }, + "_metadata": { + "type": "object", + "readOnly": true, + "properties": { + "whatIf": { + "type": "array", + "items": { + "type": "string" + } + } + } + } + } +} +``` + +## Exit codes + +The resource returns the following exit codes from operations: + +- [0](#exit-code-0) - Success +- [1](#exit-code-1) - Invalid arguments +- [2](#exit-code-2) - Invalid input +- [3](#exit-code-3) - Security policy error +- [4](#exit-code-4) - Elevation required + +### Exit code 0 + +Indicates the resource operation completed without errors. + +### Exit code 1 + +Indicates the resource operation failed because required arguments were missing or the operation +name was not recognized. + +### Exit code 2 + +Indicates the resource operation failed because the JSON input could not be deserialized into a +valid instance, the `right` isn't a known user right, an entry in `identity` isn't a valid SID or +an existing account, or `identity` was omitted from a **Set** or **Test** operation. + +### Exit code 3 + +Indicates the resource operation failed due to an error raised by the Local Security Authority or +another Windows API, or the result could not be serialized. + +### Exit code 4 + +Indicates the resource operation failed because the process isn't elevated. Reading or changing +user rights assignments requires an elevated process. + +## See also + +- [Microsoft.Windows/Registry resource][06] +- [DSC resource capabilities][01] +- [DSC resource properties][07] + + +[01]: ../../../../../concepts/resources/capabilities.md +[02]: ./examples/get-user-rights-assignment.md +[03]: ./examples/configure-user-rights-assignment.md +[04]: ../../../../../concepts/resources/properties.md#key-resource-properties +[05]: ../../../../../concepts/resources/properties.md#read-only-resource-properties +[06]: ../Registry/index.md +[07]: ../../../../../concepts/resources/properties.md diff --git a/docs/reference/resources/overview.md b/docs/reference/resources/overview.md index c6324dfa9..99a7dd1f4 100644 --- a/docs/reference/resources/overview.md +++ b/docs/reference/resources/overview.md @@ -24,6 +24,7 @@ This document lists the available resources and links to the reference documenta - [Microsoft.Windows/RebootPending](./Microsoft/Windows/RebootPending/index.md) - [Microsoft.Windows/Registry](./Microsoft/Windows/Registry/index.md) - [Microsoft.Windows/Service](./Microsoft/Windows/Service/index.md) +- [Microsoft.Windows/UserRightsAssignment](./Microsoft/Windows/UserRightsAssignment/index.md) - [Microsoft.Windows/WindowsPowerShell](./Microsoft/Windows/WindowsPowerShell/index.md) - [Microsoft.Windows/WMI](./Microsoft/Windows/WMI/index.md) @@ -66,6 +67,7 @@ The following built-in resources change the state of a machine directly: - [Microsoft.Windows/OptionalFeatureList](./Microsoft/Windows/OptionalFeatureList/index.md) - [Microsoft.Windows/Registry](./Microsoft/Windows/Registry/index.md) - [Microsoft.Windows/Service](./Microsoft/Windows/Service/index.md) +- [Microsoft.Windows/UserRightsAssignment](./Microsoft/Windows/UserRightsAssignment/index.md) ## Built-in debugging resources diff --git a/resources/windows_security_policy/.project.data.json b/resources/windows_security_policy/.project.data.json new file mode 100644 index 000000000..c2aabf7d0 --- /dev/null +++ b/resources/windows_security_policy/.project.data.json @@ -0,0 +1,14 @@ +{ + "Name": "windows_security_policy", + "Kind": "Resource", + "IsRust": true, + "SupportedPlatformOS": "Windows", + "Binaries": [ + "windows_security_policy" + ], + "CopyFiles": { + "Windows": [ + "windows_security_policy.dsc.manifests.json" + ] + } +} diff --git a/resources/windows_security_policy/Cargo.toml b/resources/windows_security_policy/Cargo.toml new file mode 100644 index 000000000..79c12abca --- /dev/null +++ b/resources/windows_security_policy/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "windows_security_policy" +version = "0.1.0" +edition = "2024" + +[package.metadata.i18n] +available-locales = ["en-us"] +default-locale = "en-us" +load-path = "locales" + +[dependencies] +rust-i18n = { workspace = true } +serde = { workspace = true } +serde_json = { workspace = true } + +[target.'cfg(windows)'.dependencies] +windows = { workspace = true } +dsc-lib-security_context = { workspace = true } diff --git a/resources/windows_security_policy/build.rs b/resources/windows_security_policy/build.rs new file mode 100644 index 000000000..315b846a1 --- /dev/null +++ b/resources/windows_security_policy/build.rs @@ -0,0 +1,7 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +fn main() { + // Always rebuild if translations are updated. + println!("cargo::rerun-if-changed=locales"); +} diff --git a/resources/windows_security_policy/locales/en-us.toml b/resources/windows_security_policy/locales/en-us.toml new file mode 100644 index 000000000..8f41dedcf --- /dev/null +++ b/resources/windows_security_policy/locales/en-us.toml @@ -0,0 +1,36 @@ +_version = 1 + +[main] +missingOperation = "Missing operation. Usage: windows_security_policy [--input ] [--what-if]" +unknownOperation = "Unknown operation: '%{operation}'. Expected: get, set, test, or export" +missingTarget = "Missing resource target. Expected: user-right" +unknownTarget = "Unknown resource target: '%{target}'. Expected: user-right" +missingInput = "Missing --input argument" +missingInputValue = "Missing value for --input argument" +invalidJson = "Invalid JSON input: %{error}" +serializeFailed = "Failed to serialize output: %{error}" +windowsOnly = "This resource is only supported on Windows" +elevationRequired = "Managing Windows user rights assignments requires elevation. Run DSC from an elevated process." + +[rights] +unknownRight = "Unknown user right '%{right}'. Specify a user right constant such as 'SeServiceLogonRight'." + +[sid] +emptyIdentity = "An entry in 'identity' is empty" +invalidSid = "'%{identity}' is not a valid SID string" +unknownAccount = "The account '%{identity}' could not be found. Specify 'DOMAIN\\Name', a well-known account name, or a SID string." +lookupNameFailed = "Failed to look up the account '%{identity}': %{error}" +lookupSidFailed = "Failed to look up the name for a SID: %{error}" +sidToStringFailed = "Failed to convert a SID to a string: %{error}" + +[lsa] +openPolicyFailed = "Failed to open the local security policy: %{error}" +stringTooLong = "The value '%{value}' is too long for the Local Security Authority" +enumerateFailed = "Failed to enumerate the accounts holding '%{right}': %{error}" +addFailed = "Failed to grant '%{right}': %{error}" +removeFailed = "Failed to revoke '%{right}': %{error}" + +[userRights] +identityRequired = "'identity' is required for the set and test operations" +whatIfGrant = "Would grant '%{right}' to '%{identity}'" +whatIfRevoke = "Would revoke '%{right}' from '%{identity}'" diff --git a/resources/windows_security_policy/src/lsa.rs b/resources/windows_security_policy/src/lsa.rs new file mode 100644 index 000000000..b12478a78 --- /dev/null +++ b/resources/windows_security_policy/src/lsa.rs @@ -0,0 +1,234 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Local Security Authority policy access: enumerate, grant and revoke account rights. +//! +//! Every buffer handed out by an `Lsa*` API is owned by the LSA and must be released with +//! `LsaFreeMemory`; the RAII guards below do that on drop so early returns cannot leak. + +use crate::sid::Sid; +use crate::types::SecurityPolicyError; +use rust_i18n::t; +use std::ffi::c_void; +use std::ptr; +use windows::Win32::Foundation::{ + NTSTATUS, STATUS_ACCESS_DENIED, STATUS_NO_MORE_ENTRIES, STATUS_NO_SUCH_PRIVILEGE, STATUS_OBJECT_NAME_NOT_FOUND, + STATUS_SUCCESS, WIN32_ERROR, +}; +use windows::Win32::Security::Authentication::Identity::{ + LSA_ENUMERATION_INFORMATION, LSA_HANDLE, LSA_OBJECT_ATTRIBUTES, LSA_UNICODE_STRING, LsaAddAccountRights, LsaClose, + LsaEnumerateAccountsWithUserRight, LsaFreeMemory, LsaNtStatusToWinError, LsaOpenPolicy, LsaRemoveAccountRights, + POLICY_CREATE_ACCOUNT, POLICY_LOOKUP_NAMES, POLICY_VIEW_LOCAL_INFORMATION, +}; +use windows::core::PWSTR; + +/// Access needed to enumerate account rights and look up names. +pub const POLICY_READ: u32 = POLICY_VIEW_LOCAL_INFORMATION.cast_unsigned() | POLICY_LOOKUP_NAMES.cast_unsigned(); +/// Access needed to grant and revoke account rights (adding a right may create an LSA account object). +pub const POLICY_WRITE: u32 = POLICY_READ | POLICY_CREATE_ACCOUNT.cast_unsigned(); + +/// The Win32 error message for an `NTSTATUS`. +fn nt_error(status: NTSTATUS) -> String { + // SAFETY: pure conversion with no preconditions. + let win32 = unsafe { LsaNtStatusToWinError(status) }; + windows::core::Error::from_hresult(WIN32_ERROR(win32).to_hresult()).to_string() +} + +/// Map an `NTSTATUS` to a resource error. Access denied means the process is not elevated. +fn check(status: NTSTATUS, context: impl FnOnce(String) -> String) -> Result<(), SecurityPolicyError> { + if status == STATUS_SUCCESS { + return Ok(()); + } + if status == STATUS_ACCESS_DENIED { + return Err(SecurityPolicyError::ElevationRequired); + } + Err(SecurityPolicyError::Policy(context(nt_error(status)))) +} + +/// An `LSA_UNICODE_STRING` together with the UTF-16 buffer it points into. +pub struct LsaString { + _buffer: Vec, + raw: LSA_UNICODE_STRING, +} + +impl LsaString { + /// Build an LSA string. `Length` counts bytes without the terminator, `MaximumLength` with it. + /// + /// # Errors + /// + /// Returns `InvalidInput` when the value does not fit the 16-bit length fields. + pub fn new(value: &str) -> Result { + let mut buffer: Vec = value.encode_utf16().chain(std::iter::once(0)).collect(); + let too_long = || SecurityPolicyError::InvalidInput(t!("lsa.stringTooLong", value = value).to_string()); + let length = u16::try_from((buffer.len() - 1) * 2).map_err(|_| too_long())?; + let maximum_length = u16::try_from(buffer.len() * 2).map_err(|_| too_long())?; + let raw = LSA_UNICODE_STRING { + Length: length, + MaximumLength: maximum_length, + Buffer: PWSTR(buffer.as_mut_ptr()), + }; + // Moving `buffer` moves the Vec header only; the heap pointer `raw.Buffer` holds stays valid. + Ok(Self { _buffer: buffer, raw }) + } + + pub fn as_raw(&self) -> &LSA_UNICODE_STRING { + &self.raw + } +} + +/// An array allocated by an `Lsa*` API and released with `LsaFreeMemory`. +struct LsaBuffer { + ptr: *mut T, + count: usize, +} + +impl LsaBuffer { + fn as_slice(&self) -> &[T] { + if self.ptr.is_null() || self.count == 0 { + &[] + } else { + // SAFETY: the LSA allocated `count` contiguous items at `ptr`; they stay valid until + // `LsaFreeMemory` runs in `drop`. + unsafe { std::slice::from_raw_parts(self.ptr, self.count) } + } + } +} + +impl Drop for LsaBuffer { + fn drop(&mut self) { + if !self.ptr.is_null() { + // SAFETY: `ptr` came from an `Lsa*` API and is released exactly once. + unsafe { + let _ = LsaFreeMemory(Some(self.ptr.cast())); + } + } + } +} + +/// An open handle to the local security policy, closed on drop. +pub struct LsaPolicy(LSA_HANDLE); + +impl LsaPolicy { + /// Open the local policy with the given access mask (`POLICY_READ` or `POLICY_WRITE`). + /// + /// # Errors + /// + /// Returns `ElevationRequired` when access is denied, or a policy error. + pub fn open(access: u32) -> Result { + let attributes = LSA_OBJECT_ATTRIBUTES::default(); + let mut handle = LSA_HANDLE::default(); + // SAFETY: an all-zero attributes block is what the API requires; `None` selects the local + // system; `handle` is a valid out pointer. + let status = unsafe { LsaOpenPolicy(None, &attributes, access, &mut handle) }; + check(status, |error| t!("lsa.openPolicyFailed", error = error).to_string())?; + Ok(Self(handle)) + } + + /// The SIDs of every account that holds `right`. An unheld right yields an empty list. + /// + /// # Errors + /// + /// Returns `InvalidInput` for a right the LSA does not know, `ElevationRequired` when the + /// process is not elevated, or a policy error. + pub fn holders(&self, right: &str) -> Result, SecurityPolicyError> { + let name = LsaString::new(right)?; + let mut buffer: *mut c_void = ptr::null_mut(); + let mut count: u32 = 0; + // SAFETY: `name` outlives the call; the out pointers are valid. + let status = unsafe { + LsaEnumerateAccountsWithUserRight(self.0, Some(ptr::from_ref(name.as_raw())), &mut buffer, &mut count) + }; + if status == STATUS_NO_MORE_ENTRIES { + return Ok(Vec::new()); + } + if status == STATUS_NO_SUCH_PRIVILEGE { + return Err(SecurityPolicyError::InvalidInput(t!("rights.unknownRight", right = right).to_string())); + } + check(status, |error| t!("lsa.enumerateFailed", right = right, error = error).to_string())?; + let entries = LsaBuffer:: { + ptr: buffer.cast(), + count: count as usize, + }; + // Copy every SID out before `entries` drops. + // SAFETY: each entry holds a valid SID that lives as long as `entries`. + let sids = entries.as_slice().iter().map(|entry| unsafe { Sid::from_psid(entry.Sid) }).collect(); + Ok(sids) + } + + /// Grant `right` to the account with `sid`. + /// + /// # Errors + /// + /// Returns `ElevationRequired` or a policy error. + pub fn add_right(&self, sid: &Sid, right: &str) -> Result<(), SecurityPolicyError> { + let name = LsaString::new(right)?; + // SAFETY: `sid` and `name` are valid for the duration of the call. + let status = unsafe { LsaAddAccountRights(self.0, sid.as_psid(), std::slice::from_ref(name.as_raw())) }; + check(status, |error| t!("lsa.addFailed", right = right, error = error).to_string()) + } + + /// Revoke `right` from the account with `sid`. Revoking from an account that holds no rights + /// at all is treated as already done. + /// + /// # Errors + /// + /// Returns `ElevationRequired` or a policy error. + pub fn remove_right(&self, sid: &Sid, right: &str) -> Result<(), SecurityPolicyError> { + let name = LsaString::new(right)?; + // `allrights` is always false: `true` would strip every right from the account. + // SAFETY: `sid` and `name` are valid for the duration of the call. + let status = + unsafe { LsaRemoveAccountRights(self.0, sid.as_psid(), false, Some(std::slice::from_ref(name.as_raw()))) }; + if status == STATUS_OBJECT_NAME_NOT_FOUND { + return Ok(()); + } + check(status, |error| t!("lsa.removeFailed", right = right, error = error).to_string()) + } +} + +impl Drop for LsaPolicy { + fn drop(&mut self) { + // SAFETY: the handle was returned by `LsaOpenPolicy` and is closed exactly once. + unsafe { + let _ = LsaClose(self.0); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::sid::parse_string_sid; + use dsc_lib_security_context::{SecurityContext, get_security_context}; + + #[test] + fn lsa_strings_count_bytes() { + let value = LsaString::new("SeServiceLogonRight").unwrap(); + assert_eq!(value.as_raw().Length, 38); + assert_eq!(value.as_raw().MaximumLength, 40); + // SAFETY: the buffer is null-terminated and owned by `value`. + let text = unsafe { value.as_raw().Buffer.to_string() }.unwrap(); + assert_eq!(text, "SeServiceLogonRight"); + let empty = LsaString::new("").unwrap(); + assert_eq!(empty.as_raw().Length, 0); + assert_eq!(empty.as_raw().MaximumLength, 2); + } + + #[test] + fn read_policy_opens_in_any_context() { + LsaPolicy::open(POLICY_READ).unwrap(); + } + + #[test] + fn enumeration_follows_the_security_context() { + let policy = LsaPolicy::open(POLICY_READ).unwrap(); + let holders = policy.holders("SeChangeNotifyPrivilege"); + if get_security_context() == SecurityContext::Admin { + let holders = holders.unwrap(); + assert!(holders.contains(&parse_string_sid("S-1-1-0").unwrap()), "Everyone should hold SeChangeNotifyPrivilege"); + assert!(matches!(policy.holders("SeNotARight"), Err(SecurityPolicyError::InvalidInput(_)))); + } else { + assert!(matches!(holders, Err(SecurityPolicyError::ElevationRequired))); + } + } +} diff --git a/resources/windows_security_policy/src/main.rs b/resources/windows_security_policy/src/main.rs new file mode 100644 index 000000000..d197155b3 --- /dev/null +++ b/resources/windows_security_policy/src/main.rs @@ -0,0 +1,217 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +#[cfg_attr(not(windows), allow(dead_code))] +mod rights; +#[cfg_attr(not(windows), allow(dead_code))] +mod types; + +#[cfg(windows)] +mod lsa; +#[cfg(windows)] +mod sid; +#[cfg(windows)] +mod user_rights; + +use rust_i18n::t; +use std::process::ExitCode; + +rust_i18n::i18n!("locales", fallback = "en-us"); + +const EXIT_SUCCESS: u8 = 0; +const EXIT_INVALID_ARGS: u8 = 1; +const EXIT_INVALID_INPUT: u8 = 2; +const EXIT_SECURITY_POLICY_ERROR: u8 = 3; +const EXIT_ELEVATION_REQUIRED: u8 = 4; + +/// Write a JSON error object to stderr: `{"error":""}` +fn write_error(message: &str) { + eprintln!("{}", serde_json::json!({"error": message})); +} + +#[cfg(not(windows))] +fn main() -> ExitCode { + write_error(&t!("main.windowsOnly")); + ExitCode::from(EXIT_SECURITY_POLICY_ERROR) +} + +#[cfg(windows)] +fn main() -> ExitCode { + use types::UserRightsAssignment; + + let args: Vec = std::env::args().collect(); + + if args.len() < 2 { + write_error(&t!("main.missingOperation")); + return ExitCode::from(EXIT_INVALID_ARGS); + } + + let operation = args[1].as_str(); + if !matches!(operation, "get" | "set" | "test" | "export") { + write_error(&t!("main.unknownOperation", operation = operation)); + return ExitCode::from(EXIT_INVALID_ARGS); + } + + let Some(target) = args.get(2) else { + write_error(&t!("main.missingTarget")); + return ExitCode::from(EXIT_INVALID_ARGS); + }; + + let input_json = match parse_input_arg(&args) { + Ok(input) => input, + Err(code) => return code, + }; + let what_if = parse_what_if_flag(&args); + + match (operation, target.as_str()) { + ("get", "user-right") => with_input(input_json, |input: UserRightsAssignment| user_rights::get(&input)), + ("set", "user-right") => with_input(input_json, |input: UserRightsAssignment| user_rights::set(&input, what_if)), + ("test", "user-right") => with_input(input_json, |input: UserRightsAssignment| user_rights::test(&input)), + ("export", "user-right") => finish_many(user_rights::export()), + (_, target) => { + write_error(&t!("main.unknownTarget", target = target)); + ExitCode::from(EXIT_INVALID_ARGS) + } + } +} + +/// Deserialize the required JSON input, or return the exit code to use. +#[cfg(windows)] +fn require_input(input_json: Option) -> Result { + let Some(json) = input_json else { + write_error(&t!("main.missingInput")); + return Err(ExitCode::from(EXIT_INVALID_ARGS)); + }; + serde_json::from_str(&json).map_err(|error| { + write_error(&t!("main.invalidJson", error = error.to_string())); + ExitCode::from(EXIT_INVALID_INPUT) + }) +} + +/// Run an operation that needs input and print its single result. +#[cfg(windows)] +fn with_input(input_json: Option, operation: impl FnOnce(T) -> Result) -> ExitCode +where + T: serde::de::DeserializeOwned, + U: serde::Serialize, +{ + match require_input::(input_json) { + Ok(input) => finish(operation(input)), + Err(code) => code, + } +} + +/// Serialize a value to JSON and print it to stdout, or return the exit code to use. +#[cfg(windows)] +fn print_json(value: &impl serde::Serialize) -> Result<(), ExitCode> { + match serde_json::to_string(value) { + Ok(json) => { + println!("{json}"); + Ok(()) + } + Err(error) => { + write_error(&t!("main.serializeFailed", error = error.to_string())); + Err(ExitCode::from(EXIT_SECURITY_POLICY_ERROR)) + } + } +} + +/// Map a resource error to its documented exit code. +#[cfg(windows)] +fn exit_code_for(error: &types::SecurityPolicyError) -> ExitCode { + use types::SecurityPolicyError; + + ExitCode::from(match error { + SecurityPolicyError::ElevationRequired => EXIT_ELEVATION_REQUIRED, + SecurityPolicyError::InvalidInput(_) => EXIT_INVALID_INPUT, + SecurityPolicyError::Policy(_) => EXIT_SECURITY_POLICY_ERROR, + }) +} + +/// Print a single result as JSON, or write the error and return its exit code. +#[cfg(windows)] +fn finish(result: Result) -> ExitCode { + match result { + Ok(value) => match print_json(&value) { + Ok(()) => ExitCode::from(EXIT_SUCCESS), + Err(code) => code, + }, + Err(error) => { + write_error(&error.to_string()); + exit_code_for(&error) + } + } +} + +/// Print each exported instance on its own line, or write the error and return its exit code. +#[cfg(windows)] +fn finish_many(result: Result, types::SecurityPolicyError>) -> ExitCode { + match result { + Ok(values) => { + for value in &values { + if let Err(code) = print_json(value) { + return code; + } + } + ExitCode::from(EXIT_SUCCESS) + } + Err(error) => { + write_error(&error.to_string()); + exit_code_for(&error) + } + } +} + +/// Parse the `--input ` argument. Arguments 1 and 2 are the operation and the target. +#[cfg(windows)] +fn parse_input_arg(args: &[String]) -> Result, ExitCode> { + let mut index = 3; + while index < args.len() { + if args[index] == "--input" || args[index] == "-i" { + if index + 1 < args.len() { + return Ok(Some(args[index + 1].clone())); + } + write_error(&t!("main.missingInputValue")); + return Err(ExitCode::from(EXIT_INVALID_ARGS)); + } + index += 1; + } + Ok(None) +} + +/// Parse the `--what-if` / `-w` flag. Arguments 1 and 2 are the operation and the target. +#[cfg(windows)] +fn parse_what_if_flag(args: &[String]) -> bool { + args.iter().skip(3).any(|arg| arg == "--what-if" || arg == "-w") +} + +#[cfg(all(test, windows))] +mod tests { + use super::{parse_input_arg, parse_what_if_flag}; + + fn args(list: &[&str]) -> Vec { + list.iter().map(ToString::to_string).collect() + } + + #[test] + fn finds_long_and_short_input_arguments_after_the_target() { + assert_eq!( + parse_input_arg(&args(&["exe", "get", "user-right", "--input", "{}"])), + Ok(Some("{}".to_string())) + ); + assert_eq!( + parse_input_arg(&args(&["exe", "set", "user-right", "-w", "-i", "[]"])), + Ok(Some("[]".to_string())) + ); + assert_eq!(parse_input_arg(&args(&["exe", "export", "user-right"])), Ok(None)); + assert!(parse_input_arg(&args(&["exe", "get", "user-right", "--input"])).is_err()); + } + + #[test] + fn finds_what_if_flag_after_the_target() { + assert!(parse_what_if_flag(&args(&["exe", "set", "user-right", "--what-if", "--input", "{}"]))); + assert!(parse_what_if_flag(&args(&["exe", "set", "user-right", "--input", "{}", "-w"]))); + assert!(!parse_what_if_flag(&args(&["exe", "set", "user-right", "--input", "{}"]))); + assert!(!parse_what_if_flag(&args(&["exe", "-w", "user-right"]))); + } +} diff --git a/resources/windows_security_policy/src/rights.rs b/resources/windows_security_policy/src/rights.rs new file mode 100644 index 000000000..2b96dc248 --- /dev/null +++ b/resources/windows_security_policy/src/rights.rs @@ -0,0 +1,106 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Static table of the Windows user rights and privileges that can be assigned to accounts. + +/// A user right or privilege. +#[derive(Debug, PartialEq, Eq)] +pub struct UserRight { + /// The constant that the LSA, `secedit` and Group Policy use, e.g. `SeServiceLogonRight`. + pub constant: &'static str, + /// The name shown in the Local Security Policy console, e.g. "Log on as a service". + pub display_name: &'static str, +} + +const fn right(constant: &'static str, display_name: &'static str) -> UserRight { + UserRight { constant, display_name } +} + +/// Every user right, ordered by display name. This is also the order `export` reports them in. +pub const USER_RIGHTS: [UserRight; 45] = [ + right("SeTrustedCredManAccessPrivilege", "Access Credential Manager as a trusted caller"), + right("SeNetworkLogonRight", "Access this computer from the network"), + right("SeTcbPrivilege", "Act as part of the operating system"), + right("SeMachineAccountPrivilege", "Add workstations to domain"), + right("SeIncreaseQuotaPrivilege", "Adjust memory quotas for a process"), + right("SeInteractiveLogonRight", "Allow log on locally"), + right("SeRemoteInteractiveLogonRight", "Allow log on through Remote Desktop Services"), + right("SeBackupPrivilege", "Back up files and directories"), + right("SeChangeNotifyPrivilege", "Bypass traverse checking"), + right("SeSystemtimePrivilege", "Change the system time"), + right("SeTimeZonePrivilege", "Change the time zone"), + right("SeCreatePagefilePrivilege", "Create a pagefile"), + right("SeCreateTokenPrivilege", "Create a token object"), + right("SeCreateGlobalPrivilege", "Create global objects"), + right("SeCreatePermanentPrivilege", "Create permanent shared objects"), + right("SeCreateSymbolicLinkPrivilege", "Create symbolic links"), + right("SeDebugPrivilege", "Debug programs"), + right("SeDenyNetworkLogonRight", "Deny access to this computer from the network"), + right("SeDenyBatchLogonRight", "Deny log on as a batch job"), + right("SeDenyServiceLogonRight", "Deny log on as a service"), + right("SeDenyInteractiveLogonRight", "Deny log on locally"), + right("SeDenyRemoteInteractiveLogonRight", "Deny log on through Remote Desktop Services"), + right("SeEnableDelegationPrivilege", "Enable computer and user accounts to be trusted for delegation"), + right("SeRemoteShutdownPrivilege", "Force shutdown from a remote system"), + right("SeAuditPrivilege", "Generate security audits"), + right("SeImpersonatePrivilege", "Impersonate a client after authentication"), + right("SeIncreaseWorkingSetPrivilege", "Increase a process working set"), + right("SeIncreaseBasePriorityPrivilege", "Increase scheduling priority"), + right("SeLoadDriverPrivilege", "Load and unload device drivers"), + right("SeLockMemoryPrivilege", "Lock pages in memory"), + right("SeBatchLogonRight", "Log on as a batch job"), + right("SeServiceLogonRight", "Log on as a service"), + right("SeSecurityPrivilege", "Manage auditing and security log"), + right("SeRelabelPrivilege", "Modify an object label"), + right("SeSystemEnvironmentPrivilege", "Modify firmware environment values"), + right("SeDelegateSessionUserImpersonatePrivilege", "Obtain an impersonation token for another user in the same session"), + right("SeManageVolumePrivilege", "Perform volume maintenance tasks"), + right("SeProfileSingleProcessPrivilege", "Profile single process"), + right("SeSystemProfilePrivilege", "Profile system performance"), + right("SeUndockPrivilege", "Remove computer from docking station"), + right("SeAssignPrimaryTokenPrivilege", "Replace a process level token"), + right("SeRestorePrivilege", "Restore files and directories"), + right("SeShutdownPrivilege", "Shut down the system"), + right("SeSyncAgentPrivilege", "Synchronize directory service data"), + right("SeTakeOwnershipPrivilege", "Take ownership of files or other objects"), +]; + +/// Find a user right by its constant, ignoring case and surrounding whitespace. +#[must_use] +pub fn find(input: &str) -> Option<&'static UserRight> { + let wanted = input.trim(); + USER_RIGHTS.iter().find(|entry| entry.constant.eq_ignore_ascii_case(wanted)) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::collections::HashSet; + + #[test] + fn table_has_forty_five_unique_rights() { + assert_eq!(USER_RIGHTS.len(), 45); + let constants: HashSet = USER_RIGHTS.iter().map(|r| r.constant.to_ascii_lowercase()).collect(); + assert_eq!(constants.len(), 45, "constants must be unique"); + let names: HashSet<&str> = USER_RIGHTS.iter().map(|r| r.display_name).collect(); + assert_eq!(names.len(), 45, "display names must be unique"); + for entry in &USER_RIGHTS { + assert!(entry.constant.starts_with("Se"), "{}", entry.constant); + assert!( + entry.constant.ends_with("Privilege") || entry.constant.ends_with("Right"), + "{}", + entry.constant + ); + assert!(!entry.display_name.is_empty(), "{}", entry.constant); + } + } + + #[test] + fn find_ignores_case_and_whitespace() { + assert_eq!(find("SeServiceLogonRight").map(|r| r.display_name), Some("Log on as a service")); + assert_eq!(find(" seservicelogonright ").map(|r| r.constant), Some("SeServiceLogonRight")); + assert!(find("SeBogusPrivilege").is_none()); + assert!(find("Log on as a service").is_none()); + assert!(find("").is_none()); + } +} diff --git a/resources/windows_security_policy/src/sid.rs b/resources/windows_security_policy/src/sid.rs new file mode 100644 index 000000000..25cad53fb --- /dev/null +++ b/resources/windows_security_policy/src/sid.rs @@ -0,0 +1,338 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Security identifiers: owned copies, string conversion and account name resolution. +//! +//! Everything here uses `advapi32` lookups that work without elevation. + +use crate::types::SecurityPolicyError; +use rust_i18n::t; +use std::collections::HashSet; +use std::ffi::c_void; +use windows::Win32::Foundation::{ERROR_INSUFFICIENT_BUFFER, ERROR_NONE_MAPPED, HLOCAL, LocalFree, WIN32_ERROR}; +use windows::Win32::Security::Authorization::{ConvertSidToStringSidW, ConvertStringSidToSidW}; +use windows::Win32::Security::{GetLengthSid, LookupAccountNameW, LookupAccountSidW, PSID, SID_NAME_USE}; +use windows::core::{PCWSTR, PWSTR}; + +/// An owned copy of a security identifier. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct Sid(Vec); + +impl Sid { + /// Copy the SID that `psid` points to. + /// + /// # Safety + /// + /// `psid` must point to a valid SID returned by a Win32 API. + pub(crate) unsafe fn from_psid(psid: PSID) -> Self { + // SAFETY: the caller guarantees a valid SID; `GetLengthSid` reads its header to size the copy. + let length = unsafe { GetLengthSid(psid) } as usize; + // SAFETY: a valid SID occupies exactly `length` readable bytes. + let bytes = unsafe { std::slice::from_raw_parts(psid.0.cast::(), length) }; + Self(bytes.to_vec()) + } + + /// A `PSID` view of this SID. The APIs this crate calls never write through it. + pub(crate) fn as_psid(&self) -> PSID { + PSID(self.0.as_ptr().cast_mut().cast()) + } + + #[cfg(test)] + pub(crate) fn from_bytes(bytes: Vec) -> Self { + Self(bytes) + } +} + +/// An account resolved from user input or from the LSA. +#[derive(Debug, Clone)] +pub struct ResolvedIdentity { + pub sid: Sid, + /// Canonical `DOMAIN\Name`, bare `Name` when the domain is empty, or the SID string when + /// the SID does not resolve to an account. + pub name: String, +} + +/// Memory allocated with `LocalAlloc` by the `Convert*Sid*` APIs, released on drop. +struct LocalBuffer(*mut c_void); + +impl Drop for LocalBuffer { + fn drop(&mut self) { + if !self.0.is_null() { + // SAFETY: the pointer came from an API documented to require `LocalFree` and is freed once. + unsafe { + let _ = LocalFree(Some(HLOCAL(self.0))); + } + } + } +} + +/// Encode a Rust string as a null-terminated UTF-16 buffer. +fn to_wide(text: &str) -> Vec { + text.encode_utf16().chain(std::iter::once(0)).collect() +} + +fn is_win32(error: &windows::core::Error, code: WIN32_ERROR) -> bool { + error.code() == code.to_hresult() +} + +/// Parse an `S-1-...` string into a SID. +/// +/// # Errors +/// +/// Returns the Win32 error when the string is not a valid SID. +pub fn parse_string_sid(text: &str) -> Result { + let wide = to_wide(text); + let mut psid = PSID::default(); + // SAFETY: `wide` is null-terminated and outlives the call; `psid` receives a `LocalAlloc`'d SID. + unsafe { ConvertStringSidToSidW(PCWSTR(wide.as_ptr()), &mut psid) }?; + let _buffer = LocalBuffer(psid.0); + // SAFETY: the API returned a valid SID that stays alive until `_buffer` drops. + Ok(unsafe { Sid::from_psid(psid) }) +} + +/// Format a SID as `S-1-...`. +/// +/// # Errors +/// +/// Returns the Win32 error when the conversion fails. +pub fn sid_to_string(sid: &Sid) -> Result { + let mut text = PWSTR::null(); + // SAFETY: `sid` is a valid SID and `text` receives a `LocalAlloc`'d null-terminated string. + unsafe { ConvertSidToStringSidW(sid.as_psid(), &mut text) }?; + let _buffer = LocalBuffer(text.as_ptr().cast()); + // SAFETY: the string is null-terminated and stays alive until `_buffer` drops. + Ok(unsafe { String::from_utf16_lossy(text.as_wide()) }) +} + +/// Resolve an account name against the local machine (and its domain, when joined). +/// +/// # Errors +/// +/// Returns `ERROR_NONE_MAPPED` when the account does not exist, or another Win32 error. +pub fn lookup_account_name(name: &str) -> Result { + let wide = to_wide(name); + let mut sid_length: u32 = 0; + let mut domain_length: u32 = 0; + let mut sid_type = SID_NAME_USE::default(); + // The first call sizes the buffers and is expected to fail with ERROR_INSUFFICIENT_BUFFER. + // SAFETY: `wide` is null-terminated; the out pointers are valid; null buffers are allowed. + let sizing = unsafe { + LookupAccountNameW(PCWSTR::null(), PCWSTR(wide.as_ptr()), None, &mut sid_length, None, &mut domain_length, &mut sid_type) + }; + if let Err(error) = sizing + && !is_win32(&error, ERROR_INSUFFICIENT_BUFFER) + { + return Err(error); + } + let mut sid = vec![0u8; sid_length as usize]; + let mut domain = vec![0u16; domain_length as usize]; + // SAFETY: the buffers have the sizes the first call asked for. + unsafe { + LookupAccountNameW( + PCWSTR::null(), + PCWSTR(wide.as_ptr()), + Some(PSID(sid.as_mut_ptr().cast())), + &mut sid_length, + Some(PWSTR(domain.as_mut_ptr())), + &mut domain_length, + &mut sid_type, + ) + }?; + sid.truncate(sid_length as usize); + Ok(Sid(sid)) +} + +/// Resolve a SID to `(domain, name)`. +/// +/// # Errors +/// +/// Returns `ERROR_NONE_MAPPED` when no account has the SID, or another Win32 error. +pub fn lookup_account_sid(sid: &Sid) -> Result<(String, String), windows::core::Error> { + let mut name_length: u32 = 0; + let mut domain_length: u32 = 0; + let mut sid_type = SID_NAME_USE::default(); + // SAFETY: `sid` is valid; the out pointers are valid; null buffers are allowed on the sizing call. + let sizing = unsafe { LookupAccountSidW(PCWSTR::null(), sid.as_psid(), None, &mut name_length, None, &mut domain_length, &mut sid_type) }; + if let Err(error) = sizing + && !is_win32(&error, ERROR_INSUFFICIENT_BUFFER) + { + return Err(error); + } + let mut name = vec![0u16; name_length as usize]; + let mut domain = vec![0u16; domain_length as usize]; + // SAFETY: the buffers have the sizes the first call asked for. + unsafe { + LookupAccountSidW( + PCWSTR::null(), + sid.as_psid(), + Some(PWSTR(name.as_mut_ptr())), + &mut name_length, + Some(PWSTR(domain.as_mut_ptr())), + &mut domain_length, + &mut sid_type, + ) + }?; + // After the second call the lengths exclude the terminator. + Ok(( + String::from_utf16_lossy(&domain[..domain_length as usize]), + String::from_utf16_lossy(&name[..name_length as usize]), + )) +} + +/// The canonical display form of a SID: `DOMAIN\Name`, bare `Name` when the domain is empty +/// (e.g. `Everyone`), or the SID string when it does not resolve. +/// +/// # Errors +/// +/// Returns an error when the lookup fails for a reason other than an unknown SID. +pub fn display_name(sid: &Sid) -> Result { + match lookup_account_sid(sid) { + Ok((domain, name)) if domain.is_empty() => Ok(name), + Ok((domain, name)) => Ok(format!("{domain}\\{name}")), + Err(error) if is_win32(&error, ERROR_NONE_MAPPED) => sid_to_string(sid).map_err(sid_to_string_error), + Err(error) => Err(SecurityPolicyError::Policy(t!("sid.lookupSidFailed", error = error.to_string()).to_string())), + } +} + +fn sid_to_string_error(error: windows::core::Error) -> SecurityPolicyError { + SecurityPolicyError::Policy(t!("sid.sidToStringFailed", error = error.to_string()).to_string()) +} + +/// Describe a SID obtained from the LSA. +/// +/// # Errors +/// +/// Returns an error when the SID cannot be converted to a string or looked up. +pub fn describe(sid: Sid) -> Result { + let name = display_name(&sid)?; + Ok(ResolvedIdentity { sid, name }) +} + +/// Whether `text` has the shape of a SID string (`S-1-...`). +fn looks_like_sid(text: &str) -> bool { + let bytes = text.as_bytes(); + bytes.len() >= 5 && bytes[0].eq_ignore_ascii_case(&b'S') && &bytes[1..4] == b"-1-" && bytes[4].is_ascii_digit() +} + +/// Resolve one `identity` entry: a name, a SID string, or the secedit `*S-1-...` form. +/// +/// # Errors +/// +/// Returns `InvalidInput` for an empty entry, a malformed SID string or an unknown account name. +pub fn resolve_identity(input: &str) -> Result { + let trimmed = input.trim(); + let text = trimmed.strip_prefix('*').unwrap_or(trimmed).trim(); + if text.is_empty() { + return Err(SecurityPolicyError::InvalidInput(t!("sid.emptyIdentity").to_string())); + } + let sid = if looks_like_sid(text) { + // A well-formed SID with no matching account is accepted: the LSA keeps rights for + // orphaned SIDs and `display_name` reports them as SID strings. + parse_string_sid(text).map_err(|_| SecurityPolicyError::InvalidInput(t!("sid.invalidSid", identity = input).to_string()))? + } else { + lookup_account_name(text).map_err(|error| { + if is_win32(&error, ERROR_NONE_MAPPED) { + SecurityPolicyError::InvalidInput(t!("sid.unknownAccount", identity = input).to_string()) + } else { + SecurityPolicyError::Policy(t!("sid.lookupNameFailed", identity = input, error = error.to_string()).to_string()) + } + })? + }; + describe(sid) +} + +/// Resolve every entry, dropping duplicates that resolve to the same SID (first occurrence wins). +/// +/// # Errors +/// +/// Returns the first resolution error. +pub fn resolve_identities(inputs: &[String]) -> Result, SecurityPolicyError> { + let mut seen = HashSet::new(); + let mut identities = Vec::with_capacity(inputs.len()); + for input in inputs { + let identity = resolve_identity(input)?; + if seen.insert(identity.sid.clone()) { + identities.push(identity); + } + } + Ok(identities) +} + +#[cfg(test)] +mod tests { + use super::*; + + const ADMINISTRATORS: &str = "S-1-5-32-544"; + + #[test] + fn sid_strings_round_trip() { + let sid = parse_string_sid(ADMINISTRATORS).unwrap(); + assert_eq!(sid.0.len(), 16, "S-1-5-32-544 is 8 header bytes plus two sub-authorities"); + assert_eq!(sid_to_string(&sid).unwrap(), ADMINISTRATORS); + assert!(parse_string_sid("S-1-X").is_err()); + assert!(parse_string_sid("Administrators").is_err()); + } + + #[test] + fn names_resolve_to_the_expected_sids() { + let administrators = parse_string_sid(ADMINISTRATORS).unwrap(); + assert_eq!(lookup_account_name("BUILTIN\\Administrators").unwrap(), administrators); + assert_eq!(lookup_account_name("Administrators").unwrap(), administrators); + assert_eq!(lookup_account_name("Everyone").unwrap(), parse_string_sid("S-1-1-0").unwrap()); + assert_eq!(lookup_account_name("NT AUTHORITY\\SYSTEM").unwrap(), parse_string_sid("S-1-5-18").unwrap()); + let missing = lookup_account_name("NoSuchAccount_zz_12345").unwrap_err(); + assert!(is_win32(&missing, ERROR_NONE_MAPPED)); + } + + #[test] + fn display_names_are_canonical() { + assert_eq!(display_name(&parse_string_sid(ADMINISTRATORS).unwrap()).unwrap(), "BUILTIN\\Administrators"); + assert_eq!(display_name(&parse_string_sid("S-1-1-0").unwrap()).unwrap(), "Everyone"); + assert_eq!(display_name(&parse_string_sid("S-1-5-18").unwrap()).unwrap(), "NT AUTHORITY\\SYSTEM"); + let orphan = "S-1-5-21-1-2-3-4567"; + assert_eq!(display_name(&parse_string_sid(orphan).unwrap()).unwrap(), orphan); + } + + #[test] + fn identities_resolve_from_every_accepted_form() { + for input in ["Administrators", "BUILTIN\\Administrators", "builtin\\administrators", ADMINISTRATORS, "*S-1-5-32-544", " s-1-5-32-544 "] { + let resolved = resolve_identity(input).unwrap_or_else(|e| panic!("{input}: {e}")); + assert_eq!(resolved.name, "BUILTIN\\Administrators", "{input}"); + assert_eq!(sid_to_string(&resolved.sid).unwrap(), ADMINISTRATORS, "{input}"); + } + let orphan = resolve_identity("S-1-5-21-1-2-3-4567").unwrap(); + assert_eq!(orphan.name, "S-1-5-21-1-2-3-4567"); + } + + #[test] + fn invalid_identities_are_invalid_input() { + for input in ["", " ", "*", "S-1-X", "NoSuchAccount_zz_12345"] { + assert!( + matches!(resolve_identity(input), Err(SecurityPolicyError::InvalidInput(_))), + "{input:?} should be invalid input" + ); + } + } + + #[test] + fn duplicate_identities_collapse_by_sid() { + let inputs: Vec = ["Administrators", "BUILTIN\\Administrators", ADMINISTRATORS, "Everyone"] + .iter() + .map(ToString::to_string) + .collect(); + let resolved = resolve_identities(&inputs).unwrap(); + assert_eq!(resolved.len(), 2); + assert_eq!(resolved[0].name, "BUILTIN\\Administrators"); + assert_eq!(resolved[1].name, "Everyone"); + } + + #[test] + fn sid_shape_detection() { + assert!(looks_like_sid("S-1-5-32-544")); + assert!(looks_like_sid("s-1-1-0")); + assert!(!looks_like_sid("S-1-")); + assert!(!looks_like_sid("S-2-5-32-544")); + assert!(!looks_like_sid("Administrators")); + assert!(!looks_like_sid("S-1-X")); + } +} diff --git a/resources/windows_security_policy/src/types.rs b/resources/windows_security_policy/src/types.rs new file mode 100644 index 000000000..b42f6c0b6 --- /dev/null +++ b/resources/windows_security_policy/src/types.rs @@ -0,0 +1,150 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use rust_i18n::t; +use serde::{Deserialize, Serialize}; + +/// How the `identity` list is applied by `set` and judged by `test`. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub enum SetAction { + /// The listed accounts become the only holders of the right. + #[default] + Replace, + /// The listed accounts are granted the right; other holders are kept. + Add, + /// The listed accounts lose the right; other holders are kept. + Remove, +} + +/// Desired or actual state of one user right (`Microsoft.Windows/UserRightsAssignment`). +#[derive(Debug, Default, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct UserRightsAssignment { + /// The user right constant, e.g. `SeServiceLogonRight`. Matched case-insensitively and echoed as given. + pub right: String, + + /// The accounts that hold the right. Required for `set` and `test`; an empty list means "No One". + #[serde(skip_serializing_if = "Option::is_none")] + pub identity: Option>, + + /// Write-only: how `identity` is applied. Never serialized. + #[serde(default, skip_serializing)] + pub set_action: SetAction, + + /// Read-only: whether the current holders satisfy `identity` and `set_action`. Set by `test`. + #[serde(rename = "_inDesiredState", skip_serializing_if = "Option::is_none")] + pub in_desired_state: Option, + + /// Metadata returned by what-if operations describing the changes that would be applied. + #[serde(rename = "_metadata", skip_serializing_if = "Option::is_none")] + pub metadata: Option, +} + +/// Metadata returned by what-if operations. +#[derive(Debug, Default, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct Metadata { + /// Human-readable descriptions of the changes that would be applied. + #[serde(skip_serializing_if = "Option::is_none")] + pub what_if: Option>, +} + +impl Metadata { + /// Wrap what-if messages, returning `None` when there is nothing to report. + #[must_use] + pub fn from_messages(messages: Vec) -> Option { + if messages.is_empty() { + None + } else { + Some(Self { what_if: Some(messages) }) + } + } +} + +/// Errors from the security policy resource. +#[derive(Debug)] +pub enum SecurityPolicyError { + /// The operation needs an elevated process. + ElevationRequired, + /// The input is well-formed JSON but not usable (unknown right, unknown account, missing property). + InvalidInput(String), + /// A Win32 or LSA operation failed. + Policy(String), +} + +impl std::fmt::Display for SecurityPolicyError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::ElevationRequired => f.write_str(&t!("main.elevationRequired")), + Self::InvalidInput(message) | Self::Policy(message) => f.write_str(message), + } + } +} + +impl std::error::Error for SecurityPolicyError {} + +impl From for SecurityPolicyError { + fn from(message: String) -> Self { + Self::Policy(message) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn set_action_defaults_to_replace_and_uses_camel_case() { + assert_eq!(SetAction::default(), SetAction::Replace); + assert_eq!(serde_json::to_string(&SetAction::Replace).unwrap(), "\"replace\""); + assert_eq!(serde_json::from_str::("\"add\"").unwrap(), SetAction::Add); + assert_eq!(serde_json::from_str::("\"remove\"").unwrap(), SetAction::Remove); + assert!(serde_json::from_str::("\"clobber\"").is_err()); + } + + #[test] + fn input_requires_right_and_rejects_unknown_fields() { + assert!(serde_json::from_str::(r#"{"identity":[]}"#).is_err()); + assert!(serde_json::from_str::(r#"{"right":"SeDebugPrivilege","bogus":1}"#).is_err()); + let input: UserRightsAssignment = serde_json::from_str(r#"{"right":"SeDebugPrivilege"}"#).unwrap(); + assert_eq!(input.right, "SeDebugPrivilege"); + assert!(input.identity.is_none()); + assert_eq!(input.set_action, SetAction::Replace); + } + + #[test] + fn set_action_is_read_but_never_written() { + let input: UserRightsAssignment = + serde_json::from_str(r#"{"right":"SeDebugPrivilege","identity":["Administrators"],"setAction":"add"}"#).unwrap(); + assert_eq!(input.set_action, SetAction::Add); + let json = serde_json::to_string(&input).unwrap(); + assert_eq!(json, r#"{"right":"SeDebugPrivilege","identity":["Administrators"]}"#); + } + + #[test] + fn empty_identity_serializes_as_an_empty_array() { + let state = UserRightsAssignment { + right: "SeTcbPrivilege".to_string(), + identity: Some(Vec::new()), + ..UserRightsAssignment::default() + }; + assert_eq!(serde_json::to_string(&state).unwrap(), r#"{"right":"SeTcbPrivilege","identity":[]}"#); + } + + #[test] + fn optional_outputs_only_serialize_when_present() { + let mut state = UserRightsAssignment { + right: "SeTcbPrivilege".to_string(), + identity: Some(vec!["BUILTIN\\Administrators".to_string()]), + ..UserRightsAssignment::default() + }; + assert!(!serde_json::to_string(&state).unwrap().contains("_inDesiredState")); + state.in_desired_state = Some(true); + state.metadata = Metadata::from_messages(vec!["Would grant".to_string()]); + let json = serde_json::to_string(&state).unwrap(); + assert!(json.contains(r#""_inDesiredState":true"#)); + assert!(json.contains(r#""_metadata":{"whatIf":["Would grant"]}"#)); + assert!(Metadata::from_messages(Vec::new()).is_none()); + } +} diff --git a/resources/windows_security_policy/src/user_rights.rs b/resources/windows_security_policy/src/user_rights.rs new file mode 100644 index 000000000..b231904b7 --- /dev/null +++ b/resources/windows_security_policy/src/user_rights.rs @@ -0,0 +1,295 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Get, test, set and export of user rights assignments. + +use crate::lsa::{LsaPolicy, POLICY_READ, POLICY_WRITE}; +use crate::rights::{self, USER_RIGHTS, UserRight}; +use crate::sid::{self, ResolvedIdentity}; +use crate::types::{Metadata, SecurityPolicyError, SetAction, UserRightsAssignment}; +use dsc_lib_security_context::{SecurityContext, get_security_context}; +use rust_i18n::t; + +/// Fail unless the process is elevated. `dsc` enforces this through the manifest; this check +/// covers direct invocation of the binary. +pub(crate) fn require_elevated() -> Result<(), SecurityPolicyError> { + if get_security_context() == SecurityContext::Admin { + Ok(()) + } else { + Err(SecurityPolicyError::ElevationRequired) + } +} + +fn find_right(input: &str) -> Result<&'static UserRight, SecurityPolicyError> { + rights::find(input).ok_or_else(|| SecurityPolicyError::InvalidInput(t!("rights.unknownRight", right = input).to_string())) +} + +/// The accounts holding `right`, sorted by name for stable output. +fn current_identities(policy: &LsaPolicy, right: &UserRight) -> Result, SecurityPolicyError> { + let mut identities = policy.holders(right.constant)?.into_iter().map(sid::describe).collect::, _>>()?; + identities.sort_by_cached_key(|identity| identity.name.to_lowercase()); + Ok(identities) +} + +fn names(identities: &[ResolvedIdentity]) -> Vec { + identities.iter().map(|identity| identity.name.clone()).collect() +} + +fn state(right: &str, current: &[ResolvedIdentity]) -> UserRightsAssignment { + UserRightsAssignment { + right: right.to_string(), + identity: Some(names(current)), + ..UserRightsAssignment::default() + } +} + +fn require_identity(input: &UserRightsAssignment) -> Result<&[String], SecurityPolicyError> { + input + .identity + .as_deref() + .ok_or_else(|| SecurityPolicyError::InvalidInput(t!("userRights.identityRequired").to_string())) +} + +fn contains(identities: &[ResolvedIdentity], wanted: &ResolvedIdentity) -> bool { + identities.iter().any(|identity| identity.sid == wanted.sid) +} + +/// The accounts to grant and to revoke so that `current` satisfies `desired` under `action`. +/// Both lists empty means the right is in the desired state. +pub(crate) fn diff<'a>( + current: &'a [ResolvedIdentity], + desired: &'a [ResolvedIdentity], + action: SetAction, +) -> (Vec<&'a ResolvedIdentity>, Vec<&'a ResolvedIdentity>) { + let missing = || desired.iter().filter(|identity| !contains(current, identity)).collect::>(); + match action { + SetAction::Replace => (missing(), current.iter().filter(|identity| !contains(desired, identity)).collect()), + SetAction::Add => (missing(), Vec::new()), + SetAction::Remove => (Vec::new(), desired.iter().filter(|identity| contains(current, identity)).collect()), + } +} + +/// The holder names after applying `grant` and `revoke` to `current`, for what-if output. +pub(crate) fn project(current: &[ResolvedIdentity], grant: &[&ResolvedIdentity], revoke: &[&ResolvedIdentity]) -> Vec { + let mut names: Vec = current + .iter() + .filter(|identity| !revoke.iter().any(|revoked| revoked.sid == identity.sid)) + .map(|identity| identity.name.clone()) + .collect(); + names.extend(grant.iter().map(|identity| identity.name.clone())); + names.sort_by_cached_key(|name| name.to_lowercase()); + names +} + +/// Get the accounts that hold a right. +/// +/// # Errors +/// +/// Returns an error when the process is not elevated, the right is unknown, or an LSA call fails. +pub fn get(input: &UserRightsAssignment) -> Result { + require_elevated()?; + let right = find_right(&input.right)?; + let policy = LsaPolicy::open(POLICY_READ)?; + Ok(state(&input.right, ¤t_identities(&policy, right)?)) +} + +/// Test whether the holders of a right satisfy `identity` under `setAction`, comparing SIDs so +/// that any accepted spelling of an account matches. +/// +/// # Errors +/// +/// Returns an error when the process is not elevated, the right or an identity is unknown, +/// `identity` is missing, or an LSA call fails. +pub fn test(input: &UserRightsAssignment) -> Result { + require_elevated()?; + let right = find_right(&input.right)?; + let desired = sid::resolve_identities(require_identity(input)?)?; + let policy = LsaPolicy::open(POLICY_READ)?; + let current = current_identities(&policy, right)?; + let (grant, revoke) = diff(¤t, &desired, input.set_action); + let mut result = state(&input.right, ¤t); + result.in_desired_state = Some(grant.is_empty() && revoke.is_empty()); + Ok(result) +} + +/// Apply `identity` to a right under `setAction`. In what-if mode nothing is changed and the +/// projected state carries `_metadata.whatIf`. +/// +/// # Errors +/// +/// Returns an error when the process is not elevated, the right or an identity is unknown, +/// `identity` is missing, or an LSA call fails. +pub fn set(input: &UserRightsAssignment, what_if: bool) -> Result { + require_elevated()?; + let right = find_right(&input.right)?; + // Resolve first so that bad input fails even in what-if mode. + let desired = sid::resolve_identities(require_identity(input)?)?; + let policy = LsaPolicy::open(if what_if { POLICY_READ } else { POLICY_WRITE })?; + + let current = match current_identities(&policy, right) { + Ok(current) => current, + Err(error) if what_if => { + return Ok(UserRightsAssignment { + right: input.right.clone(), + identity: input.identity.clone(), + metadata: Metadata::from_messages(vec![error.to_string()]), + ..UserRightsAssignment::default() + }); + } + Err(error) => return Err(error), + }; + + let (grant, revoke) = diff(¤t, &desired, input.set_action); + if grant.is_empty() && revoke.is_empty() { + return Ok(state(&input.right, ¤t)); + } + + let mut messages = Vec::with_capacity(grant.len() + revoke.len()); + messages.extend( + grant + .iter() + .map(|identity| t!("userRights.whatIfGrant", right = right.constant, identity = identity.name).to_string()), + ); + messages.extend( + revoke + .iter() + .map(|identity| t!("userRights.whatIfRevoke", right = right.constant, identity = identity.name).to_string()), + ); + if what_if { + return Ok(UserRightsAssignment { + right: input.right.clone(), + identity: Some(project(¤t, &grant, &revoke)), + metadata: Metadata::from_messages(messages), + ..UserRightsAssignment::default() + }); + } + + // Grant before revoke so that a logon right never has zero holders part-way through. + for identity in &grant { + policy.add_right(&identity.sid, right.constant)?; + } + for identity in &revoke { + policy.remove_right(&identity.sid, right.constant)?; + } + Ok(state(&input.right, ¤t_identities(&policy, right)?)) +} + +/// Export the holders of every user right. +/// +/// # Errors +/// +/// Returns an error when the process is not elevated or an LSA call fails. +pub fn export() -> Result, SecurityPolicyError> { + require_elevated()?; + let policy = LsaPolicy::open(POLICY_READ)?; + USER_RIGHTS + .iter() + .map(|right| Ok(state(right.constant, ¤t_identities(&policy, right)?))) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::sid::Sid; + + fn identity(name: &str, marker: u8) -> ResolvedIdentity { + ResolvedIdentity { + sid: Sid::from_bytes(vec![1, 1, 0, 0, 0, 0, 0, 5, marker, 0, 0, 0]), + name: name.to_string(), + } + } + + fn names_of<'a>(list: &[&'a ResolvedIdentity]) -> Vec<&'a str> { + list.iter().map(|identity| identity.name.as_str()).collect() + } + + #[test] + fn replace_grants_missing_and_revokes_extra() { + let current = [identity("A", 1), identity("B", 2)]; + let desired = [identity("B", 2), identity("C", 3)]; + let (grant, revoke) = diff(¤t, &desired, SetAction::Replace); + assert_eq!(names_of(&grant), ["C"]); + assert_eq!(names_of(&revoke), ["A"]); + + let aliases = [identity("b-alias", 2), identity("a-alias", 1)]; + let (grant, revoke) = diff(¤t, &aliases, SetAction::Replace); + assert!(grant.is_empty() && revoke.is_empty(), "same SIDs under other names are in desired state"); + + let (grant, revoke) = diff(¤t, &[], SetAction::Replace); + assert!(grant.is_empty()); + assert_eq!(names_of(&revoke), ["A", "B"]); + } + + #[test] + fn add_only_grants() { + let current = [identity("A", 1)]; + let present = [identity("A", 1)]; + let (grant, revoke) = diff(¤t, &present, SetAction::Add); + assert!(grant.is_empty() && revoke.is_empty()); + let missing = [identity("B", 2)]; + let (grant, revoke) = diff(¤t, &missing, SetAction::Add); + assert_eq!(names_of(&grant), ["B"]); + assert!(revoke.is_empty()); + let (grant, revoke) = diff(¤t, &[], SetAction::Add); + assert!(grant.is_empty() && revoke.is_empty()); + } + + #[test] + fn remove_only_revokes_listed_holders() { + let current = [identity("A", 1), identity("B", 2)]; + let absent = [identity("C", 3)]; + let (grant, revoke) = diff(¤t, &absent, SetAction::Remove); + assert!(grant.is_empty() && revoke.is_empty()); + let mixed = [identity("A", 1), identity("C", 3)]; + let (grant, revoke) = diff(¤t, &mixed, SetAction::Remove); + assert!(grant.is_empty()); + assert_eq!(names_of(&revoke), ["A"]); + let (grant, revoke) = diff(¤t, &[], SetAction::Remove); + assert!(grant.is_empty() && revoke.is_empty()); + } + + #[test] + fn projection_applies_changes_and_sorts() { + let current = [identity("Zed", 1), identity("alpha", 2)]; + let granted = identity("Mike", 3); + let revoked = identity("Zed", 1); + assert_eq!(project(¤t, &[&granted], &[&revoked]), ["alpha", "Mike"]); + assert_eq!(project(¤t, &[], &[]), ["alpha", "Zed"]); + } + + #[test] + fn operations_follow_the_security_context() { + let input = UserRightsAssignment { + right: "SeChangeNotifyPrivilege".to_string(), + identity: Some(vec!["Everyone".to_string()]), + ..UserRightsAssignment::default() + }; + if get_security_context() == SecurityContext::Admin { + let current = get(&input).unwrap(); + assert_eq!(current.right, "SeChangeNotifyPrivilege"); + assert!(current.identity.unwrap().iter().any(|name| name == "Everyone")); + let tested = test(&UserRightsAssignment { set_action: SetAction::Add, ..input.clone() }).unwrap(); + assert_eq!(tested.in_desired_state, Some(true)); + assert_eq!(export().unwrap().len(), USER_RIGHTS.len()); + } else { + assert!(matches!(get(&input), Err(SecurityPolicyError::ElevationRequired))); + assert!(matches!(test(&input), Err(SecurityPolicyError::ElevationRequired))); + assert!(matches!(set(&input, true), Err(SecurityPolicyError::ElevationRequired))); + assert!(matches!(export(), Err(SecurityPolicyError::ElevationRequired))); + } + } + + #[test] + fn unknown_right_and_missing_identity_are_invalid_input_before_any_lsa_call() { + // These checks happen after the elevation check, so only assert them when elevated. + if get_security_context() != SecurityContext::Admin { + return; + } + let unknown = UserRightsAssignment { right: "SeBogus".to_string(), ..UserRightsAssignment::default() }; + assert!(matches!(get(&unknown), Err(SecurityPolicyError::InvalidInput(_)))); + let no_identity = UserRightsAssignment { right: "SeDebugPrivilege".to_string(), ..UserRightsAssignment::default() }; + assert!(matches!(test(&no_identity), Err(SecurityPolicyError::InvalidInput(_)))); + assert!(matches!(set(&no_identity, true), Err(SecurityPolicyError::InvalidInput(_)))); + } +} diff --git a/resources/windows_security_policy/tests/windows_security_policy_user_right_export.tests.ps1 b/resources/windows_security_policy/tests/windows_security_policy_user_right_export.tests.ps1 new file mode 100644 index 000000000..2c2fb6d53 --- /dev/null +++ b/resources/windows_security_policy/tests/windows_security_policy_user_right_export.tests.ps1 @@ -0,0 +1,63 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +Describe 'User rights assignment export tests' -Skip:(!$IsWindows) { + BeforeDiscovery { + $isAdmin = if ($IsWindows) { + $identity = [Security.Principal.WindowsIdentity]::GetCurrent() + $principal = [Security.Principal.WindowsPrincipal]$identity + $principal.IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator) + } + else { + $false + } + } + + BeforeAll { + $resourceType = 'Microsoft.Windows/UserRightsAssignment' + } + + Context 'Export all rights' -Skip:(!$isAdmin) { + BeforeAll { + $raw = dsc resource export -r $resourceType 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + $script:export = $raw | ConvertFrom-Json + } + + It 'Returns every user right exactly once' { + $script:export.resources.Count | Should -Be 45 + $rights = @($script:export.resources.properties.right) + @($rights | Select-Object -Unique).Count | Should -Be 45 + foreach ($right in $rights) { + $right | Should -Match '^Se\w+(Privilege|Right)$' + } + $rights[0] | Should -BeExactly 'SeTrustedCredManAccessPrivilege' + $rights[-1] | Should -BeExactly 'SeTakeOwnershipPrivilege' + } + + It 'Returns complete entries with the correct type' { + foreach ($resource in $script:export.resources) { + $resource.type | Should -BeExactly $resourceType + $resource.properties.PSObject.Properties.Name | Should -Contain 'identity' + foreach ($name in @($resource.properties.identity)) { + $name | Should -Not -BeNullOrEmpty + } + } + } + + It 'Reports Everyone for bypass traverse checking' { + $entry = $script:export.resources | Where-Object { $_.properties.right -eq 'SeChangeNotifyPrivilege' } + @($entry.properties.identity) | Should -Contain 'Everyone' + } + + It 'Exported entries round-trip through get' { + foreach ($resource in $script:export.resources | Select-Object -First 3) { + $json = @{ right = $resource.properties.right } | ConvertTo-Json -Compress + $out = $json | dsc resource get -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + $actual = ($out | ConvertFrom-Json).actualState + @($actual.identity) | Should -Be @($resource.properties.identity) + } + } + } +} diff --git a/resources/windows_security_policy/tests/windows_security_policy_user_right_get.tests.ps1 b/resources/windows_security_policy/tests/windows_security_policy_user_right_get.tests.ps1 new file mode 100644 index 000000000..46ef03a15 --- /dev/null +++ b/resources/windows_security_policy/tests/windows_security_policy_user_right_get.tests.ps1 @@ -0,0 +1,117 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +Describe 'User rights assignment get tests' -Skip:(!$IsWindows) { + BeforeDiscovery { + $isAdmin = if ($IsWindows) { + $identity = [Security.Principal.WindowsIdentity]::GetCurrent() + $principal = [Security.Principal.WindowsPrincipal]$identity + $principal.IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator) + } + else { + $false + } + } + + BeforeAll { + $resourceType = 'Microsoft.Windows/UserRightsAssignment' + + function Get-UserRightState { + param([string]$Right) + $json = @{ right = $Right } | ConvertTo-Json -Compress + $out = $json | dsc resource get -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + return ($out | ConvertFrom-Json).actualState + } + + function ConvertTo-SidString { + param([string]$Identity) + $text = $Identity.TrimStart('*') + if ($text -match '^S-1-\d') { + return $text + } + return ([System.Security.Principal.NTAccount]$text).Translate([System.Security.Principal.SecurityIdentifier]).Value + } + + function Get-SeceditHolders { + param([string]$Right) + $inf = Join-Path $testdrive 'user-rights.inf' + $null = secedit /export /cfg $inf /areas USER_RIGHTS /quiet + $line = Get-Content $inf | Where-Object { $_ -match "^$Right\s*=" } | Select-Object -First 1 + if (-not $line) { + return @() + } + $values = ($line -split '=', 2)[1].Trim() + if (-not $values) { + return @() + } + return @($values -split ',' | ForEach-Object { ConvertTo-SidString -Identity $_.Trim() }) + } + } + + Context 'Get' -Skip:(!$isAdmin) { + It 'Returns the holders of a right' { + $result = Get-UserRightState -Right 'SeServiceLogonRight' + $result.right | Should -BeExactly 'SeServiceLogonRight' + $result.PSObject.Properties.Name | Should -Contain 'identity' + foreach ($name in @($result.identity)) { + $name | Should -Not -BeNullOrEmpty + } + } + + It 'Echoes the input casing when called directly' { + # DSC validates the schema enum, so a lower-case right only reaches the binary directly. + $out = windows_security_policy get user-right --input '{"right":"seservicelogonright"}' 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + ($out | ConvertFrom-Json).right | Should -BeExactly 'seservicelogonright' + } + + It 'Returns an empty identity list for a right that no one holds' { + $result = Get-UserRightState -Right 'SeTrustedCredManAccessPrivilege' + @($result.identity).Count | Should -Be 0 -Because 'SeTrustedCredManAccessPrivilege is unassigned by default' + } + + It 'Agrees with secedit for ' -TestCases @( + @{ right = 'SeInteractiveLogonRight' } + @{ right = 'SeChangeNotifyPrivilege' } + @{ right = 'SeServiceLogonRight' } + ) { + $result = Get-UserRightState -Right $right + $ours = @(@($result.identity) | ForEach-Object { ConvertTo-SidString -Identity $_ } | Sort-Object) + $theirs = @(Get-SeceditHolders -Right $right | Sort-Object) + $ours | Should -Be $theirs + } + } + + Context 'Input validation' -Skip:(!$isAdmin) { + It 'Fails for an unknown right' { + $null = windows_security_policy get user-right --input '{"right":"SeBogusPrivilege"}' 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 2 + Get-Content -Raw $testdrive/error.log | Should -Match 'Unknown user right' + } + + It 'Fails when right is missing' { + $json = @{ identity = @('Administrators') } | ConvertTo-Json -Compress + $null = $json | dsc resource get -r $resourceType -f - 2>&1 + $LASTEXITCODE | Should -Not -Be 0 + } + + It 'Fails when input JSON is invalid' { + $null = 'not-json' | dsc resource get -r $resourceType -f - 2>&1 + $LASTEXITCODE | Should -Not -Be 0 + } + } + + Context 'Without elevation' -Skip:($isAdmin) { + It 'Is refused by dsc because the resource requires an elevated process' { + $json = @{ right = 'SeServiceLogonRight' } | ConvertTo-Json -Compress + $null = $json | dsc resource get -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Not -Be 0 + } + + It 'Exits with code 4 when called directly' { + $null = windows_security_policy get user-right --input '{"right":"SeServiceLogonRight"}' 2>$null + $LASTEXITCODE | Should -Be 4 + } + } +} diff --git a/resources/windows_security_policy/tests/windows_security_policy_user_right_set.tests.ps1 b/resources/windows_security_policy/tests/windows_security_policy_user_right_set.tests.ps1 new file mode 100644 index 000000000..732f4a6da --- /dev/null +++ b/resources/windows_security_policy/tests/windows_security_policy_user_right_set.tests.ps1 @@ -0,0 +1,170 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +Describe 'User rights assignment set tests' -Skip:(!$IsWindows) { + BeforeDiscovery { + $isAdmin = if ($IsWindows) { + $identity = [Security.Principal.WindowsIdentity]::GetCurrent() + $principal = [Security.Principal.WindowsPrincipal]$identity + $principal.IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator) + } + else { + $false + } + } + + BeforeAll { + $resourceType = 'Microsoft.Windows/UserRightsAssignment' + # SeDenyBatchLogonRight is unassigned by default and denying batch logon to the test + # accounts below has no effect on a running system. + $testRight = 'SeDenyBatchLogonRight' + $guests = 'BUILTIN\Guests' + $guestsSid = 'S-1-5-32-546' + $anonymous = 'NT AUTHORITY\ANONYMOUS LOGON' + + function Get-UserRightState { + param([string]$Right = $testRight) + $json = @{ right = $Right } | ConvertTo-Json -Compress + $out = $json | dsc resource get -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + return ($out | ConvertFrom-Json).actualState + } + + function Set-UserRightState { + param([hashtable]$Properties) + $json = $Properties | ConvertTo-Json -Compress + $out = $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + return ($out | ConvertFrom-Json) + } + + function Restore-UserRightState { + param($Original) + if ($Original) { + $json = @{ right = $testRight; identity = @($Original.identity) } | ConvertTo-Json -Compress + $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log + } + } + + function Get-SeceditLine { + param([string]$Right) + $inf = Join-Path $testdrive 'user-rights.inf' + $null = secedit /export /cfg $inf /areas USER_RIGHTS /quiet + return (Get-Content $inf | Where-Object { $_ -match "^$Right\s*=" } | Select-Object -First 1) + } + } + + Context 'Set holders' -Skip:(!$isAdmin) { + BeforeAll { + $script:originalState = Get-UserRightState + } + + AfterAll { + Restore-UserRightState -Original $script:originalState + } + + It 'Replaces the holders with the listed account' { + $result = (Set-UserRightState -Properties @{ right = $testRight; identity = @($guests) }).afterState + $result.right | Should -BeExactly $testRight + @($result.identity) | Should -Be @($guests) + Get-SeceditLine -Right $testRight | Should -Match ([regex]::Escape("*$guestsSid")) + } + + It 'Resolves a SID to the canonical name and is idempotent' { + $output = Set-UserRightState -Properties @{ right = $testRight; identity = @($guestsSid) } + @($output.afterState.identity) | Should -Be @($guests) + $output.changedProperties | Should -BeNullOrEmpty + } + + It 'Accepts the secedit SID form' { + $output = Set-UserRightState -Properties @{ right = $testRight; identity = @("*$guestsSid") } + @($output.afterState.identity) | Should -Be @($guests) + $output.changedProperties | Should -BeNullOrEmpty + } + + It 'Adds an account without removing the existing holders' { + $result = (Set-UserRightState -Properties @{ right = $testRight; identity = @($anonymous); setAction = 'add' }).afterState + @($result.identity) | Should -Be @($guests, $anonymous) + } + + It 'Removes only the listed holders' { + $result = (Set-UserRightState -Properties @{ right = $testRight; identity = @($guests); setAction = 'remove' }).afterState + @($result.identity) | Should -Be @($anonymous) + } + + It 'Replaces with an empty list to revoke everyone' { + $result = (Set-UserRightState -Properties @{ right = $testRight; identity = @() }).afterState + @($result.identity).Count | Should -Be 0 + $line = Get-SeceditLine -Right $testRight + if ($line) { + ($line -split '=', 2)[1].Trim() | Should -BeNullOrEmpty + } + } + + It 'Collapses aliases of the same account' { + $result = (Set-UserRightState -Properties @{ right = $testRight; identity = @('Guests', $guests, $guestsSid) }).afterState + @($result.identity) | Should -Be @($guests) + } + } + + Context 'Input validation' -Skip:(!$isAdmin) { + It 'Fails when identity is missing' { + # Called directly: through dsc, the test operation would run first and reject the input. + $null = windows_security_policy set user-right --input "{`"right`":`"$testRight`"}" 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 2 + Get-Content -Raw $testdrive/error.log | Should -Match "'identity' is required" + } + + It 'Fails for an unknown account' { + $json = @{ right = $testRight; identity = @('NoSuchAccount_zz_12345') } | ConvertTo-Json -Compress + $null = $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Not -Be 0 + Get-Content -Raw $testdrive/error.log | Should -Match 'could not be found' + } + + It 'Fails for a malformed SID' { + $json = @{ right = $testRight; identity = @('S-1-X') } | ConvertTo-Json -Compress + $null = $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Not -Be 0 + Get-Content -Raw $testdrive/error.log | Should -Match 'not a valid SID' + } + + It 'Fails when input JSON is invalid' { + $null = 'not-json' | dsc resource set -r $resourceType -f - 2>&1 + $LASTEXITCODE | Should -Not -Be 0 + } + } + + Context 'Configuration document' -Skip:(!$isAdmin) { + BeforeAll { + $script:originalState = Get-UserRightState + $yaml = @" +`$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json +resources: + - name: Deny batch logon to guests + type: Microsoft.Windows/UserRightsAssignment + properties: + right: $testRight + identity: + - $guests +"@ + $script:configPath = Join-Path $testdrive 'userrights.config.dsc.yaml' + Set-Content -Path $script:configPath -Value $yaml + } + + AfterAll { + Restore-UserRightState -Original $script:originalState + } + + It 'Applies the configuration and then reports it in the desired state' { + $out = dsc config set -f $script:configPath 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + $result = ($out | ConvertFrom-Json).results[0].result.afterState + @($result.identity) | Should -Be @($guests) + + $out = dsc config test -f $script:configPath 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + ($out | ConvertFrom-Json).results[0].result.inDesiredState | Should -BeTrue + } + } +} diff --git a/resources/windows_security_policy/tests/windows_security_policy_user_right_test.tests.ps1 b/resources/windows_security_policy/tests/windows_security_policy_user_right_test.tests.ps1 new file mode 100644 index 000000000..4e43b894b --- /dev/null +++ b/resources/windows_security_policy/tests/windows_security_policy_user_right_test.tests.ps1 @@ -0,0 +1,100 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +Describe 'User rights assignment test tests' -Skip:(!$IsWindows) { + BeforeDiscovery { + $isAdmin = if ($IsWindows) { + $identity = [Security.Principal.WindowsIdentity]::GetCurrent() + $principal = [Security.Principal.WindowsPrincipal]$identity + $principal.IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator) + } + else { + $false + } + } + + BeforeAll { + $resourceType = 'Microsoft.Windows/UserRightsAssignment' + $testRight = 'SeDenyBatchLogonRight' + $guests = 'BUILTIN\Guests' + $guestsSid = 'S-1-5-32-546' + $anonymous = 'NT AUTHORITY\ANONYMOUS LOGON' + + function Get-UserRightState { + $json = @{ right = $testRight } | ConvertTo-Json -Compress + $out = $json | dsc resource get -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + return ($out | ConvertFrom-Json).actualState + } + + function Test-UserRightState { + param([hashtable]$Properties) + $json = $Properties | ConvertTo-Json -Compress + $out = $json | dsc resource test -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + return ($out | ConvertFrom-Json) + } + + function Set-UserRightIdentity { + param([string[]]$Identity) + $json = @{ right = $testRight; identity = @($Identity) } | ConvertTo-Json -Compress + $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + } + } + + Context 'Test against a right held by one account' -Skip:(!$isAdmin) { + BeforeAll { + $script:originalState = Get-UserRightState + Set-UserRightIdentity -Identity @($guests) + } + + AfterAll { + if ($script:originalState) { + Set-UserRightIdentity -Identity @($script:originalState.identity) + } + } + + It 'Replace with the same account is in the desired state' { + $result = Test-UserRightState -Properties @{ right = $testRight; identity = @($guests) } + $result.inDesiredState | Should -BeTrue + $result.actualState._inDesiredState | Should -BeTrue + @($result.actualState.identity) | Should -Be @($guests) + } + + It 'Replace compares SIDs, not spellings' { + (Test-UserRightState -Properties @{ right = $testRight; identity = @($guestsSid) }).inDesiredState | Should -BeTrue + (Test-UserRightState -Properties @{ right = $testRight; identity = @('guests') }).inDesiredState | Should -BeTrue + } + + It 'Replace with an extra account is not in the desired state' { + (Test-UserRightState -Properties @{ right = $testRight; identity = @($guests, $anonymous) }).inDesiredState | Should -BeFalse + } + + It 'Replace with an empty list is not in the desired state while someone holds the right' { + (Test-UserRightState -Properties @{ right = $testRight; identity = @() }).inDesiredState | Should -BeFalse + } + + It 'Add is in the desired state when the accounts already hold the right' { + (Test-UserRightState -Properties @{ right = $testRight; identity = @($guests); setAction = 'add' }).inDesiredState | Should -BeTrue + (Test-UserRightState -Properties @{ right = $testRight; identity = @(); setAction = 'add' }).inDesiredState | Should -BeTrue + } + + It 'Add is not in the desired state when an account is missing' { + (Test-UserRightState -Properties @{ right = $testRight; identity = @($anonymous); setAction = 'add' }).inDesiredState | Should -BeFalse + } + + It 'Remove is in the desired state when none of the accounts hold the right' { + (Test-UserRightState -Properties @{ right = $testRight; identity = @($anonymous); setAction = 'remove' }).inDesiredState | Should -BeTrue + } + + It 'Remove is not in the desired state when a listed account holds the right' { + (Test-UserRightState -Properties @{ right = $testRight; identity = @($guests); setAction = 'remove' }).inDesiredState | Should -BeFalse + } + + It 'Fails when identity is missing' { + $null = windows_security_policy test user-right --input "{`"right`":`"$testRight`"}" 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 2 + } + } +} diff --git a/resources/windows_security_policy/tests/windows_security_policy_user_right_whatif.tests.ps1 b/resources/windows_security_policy/tests/windows_security_policy_user_right_whatif.tests.ps1 new file mode 100644 index 000000000..e77163131 --- /dev/null +++ b/resources/windows_security_policy/tests/windows_security_policy_user_right_whatif.tests.ps1 @@ -0,0 +1,96 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +Describe 'User rights assignment whatif tests' -Skip:(!$IsWindows) { + BeforeDiscovery { + $isAdmin = if ($IsWindows) { + $identity = [Security.Principal.WindowsIdentity]::GetCurrent() + $principal = [Security.Principal.WindowsPrincipal]$identity + $principal.IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator) + } + else { + $false + } + } + + BeforeAll { + $testRight = 'SeDenyBatchLogonRight' + $guests = 'BUILTIN\Guests' + $anonymous = 'NT AUTHORITY\ANONYMOUS LOGON' + + function Get-UserRightState { + $out = windows_security_policy get user-right --input "{`"right`":`"$testRight`"}" 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + return ($out | ConvertFrom-Json) + } + + function Set-UserRightIdentity { + param([string[]]$Identity) + $json = @{ right = $testRight; identity = @($Identity) } | ConvertTo-Json -Compress + $null = windows_security_policy set user-right --input $json 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + } + } + + Context 'What-if' -Skip:(!$isAdmin) { + BeforeAll { + $script:originalState = Get-UserRightState + Set-UserRightIdentity -Identity @($guests) + } + + AfterAll { + if ($script:originalState) { + Set-UserRightIdentity -Identity @($script:originalState.identity) + } + } + + It 'Projects a grant without applying it' { + $json = @{ right = $testRight; identity = @($guests, $anonymous) } | ConvertTo-Json -Compress + $result = windows_security_policy set user-right -w --input $json 2>$null | ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 + + $result.right | Should -BeExactly $testRight + @($result.identity) | Should -Be @($guests, $anonymous) + $result._metadata.whatIf | Should -Be @("Would grant '$testRight' to '$anonymous'") + + @((Get-UserRightState).identity) | Should -Be @($guests) + } + + It 'Projects a revoke without applying it' { + $json = @{ right = $testRight; identity = @() } | ConvertTo-Json -Compress + $result = windows_security_policy set user-right -w --input $json 2>$null | ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 + + @($result.identity).Count | Should -Be 0 + $result._metadata.whatIf | Should -Be @("Would revoke '$testRight' from '$guests'") + + @((Get-UserRightState).identity) | Should -Be @($guests) + } + + It 'Returns no whatIf messages when nothing would change' { + $json = @{ right = $testRight; identity = @($guests) } | ConvertTo-Json -Compress + $result = windows_security_policy set user-right -w --input $json 2>$null | ConvertFrom-Json + $LASTEXITCODE | Should -Be 0 + $result._metadata | Should -BeNullOrEmpty + } + + It 'Fails for an unknown right or account even in what-if mode' { + $null = windows_security_policy set user-right -w --input '{"right":"SeBogusPrivilege","identity":[]}' 2>$null + $LASTEXITCODE | Should -Be 2 + $json = @{ right = $testRight; identity = @('NoSuchAccount_zz_12345') } | ConvertTo-Json -Compress + $null = windows_security_policy set user-right -w --input $json 2>$null + $LASTEXITCODE | Should -Be 2 + } + + It 'Works through dsc resource set --what-if' { + $json = @{ right = $testRight; identity = @($anonymous); setAction = 'add' } | ConvertTo-Json -Compress + $out = $json | dsc resource set -r Microsoft.Windows/UserRightsAssignment -w -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + $result = $out | ConvertFrom-Json + @($result.afterState.identity) | Should -Be @($guests, $anonymous) + $result.afterState._metadata.whatIf | Should -Not -BeNullOrEmpty + + @((Get-UserRightState).identity) | Should -Be @($guests) + } + } +} diff --git a/resources/windows_security_policy/windows_security_policy.dsc.manifests.json b/resources/windows_security_policy/windows_security_policy.dsc.manifests.json new file mode 100644 index 000000000..d2cce4f91 --- /dev/null +++ b/resources/windows_security_policy/windows_security_policy.dsc.manifests.json @@ -0,0 +1,182 @@ +{ + "resources": [ + { + "$schema": "https://aka.ms/dsc/schemas/v3/bundled/resource/manifest.json", + "type": "Microsoft.Windows/UserRightsAssignment", + "description": "Manage which accounts hold a Windows user right or privilege (Local Security Policy > User Rights Assignment).", + "tags": [ + "Windows", + "Security", + "UserRights" + ], + "version": "0.1.0", + "get": { + "executable": "windows_security_policy", + "args": [ + "get", + "user-right", + { + "jsonInputArg": "--input", + "mandatory": true + } + ], + "requireSecurityContext": "elevated" + }, + "set": { + "executable": "windows_security_policy", + "args": [ + "set", + "user-right", + { + "jsonInputArg": "--input", + "mandatory": true + }, + { + "whatIfArg": "--what-if" + } + ], + "implementsPretest": false, + "return": "state", + "requireSecurityContext": "elevated", + "whatIfReturns": "state" + }, + "test": { + "executable": "windows_security_policy", + "args": [ + "test", + "user-right", + { + "jsonInputArg": "--input", + "mandatory": true + } + ], + "return": "state", + "requireSecurityContext": "elevated" + }, + "export": { + "executable": "windows_security_policy", + "args": [ + "export", + "user-right" + ], + "supportsFiltering": false, + "requireSecurityContext": "elevated" + }, + "exitCodes": { + "0": "Success", + "1": "Invalid arguments", + "2": "Invalid input", + "3": "Security policy error", + "4": "Elevation required: Reading or changing user rights assignments requires an elevated process" + }, + "schema": { + "embedded": { + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "Windows User Rights Assignment", + "description": "Manage which accounts hold a Windows user right or privilege.", + "type": "object", + "required": [ + "right" + ], + "additionalProperties": false, + "properties": { + "right": { + "type": "string", + "title": "User right", + "description": "The user right or privilege constant, for example 'SeServiceLogonRight' (Log on as a service).", + "enum": [ + "SeTrustedCredManAccessPrivilege", + "SeNetworkLogonRight", + "SeTcbPrivilege", + "SeMachineAccountPrivilege", + "SeIncreaseQuotaPrivilege", + "SeInteractiveLogonRight", + "SeRemoteInteractiveLogonRight", + "SeBackupPrivilege", + "SeChangeNotifyPrivilege", + "SeSystemtimePrivilege", + "SeTimeZonePrivilege", + "SeCreatePagefilePrivilege", + "SeCreateTokenPrivilege", + "SeCreateGlobalPrivilege", + "SeCreatePermanentPrivilege", + "SeCreateSymbolicLinkPrivilege", + "SeDebugPrivilege", + "SeDenyNetworkLogonRight", + "SeDenyBatchLogonRight", + "SeDenyServiceLogonRight", + "SeDenyInteractiveLogonRight", + "SeDenyRemoteInteractiveLogonRight", + "SeEnableDelegationPrivilege", + "SeRemoteShutdownPrivilege", + "SeAuditPrivilege", + "SeImpersonatePrivilege", + "SeIncreaseWorkingSetPrivilege", + "SeIncreaseBasePriorityPrivilege", + "SeLoadDriverPrivilege", + "SeLockMemoryPrivilege", + "SeBatchLogonRight", + "SeServiceLogonRight", + "SeSecurityPrivilege", + "SeRelabelPrivilege", + "SeSystemEnvironmentPrivilege", + "SeDelegateSessionUserImpersonatePrivilege", + "SeManageVolumePrivilege", + "SeProfileSingleProcessPrivilege", + "SeSystemProfilePrivilege", + "SeUndockPrivilege", + "SeAssignPrimaryTokenPrivilege", + "SeRestorePrivilege", + "SeShutdownPrivilege", + "SeSyncAgentPrivilege", + "SeTakeOwnershipPrivilege" + ] + }, + "identity": { + "type": "array", + "title": "Identities", + "description": "The accounts that hold the right. Accepts 'DOMAIN\\Name', a bare account name, a well-known name such as 'Everyone', a SID string such as 'S-1-5-32-544', or the secedit form '*S-1-5-32-544'. Returned as the canonical 'DOMAIN\\Name', or as the SID string when the SID does not resolve to an account. An empty array means that no one holds the right. Required for set and test.", + "items": { + "type": "string", + "minLength": 1 + }, + "uniqueItems": true + }, + "setAction": { + "type": "string", + "title": "Set action", + "description": "How 'identity' is applied: 'replace' makes the listed accounts the only holders of the right, 'add' grants the right to the listed accounts and keeps the other holders, 'remove' revokes the right from the listed accounts only.", + "writeOnly": true, + "default": "replace", + "enum": [ + "replace", + "add", + "remove" + ] + }, + "_inDesiredState": { + "type": "boolean", + "title": "In desired state", + "description": "Whether the current holders of the right satisfy 'identity' and 'setAction'. Returned by the test operation only.", + "readOnly": true + }, + "_metadata": { + "type": "object", + "title": "Metadata", + "description": "Metadata returned by what-if operations describing the changes that would be applied.", + "readOnly": true, + "properties": { + "whatIf": { + "type": "array", + "items": { + "type": "string" + } + } + } + } + } + } + } + } + ] +} From 66dc903e88769dc74e222b8437506841358b2f4e Mon Sep 17 00:00:00 2001 From: "G.Reijn" <26114636+Gijsreyn@users.noreply.github.com> Date: Sun, 18 Oct 2026 05:59:35 +0200 Subject: [PATCH 2/3] Resolve remarks Copilot --- .../configure-user-rights-assignment.md | 17 ++- .../userrightsassignment.config.dsc.yaml | 2 +- .../Windows/UserRightsAssignment/index.md | 6 + .../locales/en-us.toml | 2 + resources/windows_security_policy/src/main.rs | 127 ++++++++++++------ resources/windows_security_policy/src/sid.rs | 26 +++- ...s_security_policy_user_right_set.tests.ps1 | 49 ++++++- 7 files changed, 177 insertions(+), 52 deletions(-) diff --git a/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/configure-user-rights-assignment.md b/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/configure-user-rights-assignment.md index 5464f2c3a..d8b71a0a4 100644 --- a/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/configure-user-rights-assignment.md +++ b/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/configure-user-rights-assignment.md @@ -25,7 +25,16 @@ The configuration document for this example defines three instances of the The first instance makes `BUILTIN\Guests` the only account that is denied batch logon; because `setAction` isn't specified, it defaults to `replace`. The second instance ensures that `NT SERVICE\ALL SERVICES` can log on as a service without touching the other holders. The third -instance ensures that `BUILTIN\Guests` can't log on locally, whoever else holds that right. +instance removes the direct grant of `SeInteractiveLogonRight` from `BUILTIN\Guests` and keeps +the other holders. + +> [!NOTE] +> `setAction: remove` revokes only the direct assignment. Windows grants a right to a user when +> any group the user belongs to holds it, so a member of `Guests` can still log on locally through +> another group that holds `SeInteractiveLogonRight`. To make sure that guests can't log on +> locally regardless of their other group memberships, add `BUILTIN\Guests` to +> `SeDenyInteractiveLogonRight` instead; a deny right always takes precedence over the matching +> allow right. :::code language="yaml" source="userrightsassignment.config.dsc.yaml"::: @@ -111,7 +120,7 @@ results: metadata: Microsoft.DSC: duration: PT0.0511306S - name: Guests may not log on locally + name: Remove the direct local logon grant from Guests type: Microsoft.Windows/UserRightsAssignment result: desiredState: @@ -197,7 +206,7 @@ results: metadata: Microsoft.DSC: duration: PT0.0975219S - name: Guests may not log on locally + name: Remove the direct local logon grant from Guests type: Microsoft.Windows/UserRightsAssignment result: beforeState: @@ -222,7 +231,7 @@ hadErrors: false ``` DSC skipped the second instance because its test reported it in the desired state. The third -instance only revoked the right from `Guests` and kept the other holders, because it uses +instance only revoked the direct grant from `Guests` and kept the other holders, because it uses `setAction: remove`. ## Preview the changes diff --git a/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/userrightsassignment.config.dsc.yaml b/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/userrightsassignment.config.dsc.yaml index d3f0a3210..ba681a72b 100644 --- a/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/userrightsassignment.config.dsc.yaml +++ b/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/userrightsassignment.config.dsc.yaml @@ -14,7 +14,7 @@ resources: identity: - NT SERVICE\ALL SERVICES setAction: add - - name: Guests may not log on locally + - name: Remove the direct local logon grant from Guests type: Microsoft.Windows/UserRightsAssignment properties: right: SeInteractiveLogonRight diff --git a/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/index.md b/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/index.md index 98812a7bf..35deab584 100644 --- a/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/index.md +++ b/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/index.md @@ -250,6 +250,12 @@ judges it. The following values are valid: With `replace`, an empty `identity` list revokes the right from everyone. With `add` or `remove`, an empty list changes nothing. +`remove` revokes only the direct assignment of the listed accounts. Windows grants a right to a +user when any group the user belongs to holds it, so removing a group doesn't prevent its members +from obtaining the right through another holder. To deny a right regardless of group membership, +assign the matching `SeDeny*` right instead; a deny right always takes precedence over the +matching allow right. + Because this property is write-only, DSC doesn't compare it and it never appears in the actual state. When you use `add` or `remove`, the `differingProperties` list that DSC reports may still name `identity` even though the resource reports the instance as in the desired state; rely on diff --git a/resources/windows_security_policy/locales/en-us.toml b/resources/windows_security_policy/locales/en-us.toml index 8f41dedcf..7510058a7 100644 --- a/resources/windows_security_policy/locales/en-us.toml +++ b/resources/windows_security_policy/locales/en-us.toml @@ -7,6 +7,7 @@ missingTarget = "Missing resource target. Expected: user-right" unknownTarget = "Unknown resource target: '%{target}'. Expected: user-right" missingInput = "Missing --input argument" missingInputValue = "Missing value for --input argument" +unknownOption = "Unknown option: '%{option}'. Expected: --input or --what-if" invalidJson = "Invalid JSON input: %{error}" serializeFailed = "Failed to serialize output: %{error}" windowsOnly = "This resource is only supported on Windows" @@ -17,6 +18,7 @@ unknownRight = "Unknown user right '%{right}'. Specify a user right constant suc [sid] emptyIdentity = "An entry in 'identity' is empty" +embeddedNul = "An entry in 'identity' contains an embedded null character" invalidSid = "'%{identity}' is not a valid SID string" unknownAccount = "The account '%{identity}' could not be found. Specify 'DOMAIN\\Name', a well-known account name, or a SID string." lookupNameFailed = "Failed to look up the account '%{identity}': %{error}" diff --git a/resources/windows_security_policy/src/main.rs b/resources/windows_security_policy/src/main.rs index d197155b3..30c778a01 100644 --- a/resources/windows_security_policy/src/main.rs +++ b/resources/windows_security_policy/src/main.rs @@ -57,16 +57,20 @@ fn main() -> ExitCode { return ExitCode::from(EXIT_INVALID_ARGS); }; - let input_json = match parse_input_arg(&args) { - Ok(input) => input, - Err(code) => return code, + let options = match parse_options(&args) { + Ok(options) => options, + Err(error) => { + write_error(&error.to_string()); + return ExitCode::from(EXIT_INVALID_ARGS); + } }; - let what_if = parse_what_if_flag(&args); match (operation, target.as_str()) { - ("get", "user-right") => with_input(input_json, |input: UserRightsAssignment| user_rights::get(&input)), - ("set", "user-right") => with_input(input_json, |input: UserRightsAssignment| user_rights::set(&input, what_if)), - ("test", "user-right") => with_input(input_json, |input: UserRightsAssignment| user_rights::test(&input)), + ("get", "user-right") => with_input(options.input, |input: UserRightsAssignment| user_rights::get(&input)), + ("set", "user-right") => { + with_input(options.input, |input: UserRightsAssignment| user_rights::set(&input, options.what_if)) + } + ("test", "user-right") => with_input(options.input, |input: UserRightsAssignment| user_rights::test(&input)), ("export", "user-right") => finish_many(user_rights::export()), (_, target) => { write_error(&t!("main.unknownTarget", target = target)); @@ -162,56 +166,97 @@ fn finish_many(result: Result, types::SecurityPolicy } } -/// Parse the `--input ` argument. Arguments 1 and 2 are the operation and the target. +/// The options that may follow the operation and the target. #[cfg(windows)] -fn parse_input_arg(args: &[String]) -> Result, ExitCode> { - let mut index = 3; - while index < args.len() { - if args[index] == "--input" || args[index] == "-i" { - if index + 1 < args.len() { - return Ok(Some(args[index + 1].clone())); - } - write_error(&t!("main.missingInputValue")); - return Err(ExitCode::from(EXIT_INVALID_ARGS)); +#[derive(Debug, Default, PartialEq, Eq)] +struct Options { + /// The JSON given with `--input` / `-i`. + input: Option, + /// Whether `--what-if` / `-w` was given. + what_if: bool, +} + +/// Why the argument list was rejected. +#[cfg(windows)] +#[derive(Debug, PartialEq, Eq)] +enum ArgError { + /// `--input` was the last argument. + MissingInputValue, + /// An argument that is neither a known option nor the value of `--input`. + UnknownOption(String), +} + +#[cfg(windows)] +impl std::fmt::Display for ArgError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::MissingInputValue => f.write_str(&t!("main.missingInputValue")), + Self::UnknownOption(option) => f.write_str(&t!("main.unknownOption", option = option)), } - index += 1; } - Ok(None) } -/// Parse the `--what-if` / `-w` flag. Arguments 1 and 2 are the operation and the target. +/// Parse every argument after the operation and the target (arguments 1 and 2). +/// +/// Only `--input ` / `-i ` and `--what-if` / `-w` are accepted, in any order. Every +/// other argument is an error, so that a misspelled option can never be ignored. +/// +/// # Errors +/// +/// Returns `MissingInputValue` when `--input` has no value and `UnknownOption` for any argument +/// that isn't a known option or the value of `--input`. #[cfg(windows)] -fn parse_what_if_flag(args: &[String]) -> bool { - args.iter().skip(3).any(|arg| arg == "--what-if" || arg == "-w") +fn parse_options(args: &[String]) -> Result { + let mut options = Options::default(); + let mut remaining = args.iter().skip(3); + while let Some(arg) = remaining.next() { + match arg.as_str() { + "--input" | "-i" => options.input = Some(remaining.next().ok_or(ArgError::MissingInputValue)?.clone()), + "--what-if" | "-w" => options.what_if = true, + unknown => return Err(ArgError::UnknownOption(unknown.to_string())), + } + } + Ok(options) } #[cfg(all(test, windows))] mod tests { - use super::{parse_input_arg, parse_what_if_flag}; + use super::{ArgError, Options, parse_options}; + + fn parse(list: &[&str]) -> Result { + let args: Vec = list.iter().map(ToString::to_string).collect(); + parse_options(&args) + } + + fn options(input: Option<&str>, what_if: bool) -> Options { + Options { input: input.map(ToString::to_string), what_if } + } - fn args(list: &[&str]) -> Vec { - list.iter().map(ToString::to_string).collect() + #[test] + fn accepts_input_and_what_if_in_any_order() { + assert_eq!(parse(&["exe", "get", "user-right", "--input", "{}"]), Ok(options(Some("{}"), false))); + assert_eq!(parse(&["exe", "set", "user-right", "-w", "-i", "[]"]), Ok(options(Some("[]"), true))); + assert_eq!(parse(&["exe", "set", "user-right", "--input", "{}", "--what-if"]), Ok(options(Some("{}"), true))); + assert_eq!(parse(&["exe", "export", "user-right"]), Ok(options(None, false))); + } + + #[test] + fn input_requires_a_value() { + assert_eq!(parse(&["exe", "get", "user-right", "--input"]), Err(ArgError::MissingInputValue)); + assert_eq!(parse(&["exe", "set", "user-right", "-w", "-i"]), Err(ArgError::MissingInputValue)); } #[test] - fn finds_long_and_short_input_arguments_after_the_target() { - assert_eq!( - parse_input_arg(&args(&["exe", "get", "user-right", "--input", "{}"])), - Ok(Some("{}".to_string())) - ); - assert_eq!( - parse_input_arg(&args(&["exe", "set", "user-right", "-w", "-i", "[]"])), - Ok(Some("[]".to_string())) - ); - assert_eq!(parse_input_arg(&args(&["exe", "export", "user-right"])), Ok(None)); - assert!(parse_input_arg(&args(&["exe", "get", "user-right", "--input"])).is_err()); + fn unknown_options_are_rejected_before_and_after_input() { + let unknown = |option: &str| Err(ArgError::UnknownOption(option.to_string())); + assert_eq!(parse(&["exe", "set", "user-right", "--whatif", "--input", "{}"]), unknown("--whatif")); + assert_eq!(parse(&["exe", "set", "user-right", "--input", "{}", "--whatif"]), unknown("--whatif")); + assert_eq!(parse(&["exe", "set", "user-right", "--input", "{}", "extra"]), unknown("extra")); + assert_eq!(parse(&["exe", "export", "user-right", "-x"]), unknown("-x")); } #[test] - fn finds_what_if_flag_after_the_target() { - assert!(parse_what_if_flag(&args(&["exe", "set", "user-right", "--what-if", "--input", "{}"]))); - assert!(parse_what_if_flag(&args(&["exe", "set", "user-right", "--input", "{}", "-w"]))); - assert!(!parse_what_if_flag(&args(&["exe", "set", "user-right", "--input", "{}"]))); - assert!(!parse_what_if_flag(&args(&["exe", "-w", "user-right"]))); + fn the_value_of_input_is_never_read_as_an_option() { + assert_eq!(parse(&["exe", "set", "user-right", "--input", "--what-if"]), Ok(options(Some("--what-if"), false))); } } diff --git a/resources/windows_security_policy/src/sid.rs b/resources/windows_security_policy/src/sid.rs index 25cad53fb..50eda7202 100644 --- a/resources/windows_security_policy/src/sid.rs +++ b/resources/windows_security_policy/src/sid.rs @@ -218,13 +218,17 @@ fn looks_like_sid(text: &str) -> bool { /// /// # Errors /// -/// Returns `InvalidInput` for an empty entry, a malformed SID string or an unknown account name. +/// Returns `InvalidInput` for an empty entry, an entry with an embedded NUL character, a +/// malformed SID string or an unknown account name. pub fn resolve_identity(input: &str) -> Result { let trimmed = input.trim(); let text = trimmed.strip_prefix('*').unwrap_or(trimmed).trim(); if text.is_empty() { return Err(SecurityPolicyError::InvalidInput(t!("sid.emptyIdentity").to_string())); } + if text.contains('\0') { + return Err(SecurityPolicyError::InvalidInput(t!("sid.embeddedNul").to_string())); + } let sid = if looks_like_sid(text) { // A well-formed SID with no matching account is accepted: the LSA keeps rights for // orphaned SIDs and `display_name` reports them as SID strings. @@ -270,6 +274,7 @@ mod tests { assert_eq!(sid.0.len(), 16, "S-1-5-32-544 is 8 header bytes plus two sub-authorities"); assert_eq!(sid_to_string(&sid).unwrap(), ADMINISTRATORS); assert!(parse_string_sid("S-1-X").is_err()); + assert!(parse_string_sid("S-1-5-32-abc").is_err()); assert!(parse_string_sid("Administrators").is_err()); } @@ -306,7 +311,7 @@ mod tests { #[test] fn invalid_identities_are_invalid_input() { - for input in ["", " ", "*", "S-1-X", "NoSuchAccount_zz_12345"] { + for input in ["", " ", "*", "S-1-X", "S-1-5-32-abc", "NoSuchAccount_zz_12345"] { assert!( matches!(resolve_identity(input), Err(SecurityPolicyError::InvalidInput(_))), "{input:?} should be invalid input" @@ -314,6 +319,23 @@ mod tests { } } + #[test] + fn malformed_sids_are_reported_as_invalid_sids_and_unknown_names_as_unknown_accounts() { + let error = resolve_identity("S-1-5-32-abc").unwrap_err().to_string(); + assert_eq!(error, t!("sid.invalidSid", identity = "S-1-5-32-abc")); + let error = resolve_identity("S-1-X").unwrap_err().to_string(); + assert_eq!(error, t!("sid.unknownAccount", identity = "S-1-X")); + } + + #[test] + fn embedded_nul_characters_are_rejected_before_any_lookup() { + for input in ["S-1-5-32-544\0suffix", "Administrators\0suffix", "*S-1-5-32-544\0", "\0"] { + let error = resolve_identity(input).unwrap_err(); + assert!(matches!(error, SecurityPolicyError::InvalidInput(_)), "{input:?} should be invalid input"); + assert_eq!(error.to_string(), t!("sid.embeddedNul"), "{input:?}"); + } + } + #[test] fn duplicate_identities_collapse_by_sid() { let inputs: Vec = ["Administrators", "BUILTIN\\Administrators", ADMINISTRATORS, "Everyone"] diff --git a/resources/windows_security_policy/tests/windows_security_policy_user_right_set.tests.ps1 b/resources/windows_security_policy/tests/windows_security_policy_user_right_set.tests.ps1 index 732f4a6da..154fef6e8 100644 --- a/resources/windows_security_policy/tests/windows_security_policy_user_right_set.tests.ps1 +++ b/resources/windows_security_policy/tests/windows_security_policy_user_right_set.tests.ps1 @@ -21,6 +21,7 @@ Describe 'User rights assignment set tests' -Skip:(!$IsWindows) { $guests = 'BUILTIN\Guests' $guestsSid = 'S-1-5-32-546' $anonymous = 'NT AUTHORITY\ANONYMOUS LOGON' + $anonymousSid = 'S-1-5-7' function Get-UserRightState { param([string]$Right = $testRight) @@ -59,15 +60,21 @@ Describe 'User rights assignment set tests' -Skip:(!$IsWindows) { $script:originalState = Get-UserRightState } + BeforeEach { + $null = Set-UserRightState -Properties @{ right = $testRight; identity = @($guests) } + } + AfterAll { Restore-UserRightState -Original $script:originalState } It 'Replaces the holders with the listed account' { - $result = (Set-UserRightState -Properties @{ right = $testRight; identity = @($guests) }).afterState + $result = (Set-UserRightState -Properties @{ right = $testRight; identity = @($anonymous) }).afterState $result.right | Should -BeExactly $testRight - @($result.identity) | Should -Be @($guests) - Get-SeceditLine -Right $testRight | Should -Match ([regex]::Escape("*$guestsSid")) + @($result.identity) | Should -Be @($anonymous) + $line = Get-SeceditLine -Right $testRight + $line | Should -Match ([regex]::Escape("*$anonymousSid")) + $line | Should -Not -Match ([regex]::Escape("*$guestsSid")) } It 'Resolves a SID to the canonical name and is idempotent' { @@ -88,6 +95,7 @@ Describe 'User rights assignment set tests' -Skip:(!$IsWindows) { } It 'Removes only the listed holders' { + $null = Set-UserRightState -Properties @{ right = $testRight; identity = @($guests, $anonymous) } $result = (Set-UserRightState -Properties @{ right = $testRight; identity = @($guests); setAction = 'remove' }).afterState @($result.identity) | Should -Be @($anonymous) } @@ -108,6 +116,15 @@ Describe 'User rights assignment set tests' -Skip:(!$IsWindows) { } Context 'Input validation' -Skip:(!$isAdmin) { + BeforeAll { + $script:originalState = Get-UserRightState + $null = Set-UserRightState -Properties @{ right = $testRight; identity = @($guests) } + } + + AfterAll { + Restore-UserRightState -Original $script:originalState + } + It 'Fails when identity is missing' { # Called directly: through dsc, the test operation would run first and reject the input. $null = windows_security_policy set user-right --input "{`"right`":`"$testRight`"}" 2>$testdrive/error.log @@ -115,6 +132,18 @@ Describe 'User rights assignment set tests' -Skip:(!$IsWindows) { Get-Content -Raw $testdrive/error.log | Should -Match "'identity' is required" } + It 'Rejects an unknown option --input without changing the policy' -TestCases @( + @{ position = 'before' } + @{ position = 'after' } + ) { + $json = @{ right = $testRight; identity = @($anonymous) } | ConvertTo-Json -Compress + $arguments = if ($position -eq 'before') { @('--whatif', '--input', $json) } else { @('--input', $json, '--whatif') } + $null = windows_security_policy set user-right @arguments 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 1 + Get-Content -Raw $testdrive/error.log | Should -Match "Unknown option: '--whatif'" + @((Get-UserRightState).identity) | Should -Be @($guests) + } + It 'Fails for an unknown account' { $json = @{ right = $testRight; identity = @('NoSuchAccount_zz_12345') } | ConvertTo-Json -Compress $null = $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log @@ -123,12 +152,23 @@ Describe 'User rights assignment set tests' -Skip:(!$IsWindows) { } It 'Fails for a malformed SID' { - $json = @{ right = $testRight; identity = @('S-1-X') } | ConvertTo-Json -Compress + $json = @{ right = $testRight; identity = @('S-1-5-32-abc') } | ConvertTo-Json -Compress $null = $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log $LASTEXITCODE | Should -Not -Be 0 Get-Content -Raw $testdrive/error.log | Should -Match 'not a valid SID' } + It 'Rejects an embedded null character in a without changing the policy' -TestCases @( + @{ kind = 'SID'; identity = "S-1-5-32-546`0suffix" } + @{ kind = 'account name'; identity = "Guests`0suffix" } + ) { + $json = @{ right = $testRight; identity = @($identity, $anonymous) } | ConvertTo-Json -Compress + $null = $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log + $LASTEXITCODE | Should -Not -Be 0 + Get-Content -Raw $testdrive/error.log | Should -Match 'embedded null character' + @((Get-UserRightState).identity) | Should -Be @($guests) + } + It 'Fails when input JSON is invalid' { $null = 'not-json' | dsc resource set -r $resourceType -f - 2>&1 $LASTEXITCODE | Should -Not -Be 0 @@ -138,6 +178,7 @@ Describe 'User rights assignment set tests' -Skip:(!$IsWindows) { Context 'Configuration document' -Skip:(!$isAdmin) { BeforeAll { $script:originalState = Get-UserRightState + $null = Set-UserRightState -Properties @{ right = $testRight; identity = @() } $yaml = @" `$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json resources: From bb6593d7c3404880f9323e8404796a24d0a85d4e Mon Sep 17 00:00:00 2001 From: "G.Reijn" <26114636+Gijsreyn@users.noreply.github.com> Date: Mon, 19 Oct 2026 21:55:27 +0200 Subject: [PATCH 3/3] Resolve remarks --- .../configure-user-rights-assignment.md | 91 +++++++++--- .../examples/get-user-rights-assignment.md | 6 +- .../Windows/UserRightsAssignment/index.md | 60 +++++--- .../locales/en-us.toml | 52 ++++++- resources/windows_security_policy/src/main.rs | 7 + .../windows_security_policy/src/rights.rs | 133 +++++++++++------- resources/windows_security_policy/src/sid.rs | 47 ++++++- .../src/user_rights.rs | 127 ++++++++++++++--- ...s_security_policy_user_right_set.tests.ps1 | 28 +++- ..._security_policy_user_right_test.tests.ps1 | 21 +++ ...ecurity_policy_user_right_whatif.tests.ps1 | 79 ++++++++--- ...windows_security_policy.dsc.manifests.json | 120 ++++++++++++++-- 12 files changed, 624 insertions(+), 147 deletions(-) diff --git a/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/configure-user-rights-assignment.md b/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/configure-user-rights-assignment.md index d8b71a0a4..310561619 100644 --- a/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/configure-user-rights-assignment.md +++ b/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/configure-user-rights-assignment.md @@ -2,7 +2,7 @@ description: > Example showing how to use the Microsoft.Windows/UserRightsAssignment resource in a DSC configuration document to enforce user rights assignments. -ms.date: 10/16/2026 +ms.date: 10/19/2026 ms.topic: reference title: Configure user rights assignments --- @@ -172,14 +172,12 @@ results: beforeState: right: SeDenyBatchLogonRight identity: [] - _inDesiredState: false afterState: right: SeDenyBatchLogonRight identity: - BUILTIN\Guests changedProperties: - identity - - _inDesiredState - executionInformation: duration: PT0.0509873S metadata: @@ -192,8 +190,7 @@ results: right: SeServiceLogonRight identity: - NT SERVICE\ALL SERVICES - - NT VIRTUAL MACHINE\Virtual Machines - _inDesiredState: true + setAction: add afterState: right: SeServiceLogonRight identity: @@ -216,7 +213,6 @@ results: - BUILTIN\Backup Operators - BUILTIN\Guests - BUILTIN\Users - _inDesiredState: false afterState: right: SeInteractiveLogonRight identity: @@ -225,49 +221,106 @@ results: - BUILTIN\Users changedProperties: - identity - - _inDesiredState messages: [] hadErrors: false ``` -DSC skipped the second instance because its test reported it in the desired state. The third -instance only revoked the direct grant from `Guests` and kept the other holders, because it uses +DSC skipped the second instance because its test reported it in the desired state, so DSC reports +the desired state as `beforeState` and the tested state as `afterState`. The third instance only +revoked the direct grant from `Guests` and kept the other holders, because it uses `setAction: remove`. ## Preview the changes To see which accounts would be granted or revoked without applying the change, add the -`--what-if` option. The resource reports the projected holders and describes each change under -`_metadata.whatIf`. +`--what-if` option. The resource projects the holders each instance would have under its own +`setAction` and describes each change. DSC moves those descriptions from the resource output to +the `whatIf` list in the instance's `metadata`. ```powershell dsc config set --file ./userrightsassignment.config.dsc.yaml --what-if ``` ```yaml -# Elided for brevity +executionInformation: + # Elided for brevity +metadata: + # Elided for brevity results: -- name: Deny batch logon to guests only +- executionInformation: + # Elided for brevity + metadata: + Microsoft.DSC: + # Elided for brevity + whatIf: + - Would grant 'SeDenyBatchLogonRight' (Deny log on as a batch job) to 'BUILTIN\Guests' + name: Deny batch logon to guests only type: Microsoft.Windows/UserRightsAssignment result: beforeState: right: SeDenyBatchLogonRight identity: [] - _inDesiredState: false afterState: right: SeDenyBatchLogonRight identity: - BUILTIN\Guests - _metadata: - whatIf: - - Would grant 'SeDenyBatchLogonRight' to 'BUILTIN\Guests' changedProperties: - identity - - _inDesiredState - _metadata -# Elided for brevity +- executionInformation: + # Elided for brevity + metadata: + Microsoft.DSC: + # Elided for brevity + name: Services may log on as a service + type: Microsoft.Windows/UserRightsAssignment + result: + beforeState: + right: SeServiceLogonRight + identity: + - NT SERVICE\ALL SERVICES + - NT VIRTUAL MACHINE\Virtual Machines + afterState: + right: SeServiceLogonRight + identity: + - NT SERVICE\ALL SERVICES + - NT VIRTUAL MACHINE\Virtual Machines + changedProperties: [] +- executionInformation: + # Elided for brevity + metadata: + Microsoft.DSC: + # Elided for brevity + whatIf: + - Would revoke 'SeInteractiveLogonRight' (Allow log on locally) from 'BUILTIN\Guests' + name: Remove the direct local logon grant from Guests + type: Microsoft.Windows/UserRightsAssignment + result: + beforeState: + right: SeInteractiveLogonRight + identity: + - BUILTIN\Administrators + - BUILTIN\Backup Operators + - BUILTIN\Guests + - BUILTIN\Users + afterState: + right: SeInteractiveLogonRight + identity: + - BUILTIN\Administrators + - BUILTIN\Backup Operators + - BUILTIN\Users + changedProperties: + - identity + - _metadata +messages: [] +hadErrors: false ``` +The first instance uses the default `replace`, so the preview lists `Guests` as the only holder. +The second instance uses `add` and already holds the right, so the preview keeps both holders and +describes no change. The third instance uses `remove`, so the preview revokes only `Guests` and +keeps the other holders. + [01]: ../../../../../../cli/config/test.md [02]: ../../../../../../cli/config/set.md diff --git a/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/get-user-rights-assignment.md b/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/get-user-rights-assignment.md index e824a6b1d..fa597ed14 100644 --- a/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/get-user-rights-assignment.md +++ b/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/examples/get-user-rights-assignment.md @@ -2,7 +2,7 @@ description: > Example showing how to use the Microsoft.Windows/UserRightsAssignment resource with the dsc resource commands to retrieve, test and export user rights assignments. -ms.date: 10/16/2026 +ms.date: 10/19/2026 ms.topic: reference title: Get user rights assignments --- @@ -62,6 +62,10 @@ differingProperties: - identity ``` +Because `S-1-5-32-546` isn't the name that the resource reports for the account, the resource also +writes an informational message that names the canonical form, `BUILTIN\Guests`. DSC shows the +message when you run the command with `--trace-level info`. + To check that an account holds a right regardless of the other holders, add `setAction: add`: ```powershell diff --git a/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/index.md b/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/index.md index 35deab584..106782ec0 100644 --- a/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/index.md +++ b/docs/reference/resources/Microsoft/Windows/UserRightsAssignment/index.md @@ -1,6 +1,6 @@ --- description: Microsoft.Windows/UserRightsAssignment resource reference documentation -ms.date: 10/16/2026 +ms.date: 10/19/2026 ms.topic: reference title: Microsoft.Windows/UserRightsAssignment --- @@ -134,7 +134,8 @@ The resource has the following capabilities: - `export` - You can use the resource to export the holders of every right on the system. - `whatIf` - You can use the resource with the `--what-if` option of the `dsc config set` and `dsc resource set` commands to see which accounts would be granted or revoked without applying - the change. + the change. The preview honours [setAction](#setaction), so with `add` or `remove` it keeps the + holders that you didn't list. For more information about resource capabilities, see [DSC resource capabilities][01]. @@ -206,21 +207,31 @@ IsWriteOnly : false The accounts that hold the right. This property is required for the **Set** and **Test** -operations. Each entry can be any of the following: +operations. Each entry must have one of the following forms: -- An account name with its domain, such as `CONTOSO\svc-sql`, `BUILTIN\Administrators` or - `NT SERVICE\MSSQLSERVER`. -- A bare account name that the local system can resolve, such as `Administrators`. -- A well-known name, such as `Everyone` or `NT AUTHORITY\LOCAL SERVICE`. -- A SID string, such as `S-1-5-32-544`, or the form that `secedit` exports, `*S-1-5-32-544`. +- An account name with its domain, such as `CONTOSO\svc-sql`, `BUILTIN\Administrators`, + `NT SERVICE\MSSQLSERVER` or `NT AUTHORITY\LOCAL SERVICE`. +- A bare account name that the local system can resolve, such as `Administrators`, or a + well-known name that has no domain, such as `Everyone`. Well-known names are localized, so use + the name in the language of the target system. +- A SID string, such as `S-1-5-32-544`. +- A SID string prefixed with `*`, the form that `secedit` exports, such as `*S-1-5-32-544`. + +The instance schema validates each entry against these forms, so DSC rejects entries such as +`S-1-5-32-abc`, `*Administrators` or `BUILTIN\Guests\Extra` before it invokes the resource. The resource resolves every entry to a SID. An account name that doesn't exist is an error. A SID that doesn't resolve to an account is accepted, because Windows keeps rights assigned to deleted accounts. Entries that resolve to the same SID count once. -On **Get** and **Export**, the resource returns each holder as `DOMAIN\Name`, as a bare name when -the account has no domain (for example `Everyone`), or as the SID string when the SID no longer -resolves to an account. An empty array means that no one holds the right. +On **Get** and **Export**, the resource returns each holder by its canonical name: `DOMAIN\Name`, +a bare name when the account has no domain (for example `Everyone`), or the SID string when the +SID no longer resolves to an account. An empty array means that no one holds the right. + +The resource compares accounts by SID, but DSC compares the `identity` arrays as text. When an +entry isn't the canonical name of its account, such as `Administrators` or `S-1-5-32-544` for +`BUILTIN\Administrators`, the resource writes an informational message that names the canonical +form. Specify the canonical name so that the desired state matches the actual state. ### setAction @@ -266,7 +277,7 @@ name `identity` even though the resource reports the instance as in the desired
Expand for _inDesiredState property metadata ```yaml -Type : boolean +Type : [boolean, 'null'] IsRequired : false IsKey : false IsReadOnly : true @@ -275,32 +286,41 @@ IsWriteOnly : false
-Returned by the **Test** operation only. Indicates whether the current holders of the right +This is the canonical `_inDesiredState` property. Returned by the **Test** operation only. Indicates whether the current holders of the right satisfy `identity` under the chosen `setAction`. ## Instance validating schema The following snippet contains the JSON Schema that validates an instance of the resource. The `enum` for `right` is abbreviated; it contains the 45 constants listed in the -[Description](#description). +[Description](#description). Annotations such as `title`, `description`, `markdownDescription` +and `markdownEnumDescriptions` are omitted, and the canonical `_inDesiredState` definition is +bundled under `$defs` in the manifest. ```json { + "$schema": "https://json-schema.org/draft/2020-12/schema", "type": "object", "required": ["right"], "additionalProperties": false, "properties": { "right": { "type": "string", + "x-dsc-key": true, "enum": ["SeTrustedCredManAccessPrivilege", "SeNetworkLogonRight", "..."] }, "identity": { "type": "array", + "uniqueItems": true, "items": { "type": "string", - "minLength": 1 - }, - "uniqueItems": true + "oneOf": [ + { "pattern": "^[^\\\\*]+\\\\[^\\\\*]+$" }, + { "pattern": "^[^\\\\*]+$", "not": { "pattern": "^[Ss]-1-[0-9]" } }, + { "pattern": "^[Ss]-1-([0-9]+|0[Xx][0-9A-Fa-f]+)(-[0-9]+)*$" }, + { "pattern": "^\\*[Ss]-1-([0-9]+|0[Xx][0-9A-Fa-f]+)(-[0-9]+)*$" } + ] + } }, "setAction": { "type": "string", @@ -309,8 +329,7 @@ The following snippet contains the JSON Schema that validates an instance of the "enum": ["replace", "add", "remove"] }, "_inDesiredState": { - "type": "boolean", - "readOnly": true + "$ref": "https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/v3/resource/properties/inDesiredState.json" }, "_metadata": { "type": "object", @@ -351,7 +370,8 @@ name was not recognized. Indicates the resource operation failed because the JSON input could not be deserialized into a valid instance, the `right` isn't a known user right, an entry in `identity` isn't a valid SID or -an existing account, or `identity` was omitted from a **Set** or **Test** operation. +an existing account (including `*` followed by anything other than a SID string), or `identity` +was omitted from a **Set** or **Test** operation. ### Exit code 3 diff --git a/resources/windows_security_policy/locales/en-us.toml b/resources/windows_security_policy/locales/en-us.toml index 7510058a7..376f9d6e9 100644 --- a/resources/windows_security_policy/locales/en-us.toml +++ b/resources/windows_security_policy/locales/en-us.toml @@ -16,11 +16,59 @@ elevationRequired = "Managing Windows user rights assignments requires elevation [rights] unknownRight = "Unknown user right '%{right}'. Specify a user right constant such as 'SeServiceLogonRight'." +[rights.displayName] +SeTrustedCredManAccessPrivilege = "Access Credential Manager as a trusted caller" +SeNetworkLogonRight = "Access this computer from the network" +SeTcbPrivilege = "Act as part of the operating system" +SeMachineAccountPrivilege = "Add workstations to domain" +SeIncreaseQuotaPrivilege = "Adjust memory quotas for a process" +SeInteractiveLogonRight = "Allow log on locally" +SeRemoteInteractiveLogonRight = "Allow log on through Remote Desktop Services" +SeBackupPrivilege = "Back up files and directories" +SeChangeNotifyPrivilege = "Bypass traverse checking" +SeSystemtimePrivilege = "Change the system time" +SeTimeZonePrivilege = "Change the time zone" +SeCreatePagefilePrivilege = "Create a pagefile" +SeCreateTokenPrivilege = "Create a token object" +SeCreateGlobalPrivilege = "Create global objects" +SeCreatePermanentPrivilege = "Create permanent shared objects" +SeCreateSymbolicLinkPrivilege = "Create symbolic links" +SeDebugPrivilege = "Debug programs" +SeDenyNetworkLogonRight = "Deny access to this computer from the network" +SeDenyBatchLogonRight = "Deny log on as a batch job" +SeDenyServiceLogonRight = "Deny log on as a service" +SeDenyInteractiveLogonRight = "Deny log on locally" +SeDenyRemoteInteractiveLogonRight = "Deny log on through Remote Desktop Services" +SeEnableDelegationPrivilege = "Enable computer and user accounts to be trusted for delegation" +SeRemoteShutdownPrivilege = "Force shutdown from a remote system" +SeAuditPrivilege = "Generate security audits" +SeImpersonatePrivilege = "Impersonate a client after authentication" +SeIncreaseWorkingSetPrivilege = "Increase a process working set" +SeIncreaseBasePriorityPrivilege = "Increase scheduling priority" +SeLoadDriverPrivilege = "Load and unload device drivers" +SeLockMemoryPrivilege = "Lock pages in memory" +SeBatchLogonRight = "Log on as a batch job" +SeServiceLogonRight = "Log on as a service" +SeSecurityPrivilege = "Manage auditing and security log" +SeRelabelPrivilege = "Modify an object label" +SeSystemEnvironmentPrivilege = "Modify firmware environment values" +SeDelegateSessionUserImpersonatePrivilege = "Obtain an impersonation token for another user in the same session" +SeManageVolumePrivilege = "Perform volume maintenance tasks" +SeProfileSingleProcessPrivilege = "Profile single process" +SeSystemProfilePrivilege = "Profile system performance" +SeUndockPrivilege = "Remove computer from docking station" +SeAssignPrimaryTokenPrivilege = "Replace a process level token" +SeRestorePrivilege = "Restore files and directories" +SeShutdownPrivilege = "Shut down the system" +SeSyncAgentPrivilege = "Synchronize directory service data" +SeTakeOwnershipPrivilege = "Take ownership of files or other objects" + [sid] emptyIdentity = "An entry in 'identity' is empty" embeddedNul = "An entry in 'identity' contains an embedded null character" invalidSid = "'%{identity}' is not a valid SID string" unknownAccount = "The account '%{identity}' could not be found. Specify 'DOMAIN\\Name', a well-known account name, or a SID string." +nonCanonical = "'%{identity}' resolves to '%{canonical}'. The resource reports the account as '%{canonical}', so specify that name to make the desired state match the actual state." lookupNameFailed = "Failed to look up the account '%{identity}': %{error}" lookupSidFailed = "Failed to look up the name for a SID: %{error}" sidToStringFailed = "Failed to convert a SID to a string: %{error}" @@ -34,5 +82,5 @@ removeFailed = "Failed to revoke '%{right}': %{error}" [userRights] identityRequired = "'identity' is required for the set and test operations" -whatIfGrant = "Would grant '%{right}' to '%{identity}'" -whatIfRevoke = "Would revoke '%{right}' from '%{identity}'" +whatIfGrant = "Would grant '%{right}' (%{name}) to '%{identity}'" +whatIfRevoke = "Would revoke '%{right}' (%{name}) from '%{identity}'" diff --git a/resources/windows_security_policy/src/main.rs b/resources/windows_security_policy/src/main.rs index 30c778a01..ba18dcd9d 100644 --- a/resources/windows_security_policy/src/main.rs +++ b/resources/windows_security_policy/src/main.rs @@ -29,6 +29,13 @@ fn write_error(message: &str) { eprintln!("{}", serde_json::json!({"error": message})); } +/// Write a JSON informational message to stderr: `{"info":""}`. DSC logs it at the +/// info level. +#[cfg(windows)] +fn write_info(message: &str) { + eprintln!("{}", serde_json::json!({"info": message})); +} + #[cfg(not(windows))] fn main() -> ExitCode { write_error(&t!("main.windowsOnly")); diff --git a/resources/windows_security_policy/src/rights.rs b/resources/windows_security_policy/src/rights.rs index 2b96dc248..8f15cabcb 100644 --- a/resources/windows_security_policy/src/rights.rs +++ b/resources/windows_security_policy/src/rights.rs @@ -3,66 +3,84 @@ //! Static table of the Windows user rights and privileges that can be assigned to accounts. +use rust_i18n::t; +use std::borrow::Cow; + /// A user right or privilege. -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug)] pub struct UserRight { /// The constant that the LSA, `secedit` and Group Policy use, e.g. `SeServiceLogonRight`. pub constant: &'static str, - /// The name shown in the Local Security Policy console, e.g. "Log on as a service". - pub display_name: &'static str, + /// Translates the name shown in the Local Security Policy console. See [`UserRight::display_name`]. + display_name: fn() -> Cow<'static, str>, +} + +impl UserRight { + /// The localized name shown in the Local Security Policy console, e.g. "Log on as a service". + #[must_use] + pub fn display_name(&self) -> Cow<'static, str> { + (self.display_name)() + } } -const fn right(constant: &'static str, display_name: &'static str) -> UserRight { +/// Build a table entry. +/// +/// # Arguments +/// +/// * `constant` - The user right constant. +/// * `display_name` - Calls `t!` with the literal `rights.displayName.` key, so that the +/// i18n tests can match every key in `locales/en-us.toml` to its use. +const fn right(constant: &'static str, display_name: fn() -> Cow<'static, str>) -> UserRight { UserRight { constant, display_name } } -/// Every user right, ordered by display name. This is also the order `export` reports them in. +/// Every user right, ordered by English display name. This is also the order `export` reports them in. pub const USER_RIGHTS: [UserRight; 45] = [ - right("SeTrustedCredManAccessPrivilege", "Access Credential Manager as a trusted caller"), - right("SeNetworkLogonRight", "Access this computer from the network"), - right("SeTcbPrivilege", "Act as part of the operating system"), - right("SeMachineAccountPrivilege", "Add workstations to domain"), - right("SeIncreaseQuotaPrivilege", "Adjust memory quotas for a process"), - right("SeInteractiveLogonRight", "Allow log on locally"), - right("SeRemoteInteractiveLogonRight", "Allow log on through Remote Desktop Services"), - right("SeBackupPrivilege", "Back up files and directories"), - right("SeChangeNotifyPrivilege", "Bypass traverse checking"), - right("SeSystemtimePrivilege", "Change the system time"), - right("SeTimeZonePrivilege", "Change the time zone"), - right("SeCreatePagefilePrivilege", "Create a pagefile"), - right("SeCreateTokenPrivilege", "Create a token object"), - right("SeCreateGlobalPrivilege", "Create global objects"), - right("SeCreatePermanentPrivilege", "Create permanent shared objects"), - right("SeCreateSymbolicLinkPrivilege", "Create symbolic links"), - right("SeDebugPrivilege", "Debug programs"), - right("SeDenyNetworkLogonRight", "Deny access to this computer from the network"), - right("SeDenyBatchLogonRight", "Deny log on as a batch job"), - right("SeDenyServiceLogonRight", "Deny log on as a service"), - right("SeDenyInteractiveLogonRight", "Deny log on locally"), - right("SeDenyRemoteInteractiveLogonRight", "Deny log on through Remote Desktop Services"), - right("SeEnableDelegationPrivilege", "Enable computer and user accounts to be trusted for delegation"), - right("SeRemoteShutdownPrivilege", "Force shutdown from a remote system"), - right("SeAuditPrivilege", "Generate security audits"), - right("SeImpersonatePrivilege", "Impersonate a client after authentication"), - right("SeIncreaseWorkingSetPrivilege", "Increase a process working set"), - right("SeIncreaseBasePriorityPrivilege", "Increase scheduling priority"), - right("SeLoadDriverPrivilege", "Load and unload device drivers"), - right("SeLockMemoryPrivilege", "Lock pages in memory"), - right("SeBatchLogonRight", "Log on as a batch job"), - right("SeServiceLogonRight", "Log on as a service"), - right("SeSecurityPrivilege", "Manage auditing and security log"), - right("SeRelabelPrivilege", "Modify an object label"), - right("SeSystemEnvironmentPrivilege", "Modify firmware environment values"), - right("SeDelegateSessionUserImpersonatePrivilege", "Obtain an impersonation token for another user in the same session"), - right("SeManageVolumePrivilege", "Perform volume maintenance tasks"), - right("SeProfileSingleProcessPrivilege", "Profile single process"), - right("SeSystemProfilePrivilege", "Profile system performance"), - right("SeUndockPrivilege", "Remove computer from docking station"), - right("SeAssignPrimaryTokenPrivilege", "Replace a process level token"), - right("SeRestorePrivilege", "Restore files and directories"), - right("SeShutdownPrivilege", "Shut down the system"), - right("SeSyncAgentPrivilege", "Synchronize directory service data"), - right("SeTakeOwnershipPrivilege", "Take ownership of files or other objects"), + right("SeTrustedCredManAccessPrivilege", || t!("rights.displayName.SeTrustedCredManAccessPrivilege")), + right("SeNetworkLogonRight", || t!("rights.displayName.SeNetworkLogonRight")), + right("SeTcbPrivilege", || t!("rights.displayName.SeTcbPrivilege")), + right("SeMachineAccountPrivilege", || t!("rights.displayName.SeMachineAccountPrivilege")), + right("SeIncreaseQuotaPrivilege", || t!("rights.displayName.SeIncreaseQuotaPrivilege")), + right("SeInteractiveLogonRight", || t!("rights.displayName.SeInteractiveLogonRight")), + right("SeRemoteInteractiveLogonRight", || t!("rights.displayName.SeRemoteInteractiveLogonRight")), + right("SeBackupPrivilege", || t!("rights.displayName.SeBackupPrivilege")), + right("SeChangeNotifyPrivilege", || t!("rights.displayName.SeChangeNotifyPrivilege")), + right("SeSystemtimePrivilege", || t!("rights.displayName.SeSystemtimePrivilege")), + right("SeTimeZonePrivilege", || t!("rights.displayName.SeTimeZonePrivilege")), + right("SeCreatePagefilePrivilege", || t!("rights.displayName.SeCreatePagefilePrivilege")), + right("SeCreateTokenPrivilege", || t!("rights.displayName.SeCreateTokenPrivilege")), + right("SeCreateGlobalPrivilege", || t!("rights.displayName.SeCreateGlobalPrivilege")), + right("SeCreatePermanentPrivilege", || t!("rights.displayName.SeCreatePermanentPrivilege")), + right("SeCreateSymbolicLinkPrivilege", || t!("rights.displayName.SeCreateSymbolicLinkPrivilege")), + right("SeDebugPrivilege", || t!("rights.displayName.SeDebugPrivilege")), + right("SeDenyNetworkLogonRight", || t!("rights.displayName.SeDenyNetworkLogonRight")), + right("SeDenyBatchLogonRight", || t!("rights.displayName.SeDenyBatchLogonRight")), + right("SeDenyServiceLogonRight", || t!("rights.displayName.SeDenyServiceLogonRight")), + right("SeDenyInteractiveLogonRight", || t!("rights.displayName.SeDenyInteractiveLogonRight")), + right("SeDenyRemoteInteractiveLogonRight", || t!("rights.displayName.SeDenyRemoteInteractiveLogonRight")), + right("SeEnableDelegationPrivilege", || t!("rights.displayName.SeEnableDelegationPrivilege")), + right("SeRemoteShutdownPrivilege", || t!("rights.displayName.SeRemoteShutdownPrivilege")), + right("SeAuditPrivilege", || t!("rights.displayName.SeAuditPrivilege")), + right("SeImpersonatePrivilege", || t!("rights.displayName.SeImpersonatePrivilege")), + right("SeIncreaseWorkingSetPrivilege", || t!("rights.displayName.SeIncreaseWorkingSetPrivilege")), + right("SeIncreaseBasePriorityPrivilege", || t!("rights.displayName.SeIncreaseBasePriorityPrivilege")), + right("SeLoadDriverPrivilege", || t!("rights.displayName.SeLoadDriverPrivilege")), + right("SeLockMemoryPrivilege", || t!("rights.displayName.SeLockMemoryPrivilege")), + right("SeBatchLogonRight", || t!("rights.displayName.SeBatchLogonRight")), + right("SeServiceLogonRight", || t!("rights.displayName.SeServiceLogonRight")), + right("SeSecurityPrivilege", || t!("rights.displayName.SeSecurityPrivilege")), + right("SeRelabelPrivilege", || t!("rights.displayName.SeRelabelPrivilege")), + right("SeSystemEnvironmentPrivilege", || t!("rights.displayName.SeSystemEnvironmentPrivilege")), + right("SeDelegateSessionUserImpersonatePrivilege", || t!("rights.displayName.SeDelegateSessionUserImpersonatePrivilege")), + right("SeManageVolumePrivilege", || t!("rights.displayName.SeManageVolumePrivilege")), + right("SeProfileSingleProcessPrivilege", || t!("rights.displayName.SeProfileSingleProcessPrivilege")), + right("SeSystemProfilePrivilege", || t!("rights.displayName.SeSystemProfilePrivilege")), + right("SeUndockPrivilege", || t!("rights.displayName.SeUndockPrivilege")), + right("SeAssignPrimaryTokenPrivilege", || t!("rights.displayName.SeAssignPrimaryTokenPrivilege")), + right("SeRestorePrivilege", || t!("rights.displayName.SeRestorePrivilege")), + right("SeShutdownPrivilege", || t!("rights.displayName.SeShutdownPrivilege")), + right("SeSyncAgentPrivilege", || t!("rights.displayName.SeSyncAgentPrivilege")), + right("SeTakeOwnershipPrivilege", || t!("rights.displayName.SeTakeOwnershipPrivilege")), ]; /// Find a user right by its constant, ignoring case and surrounding whitespace. @@ -82,7 +100,7 @@ mod tests { assert_eq!(USER_RIGHTS.len(), 45); let constants: HashSet = USER_RIGHTS.iter().map(|r| r.constant.to_ascii_lowercase()).collect(); assert_eq!(constants.len(), 45, "constants must be unique"); - let names: HashSet<&str> = USER_RIGHTS.iter().map(|r| r.display_name).collect(); + let names: HashSet = USER_RIGHTS.iter().map(|r| r.display_name().into_owned()).collect(); assert_eq!(names.len(), 45, "display names must be unique"); for entry in &USER_RIGHTS { assert!(entry.constant.starts_with("Se"), "{}", entry.constant); @@ -91,13 +109,22 @@ mod tests { "{}", entry.constant ); - assert!(!entry.display_name.is_empty(), "{}", entry.constant); + let name = entry.display_name(); + assert!(!name.is_empty(), "{}", entry.constant); + assert!(!name.contains("rights.displayName"), "{} has no translation", entry.constant); } } + #[test] + fn display_names_are_translated() { + let right = find("SeServiceLogonRight").unwrap(); + assert_eq!(right.display_name(), t!("rights.displayName.SeServiceLogonRight")); + assert_eq!(right.display_name(), "Log on as a service"); + } + #[test] fn find_ignores_case_and_whitespace() { - assert_eq!(find("SeServiceLogonRight").map(|r| r.display_name), Some("Log on as a service")); + assert_eq!(find("SeServiceLogonRight").map(|r| r.constant), Some("SeServiceLogonRight")); assert_eq!(find(" seservicelogonright ").map(|r| r.constant), Some("SeServiceLogonRight")); assert!(find("SeBogusPrivilege").is_none()); assert!(find("Log on as a service").is_none()); diff --git a/resources/windows_security_policy/src/sid.rs b/resources/windows_security_policy/src/sid.rs index 50eda7202..3a7b200fe 100644 --- a/resources/windows_security_policy/src/sid.rs +++ b/resources/windows_security_policy/src/sid.rs @@ -219,10 +219,13 @@ fn looks_like_sid(text: &str) -> bool { /// # Errors /// /// Returns `InvalidInput` for an empty entry, an entry with an embedded NUL character, a -/// malformed SID string or an unknown account name. +/// malformed SID string (including `*` followed by anything but a SID) or an unknown account name. pub fn resolve_identity(input: &str) -> Result { let trimmed = input.trim(); - let text = trimmed.strip_prefix('*').unwrap_or(trimmed).trim(); + let (text, secedit_form) = match trimmed.strip_prefix('*') { + Some(rest) => (rest.trim(), true), + None => (trimmed, false), + }; if text.is_empty() { return Err(SecurityPolicyError::InvalidInput(t!("sid.emptyIdentity").to_string())); } @@ -233,6 +236,9 @@ pub fn resolve_identity(input: &str) -> Result Result Result, Se let mut identities = Vec::with_capacity(inputs.len()); for input in inputs { let identity = resolve_identity(input)?; + if let Some(message) = non_canonical_message(input, &identity) { + crate::write_info(&message); + } if seen.insert(identity.sid.clone()) { identities.push(identity); } @@ -262,6 +273,18 @@ pub fn resolve_identities(inputs: &[String]) -> Result, Se Ok(identities) } +/// The informational message for an `identity` entry that differs from the name the resource +/// reports for its account, such as `Administrators` or `S-1-5-32-544` for +/// `BUILTIN\Administrators`. `None` when the entry is already canonical. +/// +/// # Arguments +/// +/// * `input` - The `identity` entry as given. +/// * `identity` - The account that `input` resolves to. +fn non_canonical_message(input: &str, identity: &ResolvedIdentity) -> Option { + (input != identity.name).then(|| t!("sid.nonCanonical", identity = input, canonical = identity.name).to_string()) +} + #[cfg(test)] mod tests { use super::*; @@ -311,7 +334,7 @@ mod tests { #[test] fn invalid_identities_are_invalid_input() { - for input in ["", " ", "*", "S-1-X", "S-1-5-32-abc", "NoSuchAccount_zz_12345"] { + for input in ["", " ", "*", "S-1-X", "S-1-5-32-abc", "NoSuchAccount_zz_12345", "*Administrators"] { assert!( matches!(resolve_identity(input), Err(SecurityPolicyError::InvalidInput(_))), "{input:?} should be invalid input" @@ -325,6 +348,8 @@ mod tests { assert_eq!(error, t!("sid.invalidSid", identity = "S-1-5-32-abc")); let error = resolve_identity("S-1-X").unwrap_err().to_string(); assert_eq!(error, t!("sid.unknownAccount", identity = "S-1-X")); + let error = resolve_identity("*Administrators").unwrap_err().to_string(); + assert_eq!(error, t!("sid.invalidSid", identity = "*Administrators"), "'*' marks a SID string, not an account name"); } #[test] @@ -348,6 +373,22 @@ mod tests { assert_eq!(resolved[1].name, "Everyone"); } + #[test] + fn only_non_canonical_entries_produce_a_message() { + let administrators = resolve_identity("BUILTIN\\Administrators").unwrap(); + assert!(non_canonical_message("BUILTIN\\Administrators", &administrators).is_none()); + for input in ["Administrators", "builtin\\administrators", ADMINISTRATORS, "*S-1-5-32-544", " BUILTIN\\Administrators "] { + let expected = t!("sid.nonCanonical", identity = input, canonical = "BUILTIN\\Administrators").to_string(); + assert_eq!(non_canonical_message(input, &administrators), Some(expected), "{input:?}"); + } + + let everyone = resolve_identity("S-1-1-0").unwrap(); + assert!(non_canonical_message("Everyone", &everyone).is_none(), "a well-known name without a domain is canonical"); + + let orphan = "S-1-5-21-1-2-3-4567"; + assert!(non_canonical_message(orphan, &resolve_identity(orphan).unwrap()).is_none(), "an orphaned SID is canonical"); + } + #[test] fn sid_shape_detection() { assert!(looks_like_sid("S-1-5-32-544")); diff --git a/resources/windows_security_policy/src/user_rights.rs b/resources/windows_security_policy/src/user_rights.rs index b231904b7..494acef77 100644 --- a/resources/windows_security_policy/src/user_rights.rs +++ b/resources/windows_security_policy/src/user_rights.rs @@ -81,6 +81,42 @@ pub(crate) fn project(current: &[ResolvedIdentity], grant: &[&ResolvedIdentity], names } +/// The what-if result of `set`: the holders `right` would have once `input.setAction` applies +/// `desired` to `current`, with a `_metadata.whatIf` message for every account that would be +/// granted or revoked. `add` and `remove` keep the holders that aren't listed; only `replace` +/// revokes them. When nothing would change, the result is the current state without metadata. +/// +/// # Arguments +/// +/// * `input` - The requested state; supplies `right` as given and `setAction`. +/// * `right` - The user right that `input.right` names. +/// * `current` - The accounts that hold the right now. +/// * `desired` - The resolved `identity` entries of `input`. +pub(crate) fn preview( + input: &UserRightsAssignment, + right: &UserRight, + current: &[ResolvedIdentity], + desired: &[ResolvedIdentity], +) -> UserRightsAssignment { + let (grant, revoke) = diff(current, desired, input.set_action); + let name = right.display_name(); + let messages = grant + .iter() + .map(|identity| t!("userRights.whatIfGrant", right = right.constant, name = name, identity = identity.name).to_string()) + .chain( + revoke + .iter() + .map(|identity| t!("userRights.whatIfRevoke", right = right.constant, name = name, identity = identity.name).to_string()), + ) + .collect(); + UserRightsAssignment { + right: input.right.clone(), + identity: Some(project(current, &grant, &revoke)), + metadata: Metadata::from_messages(messages), + ..UserRightsAssignment::default() + } +} + /// Get the accounts that hold a right. /// /// # Errors @@ -113,7 +149,7 @@ pub fn test(input: &UserRightsAssignment) -> Result Result return Err(error), }; + if what_if { + return Ok(preview(input, right, ¤t, &desired)); + } + let (grant, revoke) = diff(¤t, &desired, input.set_action); if grant.is_empty() && revoke.is_empty() { return Ok(state(&input.right, ¤t)); } - let mut messages = Vec::with_capacity(grant.len() + revoke.len()); - messages.extend( - grant - .iter() - .map(|identity| t!("userRights.whatIfGrant", right = right.constant, identity = identity.name).to_string()), - ); - messages.extend( - revoke - .iter() - .map(|identity| t!("userRights.whatIfRevoke", right = right.constant, identity = identity.name).to_string()), - ); - if what_if { - return Ok(UserRightsAssignment { - right: input.right.clone(), - identity: Some(project(¤t, &grant, &revoke)), - metadata: Metadata::from_messages(messages), - ..UserRightsAssignment::default() - }); - } - // Grant before revoke so that a logon right never has zero holders part-way through. for identity in &grant { policy.add_right(&identity.sid, right.constant)?; @@ -258,6 +278,71 @@ mod tests { assert_eq!(project(¤t, &[], &[]), ["alpha", "Zed"]); } + fn what_if_request(set_action: SetAction) -> UserRightsAssignment { + UserRightsAssignment { + right: "sedenybatchlogonright".to_string(), + identity: Some(vec!["Guests".to_string(), "Anonymous".to_string()]), + set_action, + ..UserRightsAssignment::default() + } + } + + fn grant_message(right: &UserRight, identity: &str) -> String { + t!("userRights.whatIfGrant", right = right.constant, name = right.display_name(), identity = identity).to_string() + } + + fn revoke_message(right: &UserRight, identity: &str) -> String { + t!("userRights.whatIfRevoke", right = right.constant, name = right.display_name(), identity = identity).to_string() + } + + fn what_if_messages(result: &UserRightsAssignment) -> Vec { + result.metadata.as_ref().and_then(|metadata| metadata.what_if.clone()).unwrap_or_default() + } + + #[test] + fn preview_honours_every_set_action() { + let right = rights::find("SeDenyBatchLogonRight").unwrap(); + let current = [identity("Guests", 1), identity("Users", 2)]; + let desired = [identity("Guests", 1), identity("Anonymous", 3)]; + + let replaced = preview(&what_if_request(SetAction::Replace), right, ¤t, &desired); + assert_eq!(replaced.right, "sedenybatchlogonright", "right is echoed as given"); + assert_eq!(replaced.identity.as_deref(), Some(["Anonymous", "Guests"].map(String::from).as_slice())); + assert_eq!(what_if_messages(&replaced), [grant_message(right, "Anonymous"), revoke_message(right, "Users")]); + + let added = preview(&what_if_request(SetAction::Add), right, ¤t, &desired); + assert_eq!(added.identity.as_deref(), Some(["Anonymous", "Guests", "Users"].map(String::from).as_slice())); + assert_eq!(what_if_messages(&added), [grant_message(right, "Anonymous")]); + + let removed = preview(&what_if_request(SetAction::Remove), right, ¤t, &desired); + assert_eq!(removed.identity.as_deref(), Some(["Users"].map(String::from).as_slice())); + assert_eq!(what_if_messages(&removed), [revoke_message(right, "Guests")]); + + for result in [&replaced, &added, &removed] { + assert!(result.in_desired_state.is_none()); + } + } + + #[test] + fn preview_without_changes_reports_the_current_state_without_metadata() { + let right = rights::find("SeDenyBatchLogonRight").unwrap(); + let current = [identity("Guests", 1), identity("Users", 2)]; + let (present, absent) = ([identity("Guests", 1)], [identity("Anonymous", 3)]); + for (action, desired) in [(SetAction::Replace, ¤t[..]), (SetAction::Add, &present[..]), (SetAction::Remove, &absent[..])] { + let result = preview(&what_if_request(action), right, ¤t, desired); + assert_eq!(result.identity.as_deref(), Some(["Guests", "Users"].map(String::from).as_slice()), "{action:?}"); + assert!(result.metadata.is_none(), "{action:?}"); + } + } + + #[test] + fn what_if_messages_name_the_right_by_constant_and_display_name() { + let right = rights::find("SeServiceLogonRight").unwrap(); + let message = grant_message(right, "NT SERVICE\\ALL SERVICES"); + assert!(message.contains("SeServiceLogonRight"), "{message}"); + assert!(message.contains(right.display_name().as_ref()), "{message}"); + } + #[test] fn operations_follow_the_security_context() { let input = UserRightsAssignment { diff --git a/resources/windows_security_policy/tests/windows_security_policy_user_right_set.tests.ps1 b/resources/windows_security_policy/tests/windows_security_policy_user_right_set.tests.ps1 index 154fef6e8..5471f8a8c 100644 --- a/resources/windows_security_policy/tests/windows_security_policy_user_right_set.tests.ps1 +++ b/resources/windows_security_policy/tests/windows_security_policy_user_right_set.tests.ps1 @@ -151,11 +151,27 @@ Describe 'User rights assignment set tests' -Skip:(!$IsWindows) { Get-Content -Raw $testdrive/error.log | Should -Match 'could not be found' } - It 'Fails for a malformed SID' { - $json = @{ right = $testRight; identity = @('S-1-5-32-abc') } | ConvertTo-Json -Compress + It 'Fails for ' -TestCases @( + @{ kind = 'a malformed SID'; identity = 'S-1-5-32-abc' } + @{ kind = 'an account name in the secedit SID form'; identity = '*Guests' } + ) { + # Called directly: through dsc, the schema rejects the input before the resource runs. + $json = @{ right = $testRight; identity = @($identity) } | ConvertTo-Json -Compress + $null = windows_security_policy set user-right --input $json 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 2 + Get-Content -Raw $testdrive/error.log | Should -Match 'not a valid SID' + } + + It 'Is refused by the resource schema for without changing the policy' -TestCases @( + @{ kind = 'a malformed SID'; identity = 'S-1-5-32-abc' } + @{ kind = 'an account name in the secedit SID form'; identity = '*Guests' } + @{ kind = 'a name with two domain separators'; identity = 'BUILTIN\Guests\Extra' } + ) { + $json = @{ right = $testRight; identity = @($identity, $anonymous) } | ConvertTo-Json -Compress $null = $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log $LASTEXITCODE | Should -Not -Be 0 - Get-Content -Raw $testdrive/error.log | Should -Match 'not a valid SID' + Get-Content -Raw $testdrive/error.log | Should -Match 'oneOf' + @((Get-UserRightState).identity) | Should -Be @($guests) } It 'Rejects an embedded null character in a without changing the policy' -TestCases @( @@ -163,9 +179,13 @@ Describe 'User rights assignment set tests' -Skip:(!$IsWindows) { @{ kind = 'account name'; identity = "Guests`0suffix" } ) { $json = @{ right = $testRight; identity = @($identity, $anonymous) } | ConvertTo-Json -Compress + $null = windows_security_policy set user-right --input $json 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 2 + Get-Content -Raw $testdrive/error.log | Should -Match 'embedded null character' + + # Through dsc, the schema refuses the SID form and the resource refuses the account name. $null = $json | dsc resource set -r $resourceType -f - 2>$testdrive/error.log $LASTEXITCODE | Should -Not -Be 0 - Get-Content -Raw $testdrive/error.log | Should -Match 'embedded null character' @((Get-UserRightState).identity) | Should -Be @($guests) } diff --git a/resources/windows_security_policy/tests/windows_security_policy_user_right_test.tests.ps1 b/resources/windows_security_policy/tests/windows_security_policy_user_right_test.tests.ps1 index 4e43b894b..c45f4ec70 100644 --- a/resources/windows_security_policy/tests/windows_security_policy_user_right_test.tests.ps1 +++ b/resources/windows_security_policy/tests/windows_security_policy_user_right_test.tests.ps1 @@ -96,5 +96,26 @@ Describe 'User rights assignment test tests' -Skip:(!$IsWindows) { $null = windows_security_policy test user-right --input "{`"right`":`"$testRight`"}" 2>$testdrive/error.log $LASTEXITCODE | Should -Be 2 } + + It 'Writes an informational message when is not the canonical name' -TestCases @( + @{ spelling = 'Guests' } + @{ spelling = 'builtin\guests' } + @{ spelling = 'S-1-5-32-546' } + @{ spelling = '*S-1-5-32-546' } + ) { + $json = @{ right = $testRight; identity = @($spelling) } | ConvertTo-Json -Compress + $null = windows_security_policy test user-right --input $json 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + $info = @(Get-Content $testdrive/error.log | ForEach-Object { ($_ | ConvertFrom-Json).info } | Where-Object { $_ }) + $info.Count | Should -Be 1 + $info[0] | Should -Match ([regex]::Escape("'$spelling' resolves to '$guests'")) + } + + It 'Writes no informational message for the canonical name' { + $json = @{ right = $testRight; identity = @($guests) } | ConvertTo-Json -Compress + $null = windows_security_policy test user-right --input $json 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 + Get-Content -Raw $testdrive/error.log | Should -BeNullOrEmpty + } } } diff --git a/resources/windows_security_policy/tests/windows_security_policy_user_right_whatif.tests.ps1 b/resources/windows_security_policy/tests/windows_security_policy_user_right_whatif.tests.ps1 index e77163131..230723401 100644 --- a/resources/windows_security_policy/tests/windows_security_policy_user_right_whatif.tests.ps1 +++ b/resources/windows_security_policy/tests/windows_security_policy_user_right_whatif.tests.ps1 @@ -15,9 +15,18 @@ Describe 'User rights assignment whatif tests' -Skip:(!$IsWindows) { BeforeAll { $testRight = 'SeDenyBatchLogonRight' + $testRightName = 'Deny log on as a batch job' $guests = 'BUILTIN\Guests' $anonymous = 'NT AUTHORITY\ANONYMOUS LOGON' + function Invoke-WhatIf { + param([hashtable]$Properties) + $json = $Properties | ConvertTo-Json -Compress + $out = windows_security_policy set user-right -w --input $json 2>$testdrive/error.log + $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) + return ($out | ConvertFrom-Json) + } + function Get-UserRightState { $out = windows_security_policy get user-right --input "{`"right`":`"$testRight`"}" 2>$testdrive/error.log $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) @@ -35,6 +44,9 @@ Describe 'User rights assignment whatif tests' -Skip:(!$IsWindows) { Context 'What-if' -Skip:(!$isAdmin) { BeforeAll { $script:originalState = Get-UserRightState + } + + BeforeEach { Set-UserRightIdentity -Identity @($guests) } @@ -45,32 +57,51 @@ Describe 'User rights assignment whatif tests' -Skip:(!$IsWindows) { } It 'Projects a grant without applying it' { - $json = @{ right = $testRight; identity = @($guests, $anonymous) } | ConvertTo-Json -Compress - $result = windows_security_policy set user-right -w --input $json 2>$null | ConvertFrom-Json - $LASTEXITCODE | Should -Be 0 + $result = Invoke-WhatIf -Properties @{ right = $testRight; identity = @($guests, $anonymous) } $result.right | Should -BeExactly $testRight @($result.identity) | Should -Be @($guests, $anonymous) - $result._metadata.whatIf | Should -Be @("Would grant '$testRight' to '$anonymous'") + $result._metadata.whatIf | Should -Be @("Would grant '$testRight' ($testRightName) to '$anonymous'") @((Get-UserRightState).identity) | Should -Be @($guests) } It 'Projects a revoke without applying it' { - $json = @{ right = $testRight; identity = @() } | ConvertTo-Json -Compress - $result = windows_security_policy set user-right -w --input $json 2>$null | ConvertFrom-Json - $LASTEXITCODE | Should -Be 0 + $result = Invoke-WhatIf -Properties @{ right = $testRight; identity = @() } @($result.identity).Count | Should -Be 0 - $result._metadata.whatIf | Should -Be @("Would revoke '$testRight' from '$guests'") + $result._metadata.whatIf | Should -Be @("Would revoke '$testRight' ($testRightName) from '$guests'") @((Get-UserRightState).identity) | Should -Be @($guests) } - It 'Returns no whatIf messages when nothing would change' { - $json = @{ right = $testRight; identity = @($guests) } | ConvertTo-Json -Compress - $result = windows_security_policy set user-right -w --input $json 2>$null | ConvertFrom-Json - $LASTEXITCODE | Should -Be 0 + It 'Projects an add that keeps the other holders' { + $result = Invoke-WhatIf -Properties @{ right = $testRight; identity = @($anonymous); setAction = 'add' } + + @($result.identity) | Should -Be @($guests, $anonymous) + $result._metadata.whatIf | Should -Be @("Would grant '$testRight' ($testRightName) to '$anonymous'") + + @((Get-UserRightState).identity) | Should -Be @($guests) + } + + It 'Projects a remove that keeps the other holders' { + Set-UserRightIdentity -Identity @($guests, $anonymous) + + $result = Invoke-WhatIf -Properties @{ right = $testRight; identity = @($guests); setAction = 'remove' } + + @($result.identity) | Should -Be @($anonymous) + $result._metadata.whatIf | Should -Be @("Would revoke '$testRight' ($testRightName) from '$guests'") + + @((Get-UserRightState).identity) | Should -Be @($guests, $anonymous) + } + + It 'Returns the current holders and no whatIf messages when would change nothing' -TestCases @( + @{ setAction = 'replace'; identity = @('BUILTIN\Guests') } + @{ setAction = 'add'; identity = @('BUILTIN\Guests') } + @{ setAction = 'remove'; identity = @('NT AUTHORITY\ANONYMOUS LOGON') } + ) { + $result = Invoke-WhatIf -Properties @{ right = $testRight; identity = $identity; setAction = $setAction } + @($result.identity) | Should -Be @($guests) $result._metadata | Should -BeNullOrEmpty } @@ -82,15 +113,31 @@ Describe 'User rights assignment whatif tests' -Skip:(!$IsWindows) { $LASTEXITCODE | Should -Be 2 } - It 'Works through dsc resource set --what-if' { - $json = @{ right = $testRight; identity = @($anonymous); setAction = 'add' } | ConvertTo-Json -Compress + It 'Works through dsc resource set --what-if with setAction ' -TestCases @( + @{ + setAction = 'add' + holders = @('BUILTIN\Guests') + identity = @('NT AUTHORITY\ANONYMOUS LOGON') + expected = @('BUILTIN\Guests', 'NT AUTHORITY\ANONYMOUS LOGON') + } + @{ + setAction = 'remove' + holders = @('BUILTIN\Guests', 'NT AUTHORITY\ANONYMOUS LOGON') + identity = @('BUILTIN\Guests') + expected = @('NT AUTHORITY\ANONYMOUS LOGON') + } + ) { + Set-UserRightIdentity -Identity $holders + + $json = @{ right = $testRight; identity = $identity; setAction = $setAction } | ConvertTo-Json -Compress $out = $json | dsc resource set -r Microsoft.Windows/UserRightsAssignment -w -f - 2>$testdrive/error.log $LASTEXITCODE | Should -Be 0 -Because (Get-Content -Raw $testdrive/error.log) $result = $out | ConvertFrom-Json - @($result.afterState.identity) | Should -Be @($guests, $anonymous) + @($result.beforeState.identity) | Should -Be $holders + @($result.afterState.identity) | Should -Be $expected $result.afterState._metadata.whatIf | Should -Not -BeNullOrEmpty - @((Get-UserRightState).identity) | Should -Be @($guests) + @((Get-UserRightState).identity) | Should -Be $holders } } } diff --git a/resources/windows_security_policy/windows_security_policy.dsc.manifests.json b/resources/windows_security_policy/windows_security_policy.dsc.manifests.json index d2cce4f91..fff91ec0b 100644 --- a/resources/windows_security_policy/windows_security_policy.dsc.manifests.json +++ b/resources/windows_security_policy/windows_security_policy.dsc.manifests.json @@ -71,9 +71,10 @@ }, "schema": { "embedded": { - "$schema": "http://json-schema.org/draft-07/schema#", + "$schema": "https://json-schema.org/draft/2020-12/schema", "title": "Windows User Rights Assignment", "description": "Manage which accounts hold a Windows user right or privilege.", + "markdownDescription": "The `Microsoft.Windows/UserRightsAssignment` resource manages which accounts hold a Windows user right or privilege, the settings under **Local Policies > User Rights Assignment** in the Local Security Policy console.\n\n[Online documentation][01]\n\n[01]: https://learn.microsoft.com/powershell/dsc/reference/microsoft.windows/userrightsassignment/resource\n", "type": "object", "required": [ "right" @@ -84,6 +85,8 @@ "type": "string", "title": "User right", "description": "The user right or privilege constant, for example 'SeServiceLogonRight' (Log on as a service).", + "markdownDescription": "The constant of the user right or privilege to manage, such as `SeServiceLogonRight` (_Log on as a service_). These are the names that the LSA, `secedit` templates, Group Policy and security benchmarks use. Each instance manages one right.\n\n[Online documentation][01]\n\n[01]: https://learn.microsoft.com/powershell/dsc/reference/microsoft.windows/userrightsassignment/resource#right\n", + "x-dsc-key": true, "enum": [ "SeTrustedCredManAccessPrivilege", "SeNetworkLogonRight", @@ -130,35 +133,123 @@ "SeShutdownPrivilege", "SeSyncAgentPrivilege", "SeTakeOwnershipPrivilege" + ], + "markdownEnumDescriptions": [ + "_Access Credential Manager as a trusted caller_", + "_Access this computer from the network_", + "_Act as part of the operating system_", + "_Add workstations to domain_", + "_Adjust memory quotas for a process_", + "_Allow log on locally_", + "_Allow log on through Remote Desktop Services_", + "_Back up files and directories_", + "_Bypass traverse checking_", + "_Change the system time_", + "_Change the time zone_", + "_Create a pagefile_", + "_Create a token object_", + "_Create global objects_", + "_Create permanent shared objects_", + "_Create symbolic links_", + "_Debug programs_", + "_Deny access to this computer from the network_", + "_Deny log on as a batch job_", + "_Deny log on as a service_", + "_Deny log on locally_", + "_Deny log on through Remote Desktop Services_", + "_Enable computer and user accounts to be trusted for delegation_", + "_Force shutdown from a remote system_", + "_Generate security audits_", + "_Impersonate a client after authentication_", + "_Increase a process working set_", + "_Increase scheduling priority_", + "_Load and unload device drivers_", + "_Lock pages in memory_", + "_Log on as a batch job_", + "_Log on as a service_", + "_Manage auditing and security log_", + "_Modify an object label_", + "_Modify firmware environment values_", + "_Obtain an impersonation token for another user in the same session_", + "_Perform volume maintenance tasks_", + "_Profile single process_", + "_Profile system performance_", + "_Remove computer from docking station_", + "_Replace a process level token_", + "_Restore files and directories_", + "_Shut down the system_", + "_Synchronize directory service data_", + "_Take ownership of files or other objects_" ] }, "identity": { "type": "array", "title": "Identities", "description": "The accounts that hold the right. Accepts 'DOMAIN\\Name', a bare account name, a well-known name such as 'Everyone', a SID string such as 'S-1-5-32-544', or the secedit form '*S-1-5-32-544'. Returned as the canonical 'DOMAIN\\Name', or as the SID string when the SID does not resolve to an account. An empty array means that no one holds the right. Required for set and test.", + "markdownDescription": "The accounts that hold the right. Each entry can be an account name with its domain, a bare account name, a SID string, or a SID string prefixed with `*` as `secedit` exports it.\n\nThe resource reports each holder by its canonical name: `DOMAIN\\Name`, a bare name for accounts without a domain such as `Everyone`, or the SID string when the SID doesn't resolve to an account. Specify the canonical name to make the desired state match the actual state; the resource writes an informational message for any other spelling.\n\nAn empty array means that no one holds the right. Required for **Set** and **Test**.\n\n[Online documentation][01]\n\n[01]: https://learn.microsoft.com/powershell/dsc/reference/microsoft.windows/userrightsassignment/resource#identity\n", + "uniqueItems": true, "items": { "type": "string", - "minLength": 1 - }, - "uniqueItems": true + "title": "Identity", + "description": "An account, as 'DOMAIN\\Name', a bare account name, a SID string or a SID string prefixed with '*'.", + "examples": [ + "BUILTIN\\Administrators", + "NT SERVICE\\ALL SERVICES", + "Everyone", + "S-1-5-32-544", + "*S-1-5-32-544" + ], + "oneOf": [ + { + "title": "Account name with domain", + "description": "An account name qualified with its domain or authority, such as 'BUILTIN\\Administrators' or 'NT SERVICE\\ALL SERVICES'. This is the canonical form for accounts that have a domain.", + "markdownDescription": "An account name qualified with its domain or authority, such as `BUILTIN\\Administrators`, `NT SERVICE\\ALL SERVICES` or `CONTOSO\\svc-sql`. This is the canonical form for accounts that have a domain.", + "pattern": "^[^\\\\*]+\\\\[^\\\\*]+$" + }, + { + "title": "Bare account name", + "description": "An account name without a domain, such as 'Administrators', or a well-known name that has no domain, such as 'Everyone'. Well-known names are localized, so use the name that the target system's language uses.", + "markdownDescription": "An account name without a domain, such as `Administrators`, or a well-known name that has no domain, such as `Everyone`. Well-known names are localized, so use the name that the target system's language uses. The resource reports an account that has a domain as `DOMAIN\\Name`.", + "pattern": "^[^\\\\*]+$", + "not": { + "pattern": "^[Ss]-1-[0-9]" + } + }, + { + "title": "SID string", + "description": "A security identifier, such as 'S-1-5-32-544'. A SID that doesn't resolve to an account is accepted, because Windows keeps rights assigned to deleted accounts.", + "markdownDescription": "A security identifier, such as `S-1-5-32-544`. A SID that doesn't resolve to an account is accepted, because Windows keeps rights assigned to deleted accounts.", + "pattern": "^[Ss]-1-([0-9]+|0[Xx][0-9A-Fa-f]+)(-[0-9]+)*$" + }, + { + "title": "secedit SID", + "description": "A SID string prefixed with '*', the form that 'secedit' exports, such as '*S-1-5-32-544'.", + "markdownDescription": "A SID string prefixed with `*`, the form that `secedit` exports, such as `*S-1-5-32-544`.", + "pattern": "^\\*[Ss]-1-([0-9]+|0[Xx][0-9A-Fa-f]+)(-[0-9]+)*$" + } + ] + } }, "setAction": { "type": "string", "title": "Set action", "description": "How 'identity' is applied: 'replace' makes the listed accounts the only holders of the right, 'add' grants the right to the listed accounts and keeps the other holders, 'remove' revokes the right from the listed accounts only.", + "markdownDescription": "How the **Set** operation applies `identity` and how the **Test** operation judges it. Defaults to `replace`. DSC doesn't compare this write-only property and it never appears in the actual state.\n\n[Online documentation][01]\n\n[01]: https://learn.microsoft.com/powershell/dsc/reference/microsoft.windows/userrightsassignment/resource#setaction\n", "writeOnly": true, "default": "replace", "enum": [ "replace", "add", "remove" + ], + "markdownEnumDescriptions": [ + "The listed accounts become the only holders of the right; every other holder is revoked. An empty `identity` revokes the right from everyone.", + "The listed accounts are granted the right; the other holders are kept.", + "The listed accounts lose their direct assignment of the right; the other holders are kept. Members of a listed group can still hold the right through another group." ] }, "_inDesiredState": { - "type": "boolean", - "title": "In desired state", - "description": "Whether the current holders of the right satisfy 'identity' and 'setAction'. Returned by the test operation only.", - "readOnly": true + "$ref": "https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/v3/resource/properties/inDesiredState.json" }, "_metadata": { "type": "object", @@ -174,6 +265,19 @@ } } } + }, + "$defs": { + "https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/v3/resource/properties/inDesiredState.json": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/v3/resource/properties/inDesiredState.json", + "title": "Instance is in the Desired State", + "description": "Indicates whether the instance is in the desired state. This property is only returned by the `test` method.", + "type": [ + "boolean", + "null" + ], + "readOnly": true + } } } }