Skip to content
This repository has been archived by the owner on Dec 26, 2020. It is now read-only.

sftp Match Group settings overriding global sshd_config settings #137

Closed
kekumu opened this issue Oct 27, 2017 · 6 comments
Closed

sftp Match Group settings overriding global sshd_config settings #137

kekumu opened this issue Oct 27, 2017 · 6 comments

Comments

@kekumu
Copy link
Contributor

kekumu commented Oct 27, 2017

Match Group sftponly
ForceCommand internal-sftp -l INFO -f LOCAL6
ChrootDirectory {{ sftp_chroot_dir }}
AllowTcpForwarding no
AllowAgentForwarding no
PasswordAuthentication no
PermitRootLogin no
X11Forwarding no

Lines 241-247 should be indented in order to apply to only the Match Group. Right now they are overriding the global settings higher up in the config file.

kekumu added a commit to kekumu/ansible-ssh-hardening that referenced this issue Oct 27, 2017
The "sftponly" Match Group in the sshd_config was not indented properly, so the settings that should only apply to sftp connections were overriding the global settings earlier in the file.
rndmh3ro added a commit that referenced this issue Oct 27, 2017
Issue #137: Indent sshd_config's "Match Group sftponly"
@rndmh3ro
Copy link
Member

Just as I said in the PR, indentation is not required here.
The manpage for sshd_config describes this better than I could:

Introduces a conditional block. If all of the criteria on the Match line are satisfied, the keywords on the following lines override those set in the global section of the config file, until either another Match line or the end of the file. If a keyword appears in multiple Match blocks that are satisfied, only the first instance of the keyword is applied.

@kekumu
Copy link
Contributor Author

kekumu commented Oct 27, 2017

Thanks for the clarification.

mkgin added a commit to mkgin/ansible-ssh-hardening that referenced this issue Jun 8, 2018
* use new docker images

* rm 1.9 from travis

* use centos 7 in vagrant, limit ssh conns

* use new docker images

* Replace deprecated always_run with check_mode

* Add Ed25519 SSH host key to match commit 28b4df3 in ssh-baseline InSpec profile

* update min ansible version

* Remove small dh primes

Thanks to debops!
https://github.com/debops/ansible-sshd/

* update travis to test debian 8

* change hostkey setting

* use sed isntead of perl

* fix order

* update readme

* remove debian9

Errors unrelated to this PR in travis, fill be fixed later

* actually remove debian 9

* make ChallengeResponseAuthentication configurable

* Add support for FreeBSD OpenSSH server and client

* List only one Port in ssh config

Only one Port will be used, so don't loop over a list of Ports.

* Fix ssh config to handle custom options per Host

Formerly printed Hosts on successive lines, applying all options
to the final Host only.

* Accommodate missing plugins in kitchen_vagrant_block.rb

Prevents Vagrant failures: 'Unknown configuration section <name>'.
Also accommodate missing http_proxy environment variable,
which will be nil and have no `empty?` method.

* remove duplicate section

* update changelog

* update changelog

* Adds option to set whether password authentication is allowed with sshd.

* Updates documentation.

* Added support for UseDNS config switch

* Added support for UseDNS config switch

* Added support for PermitTunnel config switch

* release new version 4.1.0

* fix validation error

* update changelog

* Template additional configuration options

All additional options are backwards compatible, and should not
introduce any unwanted vulnerability or side effect.

* PermitUserEnvironment/AcceptEnv now supports a list of accepted
  environment variables from the client.
* Compression is now configurable.
* UseDNS is now configurable.
* Both Match Group and Match User are now configurable.
* SSHD now supports being enabled and started.

* Only restart service when SSH is enabled

* finish PR

* remove 12.04 as its EOLd

* remove 12.04 as its EOLd for travis

* Update README.md

* added check_mode: no to "get openssh-version" task, so it won't fail in check-mode

* update changelog

* change vars to bool, put comments inside block

* Do not use shell when not needed + Lint whitespaces

* remove support for 12.04

* remove support for 12.04

* add back play

* use package instead of yum so the operation works on Fedora

* Adding in support for google authenticator TOTP.

This adds the PAM module package, changes the SSHd configuration and the
PAM configuration on RedHat and Debian systems to allow TOTP based
second factor auth to work with SSH keys.

* Add support to specify a list of revoked public keys

Signed-off-by: Pascal Bach <pascal.bach@siemens.com>

* update changelog

* Add comment filter to {{ansible_managed}} string

Multiline {{ansible_managed}} strings do not get properly commented
without the comment filter.
See http://docs.ansible.com/ansible/playbooks_filters.html#comment-filter

* Fix ansible.cfg settings

- using %Y-%m-%d in ansible_managed message is not recommended
as deploying from a new git checkout will change the ansible_managed
string in the template and Ansible will report the template file as changed
(see http://docs.ansible.com/ansible/intro_configuration.html#ansible-managed )
- no such setting called role_path (see
http://docs.ansible.com/ansible/intro_configuration.html )
- scp_if_ssh should be under [ssh_connection] header

* Finish the RedHat support.

This will remove password auth from RedHat distros to ensure that only
key based auth and 2FA codes can be used to authenticate users over SSH.

* Cannot have password auth when password is disabled.

* Moving the Google Auth section.

I had accidently placed this under an SELinux block rather than at the
end of it as I had planned. Now placed before the SELinux block to be
clear.

* Setting google auth to be off by default.

This will allow the current tests to run as they are failing because
this is not defined. A better step would be to add some tests.

* Don't overwrite ssh_host_key_files if set manually

before the manually set `ssh_host_key_files` would be overwritten if the openssh-version was greater than 5.3

* add bool checks to all relevant variables

* more bools

* update changelog

* Moving over to using Ansible PAM support.

Using the Ansible PAM support rather than regex based line editing for
two out of the three PAM changes required for 2FA support.

The last change does not appear to be supported by the Ansible PAM
support unless I have missed something from the documentation.

* Adding comment to config file.

This makes it clear why the ordering is public key then keyboard
interactive.

* Remove duplicate ssh_use_dns

ssh_use_dns was set twice in defaults/main.yml. This patch removes the last occurrence.

* update changelog

* corrected comments explaining the task's behaviour

* update default.yml because of broken travis

* update default.yml because of broken travis

* update default.yml because of broken travis

* force /bin/sh when getting openssh-version

(since the update to ansible 2.4.0.0) the C shell is used
on FreeBSD to run shell  tasks.  This breakes the
redirect.

Forcing /bin/sh fixes this.

* Added support for AuthorizedKeysFile config setting

* allow configuration of GatewayPorts

* Issue dev-sec#137: Fix sshd_config's "Match Group sftponly"

The "sftponly" Match Group in the sshd_config was not indented properly, so the settings that should only apply to sftp connections were overriding the global settings earlier in the file.

* test gateway ports var

* simplify macs, kex, ciphers

* divide plays to workaround gathered facts

* add ssh7.6 support

* change ssh.conf, add comments

* Remove deprecated UseLogin option

Since OpenSSH 7.4/7.4p1 (2016-12-19)[0] (The default in Debian Stretch,
CentOS 7 and others) the "UseLogin" option has been deprecated.

Setting this option originally prevented usage of a "traditional"
/usr/sbin/login-based login – but has been set to "no" by default since
quite a while, so even if this role would be applied on a host with an
older OpenSSH version, the default value should still be save.

Fixes dev-sec#140

0. https://www.openssh.com/txt/release-7.4

* remove uselogin-check

* remove merge conflict line

* fix lineending

* update MACs

* update changelog

* Add support for Amazon Linux

* Fixed trailing whitespace

* add amazon linux to travis and kitchen

* change init path on amazon

* change init path on amazon

* Update syntax to 2.4

* new parameter: ssh_max_startups

use this parameter to set 'MaxStartups' in sshd_config

* conform to current dev-sec/ssh-baseline

* Update README.md

* Use package state 'present' since 'installed' is deprecated

* Adds sshd config for keyboard-interactive pam device

- Adds configuration option for public key authentication with 2FA input
from a PAM device such as a Yubikey. This will allow keyboard
interaction from the _device only_. See the documentation on
AuthenticationMethods
[here](https://www.freebsd.org/cgi/man.cgi?sshd_config(5)).

* Added support for TrustedUserCAKeys and AuthorizedPrincipalsFile.

* ca keys & principals are now handle in a separated tasks file

* add examples for ssh_trusted_user_ca_keys & ssh_authorized_principals variables

* Handle a few deprecated OpenSSH options

RhostsRSAAuthentication + RSAAuthentication is deprecated as of 7.4
UsePrivilegeSeparation is deprecated as of 7.5 (the daemon is now
sandboxed on all modern OSes/platforms by default)

* Convert sshd_version.stdout string to float

Resolves error: '<' not supported between instances of 'AnsibleUnicode' and 'float'

* Deprecate UseRoaming after OpenSSH 7.1

* update readme for sftp, fix dev-sec#165

* Update README.md

* Update README.md

* Implement disabling chroot for sftp

By default enabled, of course, but give the option to disable sftp
chrooting.
@kiwivogel
Copy link

Question: Does this block need to be here in the first place as these are mostly covered in the global settings mentioned above. Keeping this in breaks the associated inspec ssh-baseline tests because of duplicate values.

@rndmh3ro
Copy link
Member

With this block you can enable sftp and set different options for it than in the global settings so yes, this block has a right to exist. :)

I guess the inspec tests fail, because you have sftp enabled?

@kiwivogel
Copy link

Yeah, somehow ansible refuses to pick up scp_if_ssh = True so I'm stuck on using SFTP. The question was mainly if it's safe to delete these settings (assuming they are duplicates of global settings). The alternative is that I skip them in a child profile and add matchers that are ok with the duplication but that's a bit ugly.

@rndmh3ro
Copy link
Member

The question was mainly if it's safe to delete these settings

Yes that should be safe.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants