saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Install Salt from the Salt package repositories here:
https://docs.saltproject.io/salt/install-guide/en/latest/
Apache License 2.0
14.21k stars 5.48k forks source link

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

Open willchenmark opened 1 month ago

willchenmark commented 1 month ago

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

Setup

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. ```yaml 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
welcome[bot] commented 1 month ago

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 saltproject@vmware.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!

dhs-rec commented 4 days ago

Just in case: Also happens on Server 2025, with Salt 3006.7.