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

Unique export file for security policy resource #2350

Merged
merged 2 commits into from Nov 29, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 8 additions & 4 deletions lib/resources/security_policy.rb
Expand Up @@ -14,6 +14,7 @@
# parameters. Therefore we need a combination of Registry and secedit output

require 'hashie'
require 'securerandom'

module Inspec::Resources
# known and supported MS privilege rights
Expand Down Expand Up @@ -108,22 +109,25 @@ def to_s
def read_content
return @content if defined?(@content)

# using random hex to prevent any race conditions with multiple runners
export_file = "win_secpol-#{SecureRandom.hex}.cfg"

# export the security policy
cmd = inspec.command('secedit /export /cfg win_secpol.cfg')
cmd = inspec.command("secedit /export /cfg #{export_file}")
return nil if cmd.exit_status.to_i != 0

# store file content
cmd = inspec.command('Get-Content win_secpol.cfg')
cmd = inspec.command("Get-Content #{export_file}")
return skip_resource "Can't read security policy" if cmd.exit_status.to_i != 0
@content = cmd.stdout

if @content.empty? && !file.empty?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was removed as it seems to be legacy code. We don't have any local file variable. In the future we could look into using the Train::File to capture the content of the policy.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should remove the content check completely. If the content is empty, we should not expect that to be okay, so it is probably a failure. Can we add a test that ensures the parsing is not crashing if we have an empty file?

if @content.empty?
return skip_resource "Can't read security policy"
end
@content
ensure
# delete temp file
inspec.command('Remove-Item win_secpol.cfg').exit_status.to_i
inspec.command("Remove-Item #{export_file}").exit_status.to_i
end

def read_params
Expand Down
6 changes: 3 additions & 3 deletions test/helper.rb
Expand Up @@ -214,9 +214,9 @@ def md.directory?
'ps axo label,pid,pcpu,pmem,vsz,rss,tty,stat,start,time,user:32,command' => cmd.call('ps-axoZ'),
'ps -o pid,vsz,rss,tty,stat,time,ruser,args' => cmd.call('ps-busybox'),
'ps --help' => empty.call,
'Get-Content win_secpol.cfg' => cmd.call('secedit-export'),
'secedit /export /cfg win_secpol.cfg' => cmd.call('success'),
'Remove-Item win_secpol.cfg' => cmd.call('success'),
'Get-Content win_secpol-abc123.cfg' => cmd.call('secedit-export'),
'secedit /export /cfg win_secpol-abc123.cfg' => cmd.call('success'),
'Remove-Item win_secpol-abc123.cfg' => cmd.call('success'),
'env' => cmd.call('env'),
'${Env:PATH}' => cmd.call('$env-PATH'),
# registry key test using winrm 2.0
Expand Down
2 changes: 2 additions & 0 deletions test/unit/resources/security_policy_test.rb
Expand Up @@ -8,6 +8,8 @@
describe 'Inspec::Resources::SecurityPolicy' do
it 'verify processes resource' do
resource = load_resource('security_policy')
SecureRandom.expects(:hex).returns('abc123')

_(resource.MaximumPasswordAge).must_equal 42
_(resource.send('MACHINE\Software\Microsoft\Windows NT\CurrentVersion\Setup\RecoveryConsole\SecurityLevel')).must_equal '4,0'
_(resource.SeUndockPrivilege).must_equal ["S-1-5-32-544"]
Expand Down