scale_prepare_enable_ssh_login adds duplicate conflicting config setting

[janfrode]

Describe the bug
I normally have "PermitRootLogin without-password", to only allow key-based root logins, so I tested if "scale_prepare_enable_ssh_login: true" would overwrite that to the less secure "PermitRootLogin yes". Surprisingly I then ended up with 2 lines of PermitRootLogin in the sshd_config:

# grep Root /etc/ssh/sshd_config
PermitRootLogin without-password
PermitRootLogin yes

To Reproduce

Change to "PermitRootLogin without-password":

perl -pi -e 's/^PermitRootLogin yes/PermitRootLogin without-password/' /etc/ssh/sshd_config

and the rerun core/precheck with "scale_prepare_enable_ssh_login: true".

Expected behavior
Should end up with one active line of "PermitRootLogin yes".

Environment
Please run the following an paste your output here:

# Developement
# ansible --version
ansible 2.9.18
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/root/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.6/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 3.6.8 (default, Aug 18 2020, 08:33:21) [GCC 8.3.1 20191121 (Red Hat 8.3.1-5)]

# python --version
-bash: python: command not found
# python3 --version
Python 3.6.8

Screenshots

Before running core/precheck:

# grep -n ^PermitRootLogin /etc/ssh/sshd_config
43:PermitRootLogin without-password

After:

# grep -n ^PermitRootLogin /etc/ssh/sshd_config
43:PermitRootLogin without-password
95:PermitRootLogin yes

Additional context
Not completely sure what the correct behaviour should be, but definitely not ending up with two conflicting lines of the same config setting. I think it would be best if it defaulted to changing from "no" to "without-password" as a secure default, and what we actually need for Scale.

[janfrode]

I see now that the line it's matching and changing is this one:

# the setting of "PermitRootLogin without-password".

[janfrode]

Also see from ansible lineinfile documentation that the expected behaviour is that the regexp will replace the last match.

For state=present, the pattern to replace if found. Only the last line found will be replaced.

So a simple workaround would be to improve the regexp to ignore lines that are commented out.

[janfrode]

Ideally I would set it to "without-password", which would be the secure default. But I'm afraid that might break something for existing users who expect the current behaviour.

[rajan-mis]

@janfrode Can we close this issue. Thanks