exchange12rocks / PSGPPreferences

A way to manage Group Policy Preferences through PowerShell
MIT License
36 stars 2 forks source link

Set Preferences on a brand-new GPO means settings do not show in Group Policy results and do not apply #32

Open Borgquite opened 2 years ago

Borgquite commented 2 years ago

OK, so this one may take a little longer to explain:

When setting up a brand new 'clean' Group Policy object ONLY using PSGPPreferences, without using the GPP GUI in any way, the applied Preferences settings are not visible in the Settings report under the Group Policy Management Console, and are not applied.

To reproduce:

New-GPO "TEST GPO"
New-GPPGroup -GPOName "TEST GPO" -Name "Administrators" -Create

Then start the Group Policy Management Console, and open 'TEST GPO' / 'Settings'. It shows 'No settings defined'.

The cause is described and discussed in a similar bug for a similar project here: https://github.com/hashicorp/terraform-provider-ad/issues/39 but boils down to:

[{00000000-0000-0000-0000-000000000000}{Tool Extension GUID 1}][{CSE GUID 1}{Tool Extension GUID 1}]

e.g. for Local Groups and Settings:

[{00000000-0000-0000-0000-000000000000}{79F92669-4224-476C-9C5C-6EFB4D87DF4A}][{17D89FEC-5C44-4972-B12D-241CAEF74509}{79F92669-4224-476C-9C5C-6EFB4D87DF4A}]

A few notes regarding this though:

Having played around with modifying this the following PowerShell should do the job (please feel free to use this as you see fit - released as public domain):

$GPO = Get-GPO -Name "TEST GPO"
$GPOADObject = Get-ADObject -Identity $GPO.Path -Properties gPCMachineExtensionNames

$CSEGUIDRegex = [regex]::new('(?i)\[(?<CSE>{[0-9A-F]{8}-(?:[0-9A-F]{4}-){3}[0-9A-F]{12}})(?<Exts>(?:{[0-9A-F]{8}-(?:[0-9A-F]{4}-){3}[0-9A-F]{12}})+)\]') # Regular expression - match and retrieve CSE GUIDs individually and associated Tool Extension GUIDs as a single string
$ExtGUIDRegex = [regex]::new('(?i)(?<Ext>{[0-9A-F]{8}-(?:[0-9A-F]{4}-){3}[0-9A-F]{12}})+') # Regular expression - match and retrieve Tool Extension GUIDs individually

# Set these values for each GPP Preferences item used in the policy as per https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-gppref/a00e597a-cd21-4a97-9277-f53fae251acf
$CoreCSEGUID = '{00000000-0000-0000-0000-000000000000}'
$LocalGroupsCSEGUID = '{17D89FEC-5C44-4972-B12D-241CAEF74509}'
$LocalGroupsExtGUID = '{79F92669-4224-476c-9C5C-6EFB4D87DF4A}'

Write-Warning "Existing value: $($GPOADObject.gPCMachineExtensionNames)"

$gPCMachineExtensionNamesTable = @{} # Created an hash table of arrays to store the CSE GUID and tool extension GUID values
$CSEGUIDRegex.Matches($GPOADObject.gPCMachineExtensionNames).ForEach({
    $CSE = $_.Groups['CSE'].Captures.Value
    $gPCMachineExtensionNamesTable[$CSE] = 
        $ExtGUIDRegex.Matches($_.Groups['Exts'].Captures.Value).ForEach({$_.Groups['Ext'].Captures.Value})
})

# If the GPO HAS ANY Local Users and Groups entries, run this code
if ($gPCMachineExtensionNamesTable[$CoreCSEGUID] -notcontains $LocalGroupsExtGUID) {
    Write-Warning "Adding $LocalGroupsExtGUID to $CoreCSEGUID..."
    $gPCMachineExtensionNamesTable[$CoreCSEGUID] = $gPCMachineExtensionNamesTable[$CoreCSEGUID] + @($LocalGroupsExtGUID)
}
if ($gPCMachineExtensionNamesTable[$LocalGroupsCSEGUID] -notcontains $LocalGroupsExtGUID) {
    Write-Warning "Adding $LocalGroupsExtGUID to $LocalGroupsCSEGUID..."
    $gPCMachineExtensionNamesTable[$LocalGroupsCSEGUID] = $gPCMachineExtensionNamesTable[$LocalGroupsCSEGUID] + @($LocalGroupsExtGUID)
}

# If the GPO DOES NOT HAVE ANY Local Users and Groups entries, run this code
if ($gPCMachineExtensionNamesTable[$CoreCSEGUID] -contains $LocalGroupsExtGUID) {
    Write-Warning "Removing $LocalGroupsExtGUID from $CoreCSEGUID..."
    $gPCMachineExtensionNamesTable[$CoreCSEGUID] = $gPCMachineExtensionNamesTable[$CoreCSEGUID] | Where-Object {$_ -ne $LocalGroupsExtGUID}
    if (!$gPCMachineExtensionNamesTable[$CoreCSEGUID]) {
        $gPCMachineExtensionNamesTable.Remove($CoreCSEGUID)
    }
}
if ($gPCMachineExtensionNamesTable[$LocalGroupsCSEGUID]) {
    Write-Warning "Removing $LocalGroupsCSEGUID..."
    $gPCMachineExtensionNamesTable.Remove($LocalGroupsCSEGUID)
}

# Generate the final string with each CSE guid(s) and its associated tool extension GUIDS surrounded by square brackets with each CSE GUID and the related tool extension GUIDs underneath alphanumerically sorted. Ensure Extension GUIDs are unique in case they get duplicated by mistake
$gPCMachineExtensionNames = -join ($gPCMachineExtensionNamesTable.GetEnumerator() | Sort-Object Name).ForEach({'[' + $_.Name + (-join ($_.Value | Sort-Object -Unique)) + ']'})

Write-Warning "Updated value: $gPCMachineExtensionNames"

if ($gPCMachineExtensionNames) {
    Set-ADObject -Identity $GPO.Path -Replace @{gPCMachineExtensionNames=$gPCMachineExtensionNames}
} else {
    Set-ADObject -Identity $GPO.Path -Clear gPCMachineExtensionNames
}

I guess it would be good if this could be added before and after each 'Add', 'Set' or 'Remove' operation. Adding them will make the reporting & application work in the first place. Removing them when unnecessary will speed up client-side application of the relevant GPO.

Again after testing it seems the GUI generates these values if any relevant items are 'created' or 'set', and removes them after any 'remove' operations if no items of the given type are left. I think the code is good but please do review and test as well :). Happy to explain anything if you want to know my reasoning.

Borgquite commented 2 years ago

Sorry about the huge code dump but hope it helps. If the regexes are impenetrable I can recommend https://regex101.com/

exchange12rocks commented 2 years ago

Hi @Borgquite ! Thank you very much for testing the module so thoroughly - it helps a lot! Yes, this is a known bug, I just haven't filed it here yet. I planned to fix it on this week, before giving you a new release.

Would you be interested in adding your code directly into the repository? If yes, please create a PR into the CSE branch.

Borgquite commented 2 years ago

@exchange12rocks I'll leave that to you as not sure the best place for it to go! I think this covers all the issues I've found so far (unless you're near to writing the WMI filter code, which I'd also be interested in!)

I'll look forward to the new release :)

exchange12rocks commented 2 years ago

Hi @Borgquite ! I am sorry - I haven't managed to implement this yet. I published a version 0.3.0, which includes all fixes so far, and continue to work on this one

Borgquite commented 2 years ago

Hi @exchange12rocks - no worries, appreciate you putting out the existing stuff in the new release! If you can point me where the best place for the code above to go is I can try to implement it myself. It would need to be somewhere that runs every time after a setting is added or removed. It should then be possible to check if the Groups.xml file exists (or is empty/nonexistent) and add/remove gPCMachineExtensionNames value (as appropriate)

exchange12rocks commented 2 years ago

Hey @Borgquite , I finally managed to push some work for this bug: https://github.com/exchange12rocks/PSGPPreferences/tree/CSE

I am looking to complete this by tomorrow

exchange12rocks commented 2 years ago

@borgquite The version 0.3.2 is in the gallery - please check it out

Borgquite commented 2 years ago

@exchange12rocks Thank you so much! I am in a really busy period at work at the moment but will look forward to giving it a spin when things calm down :) I'll send you some feedback when I have.

Borgquite commented 2 years ago

Hey @exchange12rocks, managed to have a play today. It seems to work OK on a 'fresh' GPO, but it messes up if other Group Policy extensions are already applied to an existing object:

For example:

$GPO = New-GPO "TEST GPO 2"
Set-GPRegistryValue -Name "TEST GPO 2" -Key "HKLM\Software\Policies\Microsoft\Windows\Personalization" -ValueName "NoLockScreen" -Type DWORD -Value 1 # Create a registry-based policy GPO setting (as used by Administrative Templates)
$GPOADObject = Get-ADObject -Identity $GPO.Path -Properties gPCMachineExtensionNames
$GPOADObject.gPCMachineExtensionNames # Returns [{35378EAC-683F-11D2-A89A-00C04FBBCFA2}{D02B1F72-3407-48AE-BA88-E8213C6761F1}]
New-GPPGroup -GPOName "TEST GPO 2" -Name "Administrators" -Create
$GPOADObject = Get-ADObject -Identity $GPO.Path -Properties gPCMachineExtensionNames # Returns [{00000000-0000-0000-0000-000000000000}{79f92669-4224-476c-9c5c-6efb4d87df4a}][{17d89fec-5c44-4972-b12d-241caef74509}{79f92669-4224-476c-9c5c-6efb4d87df4a}]
$GPOADObject.gPCMachineExtensionNames 

Please note that gPCMachineExtensionNames has the CSE IDs and Tools IDs for Group Policy Preferences, but the existing ones for Computer Policy Settings (documented under https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-gpreg/f0dba6b8-704f-45d5-999f-1a0a694a6df9) have been removed. The code I mentioned above will retain all existing CSEs and Tools (meaning existing policy settings still work) whereas the current deployed code wipes them out. It is at least partly down to your trying to match the all-zeroes CSE specifically in Get-GPOCSE whereas you should probably be treating all CSE/Tool pairs identically, leaving any existing ones in place and just adding or removing the ones you specifically need.

I would also note that the code appears to be writing out GUIDs with lower case a-f instead of upper case A-F (and the regex matches are the same). Accepting lowercase input is a good idea (I've updated my sample code above to do this), but I would change your output to upper case only to avoid any compatibility issues (in my testing the GUI tools always wrote out gPCMachineExtensionNames GUIDs in upper case, so it's possible lower case GUIDs might unexpectedly break other Group Policy extensions. It's a relatively undocumented ecosystem so best to be conservative in what your code does, I'd suggest.)

Borgquite commented 2 years ago

Also having other issues where existing GP Preferences are being changed & no longer working I'm afraid. Ran out of time today and am off on holiday for a couple of days but will be back next week. Thanks for all your work!

exchange12rocks commented 2 years ago

Hmm it should preserve all existing ones and sort them accordingly 🤔

Thank you!

exchange12rocks commented 2 years ago

Oh, of course: it does not work because Set-GPRegistryValue (and the GUI) sets the attribute without the leading zero GUID. So it just does not match the regular expression. I thought the zero GUID is always there

exchange12rocks commented 2 years ago

Ah, OK, the zero GUID is needed only for GPP 🤦‍♂️ Why did I miss that?

exchange12rocks commented 2 years ago

Fixed in the release 0.3.3 - available in the gallery

Borgquite commented 2 years ago

Thank you! :) Will take a look once I get another moment free!

Borgquite commented 2 years ago

Just been looking through the code - suggest a change to the regex to do case insensitivity using a flag rather than [A-f] (which also matches G-Z). Pull request #41

Borgquite commented 2 years ago

Based on my understanding of GP extensions I hope it's OK for me to say there are still a couple of worries I'd have about how it's implemented at present:

I'm not sure if I've made this clear or not, but if you're free sometime between 10:00-17:00 GMT one day perhaps we could discuss over a video/audio call? But basically what you want to be able to cope with is more like this, which I don't think is quite what you've got yet:

[{00000000-GPP CSE GUID}{GPP Tools GUID 1}{GPP Tools GUID 2}][{GPP CSE GUID 1}{GPP Tools GUID 1}][{GPP CSE GUID 2}{GPP Tools GUID 2}][{Another CSE GUID 1}{Another Tools GUID 1}][{Another CSE GUID 2}{Another Tools GUID 2}]

Let me know if I can explain better!

Borgquite commented 2 years ago

Hey,

Just to give you a couple of reproduction scenarios for those issues:

The issue with removing the setting (at present I think there may be another bug where Groups.xml isn't deleted even when there is no setting inside), but I think even if the XML file was empty, it might still return [{00000000-0000-0000-0000-000000000000}] - making the change via the GUI will return blank)

$GPO = New-GPO "TEST GPO"
New-GPPGroup -GPOName "TEST GPO" -Name "Administrators" -Create
Remove-GPPGroup -GPOName "TEST GPO" -Name "Administrators"
$GPOADObject = Get-ADObject -Identity $GPO.Path -Properties gPCMachineExtensionNames
$GPOADObject.gPCMachineExtensionNames # Returns [{00000000-0000-0000-0000-000000000000}{79F92669-4224-476C-9C5C-6EFB4D87DF4A}][{17D89FEC-5C44-4972-B12D-241CAEF74509}{79F92669-4224-476C-9C5C-6EFB4D87DF4A}]
# Should instead return blank

The other issue:

I could try to fix this if you want, but not sure whether it might require some rearchitecting of code!

exchange12rocks commented 1 year ago

Hi @Borgquite ! Sorry for the delay, I was moving to the Netherlands. The second issue existed because of my laziness in filtering out CSE tools - I've implemented that properly now.

I'll continue working on the first issue.

Borgquite commented 1 year ago

Thanks @exchange12rocks - I'll take a look when you're done & try to test it then!