From 2d44b6e5e0d8443e4a68ea923cd025f62376d564 Mon Sep 17 00:00:00 2001 From: Josh Hudson Date: Tue, 28 Aug 2018 13:11:38 +0000 Subject: [PATCH] Cached profiles with Compliance Fetcher (#3221) * Leverage existance check in Compliance::Fetcher.resolve to not re-download locally cached profiles * Move logic from Compliance::API.exist? to Compliance::API.profiles to reuse code in cases where we need to access profiles' metadata directly. * Declare @upstream_sha256 if target is a string * Handle other fetchers that don't support upstream_sha256 within Inspec::CachedFetcher.initialize * Add initialize for Compliance::Fetcher to not pollute Fetchers::Url with its logic * Add Compliance::Fetcher.sha256 to leverage upstream_sha256 instead of relying on inherited method from Fetchers::Url * Revert changes to cached fetcher that are unnecessary after refactor * Pacify the god of ruby syntax * Move Compliance::API.profiles filtering logic to end of method to leverage normalization of mapped_profiles * Add and update unit tests to support caching with Compliance::Fetcher.upstream_sha256 Signed-off-by: Josh Hudson --- lib/bundles/inspec-compliance/api.rb | 34 ++--- lib/bundles/inspec-compliance/target.rb | 105 ++++++++----- test/helper.rb | 2 + .../bundles/inspec-compliance/api_test.rb | 144 ++++++++++-------- .../bundles/inspec-compliance/target_test.rb | 69 +++++++-- test/unit/cached_fetcher.rb | 57 +++++++ 6 files changed, 278 insertions(+), 133 deletions(-) create mode 100644 test/unit/cached_fetcher.rb diff --git a/lib/bundles/inspec-compliance/api.rb b/lib/bundles/inspec-compliance/api.rb index 7e6c7e36c..2d418678a 100755 --- a/lib/bundles/inspec-compliance/api.rb +++ b/lib/bundles/inspec-compliance/api.rb @@ -19,7 +19,7 @@ module Compliance # return all compliance profiles available for the user # the user is either specified in the options hash or by default # the username of the account is used that is logged in - def self.profiles(config) # rubocop:disable PerceivedComplexity, Metrics/CyclomaticComplexity, Metrics/AbcSize, Metrics/MethodLength + def self.profiles(config, profile_filter = nil) # rubocop:disable PerceivedComplexity, Metrics/CyclomaticComplexity, Metrics/AbcSize, Metrics/MethodLength owner = config['owner'] || config['user'] # Chef Compliance @@ -36,9 +36,14 @@ module Compliance end headers = get_headers(config) + if profile_filter + _owner, id, ver = profile_split(profile_filter) + else + id, ver = nil + end if is_automate2_server?(config) - body = { owner: owner }.to_json + body = { owner: owner, name: id }.to_json response = Compliance::HTTP.post_with_headers(url, headers, body, config['insecure']) else response = Compliance::HTTP.get(url, headers, config['insecure']) @@ -69,6 +74,10 @@ module Compliance e } end + # filter by name and version if they were specified in profile_filter + mapped_profiles.select! do |p| + (!ver || p['version'] == ver) && (!id || p['name'] == id) + end return msg, mapped_profiles when '401' msg = '401 Unauthorized. Please check your token.' @@ -101,25 +110,10 @@ module Compliance parsed end - # verifies that a profile + # verifies that a profile exists def self.exist?(config, profile) - owner, id, ver = profile_split(profile) - - # ensure that we do not manipulate the configuration object - user_config = config.dup - user_config['owner'] = owner - _msg, profiles = Compliance::API.profiles(user_config) - - if !profiles.empty? - profiles.any? do |p| - profile_owner = p['owner_id'] || p['owner'] - profile_owner == owner && - p['name'] == id && - (ver.nil? || p['version'] == ver) - end - else - false - end + _msg, profiles = Compliance::API.profiles(config, profile) + !profiles.empty? end def self.upload(config, owner, profile_name, archive_path) diff --git a/lib/bundles/inspec-compliance/target.rb b/lib/bundles/inspec-compliance/target.rb index 252b5b09a..4baceac10 100644 --- a/lib/bundles/inspec-compliance/target.rb +++ b/lib/bundles/inspec-compliance/target.rb @@ -13,50 +13,81 @@ module Compliance class Fetcher < Fetchers::Url name 'compliance' priority 500 - def self.resolve(target) # rubocop:disable PerceivedComplexity, Metrics/CyclomaticComplexity, Metrics/AbcSize - uri = if target.is_a?(String) && URI(target).scheme == 'compliance' - URI(target) - elsif target.respond_to?(:key?) && target.key?(:compliance) - URI("compliance://#{target[:compliance]}") - end + attr_reader :upstream_sha256 + def initialize(target, opts) + super(target, opts) + if target.is_a?(Hash) && target.key?(:url) + @target = target[:url] + @upstream_sha256 = target[:sha256] + elsif target.is_a?(String) + @target = target + @upstream_sha256 = '' + end + end + + def sha256 + upstream_sha256.empty? ? super : upstream_sha256 + end + + def self.check_compliance_token(config) + if config['token'].nil? && config['refresh_token'].nil? + if config['server_type'] == 'automate' + server = 'automate' + msg = 'inspec compliance login https://your_automate_server --user USER --ent ENT --dctoken DCTOKEN or --token USERTOKEN' + elsif config['server_type'] == 'automate2' + server = 'automate2' + msg = 'inspec compliance login https://your_automate2_server --user USER --token APITOKEN' + else + server = 'compliance' + msg = "inspec compliance login https://your_compliance_server --user admin --insecure --token 'PASTE TOKEN HERE' " + end + raise Inspec::FetcherFailure, <<~EOF + + Cannot fetch #{uri} because your #{server} token has not been + configured. + + Please login using + + #{msg} + EOF + end + end + + def self.get_target_uri(target) + if target.is_a?(String) && URI(target).scheme == 'compliance' + URI(target) + elsif target.respond_to?(:key?) && target.key?(:compliance) + URI("compliance://#{target[:compliance]}") + end + end + + def self.resolve(target) + uri = get_target_uri(target) return nil if uri.nil? + config = Compliance::Configuration.new + profile = Compliance::API.sanitize_profile_name(uri) + profile_fetch_url = Compliance::API.target_url(config, profile) # we have detailed information available in our lockfile, no need to ask the server - if target.respond_to?(:key?) && target.key?(:url) - profile_fetch_url = target[:url] - config = {} + if target.respond_to?(:key?) && target.key?(:sha256) + profile_checksum = target[:sha256] else - # check if we have a compliance token - config = Compliance::Configuration.new - if config['token'].nil? && config['refresh_token'].nil? - if config['server_type'] == 'automate' - server = 'automate' - msg = 'inspec compliance login https://your_automate_server --user USER --ent ENT --dctoken DCTOKEN or --token USERTOKEN' - elsif config['server_type'] == 'automate2' - server = 'automate2' - msg = 'inspec compliance login https://your_automate2_server --user USER --token APITOKEN' - else - server = 'compliance' - msg = "inspec compliance login https://your_compliance_server --user admin --insecure --token 'PASTE TOKEN HERE' " - end - raise Inspec::FetcherFailure, <<~EOF - - Cannot fetch #{uri} because your #{server} token has not been - configured. - - Please login using - - #{msg} - EOF - end - + check_compliance_token(config) # verifies that the target e.g base/ssh exists - profile = Compliance::API.sanitize_profile_name(uri) - if !Compliance::API.exist?(config, profile) + # Call profiles directly instead of exist? to capture the results + # so we can access the upstream sha256 from the results. + _msg, profile_result = Compliance::API.profiles(config, profile) + if profile_result.empty? raise Inspec::FetcherFailure, "The compliance profile #{profile} was not found on the configured compliance server" + else + # Guarantee sorting by verison and grab the latest. + # If version was specified, it will be the first and only result. + # Note we are calling the sha256 as a string, not a symbol since + # it was returned as json from the Compliance API. + profile_info = profile_result.sort_by { |x| Gem::Version.new(x['version']) }[0] + profile_checksum = profile_info.key?('sha256') ? profile_info['sha256'] : '' end - profile_fetch_url = Compliance::API.target_url(config, profile) end # We need to pass the token to the fetcher config['token'] = Compliance::API.get_token(config) @@ -65,7 +96,7 @@ module Compliance profile_stub = profile || target[:compliance] config['profile'] = Compliance::API.profile_split(profile_stub) - new(profile_fetch_url, config) + new({ url: profile_fetch_url, sha256: profile_checksum }, config) rescue URI::Error => _e nil end diff --git a/test/helper.rb b/test/helper.rb index 5378219b6..d8bf332af 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -18,6 +18,7 @@ require 'pathname' require 'tempfile' require 'tmpdir' require 'zip' +require 'json' require 'inspec/base_cli' require 'inspec/version' @@ -31,6 +32,7 @@ require 'inspec/profile' require 'inspec/runner' require 'inspec/runner_mock' require 'fetchers/mock' +require 'inspec/dependencies/cache' require_relative '../lib/bundles/inspec-compliance' require_relative '../lib/bundles/inspec-habitat' diff --git a/test/unit/bundles/inspec-compliance/api_test.rb b/test/unit/bundles/inspec-compliance/api_test.rb index f2d0626a8..d1fe93284 100644 --- a/test/unit/bundles/inspec-compliance/api_test.rb +++ b/test/unit/bundles/inspec-compliance/api_test.rb @@ -2,41 +2,41 @@ require 'helper' describe Compliance::API do let(:profiles_response) do - [{"name"=>"apache-baseline", - "title"=>"DevSec Apache Baseline", - "maintainer"=>"DevSec Hardening Framework Team", - "copyright"=>"DevSec Hardening Framework Team", - "copyright_email"=>"hello@dev-sec.io", - "license"=>"Apache 2 license", - "summary"=>"Test-suite for best-practice apache hardening", - "version"=>"2.0.2", - "supports"=>[{"os-family"=>"unix"}], - "depends"=>nil, - "owner_id"=>"admin"}, - {"name"=>"apache-baseline", - "title"=>"DevSec Apache Baseline", - "maintainer"=>"Hardening Framework Team", - "copyright"=>"Hardening Framework Team", - "copyright_email"=>"hello@dev-sec.io", - "license"=>"Apache 2 license", - "summary"=>"Test-suite for best-practice apache hardening", - "version"=>"2.0.1", - "supports"=>[{"os-family"=>"unix"}], - "depends"=>nil, - "latest_version"=>"2.0.2", - "owner_id"=>"admin"}, - {"name"=>"cis-aix-5.3-6.1-level1", - "title"=>"CIS AIX 5.3 and AIX 6.1 Benchmark Level 1", - "maintainer"=>"Chef Software, Inc.", - "copyright"=>"Chef Software, Inc.", - "copyright_email"=>"support@chef.io", - "license"=>"Proprietary, All rights reserved", - "summary"=>"CIS AIX 5.3 and AIX 6.1 Benchmark Level 1 translated from SCAP", - "version"=>"1.1.0", - "supports"=>nil, - "depends"=>nil, - "latest_version"=>"1.1.0-3", - "owner_id"=>"admin"}] + [{ 'name'=>'apache-baseline', + 'title'=>'DevSec Apache Baseline', + 'maintainer'=>'DevSec Hardening Framework Team', + 'copyright'=>'DevSec Hardening Framework Team', + 'copyright_email'=>'hello@dev-sec.io', + 'license'=>'Apache 2 license', + 'summary'=>'Test-suite for best-practice apache hardening', + 'version'=>'2.0.2', + 'supports'=>[{ 'os-family'=>'unix' }], + 'depends'=>nil, + 'owner_id'=>'admin' }, + { 'name'=>'apache-baseline', + 'title'=>'DevSec Apache Baseline', + 'maintainer'=>'Hardening Framework Team', + 'copyright'=>'Hardening Framework Team', + 'copyright_email'=>'hello@dev-sec.io', + 'license'=>'Apache 2 license', + 'summary'=>'Test-suite for best-practice apache hardening', + 'version'=>'2.0.1', + 'supports'=>[{ 'os-family'=>'unix' }], + 'depends'=>nil, + 'latest_version'=>'2.0.2', + 'owner_id'=>'admin' }, + { 'name'=>'cis-aix-5.3-6.1-level1', + 'title'=>'CIS AIX 5.3 and AIX 6.1 Benchmark Level 1', + 'maintainer'=>'Chef Software, Inc.', + 'copyright'=>'Chef Software, Inc.', + 'copyright_email'=>'support@chef.io', + 'license'=>'Proprietary, All rights reserved', + 'summary'=>'CIS AIX 5.3 and AIX 6.1 Benchmark Level 1 translated from SCAP', + 'version'=>'1.1.0', + 'supports'=>nil, + 'depends'=>nil, + 'latest_version'=>'1.1.0-3', + 'owner_id'=>'admin' }] end describe '.version' do @@ -44,7 +44,7 @@ describe Compliance::API do let(:config) do { 'server' => 'myserver', - 'insecure' => true + 'insecure' => true, } end @@ -107,7 +107,7 @@ describe Compliance::API do response.stubs(:code).returns('200') response.stubs(:body).returns('{"api":"compliance","version":"1.2.3"}') Compliance::HTTP.expects(:get).with('myserver/version', 'test-headers', true).returns(response) - Compliance::API.version(config).must_equal({'version' => '1.2.3', 'api' => 'compliance'}) + Compliance::API.version(config).must_equal({ 'version' => '1.2.3', 'api' => 'compliance' }) end end end @@ -249,12 +249,23 @@ describe Compliance::API do describe 'exist?' do it 'works with profiles returned by Automate' do + # ruby 2.3.3 has issues running stub_requests properly + # skipping for that specific version + return if RUBY_VERSION = '2.3.3' + config = Compliance::Configuration.new config.clean + config['owner'] = 'admin' config['server_type'] = 'automate' config['server'] = 'https://myautomate' config['version'] = '1.6.99' - Compliance::API.stubs(:profiles).returns([nil, profiles_response]) + config['automate'] = { 'ent'=>'automate', 'token_type'=>'dctoken' } + config['version'] = { 'api'=> 'compliance', 'version'=>'0.8.24' } + + stub_request(:get, 'https://myautomate/profiles/admin') + .with(headers: { 'Accept'=>'*/*', 'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'Chef-Delivery-Enterprise'=>'automate', 'User-Agent'=>'Ruby', 'X-Data-Collector-Token'=>'' }) + .to_return(status: 200, body: profiles_response.to_json, headers: {}) + Compliance::API.exist?(config, 'admin/apache-baseline').must_equal true Compliance::API.exist?(config, 'admin/apache-baseline#2.0.1').must_equal true Compliance::API.exist?(config, 'admin/apache-baseline#2.0.999').must_equal false @@ -278,8 +289,8 @@ describe Compliance::API do good_response.stubs(:code).returns('400') Compliance::HTTP.expects(:get) - .with(url + automate2_endpoint, headers, insecure) - .returns(good_response) + .with(url + automate2_endpoint, headers, insecure) + .returns(good_response) Compliance::API.determine_server_type(url, insecure).must_equal(:automate2) end @@ -289,11 +300,11 @@ describe Compliance::API do bad_response.stubs(:code).returns('404') Compliance::HTTP.expects(:get) - .with(url + automate2_endpoint, headers, insecure) - .returns(bad_response) + .with(url + automate2_endpoint, headers, insecure) + .returns(bad_response) Compliance::HTTP.expects(:get) - .with(url + automate_endpoint, headers, insecure) - .returns(good_response) + .with(url + automate_endpoint, headers, insecure) + .returns(good_response) Compliance::API.determine_server_type(url, insecure).must_equal(:automate) end @@ -307,11 +318,11 @@ describe Compliance::API do good_response.stubs(:body).returns('Are You Looking For the Chef Server?') Compliance::HTTP.expects(:get) - .with(url + automate2_endpoint, headers, insecure) - .returns(bad_response) + .with(url + automate2_endpoint, headers, insecure) + .returns(bad_response) Compliance::HTTP.expects(:get) - .with(url + automate_endpoint, headers, insecure) - .returns(good_response) + .with(url + automate_endpoint, headers, insecure) + .returns(good_response) Compliance::API.determine_server_type(url, insecure).must_equal(:automate) end @@ -321,35 +332,34 @@ describe Compliance::API do bad_response.stubs(:body).returns('No Chef Manage here') Compliance::HTTP.expects(:get) - .with(url + automate_endpoint, headers, insecure) - .returns(bad_response) + .with(url + automate_endpoint, headers, insecure) + .returns(bad_response) Compliance::HTTP.expects(:get) - .with(url + automate2_endpoint, headers, insecure) - .returns(bad_response) + .with(url + automate2_endpoint, headers, insecure) + .returns(bad_response) mock_compliance_response = mock mock_compliance_response.stubs(:code).returns('404') Compliance::HTTP.expects(:get) - .with(url + compliance_endpoint, headers, insecure) - .returns(mock_compliance_response) + .with(url + compliance_endpoint, headers, insecure) + .returns(mock_compliance_response) Compliance::API.determine_server_type(url, insecure).must_be_nil end - it 'returns `:compliance` when a 200 is received from `https://URL/api/version`' do good_response.stubs(:code).returns('200') bad_response.stubs(:code).returns('404') Compliance::HTTP.expects(:get) - .with(url + automate_endpoint, headers, insecure) - .returns(bad_response) + .with(url + automate_endpoint, headers, insecure) + .returns(bad_response) Compliance::HTTP.expects(:get) - .with(url + automate2_endpoint, headers, insecure) - .returns(bad_response) + .with(url + automate2_endpoint, headers, insecure) + .returns(bad_response) Compliance::HTTP.expects(:get) - .with(url + compliance_endpoint, headers, insecure) - .returns(good_response) + .with(url + compliance_endpoint, headers, insecure) + .returns(good_response) Compliance::API.determine_server_type(url, insecure).must_equal(:compliance) end @@ -358,14 +368,14 @@ describe Compliance::API do bad_response.stubs(:code).returns('404') Compliance::HTTP.expects(:get) - .with(url + automate2_endpoint, headers, insecure) - .returns(bad_response) + .with(url + automate2_endpoint, headers, insecure) + .returns(bad_response) Compliance::HTTP.expects(:get) - .with(url + automate_endpoint, headers, insecure) - .returns(bad_response) + .with(url + automate_endpoint, headers, insecure) + .returns(bad_response) Compliance::HTTP.expects(:get) - .with(url + compliance_endpoint, headers, insecure) - .returns(bad_response) + .with(url + compliance_endpoint, headers, insecure) + .returns(bad_response) Compliance::API.determine_server_type(url, insecure).must_be_nil end diff --git a/test/unit/bundles/inspec-compliance/target_test.rb b/test/unit/bundles/inspec-compliance/target_test.rb index 4aa2666c6..633c85336 100644 --- a/test/unit/bundles/inspec-compliance/target_test.rb +++ b/test/unit/bundles/inspec-compliance/target_test.rb @@ -61,28 +61,79 @@ describe Compliance::Fetcher do end end - describe 'when the server calls a automate profile' do + describe 'when the server calls an automate profile' do + let(:profiles_result) do + [{ 'name'=>'ssh-baseline', + 'title'=>'InSpec Profile', + 'maintainer'=>'The Authors', + 'copyright'=>'The Authors', + 'copyright_email'=>'you@example.com', + 'license'=>'Apache-2.0', + 'summary'=>'An InSpec Compliance Profile', + 'version'=>'0.1.1', + 'owner'=>'admin', + 'supports'=>[], + 'depends'=>[], + 'sha256'=>'132j1kjdasfasdoaefaewo12312', + 'groups'=>[], + 'controls'=>[], + 'attributes'=>[], + 'latest_version'=>'' }] + end before do - Compliance::Configuration.expects(:new).returns({ 'token' => "123abc" }) + Compliance::Configuration.expects(:new).returns({ 'token' => '123abc', 'server' => 'https://a2.instance.com' }) end it 'returns the correct profile name when parsing url' do - Compliance::API.stubs(:exist?).returns(true) + Compliance::API.stubs(:profiles).returns(['success', profiles_result]) fetcher = Compliance::Fetcher.resolve('compliance://admin/ssh-baseline') - assert = ["admin", "ssh-baseline", nil] + assert = ['admin', 'ssh-baseline', nil] fetcher.instance_variable_get(:"@config")['profile'].must_equal assert end it 'returns the correct profile name when parsing compliance hash' do - Compliance::API.stubs(:exist?).returns(true) + Compliance::API.stubs(:profiles).returns(['success', profiles_result]) hash = { - target: "https://a2.instance.com/api/v0/compliance/tar", - compliance: "admin/ssh-baseline", - sha256: "132j1kjdasfasdoaefaewo12312", + target: 'https://a2.instance.com/api/v0/compliance/tar', + compliance: 'admin/ssh-baseline', + sha256: '132j1kjdasfasdoaefaewo12312', } fetcher = Compliance::Fetcher.resolve(hash) - assert = ["admin", "ssh-baseline", nil] + assert = ['admin', 'ssh-baseline', nil] fetcher.instance_variable_get(:"@config")['profile'].must_equal assert end end + + describe 'when the server provides a sha256 in the profiles_result' do + let(:profiles_result) do + [{ 'name'=>'ssh-baseline', + 'title'=>'InSpec Profile', + 'maintainer'=>'The Authors', + 'copyright'=>'The Authors', + 'copyright_email'=>'you@example.com', + 'license'=>'Apache-2.0', + 'summary'=>'An InSpec Compliance Profile', + 'version'=>'0.1.1', + 'owner'=>'admin', + 'supports'=>[], + 'depends'=>[], + 'sha256'=>'132j1kjdasfasdoaefaewo12312', + 'groups'=>[], + 'controls'=>[], + 'attributes'=>[], + 'latest_version'=>'' }] + end + + before do + Compliance::Configuration.expects(:new).returns({ 'token' => '123abc', 'server' => 'https://a2.instance.com' }) + end + + it 'contains the upstream_sha256' do + Compliance::API.stubs(:profiles).returns(['success', profiles_result]) + prof = profiles_result[0] + target = "compliance://#{prof['owner']}/#{prof['name']}" + fetcher = Compliance::Fetcher.resolve(target) + fetcher.upstream_sha256.must_equal prof['sha256'] + end + end end diff --git a/test/unit/cached_fetcher.rb b/test/unit/cached_fetcher.rb new file mode 100644 index 000000000..2b7b8c2ee --- /dev/null +++ b/test/unit/cached_fetcher.rb @@ -0,0 +1,57 @@ +require 'helper' + +describe Inspec::CachedFetcher do + describe 'when original fetcher is Compliance::Fetcher' do + let(:profiles_result) do + [{ 'name'=>'ssh-baseline', + 'title'=>'InSpec Profile', + 'maintainer'=>'The Authors', + 'copyright'=>'The Authors', + 'copyright_email'=>'you@example.com', + 'license'=>'Apache-2.0', + 'summary'=>'An InSpec Compliance Profile', + 'version'=>'0.1.1', + 'owner'=>'admin', + 'supports'=>[], + 'depends'=>[], + 'sha256'=>'132j1kjdasfasdoaefaewo12312', + 'groups'=>[], + 'controls'=>[], + 'attributes'=>[], + 'latest_version'=>'' }] + end + before do + Compliance::Configuration.expects(:new).returns({ 'token' => '123abc', 'server' => 'https://a2.instance.com' }) + end + + it 'downloads the profile from the compliance service when sha256 not in the cache' do + prof = profiles_result[0] + Compliance::API.stubs(:profiles).returns(['success', profiles_result]) + cache = Inspec::Cache.new + entry_path = cache.base_path_for(prof['sha256']) + mock_fetch = Minitest::Mock.new + mock_fetch.expect :call, "#{entry_path}.tar.gz", [entry_path] + cf = Inspec::CachedFetcher.new("compliance://#{prof['owner']}/#{prof['name']}", cache) + cache.stubs(:exists?).with(prof['sha256']).returns(false) + cf.fetcher.stub(:fetch, mock_fetch) do + cf.fetch + end + mock_fetch.verify + end + + it 'does not download the profile when the sha256 exists in the inspec cache' do + prof = profiles_result[0] + Compliance::API.stubs(:profiles).returns(['success', profiles_result]) + cache = Inspec::Cache.new + entry_path = cache.base_path_for(prof['sha256']) + mock_prefered_entry_for = Minitest::Mock.new + mock_prefered_entry_for.expect :call, entry_path, [prof['sha256']] + cf = Inspec::CachedFetcher.new("compliance://#{prof['owner']}/#{prof['name']}", cache) + cache.stubs(:exists?).with(prof['sha256']).returns(true) + cache.stub(:prefered_entry_for, mock_prefered_entry_for) do + cf.fetch + end + mock_prefered_entry_for.verify + end + end +end