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

os-02: Fix for SUSE environments #70

Merged
merged 1 commit into from
Jun 27, 2017
Merged

Conversation

mcgege
Copy link
Member

@mcgege mcgege commented Jun 26, 2017

Missing shadow_group + test for SUSE

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.

@mcgege thanks for this PR!

@@ -21,7 +21,7 @@
login_defs_passmaxdays = attribute('login_defs_passmaxdays', default: '60', description: 'Default password maxdays to set in login.defs')
login_defs_passmindays = attribute('login_defs_passmindays', default: '7', description: 'Default password mindays to set in login.defs')
login_defs_passwarnage = attribute('login_defs_passwarnage', default: '7', description: 'Default password warnage (days) to set in login.defs')
if os.redhat?
if os.redhat? || os.suse?
shadow_group = 'root'
elsif os.debian?
shadow_group = 'shadow'
Copy link
Member

Choose a reason for hiding this comment

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

@chris-rock @atomic111 we do not have here a sane default. It means in the situation prior to this change on a suse system shadow_group would be nil. How should we handle this? Options from my POV:

  1. introduction of else and raise, something like:
shadow_group = if os.redhat?
                              'root'
                            elsif
                              'shadow'
                            else
                              raise 'Unsupported platform'
                            end
  1. defined sane default, like
shadow_group = 'root'
if os_debian?
  shadow_group = 'shadow'
end

Ideas, opinions?

Copy link
Member

Choose a reason for hiding this comment

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

The correct way is 1., but InSpec does not support exceptions yet. This is a high-prio ticket for one of the next InSpec releases. Should we go with 2. for now?

Copy link
Member Author

Choose a reason for hiding this comment

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

@chris-rock As I had to rework my patch I already implemented the second option ...

@mcgege
Copy link
Member Author

mcgege commented Jun 26, 2017

Had to rework my patch as the default group for /etc/shadow is "shadow" and group readable on a fresh SUSE system

mcgege added a commit to mcgege/puppet-os-hardening that referenced this pull request Jun 26, 2017
- for SUSE and Ubuntu environments: group should be "shadow" and file mode "0640"
(for SUSE also corrected in Baseline, see dev-sec/linux-baseline#70)
@@ -103,7 +100,7 @@
describe file('/etc/shadow') do
it { should_not be_readable.by('group') }
end
elsif os.debian?
elsif os.debian? || os.suse?
Copy link
Member

Choose a reason for hiding this comment

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

@mcgege can we do here the same/similar like above? Maybe via if os.debian? || os.suse? & else construction

@artem-sidorenko
Copy link
Member

@mcgege and my bad, I totally forgotten to ask you to sign-off your commits :) (and we do not have a bot yet), could you please sign-off your future commits?

Signed-off-by: Michael Geiger <michael.geiger@telekom.de>
@mcgege
Copy link
Member Author

mcgege commented Jun 27, 2017

@artem-sidorenko done

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.

@mcgege thanks! looks good for me

@chris-rock any remarks?

Copy link
Member

@chris-rock chris-rock left a comment

Choose a reason for hiding this comment

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

Perfect, thank you @mcgege

@chris-rock chris-rock merged commit e192b1e into dev-sec:master Jun 27, 2017
@mcgege mcgege deleted the os-02 branch July 3, 2017 13:00
deric4 pushed a commit to Logicworks/cis-dil-benchmark that referenced this pull request Jan 5, 2021
* refactor `attribute()` -> `input()`
* fix regression/add supportfor evaluating gid on /etc/shadow and friends on
	- debian
	- suse
	- alpine
  Ref:
  	- dev-sec#33
	- dev-sec/linux-baseline#70

on-behalf-of: @Logicworks <dmiguel@logicworks.net>
Signed-off-by: Deric Miguel <dmiguel@logicworks.net>
deric4 pushed a commit to Logicworks/cis-dil-benchmark that referenced this pull request Jan 7, 2021
* refactor `attribute()` -> `input()`
* fix regression/add supportfor evaluating gid on /etc/shadow and friends on
	- debian
	- suse
	- alpine
  Ref:
  	- dev-sec#33
	- dev-sec/linux-baseline#70

on-behalf-of: @Logicworks <dmiguel@logicworks.net>
Signed-off-by: Deric Miguel <dmiguel@logicworks.net>
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.

3 participants