ansible-collection-hardening icon indicating copy to clipboard operation
ansible-collection-hardening copied to clipboard

boolean variable inconcistency ?

Open NanoPish opened this issue 4 years ago • 7 comments

Describe the bug If I want to set agent forwarding and tcp forwarding true in the last version, I need to do ssh_allow_tcp_forwarding: 'yes' ssh_allow_agent_forwarding: yes

notice the quotes Expected behavior Same boolean notation for all variables

NanoPish avatar Apr 15 '20 13:04 NanoPish

The problem is that AllowTcpForwarding accepts the strings yes, no, all and local, so a boolean is not correct here. This has to be a string. We have to make this more clear in the README.

rndmh3ro avatar Apr 15 '20 15:04 rndmh3ro

I can make a PR. Maybe different prefix for ansible variable that are passed

  • more raw, like ssh_allow_tcp_forwarding
  • more indirectly, like AllowAgentForwarding ssh_raw_allow_tcp_forwarding or string_ssh_allow_tcp_forwarding ?

Follow up question, shouldn't we make these variable the same, all strings or all bool ? And should we make this kind of catchall conditions: AllowTcpForwarding {{ ssh_allow_tcp_forwarding if (ssh_allow_tcp_forwarding in ('yes', 'no', 'local', 'all')) else 'no' }} This could lead a user to a bad configuration if he uses

ssh_allow_tcp_forwarding: TYPO

NanoPish avatar Apr 17 '20 10:04 NanoPish

And shouldn't it be sshD prefix with a D for variables that affect the server

NanoPish avatar Apr 17 '20 10:04 NanoPish

Hey @NanoPish,

sorry for not answering sooner.

And shouldn't it be sshD prefix with a D for variables that affect the server

Yes!

And should we make this kind of catchall conditions: AllowTcpForwarding {{ ssh_allow_tcp_forwarding if (ssh_allow_tcp_forwarding in ('yes', 'no', 'local', 'all')) else 'no' }}

This reads like a really good solution. Though we have to make sure to get all possible parameters.

I can make a PR.

If you're still interested - I am!

rndmh3ro avatar Jun 25 '20 11:06 rndmh3ro

Looks like the same problem exists with ssh_server_permit_environment_vars. ssh_server_permit_environment_vars: yes results in an error ("Bad yes/no argument: True") while ssh_server_permit_environment_vars: 'yes' works.

MatthiasLohr avatar Jun 30 '20 13:06 MatthiasLohr

Hey @NanoPish,

sorry for not answering sooner.

And shouldn't it be sshD prefix with a D for variables that affect the server

Yes!

And should we make this kind of catchall conditions: AllowTcpForwarding {{ ssh_allow_tcp_forwarding if (ssh_allow_tcp_forwarding in ('yes', 'no', 'local', 'all')) else 'no' }}

This reads like a really good solution. Though we have to make sure to get all possible parameters.

I can make a PR.

If you're still interested - I am!

Okay, working on it

NanoPish avatar Sep 02 '20 14:09 NanoPish

PermitUserEnvironment can have arbitrarily options passed to it, so we cannot use the same as in AllowTcpForwarding ({{ ssh_allow_tcp_forwarding if (ssh_allow_tcp_forwarding in ('yes', 'no', 'local', 'all')) else 'no' }} )

Specifies whether ~/.ssh/environment and environment= options in ~/.ssh/authorized_keys 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.

PermitTunnel however can have more options than yes/no so we should change the setting.

Specifies whether tun(4) device forwarding is allowed. The argument must be yes, point-to-point (layer 3), ethernet (layer 2), or no. Specifying yes permits both point-to-point and ethernet. The default is no.

rndmh3ro avatar Feb 07 '21 21:02 rndmh3ro