Fix: SSH regexp match commented out lines too

[dbrennand]

Fixes: https://github.com/geerlingguy/ansible-role-security/issues/66

I experienced this issue on a fresh installation of Raspberry Pi OS Lite (64-bit). The default config has the lines commented out and the role task was failing to modify the commented out lines.

[dbrennand]

Molecule tests passing and behaviour is as expected:

TASK [Build an Ansible compatible image] ***************************************
skipping: [localhost] => (item=geerlingguy/docker-ubuntu2004-ansible) 
skipping: [localhost]
...
TASK [geerlingguy.security : Update SSH configuration to be more secure.] ******
changed: [instance] => (item={'regexp': '^(#|)PasswordAuthentication', 'line': 'PasswordAuthentication no'})
changed: [instance] => (item={'regexp': '^(#|)PermitRootLogin', 'line': 'PermitRootLogin no'})
changed: [instance] => (item={'regexp': '^(#|)Port', 'line': 'Port 22'})
changed: [instance] => (item={'regexp': '^(#|)UseDNS', 'line': 'UseDNS no'})
changed: [instance] => (item={'regexp': '^(#|)PermitEmptyPasswords', 'line': 'PermitEmptyPasswords no'})
ok: [instance] => (item={'regexp': '^(#|)ChallengeResponseAuthentication', 'line': 'ChallengeResponseAuthentication no'})
changed: [instance] => (item={'regexp': '^(#|)GSSAPIAuthentication', 'line': 'GSSAPIAuthentication no'})
changed: [instance] => (item={'regexp': '^(#|)X11Forwarding', 'line': 'X11Forwarding no'})
...
PLAY RECAP *********************************************************************
instance                   : ok=15   changed=10   unreachable=0    failed=0    skipped=9    rescued=0    ignored=0

[stale[bot]]

This pull request has been marked 'stale' due to lack of recent activity. If there is no further activity, the PR will be closed in another 30 days. Thank you for your contribution!

Please read this blog post to see the reasons why I mark pull requests as stale.

[dbrennand]

Still applicable 🙂

[stale[bot]]

This issue is no longer marked for closure.

[idolize]

I'm hitting this issue as well - would be great to get this in!

[dbrennand]

I'm hitting this issue as well - would be great to get this in!

Hey @idolize - Did you test these changes on your system?

[geerlingguy]

The only concern I have is, if someone used this role before, and a new line was added with the value, then this change would match the first instance (which was the commented line, not the one this role added), and not the last one in the file... that could result in broken behavior for existing users :/

[dbrennand]

The only concern I have is, if someone used this role before, and a new line was added with the value, then this change would match the first instance (which was the commented line, not the one this role added), and not the last one in the file... that could result in broken behavior for existing users :/

Yes, this change would break for existing users.

I'm just taking a look at the lineinfile docs and I believe insertafter may be able to help here?

I'll test and report back. 🙂

[dbrennand]

Done some testing against Debian 10 and 11:

---
role_name_check: 1
dependency:
  name: galaxy
driver:
  name: podman
platforms:
  - name: debian10
    image: geerlingguy/docker-debian10-ansible
    tmpfs:
      - /run
      - /tmp
    volumes:
      - /sys/fs/cgroup:/sys/fs/cgroup:ro
    capabilities:
      - SYS_ADMIN
    command: "/lib/systemd/systemd"
    pre_build_image: true
  - name: debian11
    image: geerlingguy/docker-debian11-ansible
    tmpfs:
      - /run
      - /tmp
    volumes:
      - /sys/fs/cgroup:/sys/fs/cgroup:ro
    capabilities:
      - SYS_ADMIN
    command: "/lib/systemd/systemd"
    pre_build_image: true
provisioner:
  name: ansible
  playbooks:
    converge: ${MOLECULE_PLAYBOOK:-converge.yml}

TASK [geerlingguy.security : Update SSH configuration to be more secure.] ******
changed: [debian10] => (item={'regexp': '^PasswordAuthentication', 'insertafter': '^#PasswordAuthentication', 'line': 'PasswordAuthentication no'})
changed: [debian11] => (item={'regexp': '^PasswordAuthentication', 'insertafter': '^#PasswordAuthentication', 'line': 'PasswordAuthentication no'})
changed: [debian10] => (item={'regexp': '^PermitRootLogin', 'insertafter': '^#PermitRootLogin', 'line': 'PermitRootLogin no'})
changed: [debian11] => (item={'regexp': '^PermitRootLogin', 'insertafter': '^#PermitRootLogin', 'line': 'PermitRootLogin no'})
changed: [debian10] => (item={'regexp': '^Port', 'insertafter': '^#Port', 'line': 'Port 22'})
changed: [debian11] => (item={'regexp': '^Port', 'insertafter': '^#Port', 'line': 'Port 22'})
changed: [debian10] => (item={'regexp': '^UseDNS', 'insertafter': '^#UseDNS', 'line': 'UseDNS no'})
changed: [debian11] => (item={'regexp': '^UseDNS', 'insertafter': '^#UseDNS', 'line': 'UseDNS no'})
changed: [debian10] => (item={'regexp': '^PermitEmptyPasswords', 'insertafter': '^#PermitEmptyPasswords', 'line': 'PermitEmptyPasswords no'})
changed: [debian11] => (item={'regexp': '^PermitEmptyPasswords', 'insertafter': '^#PermitEmptyPasswords', 'line': 'PermitEmptyPasswords no'})
ok: [debian10] => (item={'regexp': '^ChallengeResponseAuthentication', 'insertafter': '^#ChallengeResponseAuthentication', 'line': 'ChallengeResponseAuthentication no'})
ok: [debian11] => (item={'regexp': '^ChallengeResponseAuthentication', 'insertafter': '^#ChallengeResponseAuthentication', 'line': 'ChallengeResponseAuthentication no'})
changed: [debian10] => (item={'regexp': '^GSSAPIAuthentication', 'insertafter': '^#GSSAPIAuthentication', 'line': 'GSSAPIAuthentication no'})
changed: [debian11] => (item={'regexp': '^GSSAPIAuthentication', 'insertafter': '^#GSSAPIAuthentication', 'line': 'GSSAPIAuthentication no'})
changed: [debian10] => (item={'regexp': '^X11Forwarding', 'insertafter': '^#X11Forwarding', 'line': 'X11Forwarding no'})
changed: [debian11] => (item={'regexp': '^X11Forwarding', 'insertafter': '^#X11Forwarding', 'line': 'X11Forwarding no'})

root@debian10:/# grep 'Port.*[0-9]' /etc/ssh/sshd_config 
#Port 22
Port 22

insertafter is inserting the line after it's commented counterpart:

insertafter is only honored if no match for regexp is found.

To check if this new behaviour works when someone has placed a value elsewhere in /etc/ssh/sshd_config, I moved Port 22 down the file:

...

#Port 22
#AddressFamily any
#ListenAddress 0.0.0.0
#ListenAddress ::

...

# Logging
#SyslogFacility AUTH
#LogLevel INFO
Port 22 <---- here

Then using molecule idempotence, checking to see there is no changed state:

TASK [geerlingguy.security : Update SSH configuration to be more secure.] ******
ok: [debian10] => (item={'regexp': '^PasswordAuthentication', 'insertafter': '^#PasswordAuthentication', 'line': 'PasswordAuthentication no'})
ok: [debian11] => (item={'regexp': '^PasswordAuthentication', 'insertafter': '^#PasswordAuthentication', 'line': 'PasswordAuthentication no'})
ok: [debian10] => (item={'regexp': '^PermitRootLogin', 'insertafter': '^#PermitRootLogin', 'line': 'PermitRootLogin no'})
ok: [debian11] => (item={'regexp': '^PermitRootLogin', 'insertafter': '^#PermitRootLogin', 'line': 'PermitRootLogin no'})
ok: [debian10] => (item={'regexp': '^Port', 'insertafter': '^#Port', 'line': 'Port 22'})
ok: [debian11] => (item={'regexp': '^Port', 'insertafter': '^#Port', 'line': 'Port 22'})
ok: [debian10] => (item={'regexp': '^UseDNS', 'insertafter': '^#UseDNS', 'line': 'UseDNS no'})
ok: [debian11] => (item={'regexp': '^UseDNS', 'insertafter': '^#UseDNS', 'line': 'UseDNS no'})
ok: [debian10] => (item={'regexp': '^PermitEmptyPasswords', 'insertafter': '^#PermitEmptyPasswords', 'line': 'PermitEmptyPasswords no'})
ok: [debian11] => (item={'regexp': '^PermitEmptyPasswords', 'insertafter': '^#PermitEmptyPasswords', 'line': 'PermitEmptyPasswords no'})
ok: [debian10] => (item={'regexp': '^ChallengeResponseAuthentication', 'insertafter': '^#ChallengeResponseAuthentication', 'line': 'ChallengeResponseAuthentication no'})
ok: [debian11] => (item={'regexp': '^ChallengeResponseAuthentication', 'insertafter': '^#ChallengeResponseAuthentication', 'line': 'ChallengeResponseAuthentication no'})
ok: [debian10] => (item={'regexp': '^GSSAPIAuthentication', 'insertafter': '^#GSSAPIAuthentication', 'line': 'GSSAPIAuthentication no'})
ok: [debian11] => (item={'regexp': '^GSSAPIAuthentication', 'insertafter': '^#GSSAPIAuthentication', 'line': 'GSSAPIAuthentication no'})
ok: [debian10] => (item={'regexp': '^X11Forwarding', 'insertafter': '^#X11Forwarding', 'line': 'X11Forwarding no'})
ok: [debian11] => (item={'regexp': '^X11Forwarding', 'insertafter': '^#X11Forwarding', 'line': 'X11Forwarding no'})

Looks OK - think this covers existing users? 🙂

[github-actions[bot]]

This pr has been marked 'stale' due to lack of recent activity. If there is no further activity, the issue will be closed in another 30 days. Thank you for your contribution!

Please read this blog post to see the reasons why I mark issues as stale.

[github-actions[bot]]

This pr has been closed due to inactivity. If you feel this is in error, please reopen the issue or file a new issue with the relevant details.