Remove the PAM deactivation enforcement #72
Conversation
| describe sshd_config do | ||
| its('UsePAM') { should eq('no') } |
There was a problem hiding this comment.
should we check for UsePAM yes?
There was a problem hiding this comment.
Do we have reasons to do that? In my eyes if we check for UsePAM yes, it gets to the hardening requirement, but its not a hardening requirement (at least anymore). Do I miss something?
There was a problem hiding this comment.
its('UsePAM') { should cmp 'yes' } would check that pam is activated, right?
There was a problem hiding this comment.
yes...but why?:)
There was a problem hiding this comment.
Its fine with me, I am just asking. Opinion @atomic111 ?
There was a problem hiding this comment.
@artem-sidorenko and @chris-rock yes we can set PAM to yes and remove the check, because we are checking PasswordAuthentication = no and ChallengeResponseAuthentication = no. But we have already the sshd-29, which checks for ChallengeResponseAuthentication = no. i would recommend to remove this PAM check and renumber the rest.
There was a problem hiding this comment.
@atomic111 good catch, I updated the PR
artem-sidorenko
changed the title
|
General question about renumbering: are we sure we want to do this? If you use profile inheritance, renumbering will break rewrites or adaptions of each test. In general we should avoid renumbering, we should just introduce new numbers. |
|
@chris-rock Hm, I was not aware of inspec's profile inheritance features. I'll update the PR and remove the renumbering |
as PAM should be enabled per default on the most distros: - dev-sec/chef-ssh-hardening#96 - dev-sec/ansible-ssh-hardening#23 - dev-sec/puppet-ssh-hardening#53
|
done |
|
This feature is quite new. It is documented here: http://inspec.io/docs/reference/profiles/ and this is an example profile https://github.com/chris-rock/acme-inspec-profile. Does not use specific control ids though. |
|
@atomic111 I like the current adaption. If you're okay, please merge it. |
|
@chris-rock thanks for the hint. @artem-sidorenko thanks for removing this test |
as PAM should be enabled per default on the most distros: