oracle_session and mssql_session improvement (#1857)

* improve database parsing
* support sqlcli
* ensure headers are downcast
* externalize database helper
* use password as argument
* feedback from @adamleff
* inline docs update + linting
* stay backwards compatible
* implement tests

docs/resources/mssql_session.md.erb

@@ -10,24 +10,20 @@ Use the `mssql_session` InSpec audit resource to test SQL commands run against a
 
 A `mssql_session` resource block declares the username and password to use for the session, and then the command to be run:
 
-    describe mssql_session(user: 'username', pass: 'password').query('QUERY') do
-      its('output') { should eq('') }
+    describe mssql_session(user: 'username', password: 'password').query('QUERY').row(0).column('result') do
+      its('value') { should eq('') }
     end
 
 where
 
 * `mssql_session` declares a username and password with permission to run the query.  Omitting the username or password parameters results in the use of Windows authentication as the user InSpec is executing as.  You may also optionally pass a host and instance name.  If omitted, they will default to host: localhost and the default instance.
 * `query('QUERY')` contains the query to be run
-* `its('output') { should eq('') }` compares the results of the query against the expected result in the test
+* `its('value') { should eq('') }` compares the results of the query against the expected result in the test
 
 ## Matchers
 
 This InSpec audit resource has the following matchers:
 
-### be
-
-<%= partial "/shared/matcher_be" %>
-
 ### cmp
 
 <%= partial "/shared/matcher_cmp" %>
@@ -36,19 +32,6 @@ This InSpec audit resource has the following matchers:
 
 <%= partial "/shared/matcher_eq" %>
 
-### include
-
-<%= partial "/shared/matcher_include" %>
-
-### match
-
-<%= partial "/shared/matcher_match" %>
-
-### output
-
-The `output` matcher tests the results of the query:
-
-    its('output') { should eq(/^0/) }
 
 ## Examples
 
@@ -56,24 +39,24 @@ The following examples show how to use this InSpec audit resource.
 
 ### Test for matching databases
 
-    sql = mssql_session(user: 'my_user', pass: 'password')
+    sql = mssql_session(user: 'my_user', password: 'password')
 
-    describe sql.query('show databases like \'test\';') do
-      its('stdout') { should_not match(/test/) }
+    describe sql.query("SELECT SERVERPROPERTY('ProductVersion') as result").row(0).column('result') do
+      its("value") { should cmp > '12.00.4457' }
     end
 
 ### Test using Windows authentication
 
     sql = mssql_session
 
-    describe sql.query('show databases like \'test\';') do
-      its('stdout') { should_not match(/test/) }
+    describe sql.query("SELECT SERVERPROPERTY('ProductVersion') as result").row(0).column('result') do
+      its("value") { should cmp > '12.00.4457' }
     end
 
 ### Test a specific host and instance
 
-    sql = mssql_session(user: 'my_user', pass: 'password', host: 'mssqlserver', instance: 'foo')
+    sql = mssql_session(user: 'my_user', password: 'password', host: 'mssqlserver', instance: 'foo')
 
-    describe sql.query('show databases like \'test\';') do
-      its('stdout') { should_not match(/test/) }
+    describe sql.query("SELECT SERVERPROPERTY('ProductVersion') as result").row(0).column('result') do
+      its("value") { should cmp > '12.00.4457' }
     end

docs/resources/oracledb_session.md.erb

@@ -10,24 +10,20 @@ Use the `oracledb_session` InSpec audit resource to test SQL commands run agains
 
 A `oracledb_session` resource block declares the username and password to use for the session with an optional service to connect to, and then the command to be run:
 
-    describe oracledb_session(user: 'username', pass: 'password').query('QUERY') do
-      its('output') { should eq('') }
+    describe oracledb_session(user: 'username', password: 'password', service: 'ORCL.localdomain').query('QUERY').row(0).column('result') do
+      its('value') { should eq('') }
     end
 
 where
 
 * `oracledb_session` declares a username and password with permission to run the query (required), and an optional parameters for host (default: `localhost`), SID (default: `nil`, which uses the default SID, and path to the sqlplus binary (default: `sqlplus`).
 * `query('QUERY')` contains the query to be run
-* `its('output') { should eq('') }` compares the results of the query against the expected result in the test
+* `its('value') { should eq('') }` compares the results of the query against the expected result in the test
 
 ## Matchers
 
 This InSpec audit resource has the following matchers:
 
-### be
-
-<%= partial "/shared/matcher_be" %>
-
 ### cmp
 
 <%= partial "/shared/matcher_cmp" %>
@@ -36,20 +32,6 @@ This InSpec audit resource has the following matchers:
 
 <%= partial "/shared/matcher_eq" %>
 
-### include
-
-<%= partial "/shared/matcher_include" %>
-
-### match
-
-<%= partial "/shared/matcher_match" %>
-
-### output
-
-The `output` matcher tests the results of the query:
-
-    its('output') { should eq(/^0/) }
-
 ## Examples
 
 The following examples show how to use this InSpec audit resource.
@@ -58,14 +40,14 @@ The following examples show how to use this InSpec audit resource.
 
     sql = oracledb_session(user: 'my_user', pass: 'password')
 
-    describe sql.query('SELECT NAME FROM v$database;') do
-      its('stdout') { should_not match(/test/) }
+    describe sql.query('SELECT NAME AS VALUE FROM v$database;').row(0).column('value') do
+      its('value') { should cmp 'ORCL' }
     end
 
 ### Test for matching databases with custom host, SID and sqlplus binary location
 
     sql = oracledb_session(user: 'my_user', pass: 'password', host: 'oraclehost', sid: 'mysid', sqlplus_bin: '/u01/app/oracle/product/12.1.0/dbhome_1/bin/sqlplus')
 
-    describe sql.query('SELECT NAME FROM v$database;') do
-      its('stdout') { should_not match(/test/) }
+    describe sql.query('SELECT NAME FROM v$database;').row(0).column('name') do
+      its('value') { should cmp 'ORCL' }
     end

inspec.gemspec

@@ -44,4 +44,5 @@ Gem::Specification.new do |spec|
   spec.add_dependency 'addressable', '~> 2.4'
   spec.add_dependency 'parslet', '~> 1.5'
   spec.add_dependency 'semverse'
+  spec.add_dependency 'htmlentities'
 end

lib/resources/mssql_session.rb

@@ -1,54 +1,102 @@
 # encoding: utf-8
+# author: Nolan Davidson
 # author: Christoph Hartmann
 # author: Dominik Richter
 
+require 'hashie/mash'
+require 'utils/database_helpers'
+
 module Inspec::Resources
+  # STABILITY: Experimental
+  # This resource needs further testing and refinement
+  #
+  # This requires the `sqlcmd` tool available on platform
+  # @see https://docs.microsoft.com/en-us/sql/relational-databases/scripting/sqlcmd-use-the-utility
+  # @see https://docs.microsoft.com/en-us/sql/linux/sql-server-linux-connect-and-query-sqlcmd
   class MssqlSession < Inspec.resource(1)
     name 'mssql_session'
     desc 'Use the mssql_session InSpec audit resource to test SQL commands run against a MS Sql Server database.'
     example "
       # Using SQL authentication
       sql = mssql_session(user: 'myuser', pass: 'mypassword')
-      describe sql.query('select * from sys.databases where name like \'*test*\') do
-        its('stdout') { should_not match(/test/) }
+      describe sql.query('SELECT * FROM table').row(0).column('columnname') do
+        its('value') { should cmp == 1 }
       end
 
       # Passing no credentials to mssql_session forces it to use Windows authentication
       sql_windows_auth = mssql_session
-      describe sql_window_auth.query('select * from sys.databases where name like \'*test*\') do
-        its('stdout') { should_not match(/test/) }
+      describe sql.query(\"SELECT SERVERPROPERTY('IsIntegratedSecurityOnly') as \\\"login_mode\\\";\").row(0).column('login_mode') do
+        its('value') { should_not be_empty }
+        its('value') { should cmp == 1 }
       end
     "
 
-    attr_reader :user, :pass, :host
-
+    attr_reader :user, :password, :host
     def initialize(opts = {})
       @user = opts[:user]
-      @pass = opts[:pass]
+      @password = opts[:password] || opts[:pass]
+      if opts[:pass]
+        warn '[DEPRECATED] use `password` option to supply password instead of `pass`'
+      end
       @host = opts[:host] || 'localhost'
       @instance = opts[:instance]
+
+      # check if sqlcmd is available
+      return skip_resource('sqlcmd is missing') if !inspec.command('sqlcmd').exist?
+      # check that database is reachable
+      return skip_resource("Can't connect to the MS SQL Server.") if !test_connection
     end
 
     def query(q)
-      escaped_query = q.gsub(/\\/, '\\\\').gsub(/"/, '\\"').gsub(/\$/, '\\$').gsub(/\@/, '`@')
-      cmd_string = "sqlcmd -Q \"#{escaped_query}\""
-      cmd_string += " -U #{@user} -P #{@pass}" unless @user.nil? or @pass.nil?
+      escaped_query = q.gsub(/\\/, '\\\\').gsub(/"/, '\\"').gsub(/\$/, '\\$')
+      # surpress 'x rows affected' in SQLCMD with 'set nocount on;'
+      cmd_string = "sqlcmd -Q \"set nocount on; #{escaped_query}\" -W -w 1024 -s ','"
+      cmd_string += " -U #{@user} -P '#{@password}'" unless @user.nil? || @password.nil?
       if @instance.nil?
         cmd_string += " -S #{@host}"
       else
         cmd_string += " -S #{@host}\\#{@instance}"
       end
-      puts cmd_string
       cmd = inspec.command(cmd_string)
       out = cmd.stdout + "\n" + cmd.stderr
-      if out =~ /Sqlcmd: Error/
-        skip_resource("Can't connect to the MS SQL Server.")
+      if cmd.exit_status != 0 || out =~ /Sqlcmd: Error/
+        # TODO: we need to throw an exception here
+        # change once https://github.com/chef/inspec/issues/1205 is in
+        warn "Could not execute the sql query #{out}"
+        DatabaseHelper::SQLQueryResult.new(cmd, Hashie::Mash.new({}))
+      else
+        DatabaseHelper::SQLQueryResult.new(cmd, parse_csv_result(cmd))
       end
-      cmd
     end
 
     def to_s
       'MSSQL session'
     end
+
+    private
+
+    def test_connection
+      !query('select getdate()').empty?
+    end
+
+    def parse_csv_result(cmd)
+      require 'csv'
+      table = CSV.parse(cmd.stdout, { headers: true })
+
+      # remove first row, since it will be a seperator line
+      table.delete(0)
+
+      # convert to hash
+      headers = table.headers
+
+      results = table.map { |row|
+        res = {}
+        headers.each { |header|
+          res[header.downcase] = row[header]
+        }
+        Hashie::Mash.new(res)
+      }
+      results
+    end
   end
 end

lib/resources/oracledb_session.rb

@@ -1,41 +1,139 @@
 # encoding: utf-8
 # author: Nolan Davidson
+# author: Christoph Hartmann
+# author: Dominik Richter
+
+require 'hashie/mash'
+require 'utils/database_helpers'
+require 'htmlentities'
+require 'rexml/document'
+require 'csv'
 
 module Inspec::Resources
+  # STABILITY: Experimental
+  # This resource needs further testing and refinement
+  #
   class OracledbSession < Inspec.resource(1)
     name 'oracledb_session'
     desc 'Use the oracledb_session InSpec resource to test commands against an Oracle database'
     example "
       sql = oracledb_session(user: 'my_user', pass: 'password')
-      describe sql.query('SELECT NAME FROM v$database;') do
-        its('stdout') { should_not match(/test/) }
+      describe sql.query(\"SELECT UPPER(VALUE) AS VALUE FROM V$PARAMETER WHERE UPPER(NAME)='AUDIT_SYS_OPERATIONS'\").row(0).column('value') do
+        its('value') { should eq 'TRUE' }
       end
     "
 
-    attr_reader :user, :pass, :host, :sid, :sqlplus_bin
-
+    attr_reader :user, :password, :host, :service
     def initialize(opts = {})
       @user = opts[:user]
-      @pass = opts[:pass]
+      @password = opts[:password] || opts[:pass]
+      if opts[:pass]
+        warn '[DEPRECATED] use `password` option to supply password instead of `pass`'
+      end
+
       @host = opts[:host] || 'localhost'
-      @sid = opts[:sid]
+      @port = opts[:port] || '1521'
+      @service = opts[:service]
+
+      # we prefer sqlci although it is way slower than sqlplus, but it understands csv properly
+      @sqlcl_bin = 'sql'
       @sqlplus_bin = opts[:sqlplus_bin] || 'sqlplus'
-      return skip_resource("Can't run Oracle checks without authentication") if @user.nil? or @pass.nil?
+
+      return skip_resource "Can't run Oracle checks without authentication" if @user.nil? || @password.nil?
+      return skip_resource 'You must provide a service name for the session' if @service.nil?
     end
 
     def query(q)
       escaped_query = q.gsub(/\\/, '\\\\').gsub(/"/, '\\"')
-      cmd = inspec.command("echo \"#{escaped_query}\" | #{@sqlplus_bin} -s #{@user}/#{@pass}@#{@host}/#{@sid}")
-      out = cmd.stdout + "\n" + cmd.stderr
-      if out.downcase =~ /^error/
-        skip_resource("Can't connect to Oracle instance for SQL checks.")
+      # escape tables with $
+      escaped_query = escaped_query.gsub('$', '\\$')
+
+      p = nil
+      # check if sqlplus is available and prefer that
+      if inspec.command(@sqlplus_bin).exist?
+        bin = @sqlplus_bin
+        opts = "SET MARKUP HTML ON\nSET FEEDBACK OFF"
+        p = :parse_html_result
+      elsif inspec.command(@sqlcl_bin).exist?
+        bin = @sqlcl_bin
+        opts = "set sqlformat csv\nSET FEEDBACK OFF"
+        p = :parse_csv_result
       end
 
-      cmd
+      return skip_resource("Can't find suitable Oracle CLI") if p.nil?
+      command = "echo \"#{opts}\n#{verify_query(escaped_query)}\nEXIT\" | #{bin} -s #{@user}/#{@password}@//#{@host}:#{@port}/#{@service}"
+      cmd = inspec.command(command)
+
+      out = cmd.stdout + "\n" + cmd.stderr
+      if out.downcase =~ /^error/
+        # TODO: we need to throw an exception here
+        # change once https://github.com/chef/inspec/issues/1205 is in
+        warn "Could not execute the sql query #{out}"
+        DatabaseHelper::SQLQueryResult.new(cmd, Hashie::Mash.new({}))
+      end
+      DatabaseHelper::SQLQueryResult.new(cmd, send(p, cmd.stdout))
     end
 
     def to_s
       'Oracle Session'
     end
+
+    private
+
+    def verify_query(query)
+      # ensure we have a ; at the end
+      query + ';' if !query.strip.end_with?(';')
+      query
+    end
+
+    def parse_csv_result(stdout)
+      output = stdout.delete(/\r/)
+      table = CSV.parse(output, { headers: true })
+
+      # convert to hash
+      headers = table.headers
+
+      results = table.map { |row|
+        res = {}
+        headers.each { |header|
+          res[header.downcase] = row[header]
+        }
+        Hashie::Mash.new(res)
+      }
+      results
+    end
+
+    def parse_html_result(stdout) # rubocop:disable Metrics/AbcSize
+      result = stdout
+      # make oracle html valid html by removing the p tag, it does not include a closing tag
+      result = result.gsub('<p>', '').gsub('</p>', '').gsub('<br>', '')
+      doc = REXML::Document.new result
+      table = doc.elements['table']
+      hash = []
+      if !table.nil?
+        rows = table.elements.to_a
+        headers = rows[0].elements.to_a('th').map { |entry| entry.text.strip }
+        rows.delete_at(0)
+
+        # iterate over each row, first row is header
+        hash = []
+        if !rows.nil? && !rows.empty?
+          hash = rows.map { |row|
+            res = {}
+            entries = row.elements.to_a('td')
+            # ignore if we have empty entries, oracle is adding th rows in between
+            return nil if entries.empty?
+            headers.each_with_index { |header, index|
+              # we need htmlentities since we do not have nokogiri
+              coder = HTMLEntities.new
+              val = coder.decode(entries[index].text).strip
+              res[header.downcase] = val
+            }
+            Hashie::Mash.new(res)
+          }.compact
+        end
+      end
+      hash
+    end
   end
 end

lib/utils/database_helpers.rb

@@ -0,0 +1,77 @@
+# encoding: utf-8
+# author: Christoph Hartmann
+# author: Dominik Richter
+
+module DatabaseHelper
+  class SQLColumn
+    def initialize(row, name)
+      @row = row
+      @name = name
+    end
+
+    def value
+      @row[@name.downcase]
+    end
+
+    def to_s
+      'SQL Column'
+    end
+  end
+
+  class SQLRow
+    def initialize(query, row)
+      @query = query
+      @row = row
+    end
+
+    def column(column)
+      SQLColumn.new(@row, column)
+    end
+
+    def to_s
+      'SQL Row'
+    end
+  end
+
+  class SQLQueryResult
+    attr_reader :error
+    def initialize(cmd, results)
+      @cmd = cmd
+      @results = results
+    end
+
+    def empty?
+      @results.empty?
+    end
+
+    def successful?
+      @cmd.exit_status == 0 && @error.nil?
+    end
+
+    def row(id)
+      SQLRow.new(self, @results[id])
+    end
+
+    def size
+      @results.size
+    end
+
+    def stdout
+      warn '[DEPRECATION] The `stdout` method is deprecated. Use `row` instead.'
+      @cmd.stdout
+    end
+
+    def stderr
+      warn '[DEPRECATION] The `stderr` method is deprecated. Use `successful?` instead.'
+      @cmd.stderr
+    end
+
+    def inspect
+      to_s
+    end
+
+    def to_s
+      'SQL ResultSet'
+    end
+  end
+end

test/helper.rb

@@ -341,6 +341,13 @@ class MockLoader
       'nc -vz -G 1 example.com 1234' => cmd.call('nc-example-com'),
       # host resource: test-netconnection for reachability check on windows
       'Test-NetConnection -ComputerName microsoft.com -WarningAction SilentlyContinue -RemotePort 1234| Select-Object -Property ComputerName, TcpTestSucceeded, PingSucceeded | ConvertTo-Json' => cmd.call('Test-NetConnection'),
+      # mssql tests
+      "bash -c 'type \"sqlcmd\"'" => cmd.call('mssql-sqlcmd'),
+      "cf33896c4bb500abc23dda5b5eddb03cd35a9c46a7358a2c0a0abe41e08a73ae" => cmd.call('mssql-getdate'),
+      "cd283a171cbd65698a2ea6a15524cb4b8566ff1caff430a51091bd5065dcbdf7" => cmd.call('mssql-result'),
+      # oracle
+      "bash -c 'type \"sqlplus\"'" => cmd.call('oracle-cmd'),
+      "ef04e5199abee80e662cc0dd1dd3bf3e0aaae9b4498217d241db00b413820911" => cmd.call('oracle-result'),
     }
     @backend
   end

test/unit/mock/cmd/mssql-getdate

@@ -0,0 +1,3 @@
+
+-
+2017-06-28 17:54:23.487

test/unit/mock/cmd/mssql-result

@@ -0,0 +1,3 @@
+result
+------
+14.0.600.250

test/unit/mock/cmd/mssql-sqlcmd

@@ -0,0 +1 @@
+sqlcmd is /usr/local/bin/sqlcmd

test/unit/mock/cmd/oracle-cmd

@@ -0,0 +1 @@
+sqlplus is /oracle//sqlplus/sqlplus

test/unit/mock/cmd/oracle-result

@@ -0,0 +1,14 @@
+<p>
+<table border='1' width='90%' align='center' summary='Script output'>
+<tr>
+<th scope="col">
+VALUE
+</th>
+</tr>
+<tr>
+<td>
+ORCL
+</td>
+</tr>
+</table>
+<p>

test/unit/resources/mssql_session_test.rb

@@ -1,13 +1,21 @@
 # encoding: utf-8
 # author: Nolan Davidson
+# author: Christoph Hartmann
 
 require 'helper'
 
 describe 'Inspec::Resources::MssqlSession' do
   it 'verify mssql_session configuration' do
-    resource = load_resource('mssql_session', user: 'myuser', pass: 'mypass', host: 'windowshost')
-    _(resource.user).must_equal 'myuser'
-    _(resource.pass).must_equal 'mypass'
-    _(resource.host).must_equal 'windowshost'
+    resource = load_resource('mssql_session', user: 'sa', password: 'yourStrong(!)Password', host: 'localhost')
+    _(resource.user).must_equal 'sa'
+    _(resource.password).must_equal 'yourStrong(!)Password'
+    _(resource.host).must_equal 'localhost'
+  end
+
+  it 'run a SQL query' do
+    resource = load_resource('mssql_session', user: 'sa', password: 'yourStrong(!)Password', host: 'localhost')
+    query = resource.query("SELECT SERVERPROPERTY('ProductVersion') as result")
+    _(query.size).must_equal 1
+    _(query.row(0).column('result').value).must_equal '14.0.600.250'
   end
 end

test/unit/resources/oracledb_session_test.rb

@@ -1,15 +1,22 @@
 # encoding: utf-8
 # author: Nolan Davidson
+# author: Christoph Hartmann
 
 require 'helper'
 
 describe 'Inspec::Resources::OracledbSession' do
   it 'verify oracledb_session configuration' do
-    resource = load_resource('oracledb_session', user: 'myuser', pass: 'mypass', host: 'oraclehost', sid: 'mysid')
-    _(resource.user).must_equal 'myuser'
-    _(resource.pass).must_equal 'mypass'
-    _(resource.host).must_equal 'oraclehost'
-    _(resource.sid).must_equal 'mysid'
-    _(resource.sqlplus_bin).must_equal 'sqlplus'
+    resource = load_resource('oracledb_session', user: 'SYSTEM', password: 'supersecurepass', host: 'localhost', service: 'ORCL.localdomain')
+    _(resource.user).must_equal 'SYSTEM'
+    _(resource.password).must_equal 'supersecurepass'
+    _(resource.host).must_equal 'localhost'
+    _(resource.service).must_equal 'ORCL.localdomain'
+  end
+
+  it 'run a SQL query' do
+    resource = load_resource('oracledb_session', user: 'SYSTEM', password: 'supersecurepass', host: '127.0.0.1', service: 'ORCL.localdomain', port: 1527)
+    query = resource.query('SELECT NAME AS VALUE FROM v$database;')
+    _(query.size).must_equal 1
+    _(query.row(0).column('value').value).must_equal 'ORCL'
   end
 end