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 may raise KeyError exception from _get_inbound_text with 3006.9 #67122

Open
2 tasks done
hurzhurz opened this issue Dec 27, 2024 · 0 comments
Open
2 tasks done
Labels
Bug broken, incorrect, or confusing behavior needs-triage

Comments

@hurzhurz
Copy link
Contributor

Description
After updating a windows minion update to 3006.9, a lgpo.set state lead to a KeyError exception.
This is related to #344a3d8 (Use Powershell instead of netsh for firewall settings).

Example:

domain_firewall_state:
    lgpo.set:
        - computer_policy:
            "Network firewall: Domain: State" : "On (recommended)"
          ID: domain_firewall_state
    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\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 422, in set_
                  current_policy[class_map[p_class]][p_name] = __salt__[
                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 9758, in get_policy
                  return _get_policy_info_setting(policy_definition)
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\modules\win_lgpo.py", line 8940, in _get_policy_info_setting
                  value = _get_netsh_value(
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\modules\win_lgpo.py", line 5703, in _get_netsh_value
                  settings = salt.utils.win_lgpo_netsh.get_all_settings(
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\utils\win_lgpo_netsh.py", line 324, in get_all_settings
                  "Inbound": _get_inbound_text(
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\utils\win_lgpo_netsh.py", line 133, in _get_inbound_text
                  return settings[rule][action]
              KeyError: 1

Important is, that on this minion, the GPO AllowInboundRules is set to True (1):

> Get-NetFirewallProfile domain -PolicyStore localhost
Name                            : Domain
Enabled                         : True
DefaultInboundAction            : Block
DefaultOutboundAction           : Allow
AllowInboundRules               : True
AllowLocalFirewallRules         : NotConfigured
AllowLocalIPsecRules            : NotConfigured
AllowUserApps                   : NotConfigured
AllowUserPorts                  : NotConfigured
AllowUnicastResponseToMulticast : NotConfigured
NotifyOnListen                  : NotConfigured
EnableStealthModeForIPsec       : NotConfigured
LogFileName                     : %SystemRoot%/System32/LogFiles/Firewall/domainfw.log
LogMaxSizeKilobytes             : 16384
LogAllowed                      : True
LogBlocked                      : True
LogIgnored                      : NotConfigured
DisabledInterfaceAliases        : {NotConfigured}

But _get_inbound_text expects AllowInboundRules to be either False (0) or NotConfigured (2). Though it treats NotConfigured like how True should be treated, if I understand the logic correctly.

def _get_inbound_text(rule, action):
"""
The "Inbound connections" setting is a combination of 2 parameters:
- AllowInboundRules
- DefaultInboundAction
The settings are as follows:
Rules Action
2 2 AllowInbound
2 4 BlockInbound
0 4 BlockInboundAlways
2 0 NotConfigured
"""
settings = {
0: {
4: "BlockInboundAlways",
},
2: {
0: "NotConfigured",
2: "AllowInbound",
4: "BlockInbound",
},
}
return settings[rule][action]

While it would be simple add that case to _get_inbound_text, it is probably necessary to consider _get_inbound_settings as well, which is used for applying changes:

def _get_inbound_settings(text):
settings = {
"allowinbound": (2, 2),
"blockinbound": (2, 4),
"blockinboundalways": (0, 4),
"notconfigured": (2, 0),
}
return settings[text.lower()]

It is probably not desirable to simple overwrite AllowInboundRules from True to NotConfigured or vice versa...

@twangboy As the mentioned commit is from you, do you have any idea how to solve this properly?
Would be great if it can be fixed in 3006.10.

Setup

  • VM (Virtualbox, KVM, etc. please specify)
  • onedir packaging

Versions Report

salt --versions-report
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: 2022Server
        system: Windows
       version: 2022Server 10.0.20348 SP0 Multiprocessor Free
@hurzhurz hurzhurz added Bug broken, incorrect, or confusing behavior needs-triage labels Dec 27, 2024
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