Adding multiple passwd/group/shadow controls#165
Adding multiple passwd/group/shadow controls#165cmhe wants to merge 7 commits intodev-sec:masterfrom
Conversation
A password changed date in the future could be used to circumvent password expiration dates. This rule checks that any password change dates are in the past. Signed-off-by: Claudius Heine <ch@denx.de>
System users should be prevented from login with exceptions for applications that are non-interactive. Signed-off-by: Claudius Heine <ch@denx.de>
This rule makes sure that the assumptions of user `root` being uid=0 is the sole member of group `root` with gid=0 are true. This prevents access to any root-owned files by non-privileged users. Signed-off-by: Claudius Heine <ch@denx.de>
'+' and '-' where prepended to lines in account files (/etc/passwd, /etc/group, /etc/shadow) to signify if fields should be overwritten or inserted from a NIS server. Since NIS is a insecure and legacy technology, that is replaced by other software, this check makes sure that no such entries exist anymore. Signed-off-by: Claudius Heine <ch@denx.de>
Signed-off-by: Claudius Heine <ch@denx.de>
Members of the shadow group could have access to password hashes and other content of the shadow files. Signed-off-by: Claudius Heine <ch@denx.de>
Signed-off-by: Claudius Heine <ch@denx.de>
56928ba to
4c607b0
Compare
chris-rock
left a comment
There was a problem hiding this comment.
Great improvement @cmhe I added a few questions
| login_defs_passmindays = attribute('login_defs_passmindays', value: '7', description: 'Default password mindays to set in login.defs') | ||
| login_defs_passwarnage = attribute('login_defs_passwarnage', value: '7', description: 'Default password warnage (days) to set in login.defs') | ||
|
|
||
| system_users = passwd.params ? passwd.params.select { |x| x['uid'].to_i < login_defs.UID_MIN.to_i && x['uid'].to_i.positive? } : [] |
There was a problem hiding this comment.
This logic should be part of the control
| control 'os-14' do | ||
| impact 1.0 | ||
| title 'All password change dates are in the past' | ||
| desc 'The password change date is used to detect expired passwords. Entering future dates might circumvent that.' |
There was a problem hiding this comment.
It would be great to add a reference to read more about the reasoning
There was a problem hiding this comment.
The internal document I have doesn't state much more information about this rule, that I wrote here. It is based on CIS, linking to it is sadly to so easy. I could drop this rule if the other rules are ok for you?
| control 'os-17' do | ||
| impact 1.0 | ||
| title 'Prevent + or - fields in passwd an related files used by NIS' | ||
| desc 'NIS is insecure and should not be used' |
There was a problem hiding this comment.
A reference to the rule and reasoning for be beneficial
There was a problem hiding this comment.
This rule is also based on CIS, but expanded upon. Cis only mentions that passwd entries should not include +, but NIS allow allows - entries. This rule tests for both. So I don't have any reference to the exact rule. I could drop this rule as well if required.
This MR bundles some additional checks related to account setting.
I can split those up into multiple MRs with an issue each, but might be easier and simpler to just discuss these changes directly.