Pretty way of keeping sensitive info out of a logged command string in Ruby?

Question

I have a long command that I am building with shovels (<<), with the intention of eventually running system(command). I'd like to log the command string, to make sure it is built properly, right before I execute the command. However, the command has to contain account credentials that I don't want to log. I'm looking for an elegant way to satisfy both requirements (1. log the meat of the command string, 2. don't log the auth credentials it contains).

This is what I have now:

command = ""
public = ""

[command, public].each {|str| str << "command_name"}
[command, public].each {|str| str << " -a a"}
[command, public].each {|str| str << " -b b"}
[command, public].each {|str| str << " -c c"}
command << " -d d"
command << " -e e"
[command, public].each {|str| str << " -f f"}
command << " -g g"
[command, public].each {|str| str << " -h h"}
[command, public].each {|str| str << " -i i"}

puts command
=> command_name -a a -b b -c c -d d -e e -f f -g g -h h -i i
puts public
=> command_name -a a -b b -c c -f f -h h -i i

system(command)

Original question from Stack Overflow

Answer 1

I see two more security issues with this code.

1. Executing a string rather than an array of command and parameters

   When building a command line programmatically, it is dangerous to build it as a string, especially when any of the parameters comes from user input. When calling Kernel#system with a string, a shell interprets the command line; when calling it with an array, the operating system kernel executes the command directly.

   In the benign failure case, the password might contain a space, in which case the shell would interpret it as two parameters, probably causing your command to be invoked incorrectly. In the worst case, the user could trigger an arbitrary command execution vulnerability.

   As an illustration, the following code will do more than just set your password to "hello":

   password_file = '.htpasswd'
   username = 'CHK'
   password = 'hello; touch me; grep; unzip; strip; finger; fsck -y; more; yes; yes; yes; umount; sleep'
   command = "htpasswd -b -c #{password_file} #{username} #{password}"
   system(command)
   

   Practice safer computing! This is not vulnerable to arbitrary command execution:

   command = ['htpasswd', '-b', '-c', password_file, username, password]
   system(*command)
   

2. Passing sensitive information on the command line

   Even if you manage to censor the logger within Ruby, it's still considered dangerous practice to pass any sensitive information as a command-line parameter. Command lines are visible to all users on a machine.

   For that reason, all reasonable commands should offer an alternative mechanism to accept secret information. For example, the recommended way to use htpasswd, since Apache 2.4, is to read the password from standard input.

   command = ['htpasswd', '-i', '-c', password_file, username]
   IO.popen(command, mode='w') do |htpasswd|
     htpasswd.puts(password)
   end
   

   Similarly, here is an example of how to securely specify a passphrase to GnuPG (in Python).

   In summary, by passing the secret information through pipes instead of the command line, you solve both the log censorship problem and the machine-wide command-line visibility problem.

Answer 2

You seem to be repeatedly performing a lot of operations on both 'command' and 'public' together. The way these variables are related make me think they should actually be part of the same object. For example:

class Command
  attr_reader :command, :loggable

  def initialize(command)
    @command = command
    @loggable = command
  end

  def concat(option)
    @command << ' ' + option
    @loggable << ' ' + option
  end
  alias_method :<<, :concat

  def concat_private(option)
    @command << ' ' + option
  end

  alias_method :to_s, :command
  alias_method :to_str, :command
end

command = Command.new('echo')
command << '-a this'
command << '-b that'
command.concat_private '--password asdf'

system(command)

This approach also makes your code more testable, gives you the flexibility of adding additional methods to Command should the need arise.

Answer 3

I agree completely with everything @200_success said, but if you do want a sensitive info solution, I would suggest something more in the lines of html_safe - a specialized string which is makes by default, and only shown when explicitly requested:

class PrivateString < String
  alias to_private_s to_s

  def to_s
    '*******'
  end

  def concat(other)
    if other.respond_to?(:to_private_s)
      super(other.to_private_s)
    else
      super
    end
  end

  alias << concat
end

class String
  def private
    PrivateString.new(self)
  end

  alias unsafe_concat concat

  def concat(other)
    if other.respond_to?(:to_private_s)
      unsafe_concat(other.to_s)
    else
      unsafe_concat(other)
    end
  end

  alias << concat
end

Now your code will look like this:

command = "".private
public = ""

[command, public].each {|str| str << "command_name"}
[command, public].each {|str| str << " -a a"}
[command, public].each {|str| str << " -b b"}
[command, public].each {|str| str << " -c c"}
[command, public].each {|str| str << " -d d".private}
[command, public].each {|str| str << " -e e".private}
[command, public].each {|str| str << " -f f"}
[command, public].each {|str| str << " -g g".private}
[command, public].each {|str| str << " -h h"}
[command, public].each {|str| str << " -i i"}

puts command
=> command_name -a a -b b -c c -d d -e e -f f -g g -h h -i i
puts public
=> command_name -a a -b b -c c************** -f f******* -h h -i i

system(command)

This is, of course, a bare bones implementation, and should probably be developed further (you can see html_safe implementation here), but it enables you to treat private strings like any other string, and only address it as private when needed.