florianutz / Ubuntu1804-CIS

Ubuntu CIS Hardening Ansible Role
MIT License
212 stars 127 forks source link

Validate SSH config file after changes #49

Closed Lirt closed 2 years ago

Lirt commented 5 years ago

When running the section5 SSH tasks, the sshd configuration file is not validated. The ssh daemon may not be restarted, because not all tasks contain the notify statement and the error remains silently in the configuration file.

This can cause, that sshd configuration file is misconfigured and ssh daemon will not start after system restart.

This is major issue, since the administrator has to log into the machine via single user mode, serial console or another type of alternative access to fix the misconfigured file. This happened to me today on GCP platform, where the end of the SSH file is by default configured properly, but if another line is added to it, it breaks the syntax.

Example of valid ending of SSH config file:

# Create chrooted directory
Match Group scponly
    ChrootDirectory %h
    passwordAuthentication yes
    ForceCommand internal-sftp
    AllowTcpForwarding no
    X11Forwarding no

By adding for example line PermitUserEnvironment no, the Match block is not closed with Match all statement and the configuration file is invalid.

# Create chrooted directory
Match Group scponly
    ChrootDirectory %h
    passwordAuthentication yes
    ForceCommand internal-sftp
    AllowTcpForwarding no
PermitUserEnvironment no

Error when running sshd -t:

/etc/ssh/sshd_config line 69: Directive 'PermitUserEnvironment' is not allowed within a Match block.

As remediation I propose:

florianutz commented 4 years ago

Hi Lirt. Your are right. There are some missing notifies. I'll try to fix that. But that will not solve your problem. Your misconfiguration is not due to the role.

Lirt commented 4 years ago

Hi Florian,

I think that in certain cases it simply changes the configuration in a way that it is not valid. The reason is due to the nature of module lineinfile, where you cannot control how the configuration file will look like completely.

Best practice for such configuration files is using templates, where the configuration file is deterministic, while lineinfile produces different results.

But I think, that your approach is correct, because if some users have their specific SSH configuration file, they would lost their changes if you used templating.

The validation sshd -t will be sufficient to avoid disasters and it is best practice to do configuration file validation everywhere it is possible. I can create PR for it, if you need to offload the work.

Thanks for reviewing the issue :-).

florianutz commented 4 years ago

Hi Lirt,

I think there are a much more solutions to implement these rules. We also played around with templates but it is hard to define a "generic template". Templates will work within your environment/company but not for all. Another point why I don't use templates for this is because I want to use the latest sshd config from the package maintainer. E.g. if there is a new default value, comment or something else I will lose that with a template. Good for a enterprise environment but bad for a community :)

If you have time to provide a PR for a handler who checks the sshd config it would be great.

Lirt commented 4 years ago

We also played around with templates but it is hard to define a "generic template". Templates will work within your environment/company but not for all. Another point why I don't use templates for this is because I want to use the latest sshd config from the package maintainer. E.g. if there is a new default value, comment or something else I will lose that with a template

I totally agree.

Thanks, I will try to find time for the PR ASAP.