Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

boolean variable inconsistency? #330

Closed
NanoPish opened this issue Apr 15, 2020 · 7 comments · Fixed by #715
Closed

boolean variable inconsistency? #330

NanoPish opened this issue Apr 15, 2020 · 7 comments · Fixed by #715

Comments

@NanoPish
Copy link

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

@rndmh3ro
Copy link
Member

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.

@NanoPish
Copy link
Author

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
Copy link
Author

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

@rndmh3ro
Copy link
Member

rndmh3ro commented Jun 25, 2020

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!

@MatthiasLohr
Copy link
Contributor

MatthiasLohr commented Jun 30, 2020

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.

@NanoPish
Copy link
Author

NanoPish commented Sep 2, 2020

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

@rndmh3ro rndmh3ro transferred this issue from dev-sec/ansible-ssh-hardening Nov 11, 2020
@rndmh3ro
Copy link
Member

rndmh3ro commented Feb 7, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants