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.19k stars 5.48k forks source link

[BUG] ssh_auth.present does not support sk-ssh-ed25519@openssh.com keys #64723

Closed jamesharr closed 1 year ago

jamesharr commented 1 year ago

Description ssh_auth.present needs to be updated to support new OpenSSH authentication methods, including the type sk-ssh-ed25519@openssh.com. The regex recognizing valid SSH pub keys has been periodically updated and I believe needs to be updated again. A link to the previous change is noted in the 'additional context' section.

Setup

This bug is manifesting on an AlmaLinux 9.2 server running Salt 3006.1 (OneDir), but I believe it'll manifest in any salt version.

Steps to Reproduce the behavior Install salt 3006.1, attempt to apply the following state:

Example state file:

# /srv/salt/ssh_auth_bug.sls
manage-some-ssh-keys:
  ssh_auth.manage:
    - user: some-user
    - ssh_keys:
      - sk-ssh-ed25519@openssh.com REDACTED_KEY some-user@host

Using the following configuration:

salt 'test-machine' state.apply ssh_auth_bug

Expected behavior I expect the key types ending in @openssh.com to be accepted

Screenshots

MACHINE:
----------
          ID: some-user
    Function: ssh_auth.manage
      Result: False
     Comment: Invalid public ssh key, most likely has spaces or invalid syntax
     Started: 21:10:29.894368
    Duration: 7.023 ms
     Changes:   

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.) ```yaml Salt Version: Salt: 3006.1 Python Version: Python: 3.10.11 (main, May 5 2023, 02:31:54) [GCC 11.2.0] Dependency Versions: cffi: 1.14.6 cherrypy: unknown dateutil: 2.8.1 docker-py: Not Installed gitdb: Not Installed gitpython: Not Installed Jinja2: 3.1.2 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.9.8 pygit2: Not Installed python-gnupg: 0.4.8 PyYAML: 5.4.1 PyZMQ: 23.2.0 relenv: 0.12.3 smmap: Not Installed timelib: 0.2.4 Tornado: 4.5.3 ZMQ: 4.3.4 System Versions: dist: almalinux 9.2 Turquoise Kodkod locale: utf-8 machine: x86_64 release: 5.14.0-284.18.1.el9_2.x86_64 system: Linux version: AlmaLinux 9.2 Turquoise Kodkod ```

Additional context

As of AlmaLinux 9.2 and OpenSSH 8.7p1, the current list of valid OpenSSH key types is as follows:

# ssh -V
OpenSSH_8.7p1, OpenSSL 3.0.7 1 Nov 2022

# ssh -Q PubkeyAcceptedAlgorithms
ssh-ed25519
ssh-ed25519-cert-v01@openssh.com
sk-ssh-ed25519@openssh.com
sk-ssh-ed25519-cert-v01@openssh.com
ssh-rsa
rsa-sha2-256
rsa-sha2-512
ssh-dss
ecdsa-sha2-nistp256
ecdsa-sha2-nistp384
ecdsa-sha2-nistp521
sk-ecdsa-sha2-nistp256@openssh.com
webauthn-sk-ecdsa-sha2-nistp256@openssh.com
ssh-rsa-cert-v01@openssh.com
rsa-sha2-256-cert-v01@openssh.com
rsa-sha2-512-cert-v01@openssh.com
ssh-dss-cert-v01@openssh.com
ecdsa-sha2-nistp256-cert-v01@openssh.com
ecdsa-sha2-nistp384-cert-v01@openssh.com
ecdsa-sha2-nistp521-cert-v01@openssh.com
sk-ecdsa-sha2-nistp256-cert-v01@openssh.com
jamesharr commented 1 year ago

As mentioned above, the commit c8cd2e64 was the previous commit that updated the regex, so it's likely a good reference for a patch.

This is the regex that c8cd2e64 updates it to:

        sshre = re.compile(r"^(.*?)\s?((?:sk-)?(?:ssh\-|ecds)[\w-]+\s.+)$")

And I believe it needs to be one of the following:

        sshre = re.compile(r"^(.*?)\s?((?:sk-)?(?:ssh\-|ecds)[\w-]+(?:@openssh\.com)?\s.+)$")
        sshre = re.compile(r"^(.*?)\s?((?:sk-)?(?:ssh\-|ecds)[\w-@.]+\s.+)$")

The former is more strict than the latter; I'm not sure if there is a preference for the salt maintainers.

jamesharr commented 1 year ago

It's also worth noting that the regex is copied throughout salt/states/ssh_auth.py, so it might be a good candidate to define & compile once in the file and then reference elsewhere.

# near top of file
RE_SSHKEY = re.compile(r"^(.*?)\s?((?:sk-)?(?:ssh\-|ecds)[\w-]+(?:@openssh\.com)?\s.+)$")
RE_SSHKEY_RELAXED = re.compile("^(sk-)?(ssh\-|ecds).*")

# later in the file
def some_function:
    # change
    sshre.search(...)
    # to
    match = RE_SSHKEY.search(...)

This could also possibly make testing a bit easier if there were a set of example keys you could test the regular expression against.

OrangeDog commented 1 year ago

Duplicates #61299

jamesharr commented 1 year ago

Closing since this is a duplicate

jamesharr commented 1 year ago

@OrangeDog - I know this is a duplicate now, but if I were to throw together a PR, do you know if the maintainers would prefer a stricter or loose regex?

OrangeDog commented 1 year ago

No idea. I wrote a PR with a looser one, but got no comments.