Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DISK: Add check for elevated privileges when DevDrive flag is set to true #292

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [6.0.1] - 2024-06-11

### Changed

- Disk:
- Add check for elevated privileges when DevDrive flag is set to true -
Fixes [Issue #284](https://github.com/dsccommunity/StorageDsc/issues/284).

### Fixed

- OpticalDiskDriveLetter:
Expand Down
3 changes: 3 additions & 0 deletions source/DSCResources/DSC_Disk/DSC_Disk.psm1
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,8 @@ function Set-TargetResource
#>
if ($DevDrive)
{
Assert-ElevatedUserWithCustomErrorMessage -CustomErrorMessage $script:localizedData.DevDriveAdminError

Assert-DevDriveFeatureAvailable
Assert-FSFormatIsReFsWhenDevDriveFlagSetToTrue -FSFormat $FSFormat

Expand Down Expand Up @@ -1222,6 +1224,7 @@ function Test-TargetResource
$($script:localizedData.CheckingDevDriveAssertions)
) -join '' )

Assert-ElevatedUserWithCustomErrorMessage -CustomErrorMessage $script:localizedData.DevDriveAdminError
Assert-DevDriveFeatureAvailable
Assert-FSFormatIsReFsWhenDevDriveFlagSetToTrue -FSFormat $FSFormat

Expand Down
31 changes: 16 additions & 15 deletions source/DSCResources/DSC_Disk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,17 @@ table for the disk has been created.
## Dev Drive

The Dev Drive feature is currently available on Windows 11 in builds 10.0.22621.2338
or later. See [the Dev Drive documentation for the latest in formation](https://learn.microsoft.com/en-us/windows/dev-drive/).
or later. See [the Dev Drive documentation for the latest in formation](https://learn.microsoft.com/windows/dev-drive/).

### What is a Dev Drive volume and how is it different from regular volumes?

Dev Drive volumes from a storage perspective are just like regular ReFS volumes
on a Windows machine. The difference However, is that most of the filter drivers
on a Windows machine. The difference however, is that most of the filter drivers
except the antivirus filter will not attach to the volume at boot time by default.
This is a low-level concept that most users will never need to interact with but
for further reading, see the documentation [here](https://learn.microsoft.com/en-us/windows/dev-drive/#how-do-i-configure-additional-filters-on-dev-drive)
for further reading.
for further reading, see the documentation [here](https://learn.microsoft.com/windows/dev-drive/#how-do-i-configure-additional-filters-on-dev-drive)
for further reading. In order to create a Dev Drive your configuration must run with
local administrator permissions or an error will be thrown by the Disk resource.

### What is the default state of the Dev Drive flag in this resource?

Expand All @@ -55,17 +56,17 @@ is `50 Gb`.

### If I have a non Dev Drive volume that is 50 Gb or more can it be reformatted as a Dev Drive volume?

Yes, since the Dev Drive volume is just like any other volume storage wise to the
Yes, since a Dev Drive volume is just like any other volume storage wise to the
Windows operating system, a non Dev Drive ReFS volume can be reformatted as a
Dev Drive volume. An NTFS volume can also be reformatted as a Dev Drive volume.
Note, the Disk resource will throw an exception, should you also attempt to resize
a ReFS volume while attempting to reformat it as a Dev Drive volume since ReFS
volumes cannot be resized. As Dev Drive volumes are also ReFS volumes, they carry
the same restrictions, see: [Resilient File System (ReFS) overview | Microsoft Learn](https://learn.microsoft.com/en-us/windows-server/storage/refs/refs-overview)
Note, the Disk resource will throw an exception, should you attempt to resize
a ReFS volume because ReFS
volumes cannot be resized. Dev Drive volumes are also ReFS volumes, so they carry
the same restrictions see: [Resilient File System (ReFS) overview | Microsoft Learn](https://learn.microsoft.com/windows-server/storage/refs/refs-overview)

### If I don't have any unallocated space available to create a Dev Drive volume, what will happen?

The Disk resource uses the Get-PartitionSupportedSize cmdlet to know which
The Disk resource uses the `Get-PartitionSupportedSize` cmdlet to know which
volume can be be resized to a safe size to create enough unallocated space for
the Dev Drive volume to be created. As long as the size parameter is used, the
Disk resource will shrink the first non ReFS Drive whose (MaxSize - MinSize) is
Expand All @@ -77,7 +78,7 @@ needed, to add to the existing unallocated space so it can be equal to the size
parameter. For example, if you wanted to create a new 50 Gb Dev Drive volume on
disk 0, and let's say on disk 0 there was only a 'C' drive that was 800 Gb in size.
Next to the 'C' drive there was only 40 Gb of free contiguous unallocated space.
The Disk resource would shrink the 'C' drive by 10 Gb, creating an addition 10
The Disk resource would shrink the 'C' drive by 10 Gb, creating an additional 10
Gb of unallocated space. Now the unallocated space would be 50 Gb in size. The
disk resource would then create a new partition and create the Dev Drive volume
into this new partition.
Expand All @@ -98,15 +99,15 @@ There are only five requirements:
> only prevent new Dev Drive volumes from being created. However, this could
> affect the `idempotence` for the Drive. For example, changes to this drive
> after disablement (e.g., reformatting the volume as an NTFS volume) would
> not be corrected by rerunning the configuration. Since the feature is
> disabled, attempting reformat the volume as a Dev Drive volume will throw an
> not be corrected by re-running the configuration. Since the feature is
> disabled, attempting to re-format the volume as a Dev Drive volume will throw an
> error advising you that it is not possible due to the feature being disabled.
1. If the `size` parameter is entered, the value must be greater than or equal to
50 Gb in size. We assert that this is true in order to format a Dev Drive
volume onto a partition.
1. Currently today, if the `size` parameter is not entered then the Disk resource
1. If the `size` parameter is not entered then the Disk resource
will use the maximum space available on the Disk. When the `DevDrive` flag is
set to `$true`, then we assert that the maximum available free unallocated space
set to `$true`. We assert that the maximum available free unallocated contiguous space
on the Disk should be `50 Gb or more in size`. This assertion only comes into
play if the volume doesn't already exist.
1. The `FSformat` parameter must be set to 'ReFS', when the `DevDrive` flag is
Expand Down
1 change: 1 addition & 0 deletions source/DSCResources/DSC_Disk/en-US/DSC_Disk.strings.psd1
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,5 @@
CheckingDevDriveAssertions = Checking system meets requirements for the Dev Drive feature.
TheVolumeIsNotConfiguredAsADevDriveVolume = The volume with path '{0}' and Drive letter '{1}' is not configured as a Dev Drive volume.
TheVolumeIsCurrentlyConfiguredAsADevDriveVolume = The volume with path '{0}' and Drive letter '{1}' is currently configured as a Dev Drive volume.
DevDriveAdminError = Creating a Dev Drive volume requires running local Administrator permissions. Please ensure this resource is being applied with an account with local Administrator permissions.
'@
11 changes: 1 addition & 10 deletions source/DSCResources/DSC_VirtualHardDisk/DSC_VirtualHardDisk.psm1
Original file line number Diff line number Diff line change
Expand Up @@ -109,16 +109,7 @@ function Set-TargetResource

Assert-ParametersValid -FilePath $FilePath -DiskSize $DiskSize -DiskFormat $DiskFormat

try
{
Assert-ElevatedUser
}
catch
{
# Use a user friendly error message specific to the virtual disk dsc resource.
throw $script:localizedData.VirtualDiskAdminError
}

Assert-ElevatedUserWithCustomErrorMessage -CustomErrorMessage $script:localizedData.VirtualDiskAdminError
$currentState = Get-TargetResource -FilePath $FilePath

if ($Ensure -eq 'Present')
Expand Down
33 changes: 32 additions & 1 deletion source/Modules/StorageDsc.Common/StorageDsc.Common.psm1
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,36 @@

}# end function Compare-SizeUsingGB

<#
.SYNOPSIS
Asserts that the configuration is running with local Administrator privileges.

.PARAMETER CustomErrorMessage
A user friendly error message specific to the caller.
#>
function Assert-ElevatedUserWithCustomErrorMessage
{
[CmdletBinding()]
param
(
[Parameter(Mandatory = $true)]
[System.String]
$CustomErrorMessage
)

try
{
Assert-ElevatedUser

Check warning on line 650 in source/Modules/StorageDsc.Common/StorageDsc.Common.psm1

View check run for this annotation

Codecov / codecov/patch

source/Modules/StorageDsc.Common/StorageDsc.Common.psm1#L650

Added line #L650 was not covered by tests

}
catch
{
# Use a user friendly error message specific to the caller
throw $CustomErrorMessage

Check warning on line 656 in source/Modules/StorageDsc.Common/StorageDsc.Common.psm1

View check run for this annotation

Codecov / codecov/patch

source/Modules/StorageDsc.Common/StorageDsc.Common.psm1#L656

Added line #L656 was not covered by tests
}

}# end function Assert-ElevatedUserWithCustomErrorMessage

Export-ModuleMember -Function @(
'Restart-ServiceIfExists',
'Assert-DriveLetterValid',
Expand All @@ -642,5 +672,6 @@
'Get-DevDriveEnablementState',
'Test-DevDriveVolume',
'Invoke-DeviceIoControlWrapperForDevDriveQuery',
'Compare-SizeUsingGB'
'Compare-SizeUsingGB',
'Assert-ElevatedUserWithCustomErrorMessage'
)
126 changes: 126 additions & 0 deletions tests/Unit/DSC_Disk.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,17 @@ try
)
}

function Assert-ElevatedUserWithCustomErrorMessageWithCustomErrorMessage
{
[CmdletBinding()]
param
(
[Parameter(Mandatory = $true)]
[System.String]
$CustomErrorMessage
)
}

Describe 'DSC_Disk\Get-TargetResource' {
Context 'When online GPT disk with a partition/volume and correct Drive Letter assigned using Disk Number' {
# verifiable (should be called) mocks
Expand Down Expand Up @@ -3039,6 +3050,9 @@ try

Context 'When the DevDrive flag is true, the AllowDestructive flag is false and there is not enough space on the disk to create the partition' {
# verifiable (should be called) mocks
Mock `
-CommandName Assert-ElevatedUserWithCustomErrorMessage

Mock `
-CommandName Get-DiskByIdentifier `
-ParameterFilter $script:parameterFilter_MockedDisk0Number `
Expand Down Expand Up @@ -3090,6 +3104,9 @@ try

Context 'When the DevDrive flag is true, AllowDestructive is false and there is enough space on the disk to create the partition' {
# verifiable (should be called) mocks
Mock `
-CommandName Assert-ElevatedUserWithCustomErrorMessage

Mock `
-CommandName Get-DiskByIdentifier `
-ParameterFilter $script:parameterFilter_MockedDisk0Number `
Expand Down Expand Up @@ -3165,6 +3182,9 @@ try

Context 'When the DevDrive flag is true, AllowDestructive flag is false and there is not enough unallocated disk space but a resize of a partition is possible to create new space' {
# verifiable (should be called) mocks
Mock `
-CommandName Assert-ElevatedUserWithCustomErrorMessage

Mock `
-CommandName Get-DiskByIdentifier `
-ParameterFilter $script:parameterFilter_MockedDisk0Number `
Expand Down Expand Up @@ -3214,6 +3234,8 @@ try

Context 'When the DevDrive flag is true, AllowDestructive flag is true and there is not enough unallocated disk space but a resize of a partition is possible to create new space' {
# verifiable (should be called) mocks
Mock `
-CommandName Assert-ElevatedUserWithCustomErrorMessage

$script:amountOfTimesGetDiskByIdentifierIsCalled = 0

Expand Down Expand Up @@ -3313,6 +3335,9 @@ try

Context 'When the DevDrive flag is true, AllowDestructive is true, and a Partition that matches the users drive letter exists' {
# verifiable (should be called) mocks
Mock `
-CommandName Assert-ElevatedUserWithCustomErrorMessage

Mock `
-CommandName Get-DiskByIdentifier `
-ParameterFilter $script:parameterFilter_MockedDisk0Number `
Expand Down Expand Up @@ -3383,6 +3408,9 @@ try

Context 'When the DevDrive flag is true, AllowDestructive is false, and a Partition that matches the users drive letter exists' {
# verifiable (should be called) mocks
Mock `
-CommandName Assert-ElevatedUserWithCustomErrorMessage

Mock `
-CommandName Get-DiskByIdentifier `
-ParameterFilter $script:parameterFilter_MockedDisk0Number `
Expand Down Expand Up @@ -3446,6 +3474,41 @@ try
}
}
}

Context 'When the DevDrive flag is true, but the configuration is not ran with Administrator permissions' {
# verifiable (should be called) mocks
$exception = [System.Exception]::new($script:localizedData.DevDriveAdminError)

Mock `
-CommandName Assert-ElevatedUserWithCustomErrorMessage `
-MockWith { throw [System.Exception]::new($exception.Message)} `
-Verifiable

Mock `
-CommandName Get-DiskByIdentifier `
-ParameterFilter $script:parameterFilter_MockedDisk0Number `
-MockWith { $script:mockedDisk0GptForDevDriveResizeNotNeededScenario } `
-Verifiable

It 'Should throw an exception' {
{
Set-TargetResource `
-DiskId $script:mockedDisk0Gpt.Number `
-Driveletter $script:testDriveLetterT `
-Size $script:mockedPartitionSize50Gb `
-FSLabel 'NewLabel' `
-FSFormat 'ReFS' `
-DevDrive $true `
-Verbose
} | Should -Throw -ExpectedMessage $exception.Message
}

It 'Should call the correct mocks' {
Assert-VerifiableMock
Assert-MockCalled -CommandName Get-DiskByIdentifier -Exactly -Times 1 `
-ParameterFilter $script:parameterFilter_MockedDisk0Number
}
}
}

Describe 'DSC_Disk\Test-TargetResource' {
Expand Down Expand Up @@ -4508,6 +4571,9 @@ try

Context 'When the DevDrive flag is true, and Size parameter is less than minimum required size for Dev Drive (50 Gb)' {
# verifiable (should be called) mocks
Mock `
-CommandName Assert-ElevatedUserWithCustomErrorMessage

Mock `
-CommandName Get-DiskByIdentifier `
-ParameterFilter $script:parameterFilter_MockedDisk0Number `
Expand Down Expand Up @@ -4546,6 +4612,9 @@ try

Context 'When the DevDrive flag is true, but the partition is effectively the same size as user inputted size and volume is NTFS' {
# verifiable (should be called) mocks
Mock `
-CommandName Assert-ElevatedUserWithCustomErrorMessage

Mock `
-CommandName Get-DiskByIdentifier `
-ParameterFilter $script:parameterFilter_MockedDisk0Number `
Expand Down Expand Up @@ -4594,6 +4663,9 @@ try

Context 'When the DevDrive flag is true, but the partition is not the same size as user inputted size, volume is ReFS formatted but not Dev Drive volume' {
# verifiable (should be called) mocks
Mock `
-CommandName Assert-ElevatedUserWithCustomErrorMessage

Mock `
-CommandName Get-DiskByIdentifier `
-ParameterFilter $script:parameterFilter_MockedDisk0Number `
Expand Down Expand Up @@ -4654,6 +4726,9 @@ try

Context 'When the DevDrive flag is true, but the partition is effectively the same size as user inputted size, volume is ReFS formatted and is Dev Drive volume' {
# verifiable (should be called) mocks
Mock `
-CommandName Assert-ElevatedUserWithCustomErrorMessage

Mock `
-CommandName Get-DiskByIdentifier `
-ParameterFilter $script:parameterFilter_MockedDisk0Number `
Expand Down Expand Up @@ -4713,6 +4788,9 @@ try

Context 'When the DevDrive flag is true, but the partition is effectively the same size as user inputted size, volume is ReFS formatted and is not Dev Drive volume' {
# verifiable (should be called) mocks
Mock `
-CommandName Assert-ElevatedUserWithCustomErrorMessage

Mock `
-CommandName Get-DiskByIdentifier `
-ParameterFilter $script:parameterFilter_MockedDisk0Number `
Expand Down Expand Up @@ -4770,6 +4848,54 @@ try
}
}
}

Context 'When the DevDrive flag is true, but the configuration is not run with Administrator permissions' {
# verifiable (should be called) mocks
$exception = [System.Exception]::new($script:localizedData.DevDriveAdminError)

Mock `
-CommandName Assert-ElevatedUserWithCustomErrorMessage `
-MockWith { throw [System.Exception]::new($exception.Message)} `
-Verifiable

Mock `
-CommandName Get-DiskByIdentifier `
-ParameterFilter $script:parameterFilter_MockedDisk0Number `
-MockWith { $script:mockedDisk0Gpt } `
-Verifiable

Mock `
-CommandName Get-Partition `
-MockWith { $script:mockedPartitionGDriveLetterAlternatePartition150Gb } `
-Verifiable

Mock `
-CommandName Get-Volume `
-MockWith { $script:mockedVolumeThatExistPriorToConfigurationRefs150Gb } `
-Verifiable

It 'Should throw an error message that the user should run resource as admin' {
{
$script:result = Test-TargetResource `
-DiskId $script:mockedDisk0Gpt.Number `
-DriveLetter $script:testDriveLetterG `
-AllocationUnitSize 4096 `
-Size $script:userDesiredSize50Gb `
-FSLabel $script:mockedVolume.FileSystemLabel `
-FSFormat $script:mockedVolumeReFS.FileSystem `
-DevDrive $true `
-Verbose
} | Should -Throw -ExpectedMessage $exception.Message
}

It 'Should call the correct mocks' {
Assert-VerifiableMock
Assert-MockCalled -CommandName Get-DiskByIdentifier -Exactly -Times 1 `
-ParameterFilter $script:parameterFilter_MockedDisk0Number
Assert-MockCalled -CommandName Get-Partition -Exactly -Times 1
Assert-MockCalled -CommandName Get-Volume -Exactly -Times 1
}
}
}
}
finally
Expand Down
Loading