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

[BUG] win_lgpo does not account for spaces in XMLNS URLs #66992

Open
2 of 9 tasks
willchenmark opened this issue Oct 23, 2024 · 1 comment
Open
2 of 9 tasks

[BUG] win_lgpo does not account for spaces in XMLNS URLs #66992

willchenmark opened this issue Oct 23, 2024 · 1 comment
Labels
Bug broken, incorrect, or confusing behavior needs-triage

Comments

@willchenmark
Copy link

Description
The win_lgpo module used does not currently attempt to escape spaces found in xmlns definitions.

Setup

  • on-prem machine (Windows 11 24H2)
  • VM (Virtualbox, KVM, etc. please specify)
  • VM running on a cloud service, please be explicit and add details
  • container (Kubernetes, Docker, containerd, etc. please specify)
  • or a combination, please be explicit
  • jails if it is FreeBSD
  • classic packaging
  • onedir packaging
  • used bootstrap to install

Steps to Reproduce the behavior
On a fresh Windows 11 24H2 system, attempt to apply any GPO that references the WindowsDefender-D0DE2C.adml definition, such as this user policy.

salt@master:/srv/salt# cat test.sls
"[BACKGROUND] Prevent desktop background from being changed":
  lgpo.set:
    - user_policy:
        Prevent changing desktop background: Enabled

The result will be an invalid URI error from lxml.

salt@master:/srv/salt# salt DEVEL-W-ea96054 state.apply test
DEVEL-W-ea96054:                                                                                                                                       
----------                                                                                                                                             
          ID: [BACKGROUND] Prevent desktop background from being changed                                                                               
    Function: lgpo.set                                                                                                                                 
      Result: False                                                                                                                                    
     Comment: An exception occurred in this state: Traceback (most recent call last):                                                                  
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\modules\win_lgpo.py", line 5143, in _parse_xml                         
                  xml_tree = lxml.etree.parse(out_file, parser=parser)                                                                                 
                File "src\lxml\etree.pyx", line 3538, in lxml.etree.parse                                                                              
                File "src\lxml\parser.pxi", line 1876, in lxml.etree._parseDocument                                                                    
                File "src\lxml\parser.pxi", line 1902, in lxml.etree._parseDocumentFromURL                                                             
                File "src\lxml\parser.pxi", line 1805, in lxml.etree._parseDocFromFile                                                                 
                File "src\lxml\parser.pxi", line 1177, in lxml.etree._BaseParser._parseDocFromFile                                                     
                File "src\lxml\parser.pxi", line 615, in lxml.etree._ParserContext._handleParseResultDoc
                File "src\lxml\parser.pxi", line 725, in lxml.etree._handleParseResult                                        
                File "src\lxml\parser.pxi", line 654, in lxml.etree._raiseParseError
                File "file:/C:/ProgramData/Salt%20Project/Salt/var/cache/salt/minion/lgpo/policy_defs/WindowsDefender-D0DE2CD.adml", line 1
              lxml.etree.XMLSyntaxError: xmlns: 'http://schemas.microsoft.com/GroupPolicy/2006/07/Policysecurity intelligence' is not a valid URI, line
 1, column 246                                                                                                                                         
                                                                                                                                                       
              During handling of the above exception, another exception occurred:                 
                                                                                                                                                       
              Traceback (most recent call last):                                                                                                       
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\state.py", line 2440, in call
                  ret = self.states[cdata["full"]](                                                                                                    
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\loader\lazy.py", line 159, in __call__                                 
                  ret = self.loader.run(run_func, *args, **kwargs)         
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\loader\lazy.py", line 1245, in run                                     
                  return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\loader\lazy.py", line 1260, in _run_as
                  ret = _func_or_method(*args, **kwargs)                
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\loader\lazy.py", line 1296, in wrapper
                  return f(*args, **kwargs)        
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\states\win_lgpo.py", line 412, in set_
                  lookup = __salt__["lgpo.get_policy_info"](      
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\loader\lazy.py", line 159, in __call__
                  ret = self.loader.run(run_func, *args, **kwargs)                                                                                     
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\loader\lazy.py", line 1245, in run    
                  return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\loader\lazy.py", line 1260, in _run_as
                  ret = _func_or_method(*args, **kwargs)
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\modules\win_lgpo.py", line 8755, in get_policy_info
                  success, policy_xml_item, policy_name_list, message = _lookup_admin_template(
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\modules\win_lgpo.py", line 8294, in _lookup_admin_template
                  admx_policy_definitions = _get_policy_definitions(language=adml_language)
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\modules\win_lgpo.py", line 5326, in _get_policy_definitions
                  _load_policy_definitions(path=path, language=language)                                                                               
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\modules\win_lgpo.py", line 5300, in _load_policy_definitions
                  xml_tree = _parse_xml(adml_file)      
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\modules\win_lgpo.py", line 5147, in _parse_xml     
                  xml_tree = _remove_unicode_encoding(out_file)
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\modules\win_lgpo.py", line 5040, in _remove_unicode_encoding
                  r' encoding=[\'"]+unicode[\'"]+', "", xml_content.decode("utf-16"), count=1
              UnicodeDecodeError: 'utf-16-le' codec can't decode byte 0x0a in position 134714: truncated data

That invalid URI error is caused by this adml file in the policy_defs cache:
C:\ProgramData\Salt Project\Salt\var\cache\salt\minion\lgpo\policy_defs\WindowsDefender-D0DE2C.adml

Specifically the Policysecurity intelligence on the first line.
https://github.com/microsoft/mdatp-devicecontrol/blob/main/windows/WindowsDefender.adml#L1

Expected behavior
The state should apply correctly, this would be the expected output.

DEVEL-W-ea96054:
----------
          ID: [BACKGROUND] Prevent desktop background from being changed
    Function: lgpo.set
      Result: True
     Comment: The following policies changed:
              Prevent changing desktop background
     Started: 14:35:35.471570
    Duration: 11736.57 ms
     Changes:   
              ----------
              new:
                  ----------
                  User Configuration:
                      ----------
                      Prevent changing desktop background:
                          Enabled
              old:
                  ----------
                  User Configuration:
                      ----------
                      Prevent changing desktop background:
                          Not Configured

Summary for DEVEL-W-ea96054
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time:  11.737 s

More specifically any spaces that appear in xmlns urls should be escaped with '%20'. Such that
<policyDefinitionResources xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" revision="1.0" schemaVersion="1.0" xmlns="http://schemas.microsoft.com/GroupPolicy/2006/07/Policysecurity intelligence">
becomes
<policyDefinitionResources xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" revision="1.0" schemaVersion="1.0" xmlns="http://schemas.microsoft.com/GroupPolicy/2006/07/Policysecurity%20intelligence">

Versions Report

salt --versions-report

This is from the minion, as the issue is specific to the Windows salt-minion.

Salt Version:
          Salt: 3006.9
 
Python Version:
        Python: 3.10.14 (heads/main:9f7d197, Jun 26 2024, 11:42:40) [MSC v.1940 64 bit (AMD64)]
 
Dependency Versions:
          cffi: 1.14.6
      cherrypy: 18.6.1
  cryptography: 42.0.5
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: 4.0.7
     gitpython: Not Installed
        Jinja2: 3.1.4
       libgit2: Not Installed
  looseversion: 1.0.2
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     packaging: 22.0
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.19.1
        pygit2: Not Installed
  python-gnupg: 0.4.8
        PyYAML: 6.0.1
         PyZMQ: 25.0.2
        relenv: 0.17.0
         smmap: 4.0.0
       timelib: 0.2.4
       Tornado: 4.5.3
           ZMQ: 4.3.4
 
System Versions:
          dist:   
        locale: utf-8
       machine: AMD64
       release: 10
        system: Windows
       version: 10 10.0.26100 SP0 Multiprocessor Free

Additional context

This might not be the best way to handle this, but I was able to correct the error by adding an additional function to modules/win_lgpo.py to escape those spaces, and then added it to the for line iterator in _parse_xml.

--- a/salt/modules/win_lgpo.py	2024-10-22 15:39:57.480400553 -0400
+++ b/salt/modules/win_lgpo.py	2024-10-23 15:29:04.600779526 -0400
@@ -5060,6 +5060,16 @@
     xml_tree = lxml.etree.parse(io.StringIO(modified_xml))
     return xml_tree
 
+def _encode_xmlns_url(match):
+    """
+    Escape spaces in xmlns urls
+    """
+    before_xmlns = match.group(1)
+    xmlns = match.group(2)
+    url = match.group(3)
+    after_url = match.group(4)
+    encoded_url = re.sub(r'\s+', '%20', url)
+    return f'{before_xmlns}{xmlns}="{encoded_url}"{after_url}'
 
 def _parse_xml(adm_file):
     """
@@ -5107,6 +5117,8 @@
                 encoding = "utf-16"
                 raw = raw.decode(encoding)
             for line in raw.split("\r\n"):
+                if 'xmlns="' in line:
+                    line = re.sub(r'(.*)(\bxmlns(?::\w+)?)\s*=\s*"([^"]+)"(.*)', _encode_xmlns_url, line)
                 if 'key="' in line:
                     start = line.index('key="')
                     q1 = line[start:].index('"') + start

@willchenmark willchenmark added Bug broken, incorrect, or confusing behavior needs-triage labels Oct 23, 2024
Copy link

welcome bot commented Oct 23, 2024

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at [email protected]. We’re glad you’ve joined our community and look forward to doing awesome things with you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior needs-triage
Projects
None yet
Development

No branches or pull requests

1 participant