dev-sec / ansible-ssh-hardening

This Ansible role provides numerous security-related ssh configurations, providing all-round base protection.
http://dev-sec.io/
776 stars 199 forks source link

PermitUserEnvironment should not be conflated with AcceptEnv #232

Closed gregsymons closed 5 years ago

gregsymons commented 5 years ago

Describe the bug Currently, the template for the ssh server configuration will enable the PermitUserEnvironment directive if an array of allowed environment variables are passed in the ssh_server_permit_environment_vars list in addition to setting AcceptEnv directives for each of the entries in the list. However PermitUserEnvironment and AcceptEnv are unrelated settings. According to the man page, the AcceptEnv directive only deals with environment variables sent by the client using the SendEnv command at the protocol level:

AcceptEnv Specifies what environment variables sent by the client will be copied into the session's environ(7). See SendEnv and SetEnv in ssh_config(5) for how to configure the client. The TERM environment variable is always accepted whenever the client requests a pseudo-terminal as it is required by the protocol. Variables are specified by name, which may contain the wildcard characters ‘*’ and ‘?’. Multiple environment variables may be separated by whitespace or spread across multiple AcceptEnv directives. Be warned that some environment variables could be used to bypass restricted user environments. For this reason, care should be taken in the use of this directive. The default is not to accept any environment variables.

Whereas the PermitUserEnvironment directive controls whether or not user-supplied custom environment files are processed:

PermitUserEnvironment Specifies whether ~/.ssh/environment and environment= options in ~/.ssh/authorizedkeys are processed by sshd(8). Valid options are yes, no or a pattern-list specifying which environment variable names to accept (for example “LANG,LC*”). The default is no. Enabling environment processing may enable users to bypass access restrictions in some configurations using mechanisms such as LD_PRELOAD.

Expected behavior Specifying a list of environment variables for AcceptEnv should not enable PermitUserEnvironment. Additionally, a separate variable should probably allow either enabling the PermitUserEnvironment directive on a blanket level (i.e. set it to yes) or supply a pattern list similarly to AcceptEnv. Something similar to this (using the example play below) would be what I expect in /etc/ssh/sshd_conf after executing the play below:

...
# It is unclear from the man page whether
# PermitUserEnvironment works similarly 
# to AcceptEnv and can be specified multiple 
# times, so the patterns should be joined with
# a ",".
PermitUserEnvironment LC_*,EDITOR 

AcceptEnv LC_*
AcceptEnv SOME_ARBITRARY_ENVVAR
...

Actual behavior I haven't actually done a run with this setting, I simply inspected the template in templates/opensshd.conf.j2. But I expect the content of /etc/ssh/sshd_conf from the play copied below to be similar to this:

...

PermitUserEnvironment yes
AcceptEnv LC_*
AcceptEnv SOME_ARBITRARY_ENVVAR
...

Example Playbook The following example would trigger the behavior discussed:

- name: Harden SSh
  hosts: all
  vars:
    ssh_server_permit_environment_vars:
      - LC_*
      - SOME_ARBITRARY_ENVVAR
    ssh_server_permit_user_environment_vars:
      - LC_*
      - EDITOR
  roles:
    - devsec.ssh-hardening

OS / Environment All supported

Ansible Version

ansible 2.6.13
  config file = ~/.ansible.cfg
  configured module search path = [u'~/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = ~/.pyenv/versions/2.7.13/envs/ansible/lib/python2.7/site-packages/ansible
  executable location = ~/.pyenv/versions/ansible/bin/ansible
  python version = 2.7.13 (default, May 31 2017, 10:34:24) [GCC 4.2.1 Compatible Apple LLVM 8.1.0 (clang-802.0.42)]

Role Version master

rndmh3ro commented 5 years ago

Hey @gregsymons,

thanks for your great Issue! @szEvEz fixed this issue!