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

WIP: ssh baseline refactoring. #126

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

JHeinzde
Copy link

@JHeinzde JHeinzde commented Apr 6, 2019

This is a WIP refactoring of the ssh baseline to match the chef-ssh-hardening implementation.

@chris-rock
Copy link
Member

@JHeinzde Very nice. I am looking forward to see this work completed

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.

@JHeinzde great work! Lets maybe try to split it to different PRs to be able to move forward fast. As suggestion/idea:

  • one PR related to the linting/rubocop stuff
  • another PR with renaming of ssh_version to real_ssh_version and switch of current controls to it
  • next PR with first implementation of ssh_version and only for privlege_separation part
  • next PR or PRs with crypto stuff, algorithms etc

What do you think?

.rubocop.yml Outdated Show resolved Hide resolved

FALLBACK_SSH_VERSION ||= 5.9

def real_ssh_version
Copy link
Member

Choose a reason for hiding this comment

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

I guess we need that only as a intermediate step to avoid breaking the baseline and do not need it when everything is complete, right? (I see this command as part of ssh_version)

Copy link
Author

Choose a reason for hiding this comment

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

I hope so yes. This is indeed only temporary to avoid any breaking changes to the baseline.

jheinz added 3 commits June 20, 2019 13:52
Signed-off-by: jheinz <heinzjonathan95@gmail.com>
Replacing the ssh_version in the test with real_ssh_version.

Signed-off-by: jheinz <heinzjonathan95@gmail.com>
…recipe

Signed-off-by: jheinz <heinzjonathan95@gmail.com>
…ued in the get_ssh_version instead of the find_ssh_version. Also adding logoutput to find_ssh_version, since we can't throw an error there like in the devsec_shh.rb in the chef ssh hardening cookbook

Signed-off-by: jheinz <heinzjonathan95@gmail.com>
@JHeinzde
Copy link
Author

JHeinzde commented Jun 20, 2019

Hello @artem-sidorenko, I have put more work into this and will honor the plan you described here, but modify it a bit:

one PR related to the linting/rubocop stuff
another PR with the renaming of ssh_version to real_ssh_version and switch of current controls to it
next PR with a first implementation of ssh_version and only for privlege_separation part
next PR or PRs with crypto stuff, algorithms etc.

Since I think no rename is required to ssh_version its going to stay like this.
I will first submit 2 pull requests. The first PR is going to be aimed at find_ssh_version, guess_ssh_version and PRIVILEGE_SEPARATION and HOSTKEY Algorithms. The second PR is going to introduce the cryptologic of devsec_ssh.rb.

The last one is going to be related to rubocop/other stuff, when I can figure out the consequences of this, since at least for me currently the travis build is broken with these changes I've done

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

3 participants