-
Notifications
You must be signed in to change notification settings - Fork 682
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
auditd_rules resource: fix get_keys error on lines that have no keys #2103
Conversation
…lidate bug fix Signed-off-by: Jennifer Burns <jburns@mitre.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix, @jburns12! I'd like to protect against an edge case in this fix.
lib/resources/auditd_rules.rb
Outdated
@@ -177,7 +177,7 @@ def get_action_list(line) | |||
|
|||
# NB only in file lines | |||
def get_key(line) | |||
line.match(/-k ([^ ]+)/)[1] | |||
line.match(/-k ([^ ]+)/)[1] if line =~ /-k/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should be more specific about this condition. What is the filename in the line is /etc/private-keys
but has no -k SOMEKEY
indication? This will still match the /-k/
condition.
Perhaps we should change that to: if line.include?('-k ')
so we directly match on the trailing space? And add a line to your mock file that has a filename that includes -k
in the file name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thing...thanks!
Signed-off-by: Jennifer Burns <jburns@mitre.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great fix, @jburns12 - thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sweet fix, thank you Jennifer!!
Currently, if a file rule found by audtictl -l does not contain a key, the get_key function in auditd_rules.rb generates a NilClass error. This PR alters the single line in get_key to be sure that a key exists before line.match can throw the error. The unit tests and mock auditctl command file have also been updated to verify this fix prevents the NilClass error.
Signed-off-by: Jennifer Burns jburns@mitre.org