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

[win_lgpo] Causes corrupt Registry.pol file #48782

Closed
mike2523 opened this issue Jul 26, 2018 · 19 comments

Comments

Projects
None yet
4 participants
@mike2523
Copy link

commented Jul 26, 2018

Description of Issue/Question

When applying below SLS files, causes Registry.pol to get corrupted. Using LGPO.exe /parse /m C:\Windows\System32\GroupPolicy\Machine\Registry.pol to verify. All other consecutive states afterwards will fail with CommandExecutionError: Error while attempting to write Administrative Template Policy data. Some changes may not be applied as expected

Before:

; PARSING COMPLETED.
; ----------------------------------------------------------------------

After

; PARSING Computer POLICY
; Source file:  C:\Windows\System32\GroupPolicy\Machine\Registry.pol

Computer
Software\Policies\Microsoft\MicrosoftEdge\PhishingFilter
EnabledV9
DWORD:1

_Invalid file format.  Expected '[', found character 0x00005b00_

Setup and steps to repo

I have tried this line by line to see which of these items is causing this, but this is the only combo that can do this. I don't know what other combo can cause this. Just need to keep a close eye on the changes we make moving forward.

Step1: Delete C:\Windows\System32\GroupPolicy\Machine\Registry.pol, then apply this SLS file first

# Microsoft Edge Security requirements
edge_sec_requirements:
  lgpo.set:
    - computer_policy:
        "Windows Components\\Microsoft Edge\\Allow Address bar drop-down list suggestions": Disabled
        "Windows Components\\Microsoft Edge\\Configure Password Manager": Disabled
        "Windows Components\\Microsoft Edge\\Configure search suggestions in Address bar": Disabled
        "Windows Components\\Microsoft Edge\\Prevent access to the about:flags page in Microsoft Edge": Enabled
        "Windows Components\\Microsoft Edge\\Prevent using Localhost IP address for WebRTC": Enabled

# Microsoft Edge Smart Screen settings
enable_edge_defender_smartscreen:
  lgpo.set:
    - computer_policy:
        Configure Windows Defender SmartScreen: Enabled
        Prevent bypassing Windows Defender SmartScreen prompts for files: Enabled
        Prevent bypassing Windows Defender SmartScreen prompts for sites: Enabled

Step2: Verified that Registry.pol is still valid
Step3: Run this second SLS file:

gpo_log_trace:
  lgpo.set:
    - computer_policy:
        "System\\Group Policy\\Configure registry policy processing":
            "Do not apply during periodic background processing": False
            "Process even if the Group Policy objects have not changed": True
        "System\\Group Policy\\Continue experiences on this device": Disabled
        "System\\Group Policy\\Turn off background refresh of Group Policy": Disabled

Step4: Registry.pol file is now corrupt due to Allow Address bar drop-down list suggestions being overwritten by this SLS file:

local:
----------
          ID: gpo_log_trace
    Function: lgpo.set
      Result: True
     Comment:
     Started: 09:16:55.187000
    Duration: 11854.0 ms
     Changes:
              ----------
              new:
                  ----------
                  Computer Configuration:
                      ----------
                      System\Group Policy\Configure registry policy processing:
                          ----------
                          Do not apply during periodic background processing:
                              False
                          Process even if the Group Policy objects have not changed:
                              True
                      System\Group Policy\Continue experiences on this device:
                          Disabled
                      System\Group Policy\Turn off background refresh of Group Policy:
                          Disabled
              old:
                  ----------
                  Computer Configuration:
                      ----------
                      Windows Components\Microsoft Edge\Allow Address bar drop-down list suggestions:
                          Disabled

Summary for local
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time:  11.854 s

Versions Report

Salt Version:
           Salt: 2018.3.2

Dependency Versions:
           cffi: 1.10.0
       cherrypy: 10.2.1
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: 2.0.3
      gitpython: 2.1.3
          ioflo: Not Installed
         Jinja2: 2.9.6
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: 1.0.6
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: Not Installed
      pycparser: 2.17
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.14 (v2.7.14:84471935ed, Sep 16 2017, 20:25:58) [MSC v.1500 64 bit (AMD64)]
   python-gnupg: 0.4.1
         PyYAML: 3.12
          PyZMQ: 16.0.3
           RAET: Not Installed
          smmap: 2.0.3
        timelib: 0.2.4
        Tornado: 4.5.1
            ZMQ: 4.1.6

System Versions:
           dist:
         locale: cp1252
        machine: AMD64
        release: 10
         system: Windows
        version: 10 10.0.17134  Multiprocessor Free
@mike2523

This comment has been minimized.

Copy link
Author

commented Jul 27, 2018

Looks like smartscreen configs are the show stopper:

[DEBUG   ] working on admPolicy ShellConfigureSmartScreen
[DEBUG   ] time to enable and set the policy "ShellConfigureSmartScreen"
[DEBUG   ] found this_policy == [<Element {Microsoft.Policies.SmartScreen}policy at 0x7c36a08>]
[DEBUG   ] item value name is  E n a b l e S m a r t S c r e e n
[ERROR   ] Unhandled exception %s occurred while attempting to write Adm Template Policy File
[ERROR   ] An exception occurred in this state: Traceback (most recent call last):
  File "C:\salt\bin\lib\site-packages\salt\state.py", line 1905, in call
    **cdata['kwargs'])
  File "C:\salt\bin\lib\site-packages\salt\loader.py", line 1830, in wrapper
    return f(*args, **kwargs)
  File "C:\salt\bin\lib\site-packages\salt\states\win_lgpo.py", line 306, in set_
    adml_language=adml_language)
  File "C:\salt\bin\lib\site-packages\salt\modules\win_lgpo.py", line 5556, in set_
    raise CommandExecutionError(msg)
CommandExecutionError: Error while attempting to write Administrative Template Policy data.  Some changes may not be applied as expected

reverting using this fixes all lgpo consecutive run issues(for certain line items). Although Registry.pol will still need to be fixed (deleted or parsed for deletion if it's in an invalid format state):

# Undo Smart Screen settings as this foobar'ed other lgpo items
dont_configure_smartscreen:
  lgpo.set:
    - computer_policy:
        AllowSmartScreen: 'Not Configured'
        ShellConfigureSmartScreen: 'Not Configured'
        EdgeConfigureSmartScreen: 'Not Configured'
        EnableSmartScreen: 'Not Configured'
        PreventSmartScreenPromptOverride: 'Not Configured'
        PreventSmartScreenPromptOverrideForFiles: 'Not Configured'
        EdgePreventOverrideForNav: 'Not Configured'

example line item that this breaks and fixes:

disable_app_data_sharing:
  lgpo.set:
    - computer_policy:
        Allow a Windows app to share application data between users: Disabled
@garethgreenaway

This comment has been minimized.

Copy link
Member

commented Jul 27, 2018

@dwoz @twangboy @saltstack/team-windows Thoughts on this one?

@twangboy

This comment has been minimized.

Copy link
Contributor

commented Jul 27, 2018

@lomeroe ^^^

@twangboy twangboy self-assigned this Jul 27, 2018

@lomeroe

This comment has been minimized.

Copy link
Contributor

commented Jul 28, 2018

@mike2523 could you possibly share the ADMX/ADML files that have those policies in them with me? The Win10 systems I have have access to don't have the policies available...

@mike2523

This comment has been minimized.

Copy link
Author

commented Jul 30, 2018

This is on a Windows 10 ENT Build 1803.

PolicyDefinitions.zip

@mike2523

This comment has been minimized.

Copy link
Author

commented Jul 30, 2018

Another SLS that causes issue (this time, gpedit console stopped working when disabled), setting these to Not Configured fixes it back up:

remote_assistance:
  lgpo.set:
    - computer_policy:
        RA_Solicit: 'Not configured'
        RA_Unsolicit: 'Not configured'
@mike2523

This comment has been minimized.

Copy link
Author

commented Jul 30, 2018

Last one i promise :)

Windows Components -> Delivery Optimization -> Download Mode. Enable, Download Mode set to LAN (1)

when enabling Download mode via gpedit console first, stops lgpo.get module with error below:
PS C:\Windows\system32> salt-call.bat lgpo.get machine return_full_policy_names=True

PS C:\Windows\system32> salt-call.bat lgpo.get machine return_full_policy_names=True
[ERROR   ] An un-handled exception was caught by salt's global exception handler:
AttributeError: 'NoneType' object has no attribute 'rstrip'
Traceback (most recent call last):
  File "C:\salt\bin\Scripts\salt-call", line 11, in <module>
    salt_call()
  File "C:\salt\bin\lib\site-packages\salt\scripts.py", line 400, in salt_call
    client.run()
  File "C:\salt\bin\lib\site-packages\salt\cli\call.py", line 57, in run
    caller.run()
  File "C:\salt\bin\lib\site-packages\salt\cli\caller.py", line 134, in run
    ret = self.call()
  File "C:\salt\bin\lib\site-packages\salt\cli\caller.py", line 212, in call
    ret['return'] = func(*args, **kwargs)
  File "C:\salt\bin\lib\site-packages\salt\modules\win_lgpo.py", line 5132, in get
    return_not_configured=return_not_configured))
  File "C:\salt\bin\lib\site-packages\salt\modules\win_lgpo.py", line 3785, in _checkAllAdmxPolicies
    adml_policy_resources)
  File "C:\salt\bin\lib\site-packages\salt\modules\win_lgpo.py", line 3109, in _getFullPolicyName
    fullPolicyName = _getAdmlPresentationRefId(adml_data, policy_item.attrib['id'])
  File "C:\salt\bin\lib\site-packages\salt\modules\win_lgpo.py", line 3074, in _getAdmlPresentationRefId
    prepended_text = p_item.text.rstrip()
AttributeError: 'NoneType' object has no attribute 'rstrip'
Traceback (most recent call last):
  File "C:\salt\bin\Scripts\salt-call", line 11, in <module>
    salt_call()
  File "C:\salt\bin\lib\site-packages\salt\scripts.py", line 400, in salt_call
    client.run()
  File "C:\salt\bin\lib\site-packages\salt\cli\call.py", line 57, in run
    caller.run()
  File "C:\salt\bin\lib\site-packages\salt\cli\caller.py", line 134, in run
    ret = self.call()
  File "C:\salt\bin\lib\site-packages\salt\cli\caller.py", line 212, in call
    ret['return'] = func(*args, **kwargs)
  File "C:\salt\bin\lib\site-packages\salt\modules\win_lgpo.py", line 5132, in get
    return_not_configured=return_not_configured))
  File "C:\salt\bin\lib\site-packages\salt\modules\win_lgpo.py", line 3785, in _checkAllAdmxPolicies
    adml_policy_resources)
  File "C:\salt\bin\lib\site-packages\salt\modules\win_lgpo.py", line 3109, in _getFullPolicyName
    fullPolicyName = _getAdmlPresentationRefId(adml_data, policy_item.attrib['id'])
  File "C:\salt\bin\lib\site-packages\salt\modules\win_lgpo.py", line 3074, in _getAdmlPresentationRefId
    prepended_text = p_item.text.rstrip()
AttributeError: 'NoneType' object has no attribute 'rstrip'
@lomeroe

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2018

It looks like there are multiple "Configure Windows Defender SmartScreen" policies, but it isn't being caught.

PS 48782_PolicyDefinitions> get-childitem -recurse | select-string "Configure Windows Defender SmartScreen"

PolicyDefinitions\en-US\MicrosoftEdge.adml:127:      <string id="AllowSmartScreen">Configure Windows Defender
SmartScreen</string>
PolicyDefinitions\en-US\SmartScreen.adml:13:      <string id="ShellConfigureSmartScreen">Configure Windows Defender
SmartScreen</string>
PolicyDefinitions\en-US\SmartScreen.adml:41:      <string id="EdgeConfigureSmartScreen">Configure Windows Defender
SmartScreen</string>
PolicyDefinitions\en-US\WindowsExplorer.adml:446:      <string id="EnableSmartScreen">Configure Windows Defender
SmartScreen</string>


PS C:\Users\Administrator> salt-call lgpo.get_policy_info 'Configure Windows Defender SmartScreen' machine --local
local:
    ----------
    message:
    policy_aliases:
        - Configure Windows Defender SmartScreen
        - AllowSmartScreen
        - Windows Components\Microsoft Edge\Configure Windows Defender SmartScreen
    policy_class:
        machine
    policy_elements:
    policy_found:
        True
    policy_name:
        Configure Windows Defender SmartScreen
    rights_assignment:
        False


PS C:\Users\Administrator> salt-call lgpo.get_policy_info 'ShellConfigureSmartScreen' machine --local
local:
    ----------
    message:
    policy_aliases:
        - Configure Windows Defender SmartScreen
        - ShellConfigureSmartScreen
        - Windows Components\Windows Defender SmartScreen\Explorer\Configure Windows Defender SmartScreen
    policy_class:
        machine
    policy_elements:
        |_
          ----------
          element_aliases:
              - ShellConfigureSmartScreen_Dropdown
              - Pick one of the following settings
          element_id:
              ShellConfigureSmartScreen_Dropdown
    policy_found:
        True
    policy_name:
        ShellConfigureSmartScreen
    rights_assignment:
        False


PS C:\Users\Administrator> salt-call lgpo.get_policy_info 'EdgeConfigureSmartScreen' machine --local
local:
    ----------
    message:
    policy_aliases:
        - Configure Windows Defender SmartScreen
        - EdgeConfigureSmartScreen
        - Windows Components\Windows Defender SmartScreen\Microsoft Edge\Configure Windows Defender SmartScreen
    policy_class:
        machine
    policy_elements:
    policy_found:
        True
    policy_name:
        EdgeConfigureSmartScreen
    rights_assignment:
        False

If you use one of the unique policy aliases, do you get the same results, or does it work as expected?

Edit to add: you can also test making the updates from this commit, which should resolve both items you've mentioned here (that commit is for 2017.7, so you'll have to patch your 2018.3 version - you won't be able to take the file wholesale)

lomeroe added a commit to lomeroe/salt that referenced this issue Aug 6, 2018

test that presentation element "text" property returns a string before
attempting to add it to the prepended text

properly test if multiple adml entries were found when searching for an
admx policy based on an ADML name

fixes saltstack#48782
@mike2523

This comment has been minimized.

Copy link
Author

commented Aug 7, 2018

Using unique policy aliases or non-aliases works just fine, but always results in corrupt registry.pol

Caveat that happens tho is this:

  1. When enabling Configure Windows Defender SmartScreen in:
  • Windows Components\Microsoft Edge\Configure Windows Defender SmartScreen
  • Windows Components\File Explorer\Configure Windows Defender SmartScreen

These paths automatically gets enabled as well (and vise-versa on the above):

  • Windows Components\Windows Defender SmartScreen\Microsoft Edge\Configure Windows Defender SmartScreen
  • Windows Components\Windows Defender SmartScreen\Explorer\Configure Windows Defender SmartScreen

I don't know if this automagically enabling the other item is what's causing the corruption...

I'll test this out on 2017.7, but worked around this using registry instead for now. Will this be rolled-out to 2018.3 releases? and is the stable release 2017 or 2018?

@lomeroe

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2018

@mike2523 -- I finally got back to working on this. I think you're hitting multiple issues, some that were corrected (but perhaps not in an actual release yet) and some new ones.

Can you test with this version of win_lgpo.py, it is for 2018.3. If it resolves your issue, I'll backport to 2017.7 as well.

https://github.com/lomeroe/salt/blob/issue48782_2018.3/salt/modules/win_lgpo.py

Let me know if you have any questions/etc.

@mike2523

This comment has been minimized.

Copy link
Author

commented Oct 5, 2018

i get this error when trying out that version:

[ERROR ] Handle this explicitly Traceback (most recent call last): File "C:\Users\MichaelOblena\it-co\itco-salt\salt\_modules\win_lgpo.py", line 2586, in _sidConversion userSid = win32security.LookupAccountSid('', _sid) error: (1332, 'LookupAccountSid', 'No mapping between account names and security IDs was done.')

@lomeroe

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2018

@mike2523 is that the only error returned? Is the registry.pol file still corrupt? Do you have user rights assignments with a user/group listed that no longer exists? If so, that error would actually be expected (though it shouldn't keep anything from applying)

@lomeroe

This comment has been minimized.

Copy link
Contributor

commented Oct 10, 2018

@mike2523 it's likely that the registry.pol could still be corrupted, I found at least one more issue that could have caused it and pushed another update to the above module version. Please test it out again for me...

@mike2523

This comment has been minimized.

Copy link
Author

commented Oct 10, 2018

my registry.pol is still valid, but still shows up with the trace error on this new update. trace error happens either on an empty registry.pol file or a valid registry.pol file.

- cumulative_rights_assignments: False is set in SLS file.

here's the full output:

[ERROR   ] Handle this explicitly
Traceback (most recent call last):
  File "C:\Users\MichaelOblena\it-co\itco-salt\salt\_modules\win_lgpo.py", line 2586, in _sidConversion
    userSid = win32security.LookupAccountSid('', _sid)
error: (1332, 'LookupAccountSid', 'No mapping between account names and security IDs was done.')
[ERROR   ] Handle this explicitly
Traceback (most recent call last):
  File "C:\Users\MichaelOblena\it-co\itco-salt\salt\_modules\win_lgpo.py", line 2586, in _sidConversion
    userSid = win32security.LookupAccountSid('', _sid)
error: (1332, 'LookupAccountSid', 'No mapping between account names and security IDs was done.')
[ERROR   ] Handle this explicitly
Traceback (most recent call last):
  File "C:\Users\MichaelOblena\it-co\itco-salt\salt\_modules\win_lgpo.py", line 2586, in _sidConversion
    userSid = win32security.LookupAccountSid('', _sid)
error: (1332, 'LookupAccountSid', 'No mapping between account names and security IDs was done.')
[ERROR   ] Handle this explicitly
Traceback (most recent call last):
  File "C:\Users\MichaelOblena\it-co\itco-salt\salt\_modules\win_lgpo.py", line 2586, in _sidConversion
    userSid = win32security.LookupAccountSid('', _sid)
error: (1332, 'LookupAccountSid', 'No mapping between account names and security IDs was done.')
[ERROR   ] Handle this explicitly
Traceback (most recent call last):
  File "C:\Users\MichaelOblena\it-co\itco-salt\salt\_modules\win_lgpo.py", line 2586, in _sidConversion
    userSid = win32security.LookupAccountSid('', _sid)
error: (1332, 'LookupAccountSid', 'No mapping between account names and security IDs was done.')
[ERROR   ] Handle this explicitly
Traceback (most recent call last):
  File "C:\Users\MichaelOblena\it-co\itco-salt\salt\_modules\win_lgpo.py", line 2586, in _sidConversion
    userSid = win32security.LookupAccountSid('', _sid)
error: (1332, 'LookupAccountSid', 'No mapping between account names and security IDs was done.')
[ERROR   ] Handle this explicitly
Traceback (most recent call last):
  File "C:\Users\MichaelOblena\it-co\itco-salt\salt\_modules\win_lgpo.py", line 2586, in _sidConversion
    userSid = win32security.LookupAccountSid('', _sid)
error: (1332, 'LookupAccountSid', 'No mapping between account names and security IDs was done.')
[ERROR   ] Handle this explicitly
Traceback (most recent call last):
  File "C:\Users\MichaelOblena\it-co\itco-salt\salt\_modules\win_lgpo.py", line 2586, in _sidConversion
    userSid = win32security.LookupAccountSid('', _sid)
error: (1332, 'LookupAccountSid', 'No mapping between account names and security IDs was done.')
[ERROR   ] Handle this explicitly
Traceback (most recent call last):
  File "C:\Users\MichaelOblena\it-co\itco-salt\salt\_modules\win_lgpo.py", line 2586, in _sidConversion
    userSid = win32security.LookupAccountSid('', _sid)
error: (1332, 'LookupAccountSid', 'No mapping between account names and security IDs was done.')
[ERROR   ] Handle this explicitly
Traceback (most recent call last):
  File "C:\Users\MichaelOblena\it-co\itco-salt\salt\_modules\win_lgpo.py", line 2586, in _sidConversion
    userSid = win32security.LookupAccountSid('', _sid)
error: (1332, 'LookupAccountSid', 'No mapping between account names and security IDs was done.')
local:
----------
          ID: user_rights_assignments
    Function: lgpo.set
      Result: True
     Comment: "Take ownership of files and other objects" is already set."Restore files and directories" is already set."Replace a process level token" is already set."Profile system performance" is already set."Profile single process" is already set."Perform volume maintenance tasks" is already set."Modify firmware environment values" is already set."Modify an object label" is already set."Manage auditing and security log" is already set."Log on as a batch job" is already set."Lock pages in memory" is already set."Load and unload device drivers" is already set."Increase scheduling priority" is already set."Impersonate a client after authentication" is already set."Generate security audits" is already set."Force shutdown from a remote system" is already set."Enable computer and user accounts to be trusted for delegation" is already set."Deny log on through Remote Desktop Services" is already set."Deny log on locally" is already set."Deny log on as a service" is already set."Deny log on as a batch job" is already set."Deny access to this computer from the network" is already set."Debug programs" is already set."Create symbolic links" is already set."Create permanent shared objects" is already set."Create a token object" is already set."Create a pagefile" is already set."Change the time zone" is already set."Change the system time" is already set."Backup files and directories" is already set."Allow log on through Remote Desktop Services" is already set."Allow log on locally" is already set."Act as part of the operating system" is already set."Access this computer from the network" is already set."Access Credential Manager as a trusted caller" is already set.
     Started: 12:45:16.831000
    Duration: 108929.0 ms
     Changes:

Summary for local
------------
Succeeded: 1
Failed:    0
------------
Total states run:     1
Total run time: 108.929 s

still continues to completion, but error trace everytime.

@lomeroe

This comment has been minimized.

Copy link
Contributor

commented Oct 11, 2018

@mike2523 Ok, good deal that the registry.pol is good. User rights assignments aren't stored there, so if only user rights assignment changes are being made, the registry.pol shouldn't be modified.

In PR #50006, I changed that code to only log a warning instead of an exception, so the inability to convert the SID to a user/group name shouldn't generate a traceback like that...

@twangboy

This comment has been minimized.

Copy link
Contributor

commented Oct 11, 2018

@lomeroe Shouldn't it raise an error if it can't resolve the SID? Does it allow you to add a non-existing user/group from the GUI?

@lomeroe

This comment has been minimized.

Copy link
Contributor

commented Oct 12, 2018

@twangboy No, the _sidConversion function is only used when retrieving the existing user rights assignments (they are stored as SIDs) and converting them to the friendly names for display/etc. To grant a user/group a right, it does have to be a valid group (the _usernamesToSidObjects function will error if the user/group name cannot be converted to a SID when attempting to set a right).

@twangboy

This comment has been minimized.

Copy link
Contributor

commented Oct 13, 2018

@lomeroe Excellent. Thanks for the clarification.

@twangboy twangboy closed this Nov 2, 2018

@twangboy

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2018

With the merge of #50006 I'm closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.