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

use simple config for security policy resource #1044

Merged
merged 1 commit into from
Sep 12, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 67 additions & 38 deletions lib/resources/security_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,72 +13,101 @@
# All local GPO parameters can be examined via Registry, but not all security
# parameters. Therefore we need a combination of Registry and secedit output

require 'hashie'

module Inspec::Resources
class SecurityPolicy < Inspec.resource(1)
name 'security_policy'
desc 'Use the security_policy InSpec audit resource to test security policies on the Microsoft Windows platform.'
example "
describe security_policy do
its('SeNetworkLogonRight') { should eq '*S-1-5-11' }
its('SeNetworkLogonRight') { should include 'S-1-5-11' }
end
"
def initialize
@loaded = false
@policy = nil
@exit_status = nil

def content
read_content
end

def params(*opts)
Copy link
Contributor

Choose a reason for hiding this comment

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

A number of the resources include a params function that feels similar. Might be nice to pull this out into a module where the behavior can be tested once.

It's a bit strange that some of the resources take arguments for their params method while others return a hash.

Fine for now, just figured it was worth pointing out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, resources need some harmonization here and it would be nice to extract some basics. This improvement makes sense to me...

opts.inject(read_params) do |res, nxt|
res.respond_to?(:key) ? res[nxt] : nil
end
end

def method_missing(name)
params = read_params
return nil if params.nil?

# deep search for hash key
params.extend Hashie::Extensions::DeepFind
res = params.deep_find(name.to_s)
res
end

def to_s
'Security Policy'
end

# load security content
def load
private

def read_content
return @content if defined?(@content)

# export the security policy
cmd = inspec.command('secedit /export /cfg win_secpol.cfg')
Copy link
Contributor

Choose a reason for hiding this comment

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

This is existing code, but is there any concern about using a known filename here rather than a temporary file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I'd like to do that in another iteration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#1047 covers the requirement to use a temporary generated file

return nil if cmd.exit_status.to_i != 0

# store file content
cmd = inspec.command('Get-Content win_secpol.cfg')
@exit_status = cmd.exit_status.to_i
return nil if @exit_status != 0
@policy = cmd.stdout
@loaded = true

# returns self
self
return skip_resource "Can't read security policy" if cmd.exit_status.to_i != 0
@content = cmd.stdout

if @content.empty? && file.size > 0
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
end

def method_missing(method)
# load data if needed
if @loaded == false
load
end

# find line with key
key = Regexp.escape(method.to_s)
target = ''
@policy.each_line {|s|
target = s.strip if s =~ /^\s*#{key}\s*=\s*(.*)\b/
}
def read_params
return @params if defined?(@params)
return @params = {} if read_content.nil?

# extract variable value
result = target.match(/[=]{1}\s*(?<value>.*)/)
conf = SimpleConfig.new(
@content,
assignment_re: /^\s*(.*)=\s*(\S*)\s*$/,
)
@params = convert_hash(conf.params)
end

if !result.nil?
val = result[:value]
val = val.to_i if val =~ /^\d+$/
# extracts the values, this methods detects:
# numbers and SIDs and optimizes them for further usage
def extract_value(val)
if val =~ /^\d+$/
val.to_i
# special handling for SID array
elsif val =~ /^\*\S/
val.split(',').map { |v|
v.sub('*S', 'S')
}
# special handling for string values with "
elsif !(m = /^\"(.*)\"$/.match(val)).nil?
m[1]
else
# TODO: we may need to return skip or failure if the
# requested value is not available
val = nil
val
end

val
end

def to_s
'Security Policy'
def convert_hash(hash)
new_hash = {}
hash.each do |k, v|
v.is_a?(Hash) ? value = convert_hash(v) : value = extract_value(v)
new_hash[k.strip] = value
end
new_hash
end
end
end
4 changes: 2 additions & 2 deletions test/unit/resources/security_policy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
resource = load_resource('security_policy')
_(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'
_(resource.SeRemoteInteractiveLogonRight).must_equal '*S-1-5-32-544,*S-1-5-32-555'
_(resource.SeUndockPrivilege).must_equal ["S-1-5-32-544"]
_(resource.SeRemoteInteractiveLogonRight).must_equal ["S-1-5-32-544","S-1-5-32-555"]
end
end