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

Describe Block Update Fix #21

Merged
merged 13 commits into from
Mar 31, 2023
82 changes: 74 additions & 8 deletions src/objects/control.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,21 +84,85 @@ export default class Control {
return new Control(unflatten(flattened));
}

toRuby() {
// WIP - provides the ability to get the control in its raw form
em-c-rod marked this conversation as resolved.
Show resolved Hide resolved
toString() {
em-c-rod marked this conversation as resolved.
Show resolved Hide resolved
let result = '';
result += `control '${this.id}' do\n`;

if (this.title) {
result += ` title "${this.title}"\n`;
}
// This is the known 'default' description - on previous version this content was repeated on descriptions processed by "descs"
if (this.desc) {
result += ` desc "${this.desc}"\n`;
}

if (this.descs) {
Object.entries(this.descs).forEach(([key, subDesc]) => {
if (subDesc) {
result += ` desc '${key}', "${subDesc}"\n`;
}
});
}

if (this.impact) {
result += ` impact ${this.impact}\n`;
}

if (this.refs) {
this.refs.forEach((ref) => {
if (typeof ref === 'string') {
result += ` ref "${ref}"\n`;
} else {
result += ` ref ${ref.ref?.toString() || ''}, url: ${ref.url || ''}`
}
});
}

Object.entries(this.tags).forEach(([tag, value]) => {
if (typeof value === 'object') {
if (Array.isArray(value) && typeof value[0] === 'string') {
result += ` tag ${tag}: ${JSON.stringify(value)}\n`
} else {
result += ` tag '${tag}': ${(value==null?'nil':value)}\n`
}
} else if (typeof value === 'string') {
if (value.includes('"')) {
result += ` tag "${tag}": "${value}"\n`;
} else {
result += ` tag '${tag}': '${value}'\n`;
}
}
});

if (this.describe) {
result += '\n';
result += this.describe
}

if (!result.slice(-1).match('\n')) {
result += '\n';
}
result += 'end\n';

return result;
}

toRuby(verbose = true) {
let result = '';

result += `control '${this.id}' do\n`;
if (this.title) {
result += ` title ${escapeQuotes(this.title)}\n`;
} else {
console.error(`${this.id} does not have a title`);
if (verbose) {console.error(`${this.id} does not have a title`);}
}

// This is the known 'default' description - on previous version this content was repeated on descriptions processed by "descs"
if (this.desc) {
result += ` desc ${escapeQuotes(this.desc)}\n`;
} else {
console.error(`${this.id} does not have a desc`);
if (verbose) {console.error(`${this.id} does not have a desc`);}
}

if (this.descs) {
Expand All @@ -109,22 +173,22 @@ export default class Control {
// The "default" keyword may have the same content as the desc content for backward compatibility with different historical InSpec versions.
// In that case, we can ignore writing the "default" subdescription field.
// If they are different, however, someone may be trying to use the keyword "default" for a unique subdescription, which should not be done.
console.error(`${this.id} has a subdescription called "default" with contents that do not match the main description. "Default" should not be used as a keyword for unique sub-descriptions.`);
if (verbose) {console.error(`${this.id} has a subdescription called "default" with contents that do not match the main description. "Default" should not be used as a keyword for unique sub-descriptions.`);}
}
}
else {
result += ` desc '${key}', ${escapeQuotes(subDesc)}\n`;
}
} else {
console.error(`${this.id} does not have a desc for the value ${key}`);
if (verbose) {console.error(`${this.id} does not have a desc for the value ${key}`);}
}
});
}

if (this.impact) {
result += ` impact ${this.impact}\n`;
if (this.impact !== undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Great catch

result += ` impact ${(this.impact<=0?this.impact.toFixed(1):this.impact)}\n`
} else {
console.error(`${this.id} does not have an impact`);
if (verbose) {console.error(`${this.id} does not have an impact`);}
}

if (this.refs) {
Expand Down Expand Up @@ -161,6 +225,8 @@ export default class Control {
} else if (typeof value === 'string') {
result += ` tag ${tag}: ${escapeQuotes(value)}\n`;
}
} else {
result += ` tag ${tag}: nil\n`;
}
});

Expand Down
1 change: 0 additions & 1 deletion src/parsers/json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ export function processProfileJSON(
tags: control.tags,
descs: objectifyDescriptions(control.descriptions),
})

newControl.describe = getExistingDescribeFromControl(newControl);

// Migrate check and fix text from tags to descriptions
Expand Down
6 changes: 5 additions & 1 deletion src/utilities/global.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ export function removeWhitespace(input: string): string {
return input.replace(/\s/gi, '')
}

const escapeSpecialCaseBackslashes = (s: string) => {
return s.replace(/\\\)/g, '\\\\)'); // Escape backslashes if preceding close parentheses
}

const escapeSingleQuotes = (s: string) => {
return s.replace(/\\/g, '\\\\').replace(/'/g, "\\'"); // Escape backslashes and quotes
}
Expand All @@ -51,7 +55,7 @@ const escapeDoubleQuotes = (s: string) => {

export function escapeQuotes(s: string): string {
if (s.includes("'") && s.includes('"')) {
return `%q(${removeNewlinePlaceholders(s)})`
return `%q(${escapeSpecialCaseBackslashes(removeNewlinePlaceholders(s))})`
em-c-rod marked this conversation as resolved.
Show resolved Hide resolved
} else if (s.includes("'")) {
return `"${escapeDoubleQuotes(removeNewlinePlaceholders(s))}"`
} else {
Expand Down
40 changes: 30 additions & 10 deletions src/utilities/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@

import _ from 'lodash'
import winston from 'winston';
import {diffProfile} from './diff'
import Control from '../objects/control'
import Profile from '../objects/profile'
import {processXCCDF} from '../parsers/xccdf'
import {ProfileDiff} from '../types/diff'
import {processXCCDF} from '../parsers/xccdf'
import {OvalDefinitionValue} from '../types/oval'
import {diffProfile} from './diff'
import {createDiffMarkdown} from './diffMarkdown'

export type UpdatedProfileReturn = {
Expand Down Expand Up @@ -59,7 +59,7 @@ function getRangesForLines(text: string): number[][] {
- Percent literals (%; delimiters: (), {}, [], <>, most non-
alphanumeric characters); (e.g., "%()")
- Multi-line comments (e.g., =begin\nSome comment\n=end)
- Variable delimiters (i.e., paranthesis: (); array: []; hash: {})
- Variable delimiters (i.e., parenthesis: (); array: []; hash: {})
*/
const stringDelimiters: {[key: string]: string} = {'(': ')', '{': '}', '[': ']', '<': '>'}
const variableDelimiters: {[key: string]: string} = {'(': ')', '{': '}', '[': ']'}
Expand All @@ -68,7 +68,8 @@ function getRangesForLines(text: string): number[][] {
enum skipCharLength {
string = '('.length,
percentString = 'q('.length,
commentBegin = '=begin'.length
commentBegin = '=begin'.length,
inlineInterpolationBegin = '{'.length
}

const stack: string[] = []
Expand All @@ -91,6 +92,8 @@ function getRangesForLines(text: string): number[][] {
const isVariableDelimiterChar = Object.keys(variableDelimiters).includes(char)
const isStringDelimiterChar = ((j < line.length - 1) && (/^[^A-Za-z0-9]$/.test(line[j + 1])))
const isCommentBeginChar = ((j == 0) && (line.length >= 6) && (line.slice(0, 6) == '=begin'))
const isCommentChar = /^\s*#/.test(line)
const isInlineInterpolation = (char == '#' && ((j < line.length - 1) && line[j + 1] == '{'))

const isPercentStringKeyChar = ((j < line.length - 1) && (strings.includes(line[j + 1])))
const isPercentStringDelimiterChar = ((j < line.length - 2) && (/^[^A-Za-z0-9]$/.test(line[j + 2])))
Expand All @@ -102,13 +105,21 @@ function getRangesForLines(text: string): number[][] {
const stringPushCondition = (baseCondition && isPercentChar && isStringDelimiterChar)
const percentStringPushCondition = (baseCondition && isPercentChar && isPercentString)
const commentBeginCondition = (baseCondition && isCommentBeginChar)
const commentCondition = (baseCondition && isCommentChar)
const inlineInterpolationCondition = (isNotEmptyStack && isInlineInterpolation)

if (commentCondition) {
break
}

if (stringPushCondition) {
j += skipCharLength.string // j += 1
} else if (percentStringPushCondition) {
j += skipCharLength.percentString // j += 2
} else if (commentBeginCondition) {
j += skipCharLength.commentBegin // j += 6
} else if (inlineInterpolationCondition) {
j += skipCharLength.inlineInterpolationBegin // j += 1
}
char = line[j]

Expand All @@ -122,7 +133,7 @@ function getRangesForLines(text: string): number[][] {

const popCondition = (basePopCondition || delimiterPopCondition || commentEndCondition)
const pushCondition = (quotePushCondition || variablePushCondition || stringPushCondition ||
percentStringPushCondition || delimiterPushCondition || commentBeginCondition)
percentStringPushCondition || delimiterPushCondition || commentBeginCondition || inlineInterpolationCondition)

if (popCondition) {
stack.pop()
Expand All @@ -134,7 +145,9 @@ function getRangesForLines(text: string): number[][] {
} else if (pushCondition) {
if (commentBeginCondition) {
stack.push('=begin')
} else {
} else if (inlineInterpolationCondition) {
stack.push('{')
} else {
stack.push(char)
}
rangeStack.push([i])
Expand Down Expand Up @@ -185,6 +198,7 @@ function getMultiLineRanges(ranges: number[][]): number[][] {
return multiLineRanges
}


/*
This is the most likely thing to break if you are getting code formatting issues.
Extract the existing describe blocks (what is actually run by inspec for validation)
Expand All @@ -193,10 +207,14 @@ export function getExistingDescribeFromControl(control: Control): string {
if (control.code) {
// Join multi-line strings in InSpec control.
const ranges = getRangesForLines(control.code)

// Get the entries that have delimiters that span multi-lines
const multiLineRanges = getMultiLineRanges(ranges)
const lines = joinMultiLineStringsFromRanges(control.code, multiLineRanges) // Array of lines representing the full InSpec control, with multi-line strings collapsed

// Define RegExp for lines to skip when searching for describe block.
// Array of lines representing the full InSpec control, with multi-line strings collapsed
const lines = joinMultiLineStringsFromRanges(control.code, multiLineRanges)

// Define RegExp for lines to skip.
const skip = ['control\\W', ' title\\W', ' desc\\W', ' impact\\W', ' tag\\W', ' ref\\W']
const skipRegExp = RegExp(skip.map(x => `(^${x})`).join('|'))

Expand All @@ -206,7 +224,7 @@ export function getExistingDescribeFromControl(control: Control): string {
for (const line of lines) {
const checkRegExp = ((line.trim() !== '') && !skipRegExp.test(line))
const checkNewLine = ((line.trim() === '') && !ignoreNewLine)

// Include '\n' if it is part of describe block, otherwise skip line.
if (checkRegExp || checkNewLine) {
describeBlock.push(line)
Expand All @@ -215,7 +233,9 @@ export function getExistingDescribeFromControl(control: Control): string {
ignoreNewLine = true
}
}
return describeBlock.slice(0, describeBlock.length - 2).join('\n') // Drop trailing ['end', '\n'] from Control block.

// Return synthesized logic as describe block
return describeBlock.slice(0, describeBlock.lastIndexOf('end')).join('\n') // Drop trailing ['end', '\n'] from Control block.
} else {
return ''
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,16 @@
tag subsystems: ['init_files']

if virtualization.system.eql?('docker')
impact 0.0
describe 'Control not applicable to a container' do
skip 'Control not applicable to a container'
end
else

exempt_home_users = input('exempt_home_users')
non_interactive_shells = input('non_interactive_shells')

ignore_shells = non_interactive_shells.join('|')

findings = Set[]
users.where { !shell.match(ignore_shells) && (uid >= 1000 || uid == 0) }.entries.each do |user_info|
next if exempt_home_users.include?(user_info.username.to_s)

findings += command("find #{user_info.home} -name '.*' -not -user #{user_info.username} -a -not -user root").stdout.split("\n")
end
describe 'Files and Directories not owned by the user or root of the parent home directory' do
Expand Down
27 changes: 27 additions & 0 deletions test/sample_data/controls-cookstyle/controls/SV-205653.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
control 'SV-205653' do
title 'Windows Server 2019 reversible password encryption must be disabled.'
desc 'Storing passwords using reversible encryption is essentially the same as storing clear-text versions of the passwords, which are easily compromised. For this reason, this policy must never be enabled.'
desc 'check', 'Verify the effective setting in Local Group Policy Editor.

Run "gpedit.msc".

Navigate to Local Computer Policy >> Computer Configuration >> Windows Settings >> Security Settings >> Account Policies >> Password Policy.
If the value for "Store passwords using reversible encryption" is not set to "Disabled", this is a finding.

For server core installations, run the following command:
Secedit /Export /Areas SecurityPolicy /CFG C:\\Path\\FileName.Txt
If "ClearTextPassword" equals "1" in the file, this is a finding.'
desc 'fix', 'Configure the policy value for Computer Configuration >> Windows Settings >> Security Settings >> Account Policies >> Password Policy >> "Store passwords using reversible encryption" to "Disabled".'
impact 0.7
tag gtitle: 'SRG-OS-000073-GPOS-00041'
tag gid: 'V-93465'
tag rid: 'SV-103551r1_rule'
tag stig_id: 'WN19-AC-000090'
tag fix_id: 'F-99709r1_fix'
tag cci: ['CCI-000196']
tag nist: ['IA-5 (1) (c)', 'Rev_4']

describe security_policy do
its('ClearTextPassword') { should eq 0 }
end
end
Loading