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

permit_tunnel attribute - allow tun device forwarding #211

Merged
merged 6 commits into from
Nov 21, 2018
Merged

permit_tunnel attribute - allow tun device forwarding #211

merged 6 commits into from
Nov 21, 2018

Conversation

bobchaos
Copy link

No description provided.

Copy link
Member

@artem-sidorenko artem-sidorenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bobchaos thank you! it looks good for me, we need to fix some minor things:

  • can you please sign-off your commit?
git commit --amend --signoff
git push --force

README.md Outdated
@@ -56,6 +56,7 @@ override['ssh-hardening']['ssh']['server']['listen_to'] = node['ipaddress']
* `['ssh-hardening']['ssh']['server']['allow_tcp_forwarding']` - `false`. Set to `true` to allow TCP Forwarding
* `['ssh-hardening']['ssh']['server']['allow_agent_forwarding']` - `false`. Set to `true` to allow Agent Forwarding
* `['ssh-hardening']['ssh']['server']['allow_x11_forwarding']` - `false`. Set to `true` to allow X11 Forwarding
* `['ssh-hardening']['ssh']['server']['permit_tunnel']` - `false` to disable tun device forwarding. Set to `true` to allow tun device forwarding. Other accepted values: 'yes', 'no', 'point-to-point', 'ethernet', see man sshd\_config pages for exact behaviors. Do note you'll also need to allow TCP forwarding.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please change it this way in order to match the style of README.md:

* `['ssh-hardening']['ssh']['server']['permit_tunnel']` - `false`. Set to `true` to allow tun device forwarding. Other accepted values: 'yes', 'no', 'point-to-point', 'ethernet', see `man sshd_config` for more details. Note: `allow_tcp_forwarding` should be enabled together with this option.

Something like this?

@artem-sidorenko
Copy link
Member

@bobchaos are your familiar with chefspec tests? If yes, can you please add some tests for this option? (see this as a possible example).

If not and you don't have much time to learn it - its not a problem, I'll add them by myself just on top to this PR.

@coveralls
Copy link

coveralls commented Nov 20, 2018

Coverage Status

Coverage decreased (-0.8%) to 99.024% when pulling 69b6330 on pbsc:permittunnel into 3b7f92f on dev-sec:master.

spec/recipes/server_spec.rb Outdated Show resolved Hide resolved
spec/recipes/server_spec.rb Outdated Show resolved Hide resolved
spec/recipes/server_spec.rb Outdated Show resolved Hide resolved
Copy link
Member

@artem-sidorenko artem-sidorenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bobchaos this looks very good! Thanks!

If you would make rubocop happy and (optionally) squash your commits (but only your commits) to one commit (to avoid fixup commits to previsious commits) we are ready to merge! And you can be proud of a very good PR with usage of advanced git features ;-)

@bobchaos
Copy link
Author

fixed and squashed!

runningman84 and others added 6 commits November 21, 2018 19:22
Signed-off-by: Marc Chamberland <mchamberland@pbsc.com>
somehow the exception is not really catched, maybe because of the lazy execution somehow.
If expect is called in the usual way with () instead of {} you see the exception. Weird thing.

Signed-off-by: Artem Sidorenko <artem@posteo.de>
@artem-sidorenko artem-sidorenko merged commit 51c18f2 into dev-sec:master Nov 21, 2018
@artem-sidorenko artem-sidorenko changed the title Permittunnel permit_tunnel attribute - allow tun device forwarding Nov 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants