ansible / proposals

Repository for sharing and tracking progress on enhancement proposals for Ansible.
Creative Commons Zero v1.0 Universal
93 stars 19 forks source link

Configurable no_log at variable level in playbook and/or tasks #109

Closed ahuffman closed 6 years ago

ahuffman commented 6 years ago

Proposal: Configurable no_log at variable level in playbook and/or tasks

Author: Andrew J. Huffman <@ahuffman> IRC: N/A

Date: 2018/04/23

Motivation

To be able to define variables that should not ever be logged to standard out with or without debugging turned on in a playbook or task, or during a task failure. This is a huge security problem, especially when using log aggregation with Ansible Tower. Users who are able to consume log aggregation output can inadvertently be exposed to sensitive high-level account details.

Problems

What problems exist that this proposal will solve?

Solution proposal

Example 1

Example 2

The answer should be "no" on all the above tests, and should instead be a value of "obfuscated" or similar.

Documentation

Yes, this would be required.

abadger commented 6 years ago

I don't see us doing this ever as there's no way to do this with even a modicum of knowing that it was secure as even easy to create bugs in modules would cause variables to escape the boundaries set up for them controller side.

Perhaps a less ambitious feature like "automatically turn on no_log for tasks that use variables that were vaulted" would be implementable. However, I don't think jinja will make even that easy to implement. We'd need some method of having jinja tell us that a tainted variable was used to create a new variable so that we can taint the new one. That's not a facitity that jinja provides. So we'd have resort to something like templating every variable twice (once with known tainted vars and once without) and then comparing the output to decide if the new variable is tainted. That will get prohibitively expensive for sites that use large numbers of vars.

abadger commented 6 years ago

We discussed and voted at today's meeting. seven voted against, none voted for. We don't have a problem with the concept but do not see any way to implement this that would be secure without every plugin (written by third parties) doing things in a secure manner. In the light of that, we believe that it is better not to write and promote a feature for security which will leak information at the slightest misuse.

Two additional thoughts that came up:

ahuffman commented 6 years ago

@abadger Thanks for looking into this. I'll file a bug report for where I've seen no_log failing.

To be clear: Namely no_log fails to be secure if a task fails. The full task failure output gets spit out to standard out with the password staring you in the face. I've seen this specifically on the command/shell module. Then it ends up in my customer's Splunk database which is fun explaining.

I guess my question is how are some modules obfuscating password fields and could something like that be utilized?

Also, another argument for this is when wrting uri heavy roles that require a token be passed, I literally have to add no_log to every single task in the role. There's no way to have a no_log apply to all tasks within a role. The only tool I have today is to put all of the tasks into a block with no_log: True.

abadger commented 6 years ago

EDITED: added no_log: True to the ping: data=crash example.

Okay, we'll need a reproducer for no_log failing. I'm not able to reproduce it. You can ping me in IRC if you want me to look at it, I'm abadger1999 in the #ansible-devel channel on irc.freenode.net. The ping command is handy for testing a failure, btw:

---
- hosts: localhost
  no_log: True
  tasks:
  - ping:
      data: crash

ansible-playbook test.yml -vvvvv | grep crash does not show me any output so there must be more to your problem than that.

For your use case, how are you trying to add no_log to the role? At first glance, it should work to add no_log to the role invocation to make it apply to tasks inside of the role:

$ cat playbook.yml
---
- hosts: localhost
  roles:
  - role: myrole
    no_log: True

$ cat myrole/tasks/main.yml
---
- ping:
    data: secret
caphrim007 commented 6 years ago

@abadger what if there were a python traceback or some other more catastrophic error? Something that is not well-formed json?

abadger commented 6 years ago

@caphrim007

- ping:
    data: crash 

Tests the case of a python traceback.

abadger commented 6 years ago

Another note since I read your initial comment again and noticed you mentioned "debugging":

ANSIBLE_DEBUG ignores no_log: http://docs.ansible.com/ansible/latest/reference_appendices/faq.html#how-do-i-keep-secret-data-in-my-playbook This is on purpose as the debugging switch is intended to make all of ansible's decisions transparent for easier debugging of ansible itself. It should not be used in production. (Especially with tower which would save all of that debugging information if it was turned on).

I'm opening up a PR to add a similar warning to what's in the FAQ to the documentation for the ANSIBLE_DEBUG environment variable.

sirkubax commented 6 years ago

Well, I don't know about @abadger voting :) but how about the idea I suggested in https://github.com/ansible/ansible/issues/38942 - 'mute' (never display) variables that (part of the) name is 'blacklisted' in a configuration.

This blacklisting should be valid also for high verbosity output.
I believe that my suggestion can be done on 'ansible side' printing mechanism - that is - not playing around with the modules logic.

ansible.cfg

never print variables with this name (should we support regex?)

hidenn_name_output_list: password*;_vault;_hidden;db_string;credentials;pwd

bcoca commented 6 years ago

@sirkubax its the same issue, there is no good way to create a 'taint' that will be observed by every subsystem and/or plugin, the idea is nice, but there is no sane implementation we can see.

If you figure one out, we are open to PRs being submitted on this subject, we just don't see a way to sanely implement this ourselves.

sirkubax commented 6 years ago

@bcoca I thing that this is 'good enough' idea that would solve our problems.

I think if we modify the 'printing/logging' mechanism, so that we hide the 'protected phrases' from beeing logged in - it should be enough.

Please guide me with regards the printing mechanism: Is the file

./lib/ansible/utils/display.py

the right one to look at? Or shall we look at callbacks too?

We are looking for a place where the strings (to be printed on the terminal (into logs)) are created, colored, cowsay'ed, - so at the very end - we modify the output and ** the blacklisted phrases (variables, dictionaries, etc).

I've found this nicely named Class :)

class FilterBlackList(logging.Filter):
    def __init__(self, blacklist):
        self.blacklist = [logging.Filter(name) for name in blacklist]

    def filter(self, record):
        return not any(f.filter(record) for f in self.blacklist)
sivel commented 6 years ago

"good enough" isn't sufficient. Any "good enough" solution is going to produce bug reports and security issues.

Additionally, it's more than just printing locally. This would also have to work with modules, transferring state along with a remote module execution, to ensure that nothing is recorded remotely as well.

Additionally, it would need to track all reassignments and modifications of that data, not only the original.

sirkubax commented 6 years ago

well - you can always find a way to print a hidden variable For now - I think this is way better than the no_log feature...

abadger commented 6 years ago

The no_log feature is more maintainable to us. If you'd like to maintain a patch to your copy of Ansible that does that, however, I'd replace sys.stdout and sys.stderr with a filter that does what you want. Here's an example of how to do that:

The toplevel of utils/display.py is a logical place to put that as it keeps display related things together however it doesn't have to be there. Pretty much any place that is early in the run (so that it is set up before you have an opportunity to start outputting private information) will do. Filtering the display is not likely to be the hard part, though. It will be keeping track of things that need to be locked down and making sure that the filter has access to those that is hard. You will likely also quickly want to add on to this the ability to protect information on the managed machines; there is a standard feature that allows logging information remotely and that's probably something you'll want to prevent for these values. Simply pushing all of the to-be-secured values to the remote machines is definitely not a good idea as the remote machines could be compromised so you'll want to make sure that they only have access to the secrets that they need and nothing more.

That's all the help I can think of to give you. Good luck!