No security creds should be passed from the commandline #2

Open
mattyjones opened this Issue Feb 12, 2015 · 11 comments

Projects

None yet

4 participants

@mattyjones
Member

All tokens, keys, etc should be read from a local config file, databag, etc. The less these tokens see the light of day the better

@mattyjones mattyjones added this to the v0.0.1 milestone Feb 12, 2015
@bkett
bkett commented Feb 12, 2015

@mattyjones I can add this to the queue, just so you're aware implementing this would be a breaking change.

@bkett
bkett commented Feb 12, 2015

The aws-sdk gem provides all the necessary outlets for setting credentials. It will either a) pull them from the metadata server if you're using a role (preferred), b) pull them from environment variables or c) pull them from a file (I think it's ~/.aws/credentials). As part of the refactoring issue I'll pull out the key options and that should resolve this.

@mattyjones
Member

@bkett What are your thoughts on the matter, I don't want to come off as pushy but Sensu is built for the cloud as are many of these checks. Taking that into account, tokens, keys, creds should be highly discouraged from being commandline args.

What I ended up doing with the github plugin and a few others I wrote for myself was leave the current functionality intact and then set

 option :token,
         short: '-t TOKEN',
         long: '--token TOKEN',
         description: 'Github OAuth Token',
         default: SensuPluginsGithub.acquire_git_token
def acquire_git_token
  File.readlines(File.expand_path('~/.ssh/git_token')).each do |line|
     return line
  end

It's quick, dirty, and cheap but it does the trick for now.

I am going to reach out to Sean as well and get his thoughts. The gems are in an alpha state for this very reason, they should be considered unstable at this point. When the move to 0.0.1, is made, breaking changes will not be as easy, if at all possible so I am trying to get as much refactoring out of the way now before everybody has to make configuration changes every few days because we break something else.

@mattyjones
Member

@bkett I use roles for this in my Chef env. The standard why we do things is set

default[aws][conf][security][key]   = ''

Then we overwrite it with the role when being deployed or locally in the Vagrantfile for development and testing.

@bkett
bkett commented Feb 12, 2015

@mattyjones So to address the first point, this is something we can't really have both ways; either we will allow keys to be specified via CLI or we won't. I like the idea of following the aws-sdk practices and saying for requests to the aws api you should follow one of the three options I outlined in my earlier comment. So I'm for removing these options.

As for your second point, I am not quite sure that applies to these checks. For one thing, Sensu isn't going to be pulling aws credentials from your role/data bag because it's making a call directly to the AWS sdk which as I outlined in my above post is only going to pull credentials from one of those three places. Now if the sensu wrapper cookbook is setting credentials via data bag or role, then that will continue to work just fine.

Really the only people removing these options will affect are those who are hardcoding the credentials into their wrapper cookbook recipes, which we should not be encouraging and is not the recommended practice by Amazon.

EDIT:
As soon as I submitted this I realized that you could be pulling the credentials into the check via databag or role on the CLI. Not sure if we should continue supporting this use case.

@mattyjones
Member

@bkett I sent something out to the mailing list on this to try and get a wider feedback loop going.

As to your first point, if I can't have my cake and eat than thats fine, I say going all in with the SDK and say no to CLI period. Again I am open to thoughts but even setting the key as an attribute at some point exposes it on the commandline.

EDIT: Not sure how the aws-sdk works but in Python when using boto, I had a boto.cfg file that stored everything for me and I would just call it. Possibly the aws-sdk allows this as well.

@bkett
bkett commented Feb 12, 2015

@mattyjones Yeah, if we allow my edited case where you can supply the key via data bag, that also allows you to hardcode it. I just don't see a way to avoid that situation without removing the option completely. I'm going to proceed along these lines and if it's decided we don't want to go down this route then after I submit the PR we can adjust accordingly.

@bkett
bkett commented Feb 12, 2015

@mattyjones It does, hence my note about ~/.aws/config

@miah
miah commented Feb 12, 2015

I am very +1 on 'use environment' etc. See: http://12factor.net/config

With EC2, you can use IAM Profiles to completely remove the need for sharing access keys at all.

There are certainly libraries out there that do not respect this. Which means in some cases it may seem like you have to pass that data in via the command line. But Ruby has the ENV class as well, so you can simply ENV.fetch('FOO') and be on your way.

You can still pass data down from Chef node attributes and store it in a way that it is present in the working environment of an application. That may require refactoring some Chef recipes, and it may require you to think about your deployment differently.

@miah
miah commented Feb 12, 2015

If you're truly paranoid you can employ a tool like https://github.com/whilp/envcrypt to secure your environment variables at rest. While your application is running, its environment as well as command line arguments are available with a few arguments passed to ps, and they're readable in /proc.

@sstarcher
Contributor

I would like to see this implemented and us relying on aws-sdk to pull the credentials. The ruby sdk already supports environment variables and several other mechanisms.

http://docs.aws.amazon.com/sdkforruby/api/

@mattyjones mattyjones removed the Enhancement label Aug 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment