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

windows-011 array order failure #63

Open
spencer-cdw opened this issue Nov 15, 2022 · 11 comments
Open

windows-011 array order failure #63

spencer-cdw opened this issue Nov 15, 2022 · 11 comments
Labels

Comments

@spencer-cdw
Copy link
Contributor

spencer-cdw commented Nov 15, 2022

Description

Window-011 check is failing due to an array ordering problem

Screenshot 2022-11-15 at 12 57 17 PM

It is looking for expected: ["S-1-5-9", "S-1-5-32-544"] but is finding got: ["S-1-5-32-544", "S-1-5-9"]

Reproduction steps

This is with a stock windows 2016 build, using the stock inspec defaults

https://github.com/dev-sec/windows-baseline/blob/master/inspec.yml#L34-L38

Current Behavior

expected: ["S-1-5-9", "S-1-5-32-544"]
got: ["S-1-5-32-544", "S-1-5-9"]

Expected Behavior

The array order should not change.
If the array order does change, it should not be considered a failure as long as the contents are equivalent.

OS / Environment

windows 2016

Inspec Version

5.18.14

Baseline Version

name: .
title: InSpec Profile
maintainer: The Authors
copyright: The Authors
copyright_email: [email protected]
license: Apache-2.0
summary: An InSpec Compliance Profile
version: 0.1.0
supports:
  platform-family: windows
depends:
  - name: windows-baseline
    git: https://github.com/dev-sec/windows-baseline
    tag: 2.1.9

Additional information

No response

@spencer-cdw
Copy link
Contributor Author

A quick workaround may be to just override the default ordering in the inspec.yml file

name: .
title: InSpec Profile
maintainer: The Authors
copyright: The Authors
copyright_email: [email protected]
license: Apache-2.0
summary: An InSpec Compliance Profile
version: 0.1.0
supports:
  platform-family: windows
depends:
  - name: windows-baseline
    git: https://github.com/dev-sec/windows-baseline
    tag: 2.1.9
inputs:
  - name: se_network_logon_right
    value: ['S-1-5-32-544', 'S-1-5-9']

@aaronlippold
Copy link
Member

aaronlippold commented Nov 18, 2022 via email

@aaronlippold
Copy link
Member

aaronlippold commented Nov 18, 2022 via email

@spencer-cdw
Copy link
Contributor Author

Thanks @aaronlippold Does be_in support comparing 2 arrays?

Here is a POC leveraging be_in that loops over the first array and checks each value in the second array

inspec shell

foo=['42','9000']
bar=['9000','42']
foo.each do |f|
  describe f do
    it {should be_in bar}
  end
end
Profile:   inspec-shell
Version:   (not specified)
Target ID: 

  42
     ✔  is expected to be in "9000" and "42"
  9000
     ✔  is expected to be in "9000" and "42"

Test Summary: 2 successful, 0 failures, 0 skipped

@aaronlippold
Copy link
Member

The being matcher will compare a list to an array and make sure it is a proper subset. So if I had an array, which was ABC, I would expect that a would be in ABC or that a C or CA would be in ABC. It doesn't care about order. I built this specifically to solve the list of Cyphers problem.

@aaronlippold
Copy link
Member

So you don't really need to do a loop

@schurzi
Copy link
Contributor

schurzi commented Nov 19, 2022

As I understand the be_in matcher returns true even for subsets, but we want to check for equality.
Since the order of the elements is of no consequence, why don't we simply sort the input? :)

This should do the trick:

  describe security_policy do
    its('SeNetworkLogonRight.sort') { should eq input('se_network_logon_right') }
  end

@spencer-cdw
Copy link
Contributor Author

I've tested .sort with no luck.

Screenshot 2022-11-23 at 12 26 47 PM
Screenshot 2022-11-23 at 12 28 27 PM


I setup an additional test for the .sort method in inspec-shell. (Using the ini resource instead of the security_policy for simplicity)

touch foo.ini
echo "foo=['42','9000']" > foo.ini
bar=['9000','42']
describe ini('foo.ini') do
  its('foo.sort') {should eq bar}
end

Which returned error undefined method 'sort'

  INI foo.ini
     ×  foo.sort 
     undefined method `sort' for "['42','9000']":String

Looking at the Rspec documentation, I'm not finding a .sort method that could be used. https://rubydoc.info/gems/rspec-core/RSpec/Core/ExampleGroup

However there are examples of the azurerm_virtual_machines resource using os_disks.sort

its('os_disks.sort') { should eq [['MyDisk1'], ['MyDisk2']] }

https://docs.chef.io/inspec/resources/azurerm_virtual_machines/#os_disks

https://github.com/inspec/inspec-azure/blob/98d93642bdb5afb856a95edd9ac4e56b3e1fe1b6/test/integration/verify/controls/azurerm_virtual_machines.rb#L3

Looking at the code for security_policy I see the following methods

  • initialize
  • content
  • params
  • method_missing
  • to_s
  • resource_id

But I'm not seeing a .sort method 🤔

@schurzi
Copy link
Contributor

schurzi commented Nov 28, 2022

This is very stranke. I also did some tests on a temporary system. My cinc-auditor shell behaves a bit different.

I'm using a default Windows 10 with the current version of cinc-auditor

PS C:\cinc-project\cinc-auditor\bin> .\cinc-auditor shell
Welcome to the interactive InSpec Shell
To find out how to use it, type: help

You are currently running on:

    Name:      windows_10_enterprise
    Families:  windows, os
    Release:   10.0.19044
    Arch:      x86_64

The security policy is set and is an array

inspec> security_policy.SeNetworkLogonRight
=> ["S-1-1-0", "S-1-5-32-544", "S-1-5-32-545", "S-1-5-32-551"]
inspec> security_policy.SeNetworkLogonRight.instance_of? Array
=> true

So I took the reported values and reversed them ["S-1-5-32-551", "S-1-5-32-545", "S-1-5-32-544", "S-1-1-0"]

inspec> describe security_policy do
inspec>   its('SeNetworkLogonRight.sort') { should eq ["S-1-5-32-551", "S-1-5-32-545", "S-1-5-32-544", "S-1-1-0"] }
inspec> end

Profile: inspec-shell
Version: (not specified)

  Security Policy
     [FAIL]  SeNetworkLogonRight.sort should eq ["S-1-5-32-551", "S-1-5-32-545", "S-1-5-32-544", "S-1-1-0"]

     expected: ["S-1-5-32-551", "S-1-5-32-545", "S-1-5-32-544", "S-1-1-0"]
          got: ["S-1-1-0", "S-1-5-32-544", "S-1-5-32-545", "S-1-5-32-551"]

     (compared using ==)


Test Summary: 0 successful, 1 failure, 0 skipped
inspec> describe security_policy do
inspec>   its('SeNetworkLogonRight.reverse') { should eq ["S-1-5-32-551", "S-1-5-32-545", "S-1-5-32-544", "S-1-1-0"] }
inspec> end

Profile: inspec-shell
Version: (not specified)

  Security Policy
     [PASS]  SeNetworkLogonRight.reverse should eq ["S-1-5-32-551", "S-1-5-32-545", "S-1-5-32-544", "S-1-1-0"]

Test Summary: 1 successful, 0 failures, 0 skipped

So the basic idea seems to work. I have no clue, why it does not work for your environment.

@spencer-cdw
Copy link
Contributor Author

@schurzi Thank you for testing that. Your test looks very concise and logical. It appears that .sort and .reverse are valid methods on security_policy.

My tests were done through a layer of abstraction so I trust your results over mine.

Changing the order should fix it for me, but there is a possibility it will break the benchmark for other users if they have different ordering.

Would sorting both sides of the comparator be safer?

e.g.

  describe security_policy do
    its('SeNetworkLogonRight.sort') { should eq input('se_network_logon_right').sort }
  end

@schurzi
Copy link
Contributor

schurzi commented Dec 30, 2022

Changing the order should fix it for me, but there is a possibility it will break the benchmark for other users if they have different ordering.

very good point. Are you up to generate a PR for this and do some testing in your environment?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants