From d92255fffc411b25ebba7ef3fe81f66b00da8007 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexey=20ALERT=20Rubash=D1=91ff?= Date: Wed, 26 Aug 2026 19:01:15 +0300 Subject: [PATCH 1/2] fix: hand BitLocker only passwords it will accept Read-StrongPassword had three defects and became two functions. The policy was inline in a prompt loop where no test could reach it. It is Resolve-UnmetPasswordRequirement now, which answers the unmet requirements as data. Two of its five rules rejected nothing: -notmatch is case-insensitive, so [A-Z] matched a lowercase letter. abcdefg1! was accepted as containing an uppercase one. Both use -cnotmatch now, and a password accepted by the old build can be refused by this one. Two more rules were missing entirely. BitLocker refuses anything outside printable ASCII (0x803100A4) and anything past 256 characters (0x803100AA), and the pre-check knew neither, so a password with a Cyrillic letter passed here and was refused by Windows a moment later. Every class is spelled out in ASCII for the same reason: \d and the old negated class both match beyond it. The BSTR from SecureStringToBSTR was never freed, leaving the plaintext in unmanaged memory for the life of the process. It is freed with ZeroFreeBSTR in a finally, so a throw in between cannot skip it. What that erases is the unmanaged copy; $plain is a .NET string and can only be released. Renamed to Request-StrongPassword: it asks the user, which is what Request-* means here. Closes #75 Co-Authored-By: Claude Opus 5 --- README.md | 2 +- dev_drive.Tests.ps1 | 233 ++++++++++++++++++++++++++++++++++++++++++++ dev_drive.ps1 | 82 ++++++++++++---- 3 files changed, 297 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index f3e3557..7ca2beb 100644 --- a/README.md +++ b/README.md @@ -118,7 +118,7 @@ After the drive exists, every setting is read back off the volume and reported - - **On a machine that denies write access to unencrypted fixed drives, BitLocker is not optional.** The setting is `FDVDenyWriteAccess` under `HKLM\SYSTEM\CurrentControlSet\Policies\Microsoft\FVE`. Without encryption the new drive mounts read-only and the run stops at the write check, having already made the partition. The run reads that setting and says so before the BitLocker question - **Windows may open its own encryption prompt** on such a machine while the script is already encrypting. Leave it alone - answering it only produces "BitLocker encryption already enabled" - **A BitLocker failure does not end the run.** It offers retry, carry on without it, or stop, and says what state the drive is in either way. A refusal by group policy is not offered a retry that would meet the same refusal -- **The recovery key is printed once and must be acknowledged.** Outside Entra ID it exists nowhere but on the volume and on the paper you write it on. A password is asked for in virtual hard disk mode only, and must be complex - 8+ characters with upper, lower, digit and special +- **The recovery key is printed once and must be acknowledged.** Outside Entra ID it exists nowhere but on the volume and on the paper you write it on. A password is asked for in virtual hard disk mode only, and must be complex - 8 to 256 printable ASCII characters with upper, lower, digit and special. Every one of those is a refusal BitLocker answers by its own error code, so a password this accepts is not one Windows is known to reject - **Where automatic unlocking cannot be set up** - Windows requires the operating system drive to be BitLocker-protected first - the drive has to be unlocked by hand after every restart - **A `.vhdx` carried to another machine loses its trusted designation.** See [Carrying the file to another machine](#carrying-the-file-to-another-machine) - **Do not run `compact vdisk` against a deduplicated `.vhdx`** without a backup. See [BitLocker and deduplication inside a virtual hard disk](#bitlocker-and-deduplication-inside-a-virtual-hard-disk) diff --git a/dev_drive.Tests.ps1 b/dev_drive.Tests.ps1 index bcaccd1..e5eb66b 100644 --- a/dev_drive.Tests.ps1 +++ b/dev_drive.Tests.ps1 @@ -40,6 +40,33 @@ BeforeAll { if (-not $policyPath.Success) { throw 'cannot find $FixedDriveWritePolicyPath in dev_drive.ps1' } $script:FixedDriveWritePolicyPath = $policyPath.Groups[1].Value + # Read out of the script for the same reason: a literal here would let the two drift, and the + # password tests would then keep passing against a floor the script no longer uses. + $passwordFloor = [regex]::Match((Get-Content -Path $script:ScriptPath -Raw), + "(?m)^\`$PasswordMinLength\s*=\s*(\d+)") + if (-not $passwordFloor.Success) { throw 'cannot find $PasswordMinLength in dev_drive.ps1' } + $script:PasswordFloor = [int]$passwordFloor.Groups[1].Value + + $passwordCeiling = [regex]::Match((Get-Content -Path $script:ScriptPath -Raw), + "(?m)^\`$PasswordMaxLength\s*=\s*(\d+)") + if (-not $passwordCeiling.Success) { throw 'cannot find $PasswordMaxLength in dev_drive.ps1' } + $script:PasswordCeiling = [int]$passwordCeiling.Groups[1].Value + + function Get-ScriptFunction { + <# One named function of dev_drive.ps1 as a syntax-tree node, so a test can ask about the + text of that function alone. A whole-file match prints the whole file when it fails. #> + param([Parameter(Mandatory)][string]$Name) + + $tree = [System.Management.Automation.Language.Parser]::ParseFile($script:ScriptPath, [ref]$null, [ref]$null) + $found = @($tree.FindAll({ + param($node) + $node -is [System.Management.Automation.Language.FunctionDefinitionAst] -and + $node.Name -eq $Name + }, $false)) + if ($found.Count -ne 1) { throw "dev_drive.ps1 defines $($found.Count) functions named $Name" } + return $found[0] + } + function New-PlanAnswerSet { <# A complete, valid answer table for one mode, so a test can change the single field it is about. Mirrors what the body assembles: a key exists only where its question was asked. #> @@ -128,6 +155,90 @@ Describe 'The script itself' { [int]$line.Matches[0].Groups[1].Value | Should -Be 50 } + It 'declares each password length bound exactly once, at ' -TestCases @( + @{ Name = 'the floor'; Variable = 'PasswordMinLength'; Value = 8 } + # BitLocker refuses past 256 with 0x803100AA, so the ceiling is its number, not a choice. + @{ Name = 'the ceiling'; Variable = 'PasswordMaxLength'; Value = 256 } + ) { + $line = Select-String -Path $script:ScriptPath -Pattern "^\`$$Variable\s*=\s*(\d+)" + @($line).Count | Should -Be 1 + [int]$line.Matches[0].Groups[1].Value | Should -Be $Value + } + + It 'builds the password prompt from the bounds it was given, never from a second copy of them' { + $reader = Get-ScriptFunction -Name 'Request-StrongPassword' + # Positive, not a negative against today's wording: a reworded prompt carrying a fresh digit + # would slip past "does not say 8", and so would a parameter default. + $reader.Extent.Text | Should -Match '\$MinimumLength-\$MaximumLength printable ASCII chars' + $reader.Extent.Text | Should -Not -Match '\$M(in|ax)imumLength\s*=\s*\d' + } + + It 'has one password caller, and it passes the constants rather than numbers' { + $ast = [System.Management.Automation.Language.Parser]::ParseFile($script:ScriptPath, [ref]$null, [ref]$null) + $calls = @($ast.FindAll({ + param($node) + $node -is [System.Management.Automation.Language.CommandAst] -and + $node.GetCommandName() -eq 'Request-StrongPassword' + }, $true)) + $calls.Count | Should -Be 1 + # The arguments themselves, so a second caller under any variable name cannot pass a literal. + $passed = @{} + for ($i = 1; $i -lt $calls[0].CommandElements.Count - 1; $i++) { + $element = $calls[0].CommandElements[$i] + if ($element -is [System.Management.Automation.Language.CommandParameterAst]) { + $passed[$element.ParameterName] = $calls[0].CommandElements[$i + 1] + } + } + foreach ($pair in @{ MinimumLength = 'PasswordMinLength'; MaximumLength = 'PasswordMaxLength' }.GetEnumerator()) { + $passed[$pair.Key] | Should -BeOfType [System.Management.Automation.Language.VariableExpressionAst] + $passed[$pair.Key].VariablePath.UserPath | Should -Be $pair.Value + } + } + + It 'frees the unmanaged password copy in a finally, so a throw cannot leave it behind' { + # A ZeroFreeBSTR call sitting after the checks instead of in a finally leaks the plaintext + # on every path that throws in between, which is exactly the defect this replaced. + $ast = [System.Management.Automation.Language.Parser]::ParseFile($script:ScriptPath, [ref]$null, [ref]$null) + $tries = @($ast.FindAll({ + param($node) + $node -is [System.Management.Automation.Language.TryStatementAst] -and + $node.Finally -and + $node.Finally.Extent.Text -match 'ZeroFreeBSTR' + }, $true)) + $tries.Count | Should -Be 1 + # The conversion must sit in the guarded body, not before the try where nothing frees it. + $tries[0].Body.Extent.Text | Should -Match '::PtrToStringBSTR\(' + # And the pointer must be taken outside it: a throw from that call with it inside would hand + # the finally the previous pass's pointer, freeing it twice. + $tries[0].Body.Extent.Text | Should -Not -Match '::SecureStringToBSTR\(' + # The managed copy is released in the same place, so neither survives the loop. + $tries[0].Finally.Extent.Text | Should -Match '\$plain\s*=\s*\$null' + # The call form, and only within the function: the Auto variant reads to the first null + # instead of taking the BSTR length prefix, and a whole-file match would print the file. + $reader = Get-ScriptFunction -Name 'Request-StrongPassword' + $reader.Extent.Text | Should -Not -Match '::PtrToStringAuto\(' + # The refused attempt goes too, and before the loop overwrites the variable holding it. + $text = $reader.Extent.Text + $text.IndexOf('$secure.Dispose()') | Should -BeGreaterThan $text.IndexOf('return $secure') + } + + It 'wraps the requirement check so an empty answer cannot throw under strict mode' { + # Measured: return @() arrives as $null and return @('one') as a bare string, so .Count on + # the bare result throws here. The @() around the call is what makes the count safe. + $ast = [System.Management.Automation.Language.Parser]::ParseFile($script:ScriptPath, [ref]$null, [ref]$null) + $calls = @($ast.FindAll({ + param($node) + $node -is [System.Management.Automation.Language.CommandAst] -and + $node.GetCommandName() -eq 'Resolve-UnmetPasswordRequirement' + }, $true)) + $calls.Count | Should -Be 1 + # The exact chain, not any ancestor: a call nested inside some other array expression + # somewhere up the tree is not the wrap that makes this .Count safe. + $calls[0].Parent | Should -BeOfType [System.Management.Automation.Language.PipelineAst] + $calls[0].Parent.Parent | Should -BeOfType [System.Management.Automation.Language.StatementBlockAst] + $calls[0].Parent.Parent.Parent | Should -BeOfType [System.Management.Automation.Language.ArrayExpressionAst] + } + It 'declares the shrink head-room exactly once' { @(Select-String -Path $script:ScriptPath -Pattern '^\$ShrinkSpareBytes\s*=').Count | Should -Be 1 } @@ -4425,3 +4536,125 @@ Describe 'Get-VolumeWriteState' { $reason | Should -Not -Match 'Exception calling' } } + +Describe 'Resolve-UnmetPasswordRequirement' { + BeforeAll { + # The floor and ceiling are read out of the script in the file-level BeforeAll, so the + # fixtures that are about length are built from them rather than from numbers of their own. + $script:AtFloor = 'Ab1!' + ('c' * [math]::Max(0, $script:PasswordFloor - 4)) + $script:BelowFloor = $script:AtFloor.Substring(0, $script:AtFloor.Length - 1) + $script:FloorMessage = "at least $script:PasswordFloor characters" + $script:CeilingMessage = "at most $script:PasswordCeiling characters" + + function Get-Unmet { + <# The call every test makes, so the two length arguments are not repeated in each. It + unrolls like any function, so a caller wanting .Count wraps it in @() as the script + does - the array a return statement carries out of a function is not one. #> + param([Parameter(Mandatory)][AllowEmptyString()][string]$Plain) + + return @(Resolve-UnmetPasswordRequirement -Plain $Plain ` + -MinimumLength $script:PasswordFloor -MaximumLength $script:PasswordCeiling) + } + } + + It 'answers nothing for a password that meets every rule' { + @(Get-Unmet -Plain $script:AtFloor).Count | Should -Be 0 + } + + It 'names the length requirement one character below the floor, and not at it' { + Get-Unmet -Plain $script:BelowFloor | Should -Contain $script:FloorMessage + Get-Unmet -Plain $script:AtFloor | Should -Not -Contain $script:FloorMessage + } + + It 'names the ceiling one character above it, and not at it' { + # BitLocker answers 0x803100AA past its ceiling, so a longer password is refused here first. + $atCeiling = 'Ab1!' + ('c' * ($script:PasswordCeiling - 4)) + Get-Unmet -Plain $atCeiling | Should -Not -Contain $script:CeilingMessage + Get-Unmet -Plain ($atCeiling + 'c') | Should -Contain $script:CeilingMessage + } + + It 'refuses an all-lowercase password for want of an uppercase letter' { + # -notmatch would accept this: it is case-insensitive, so [A-Z] matches a lowercase letter. + Get-Unmet -Plain 'abcdefg1!' | Should -Contain 'at least one uppercase letter' + } + + It 'refuses an all-uppercase password for want of a lowercase letter' { + Get-Unmet -Plain 'ABCDEFG1!' | Should -Contain 'at least one lowercase letter' + } + + It 'names the digit requirement when there is no digit' { + Get-Unmet -Plain 'Abcdefgh!' | Should -Contain 'at least one digit' + } + + It 'names the special-character requirement when every character is alphanumeric' { + Get-Unmet -Plain 'Abcdefg12' | Should -Contain 'at least one special character' + } + + It 'does not accept a space as the special character' { + Get-Unmet -Plain 'Abcdefg1 ' | Should -Contain 'at least one special character' + } + + It 'accepts a space inside an otherwise valid password' { + # Space is printable ASCII, so only the special-character rule refuses to count it. + @(Get-Unmet -Plain 'Ab 1!cde').Count | Should -Be 0 + } + + It 'refuses a password Windows would refuse as non-ASCII, for ' -TestCases @( + @{ Name = 'a Cyrillic capital'; Code = 0x0416 } + @{ Name = 'an Arabic-Indic digit'; Code = 0x0665 } + @{ Name = 'an accented Latin letter'; Code = 0x00E9 } + ) { + # Built from code points so this file stays ASCII. Each of these satisfied the old negated + # class or \d and was then refused by BitLocker as non-printable ASCII (0x803100A4). + $unmet = @(Get-Unmet -Plain ('Abcdefg1!' + [char]$Code)) + $unmet.Count | Should -Be 1 + $unmet | Should -Contain 'printable ASCII characters only' + } + + It 'refuses a control character, which is ASCII but not printable' { + Get-Unmet -Plain ('Abcdefg1!' + [char]9) | Should -Contain 'printable ASCII characters only' + } + + It 'does not count a non-ASCII digit as the digit' { + # \d is Unicode in .NET, so it would take an Arabic-Indic five and call the rule met while + # BitLocker refuses the password outright. The class is [0-9] for that reason. + $unmet = @(Get-Unmet -Plain ('Abcdefg!' + [char]0x0665)) + $unmet | Should -Contain 'at least one digit' + $unmet | Should -Contain 'printable ASCII characters only' + } + + It 'counts only ASCII punctuation as the special character' { + # The old class counted any non-Latin letter; this one is the printable ASCII range less + # space and alphanumerics, so a Cyrillic capital no longer stands in for punctuation. + $unmet = @(Get-Unmet -Plain ('Abcdefg1' + [char]0x0416)) + $unmet | Should -Contain 'at least one special character' + } + + It 'names every unmet requirement at once rather than the first' { + @(Get-Unmet -Plain '').Count | Should -Be 5 + } + + It 'answers in the order the prompt lists the requirements' { + Get-Unmet -Plain '' | Should -Be @( + $script:FloorMessage + 'at least one uppercase letter' + 'at least one lowercase letter' + 'at least one digit' + 'at least one special character' + ) + } + + It 'takes the lengths from its arguments rather than from literals, for ' -TestCases @( + @{ Floor = 4; Ceiling = 64; Plain = 'Ab1!'; Met = $true } + @{ Floor = 12; Ceiling = 64; Plain = 'Abcdefg1!'; Met = $false; Message = 'at least 12 characters' } + @{ Floor = 4; Ceiling = 8; Plain = 'Abcdefg1!'; Met = $false; Message = 'at most 8 characters' } + ) { + $unmet = @(Resolve-UnmetPasswordRequirement -Plain $Plain -MinimumLength $Floor -MaximumLength $Ceiling) + if ($Met) { + $unmet.Count | Should -Be 0 + } else { + $unmet | Should -Contain $Message + } + } +} + diff --git a/dev_drive.ps1 b/dev_drive.ps1 index 94c8432..3d7e180 100644 --- a/dev_drive.ps1 +++ b/dev_drive.ps1 @@ -1918,29 +1918,66 @@ function Request-DevDriveSizeGB { } } -function Read-StrongPassword { - while ($true) { - $secure = Read-Host "Enter password (min 8 chars, incl. upper, lower, digit, special)" -AsSecureString - $plain = [Runtime.InteropServices.Marshal]::PtrToStringAuto( - [Runtime.InteropServices.Marshal]::SecureStringToBSTR($secure) - ) +function Resolve-UnmetPasswordRequirement { + <# Answers the requirements this password does not meet, in the order the prompt lists them, and + nothing when it meets them all. Every rule mirrors one BitLocker refuses by its own error + code, so nothing obviously refusable is handed over; its policy may still ask for more. #> + param( + [Parameter(Mandatory)][AllowEmptyString()][string]$Plain, + [Parameter(Mandatory)][int]$MinimumLength, + [Parameter(Mandatory)][int]$MaximumLength + ) - # Build validation flags - $errors = @() - if ($plain.Length -lt 8) { $errors += "at least 8 characters" } - if ($plain -notmatch '[A-Z]') { $errors += "at least one uppercase letter" } - if ($plain -notmatch '[a-z]') { $errors += "at least one lowercase letter" } - if ($plain -notmatch '\d') { $errors += "at least one digit" } - if ($plain -notmatch '[^a-zA-Z\d\s]') { $errors += "at least one special character" } + # Every class is spelled out in ASCII rather than left to \d or a negated class, because both of + # those match beyond ASCII and BitLocker refuses anything outside printable ASCII (0x803100A4). + $unmet = @() + if ($Plain.Length -lt $MinimumLength) { $unmet += "at least $MinimumLength characters" } + if ($Plain.Length -gt $MaximumLength) { $unmet += "at most $MaximumLength characters" } + if ($Plain -cnotmatch '\A[\x20-\x7E]*\z') { $unmet += "printable ASCII characters only" } + # -cnotmatch, because the case-insensitive form lets [A-Z] match a lowercase letter and vice versa. + if ($Plain -cnotmatch '[A-Z]') { $unmet += "at least one uppercase letter" } + if ($Plain -cnotmatch '[a-z]') { $unmet += "at least one lowercase letter" } + if ($Plain -cnotmatch '[0-9]') { $unmet += "at least one digit" } + # Printable ASCII less space and alphanumerics, which is what is left to be a special character. + if ($Plain -cnotmatch '[\x21-\x2F\x3A-\x40\x5B-\x60\x7B-\x7E]') { $unmet += "at least one special character" } + return $unmet +} + +function Request-StrongPassword { + <# Prompts until the password meets every requirement, then answers it as a SecureString. The + pointer is freed in a finally: a throw in between would leave the plaintext in unmanaged + memory for the life of the process. #> + param( + [Parameter(Mandatory)][int]$MinimumLength, + [Parameter(Mandatory)][int]$MaximumLength + ) - if ($errors.Count -eq 0) { - return $secure # All good + while ($true) { + $secure = Read-Host "Enter password ($MinimumLength-$MaximumLength printable ASCII chars, incl. upper, lower, digit, special)" -AsSecureString + # Outside the try: inside it, a throw from this call would hand the finally a stale pointer. + $bstr = [Runtime.InteropServices.Marshal]::SecureStringToBSTR($secure) + try { + # PtrToStringBSTR takes the length from the BSTR prefix; the Auto form stops at a null. + $plain = [Runtime.InteropServices.Marshal]::PtrToStringBSTR($bstr) + # @(): an empty array arrives from a function as $null, and .Count on it throws here. + $unmet = @(Resolve-UnmetPasswordRequirement -Plain $plain ` + -MinimumLength $MinimumLength -MaximumLength $MaximumLength) + } + finally { + # Erases the unmanaged copy. $plain is a .NET string, so it can only be released. + [Runtime.InteropServices.Marshal]::ZeroFreeBSTR($bstr) + $plain = $null + } + + if ($unmet.Count -eq 0) { + return $secure } - # Output errors + # The refused attempt is encrypted rather than plaintext, but nothing needs it after this. + $secure.Dispose() Write-Host "Password does not meet the following requirement(s):" -ForegroundColor Red - foreach ($e in $errors) { - Write-Host " - $e" -ForegroundColor Yellow + foreach ($requirement in $unmet) { + Write-Host " - $requirement" -ForegroundColor Yellow } } } @@ -2403,6 +2440,12 @@ $DevDriveDefaultLabel = "DevDrive" # Format-Volume -NewFileSystemLabel, so there is nothing to cite and nothing to read off a volume. $DevDriveLabelMaxLength = 32 +# This script's own floor for the BitLocker password; Windows applies its own policy afterwards. +$PasswordMinLength = 8 + +# BitLocker's own ceiling, from the refusal it answers past it: 0x803100AA, "over 256 characters". +$PasswordMaxLength = 256 + # Head-room kept on the volume hosting a fixed size .vhdx, so it is never filled to the last byte. $VhdxHostSpareBytes = 1GB @@ -2948,7 +2991,8 @@ try { if ($bitLockerPlan.UsePasswordProtector -and $protectorPlan.TypesToAdd -contains 'Password') { Write-Host "Enter BitLocker password for the new volume. It must be a complex one." -ForegroundColor Yellow - $SecurePassword = Read-StrongPassword + $SecurePassword = Request-StrongPassword -MinimumLength $PasswordMinLength ` + -MaximumLength $PasswordMaxLength Write-Host "Adding BitLockerKeyProtector PasswordProtector" -ForegroundColor Green Add-BitLockerKeyProtector -MountPoint $devLetterColon -PasswordProtector -Password $SecurePassword -ErrorAction Stop } From 53fd653a148476dfe1ed294850b352705f34e96b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexey=20ALERT=20Rubash=D1=91ff?= Date: Wed, 26 Aug 2026 19:41:06 +0300 Subject: [PATCH 2/2] fix: stop the rule copying the password, and say whose rules these are From an independent review of the pull request. The printable-ASCII rule tested its pattern with an operator, and an operator copies what it matched into $Matches. That pattern captures the whole password, so checking it left a second plaintext copy behind - in a change whose point is not leaving one. Every pattern rule uses [regex]::IsMatch now, which touches nothing and is case-sensitive by default, so the defect this issue started from cannot come back by forgetting a letter. The BitLocker retry loop asked for a password again without disposing the one BitLocker had refused, so up to ten SecureStrings stayed alive for the rest of the run. The prompt loop was already doing this; the outer one was not. Two claims were overstated and are corrected. Only the ceiling and the ASCII rule are BitLocker's own refusals; the floor and the four character classes are this script's, and are stricter than BitLocker's default policy, which asks only for a length unless complexity is configured. So a long passphrase Windows would have taken is refused here. The docstring and README say that now, in both directions. Tests: the disposal assertion compared text offsets, where a reworded return would have made IndexOf answer -1 and the comparison pass on nothing; it asks the tree for statement positions instead. Two test cases rendered the same name. The all-spaces case the issue named was never covered. Three copies of the read-a-constant-out-of-the-script block became Get-ScriptConstant, and the new syntax-tree tests share the tree the harness already parsed. Co-Authored-By: Claude Opus 5 --- README.md | 2 +- dev_drive.Tests.ps1 | 116 +++++++++++++++++++++++++++++++------------- dev_drive.ps1 | 29 ++++++----- 3 files changed, 100 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index 7ca2beb..09a9972 100644 --- a/README.md +++ b/README.md @@ -118,7 +118,7 @@ After the drive exists, every setting is read back off the volume and reported - - **On a machine that denies write access to unencrypted fixed drives, BitLocker is not optional.** The setting is `FDVDenyWriteAccess` under `HKLM\SYSTEM\CurrentControlSet\Policies\Microsoft\FVE`. Without encryption the new drive mounts read-only and the run stops at the write check, having already made the partition. The run reads that setting and says so before the BitLocker question - **Windows may open its own encryption prompt** on such a machine while the script is already encrypting. Leave it alone - answering it only produces "BitLocker encryption already enabled" - **A BitLocker failure does not end the run.** It offers retry, carry on without it, or stop, and says what state the drive is in either way. A refusal by group policy is not offered a retry that would meet the same refusal -- **The recovery key is printed once and must be acknowledged.** Outside Entra ID it exists nowhere but on the volume and on the paper you write it on. A password is asked for in virtual hard disk mode only, and must be complex - 8 to 256 printable ASCII characters with upper, lower, digit and special. Every one of those is a refusal BitLocker answers by its own error code, so a password this accepts is not one Windows is known to reject +- **The recovery key is printed once and must be acknowledged.** Outside Entra ID it exists nowhere but on the volume and on the paper you write it on. A password is asked for in virtual hard disk mode only, and must be complex - 8 to 256 printable ASCII characters with upper, lower, digit and special. The ceiling and the ASCII rule are refusals BitLocker answers by its own error code; the floor and the four character classes are this script's own and are stricter than BitLocker's default, so a long passphrase it would have taken is refused here, and group policy can still refuse one this accepts - **Where automatic unlocking cannot be set up** - Windows requires the operating system drive to be BitLocker-protected first - the drive has to be unlocked by hand after every restart - **A `.vhdx` carried to another machine loses its trusted designation.** See [Carrying the file to another machine](#carrying-the-file-to-another-machine) - **Do not run `compact vdisk` against a deduplicated `.vhdx`** without a backup. See [BitLocker and deduplication inside a virtual hard disk](#bitlocker-and-deduplication-inside-a-virtual-hard-disk) diff --git a/dev_drive.Tests.ps1 b/dev_drive.Tests.ps1 index e5eb66b..4776a86 100644 --- a/dev_drive.Tests.ps1 +++ b/dev_drive.Tests.ps1 @@ -18,10 +18,12 @@ BeforeAll { # dev_drive.ps1 is a linear script that starts asking questions when it runs, so its functions # are lifted out of the syntax tree instead of dot-sourcing the file. $parseErrors = $null - $ast = [System.Management.Automation.Language.Parser]::ParseFile($script:ScriptPath, [ref]$null, [ref]$parseErrors) + # Kept for the tests that ask the tree questions, so none of them parses the file again. + $script:Ast = [System.Management.Automation.Language.Parser]::ParseFile($script:ScriptPath, [ref]$null, [ref]$parseErrors) if ($parseErrors) { throw "dev_drive.ps1 does not parse: $($parseErrors[0].Message)" } + $ast = $script:Ast $functions = $ast.FindAll({ param($node) $node -is [System.Management.Automation.Language.FunctionDefinitionAst] }, $false) foreach ($function in $functions) { @@ -33,32 +35,29 @@ BeforeAll { # Where the body prints what the plan function decided; several source-order tests anchor on it. $script:PlanPrintLoop = 'foreach ($planLine in (Format-CreationPlan' - # Lifted functions do not bring the body's constants; the write-access advice defaults to this - # one. Read out of the script so the two cannot drift, which a second literal here would allow. - $policyPath = [regex]::Match((Get-Content -Path $script:ScriptPath -Raw), - "(?m)^\`$FixedDriveWritePolicyPath\s*=\s*'([^']+)'") - if (-not $policyPath.Success) { throw 'cannot find $FixedDriveWritePolicyPath in dev_drive.ps1' } - $script:FixedDriveWritePolicyPath = $policyPath.Groups[1].Value - - # Read out of the script for the same reason: a literal here would let the two drift, and the - # password tests would then keep passing against a floor the script no longer uses. - $passwordFloor = [regex]::Match((Get-Content -Path $script:ScriptPath -Raw), - "(?m)^\`$PasswordMinLength\s*=\s*(\d+)") - if (-not $passwordFloor.Success) { throw 'cannot find $PasswordMinLength in dev_drive.ps1' } - $script:PasswordFloor = [int]$passwordFloor.Groups[1].Value - - $passwordCeiling = [regex]::Match((Get-Content -Path $script:ScriptPath -Raw), - "(?m)^\`$PasswordMaxLength\s*=\s*(\d+)") - if (-not $passwordCeiling.Success) { throw 'cannot find $PasswordMaxLength in dev_drive.ps1' } - $script:PasswordCeiling = [int]$passwordCeiling.Groups[1].Value + $script:ScriptText = Get-Content -Path $script:ScriptPath -Raw + + function Get-ScriptConstant { + <# The value the body assigns to one top-level constant, so no test carries a copy of it. + Pattern is the capture for the value; a constant that has moved or gone throws here. #> + param([Parameter(Mandatory)][string]$Name, [Parameter(Mandatory)][string]$Pattern) + + $found = [regex]::Match($script:ScriptText, "(?m)^\`$$Name\s*=\s*$Pattern") + if (-not $found.Success) { throw "cannot find `$$Name in dev_drive.ps1" } + return $found.Groups[1].Value + } + + # Lifted functions do not bring the body's constants, and a literal here would let the two drift. + $script:FixedDriveWritePolicyPath = Get-ScriptConstant -Name 'FixedDriveWritePolicyPath' -Pattern "'([^']+)'" + $script:PasswordFloor = [int](Get-ScriptConstant -Name 'PasswordMinLength' -Pattern '(\d+)') + $script:PasswordCeiling = [int](Get-ScriptConstant -Name 'PasswordMaxLength' -Pattern '(\d+)') function Get-ScriptFunction { <# One named function of dev_drive.ps1 as a syntax-tree node, so a test can ask about the text of that function alone. A whole-file match prints the whole file when it fails. #> param([Parameter(Mandatory)][string]$Name) - $tree = [System.Management.Automation.Language.Parser]::ParseFile($script:ScriptPath, [ref]$null, [ref]$null) - $found = @($tree.FindAll({ + $found = @($script:Ast.FindAll({ param($node) $node -is [System.Management.Automation.Language.FunctionDefinitionAst] -and $node.Name -eq $Name @@ -165,7 +164,7 @@ Describe 'The script itself' { [int]$line.Matches[0].Groups[1].Value | Should -Be $Value } - It 'builds the password prompt from the bounds it was given, never from a second copy of them' { + It 'writes the password prompt with the bounds interpolated, never with numbers of its own' { $reader = Get-ScriptFunction -Name 'Request-StrongPassword' # Positive, not a negative against today's wording: a reworded prompt carrying a fresh digit # would slip past "does not say 8", and so would a parameter default. @@ -174,7 +173,8 @@ Describe 'The script itself' { } It 'has one password caller, and it passes the constants rather than numbers' { - $ast = [System.Management.Automation.Language.Parser]::ParseFile($script:ScriptPath, [ref]$null, [ref]$null) + # The tree the file-level BeforeAll already parsed, rather than a fourth parse of the file. + $ast = $script:Ast $calls = @($ast.FindAll({ param($node) $node -is [System.Management.Automation.Language.CommandAst] -and @@ -198,7 +198,8 @@ Describe 'The script itself' { It 'frees the unmanaged password copy in a finally, so a throw cannot leave it behind' { # A ZeroFreeBSTR call sitting after the checks instead of in a finally leaks the plaintext # on every path that throws in between, which is exactly the defect this replaced. - $ast = [System.Management.Automation.Language.Parser]::ParseFile($script:ScriptPath, [ref]$null, [ref]$null) + # The tree the file-level BeforeAll already parsed, rather than a fourth parse of the file. + $ast = $script:Ast $tries = @($ast.FindAll({ param($node) $node -is [System.Management.Automation.Language.TryStatementAst] -and @@ -217,15 +218,54 @@ Describe 'The script itself' { # instead of taking the BSTR length prefix, and a whole-file match would print the file. $reader = Get-ScriptFunction -Name 'Request-StrongPassword' $reader.Extent.Text | Should -Not -Match '::PtrToStringAuto\(' - # The refused attempt goes too, and before the loop overwrites the variable holding it. - $text = $reader.Extent.Text - $text.IndexOf('$secure.Dispose()') | Should -BeGreaterThan $text.IndexOf('return $secure') + # The rule function tests none of its patterns with an operator: -match and -notmatch are + # case-insensitive, which is the original defect, and they copy what matched into $Matches - + # for the ASCII pattern that copy is the whole password. + $rule = Get-ScriptFunction -Name 'Resolve-UnmetPasswordRequirement' + # One per pattern rule: printable ASCII, uppercase, lowercase, digit, special. + @([regex]::Matches($rule.Extent.Text, '\[regex\]::IsMatch\(')).Count | Should -Be 5 + # The operators by node, not by text: the comment above them names the ones being avoided. + @($rule.FindAll({ + param($node) + $node -is [System.Management.Automation.Language.BinaryExpressionAst] -and + $node.Operator.ToString() -match '^(I|C)not?match$' + }, $true)).Count | Should -Be 0 + } + + It 'disposes the refused attempt inside the loop, after the accepted one has left' { + # By position in the loop body, not by text offset: an IndexOf that finds nothing answers + # -1, and -1 is less than any real offset, so a reworded return would pass this silently. + $loop = @((Get-ScriptFunction -Name 'Request-StrongPassword').FindAll({ + param($node) $node -is [System.Management.Automation.Language.WhileStatementAst] + }, $true)) + $loop.Count | Should -Be 1 + $statements = @($loop[0].Body.Statements) + $disposeAt = [array]::FindIndex($statements, [Predicate[object]] { $args[0].Extent.Text -match '\$secure\.Dispose\(\)' }) + $returnAt = [array]::FindIndex($statements, [Predicate[object]] { $args[0].Extent.Text -match 'return \$secure' }) + $disposeAt | Should -BeGreaterThan 0 + $returnAt | Should -BeGreaterThan 0 + $disposeAt | Should -BeGreaterThan $returnAt + } + + It 'disposes the attempt BitLocker refused before the retry asks for another' { + # The prompt loop is not the whole story: the BitLocker loop comes back to the same line and + # would otherwise leave one SecureString per attempt alive for the rest of the run. + $calls = @($script:Ast.FindAll({ + param($node) + $node -is [System.Management.Automation.Language.CommandAst] -and + $node.GetCommandName() -eq 'Request-StrongPassword' + }, $true)) + $calls.Count | Should -Be 1 + $before = $script:ScriptText.Substring(0, $calls[0].Extent.StartOffset) + $before.LastIndexOf('$SecurePassword.Dispose()') | + Should -BeGreaterThan $before.LastIndexOf('$SecurePassword = $null') } It 'wraps the requirement check so an empty answer cannot throw under strict mode' { # Measured: return @() arrives as $null and return @('one') as a bare string, so .Count on # the bare result throws here. The @() around the call is what makes the count safe. - $ast = [System.Management.Automation.Language.Parser]::ParseFile($script:ScriptPath, [ref]$null, [ref]$null) + # The tree the file-level BeforeAll already parsed, rather than a fourth parse of the file. + $ast = $script:Ast $calls = @($ast.FindAll({ param($node) $node -is [System.Management.Automation.Language.CommandAst] -and @@ -4594,6 +4634,15 @@ Describe 'Resolve-UnmetPasswordRequirement' { Get-Unmet -Plain 'Abcdefg1 ' | Should -Contain 'at least one special character' } + It 'refuses a password of nothing but spaces on all four class rules and no other' { + # Named in the issue as untested. Spaces are printable ASCII and reach the floor, so this is + # the one input where every class rule fires while the length and ASCII rules pass. + $unmet = @(Get-Unmet -Plain (' ' * $script:PasswordFloor)) + $unmet.Count | Should -Be 4 + $unmet | Should -Not -Contain $script:FloorMessage + $unmet | Should -Not -Contain 'printable ASCII characters only' + } + It 'accepts a space inside an otherwise valid password' { # Space is printable ASCII, so only the special-character rule refuses to count it. @(Get-Unmet -Plain 'Ab 1!cde').Count | Should -Be 0 @@ -4644,10 +4693,12 @@ Describe 'Resolve-UnmetPasswordRequirement' { ) } - It 'takes the lengths from its arguments rather than from literals, for ' -TestCases @( - @{ Floor = 4; Ceiling = 64; Plain = 'Ab1!'; Met = $true } - @{ Floor = 12; Ceiling = 64; Plain = 'Abcdefg1!'; Met = $false; Message = 'at least 12 characters' } - @{ Floor = 4; Ceiling = 8; Plain = 'Abcdefg1!'; Met = $false; Message = 'at most 8 characters' } + It 'takes the lengths from its arguments rather than from literals, ' -TestCases @( + @{ Name = 'inside both'; Floor = 4; Ceiling = 64; Plain = 'Ab1!'; Met = $true } + @{ Name = 'under a raised floor'; Floor = 12; Ceiling = 64; Plain = 'Abcdefg1!'; Met = $false + Message = 'at least 12 characters' } + @{ Name = 'over a lowered ceiling'; Floor = 4; Ceiling = 8; Plain = 'Abcdefg1!'; Met = $false + Message = 'at most 8 characters' } ) { $unmet = @(Resolve-UnmetPasswordRequirement -Plain $Plain -MinimumLength $Floor -MaximumLength $Ceiling) if ($Met) { @@ -4657,4 +4708,3 @@ Describe 'Resolve-UnmetPasswordRequirement' { } } } - diff --git a/dev_drive.ps1 b/dev_drive.ps1 index 3d7e180..fd6e7b9 100644 --- a/dev_drive.ps1 +++ b/dev_drive.ps1 @@ -1920,26 +1920,27 @@ function Request-DevDriveSizeGB { function Resolve-UnmetPasswordRequirement { <# Answers the requirements this password does not meet, in the order the prompt lists them, and - nothing when it meets them all. Every rule mirrors one BitLocker refuses by its own error - code, so nothing obviously refusable is handed over; its policy may still ask for more. #> + nothing when it meets them all. The ceiling and the ASCII rule are BitLocker's own refusals; + the floor and the four classes are this script's, and are stricter than its default policy. #> param( [Parameter(Mandatory)][AllowEmptyString()][string]$Plain, [Parameter(Mandatory)][int]$MinimumLength, [Parameter(Mandatory)][int]$MaximumLength ) - # Every class is spelled out in ASCII rather than left to \d or a negated class, because both of - # those match beyond ASCII and BitLocker refuses anything outside printable ASCII (0x803100A4). + # IsMatch throughout, never -match or -notmatch: those are case-insensitive, which is how [A-Z] + # came to accept a lowercase letter, and they leave what they matched in $Matches - for the ASCII + # pattern that is the whole password. Classes are spelled out in ASCII because \d and a negated + # class both reach past it, and BitLocker refuses anything outside printable ASCII (0x803100A4). $unmet = @() - if ($Plain.Length -lt $MinimumLength) { $unmet += "at least $MinimumLength characters" } - if ($Plain.Length -gt $MaximumLength) { $unmet += "at most $MaximumLength characters" } - if ($Plain -cnotmatch '\A[\x20-\x7E]*\z') { $unmet += "printable ASCII characters only" } - # -cnotmatch, because the case-insensitive form lets [A-Z] match a lowercase letter and vice versa. - if ($Plain -cnotmatch '[A-Z]') { $unmet += "at least one uppercase letter" } - if ($Plain -cnotmatch '[a-z]') { $unmet += "at least one lowercase letter" } - if ($Plain -cnotmatch '[0-9]') { $unmet += "at least one digit" } - # Printable ASCII less space and alphanumerics, which is what is left to be a special character. - if ($Plain -cnotmatch '[\x21-\x2F\x3A-\x40\x5B-\x60\x7B-\x7E]') { $unmet += "at least one special character" } + if ($Plain.Length -lt $MinimumLength) { $unmet += "at least $MinimumLength characters" } + if ($Plain.Length -gt $MaximumLength) { $unmet += "at most $MaximumLength characters" } + if (-not [regex]::IsMatch($Plain, '\A[\x20-\x7E]*\z')) { $unmet += "printable ASCII characters only" } + if (-not [regex]::IsMatch($Plain, '[A-Z]')) { $unmet += "at least one uppercase letter" } + if (-not [regex]::IsMatch($Plain, '[a-z]')) { $unmet += "at least one lowercase letter" } + if (-not [regex]::IsMatch($Plain, '[0-9]')) { $unmet += "at least one digit" } + # Printable ASCII less space and alphanumerics. + if (-not [regex]::IsMatch($Plain, '[\x21-\x2F\x3A-\x40\x5B-\x60\x7B-\x7E]')) { $unmet += "at least one special character" } return $unmet } @@ -2991,6 +2992,8 @@ try { if ($bitLockerPlan.UsePasswordProtector -and $protectorPlan.TypesToAdd -contains 'Password') { Write-Host "Enter BitLocker password for the new volume. It must be a complex one." -ForegroundColor Yellow + # The retry loop comes back here, so the attempt BitLocker refused goes first. + if ($SecurePassword) { $SecurePassword.Dispose() } $SecurePassword = Request-StrongPassword -MinimumLength $PasswordMinLength ` -MaximumLength $PasswordMaxLength Write-Host "Adding BitLockerKeyProtector PasswordProtector" -ForegroundColor Green