Skip to content

Commit

Permalink
adds better validation for commands passed in
Browse files Browse the repository at this point in the history
If you use a non absolute path (or a `Cmnd_Alias` which is represented in ALL_CAPS) the sudoers validation will fail it but not tell you why. This makes it easier/quicker to spot the issue.

Signed-off-by: Ben Abrams <me@benabrams.it>
  • Loading branch information
majormoses committed Sep 21, 2018
1 parent 11316a6 commit 5695257
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 1 deletion.
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -42,7 +42,7 @@ Use the sudo resource to add or remove individual sudo entries using sudoers.d f
Property | Description | Example Value | Default Value
------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------- | ---------------
`filename` | name of the `/etc/sudoers.d` file | restart-tomcat | resource's name
`commands` | array of commands this sudoer can execute | ['/etc/init.d/tomcat restart'] | ['ALL']
`commands` | array of commands this sudoer can execute, they must contain a full path. Example: use `/usr/bin/tail` over `tail` | ['/etc/init.d/tomcat restart'] | ['ALL']
`groups` | group(s) to provide sudo privileges to. This accepts either an array or a comma separated list. Leading % on group names is optional. This property was named 'group' prior to the 5.1 cookbook release. | %admin,superadmin | []
`nopasswd` | allow running sudo without specifying a password sudo | true | false
`noexec` | prevents commands from shelling out | true | false
Expand Down
22 changes: 22 additions & 0 deletions resources/default.rb
Expand Up @@ -80,6 +80,24 @@ def platform_config_prefix
end
end

# Validates if each element in an array starts with `/` or is in
# ALL_CAPS. This is helpful for ensuring that the commands
# passing into the sudoers resource as they need a full path or a
# `Cmnd_Alias`. This should help people more easily catch issues
# where the user requested `tail SOME_ARGS SOME_FILE` where they
# need to use `/usr/bin/tail SOME_ARGS SOME_FILE`.
# return [TrueClass, FalseClass]
def validate_commands_path(commands)
commands.each do |command|
cmd = command.split(' ').first
if command.starts_with('/') || cmd.upcase == cmd

This comment has been minimized.

Copy link
@zepheiryan

zepheiryan Oct 10, 2018

This should be start_with?, not starts_with.

This comment has been minimized.

Copy link
@tas50

tas50 Oct 10, 2018

Contributor

Fixed and shipped. Thanks

This comment has been minimized.

Copy link
@majormoses

majormoses Oct 11, 2018

Author Contributor

good catch

true
else
false
end
end
end

# Default action - install a single sudoer
action :create do
validate_properties
Expand All @@ -96,6 +114,10 @@ def platform_config_prefix

Chef::Log.warn("#{new_resource.filename} will be rendered, but will not take effect because the #{new_resource.config_prefix}/sudoers config lacks the includedir directive that loads configs from #{new_resource.config_prefix}/sudoers.d/!") if ::File.readlines("#{new_resource.config_prefix}/sudoers").grep(/includedir/).empty?

if new_resource.commands && !validate_commands_path(new_resource.commands)
Chef::Log.fatal('To restrict sudoer commands you must use absolute paths. For example to use `tail` you must specify `/usr/bin/tail` or whatever the appropriate path is for your system. This is becase someone could create a command called `tail` and put it in their path, sudo does not know which one to allow.')
end

if new_resource.template
Chef::Log.debug('Template property provided, all other properties ignored.')

Expand Down

0 comments on commit 5695257

Please sign in to comment.