From 6b81c365330c55824902e5148deefed9a7340e1b Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Tue, 14 Aug 2018 12:58:00 -0700 Subject: [PATCH] restore task arg splatting (#43798) * restore task arg splatting * reverts #41804 * supersedes #41295 * fixes #42192 * after lots of discussion amongst the core team, we decided to preserve this feature, clarify the runtime warnings/docs, and prioritize a path toward fixing the underlying behavior that causes this feature to be insecure (un-namespaced facts). * update faq text note that warning is disabled when inject_facts_as_vars is * wordsmithing FAQ entry --- docs/docsite/rst/reference_appendices/faq.rst | 34 ++++++++++++++++++- lib/ansible/executor/task_executor.py | 6 +++- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/docs/docsite/rst/reference_appendices/faq.rst b/docs/docsite/rst/reference_appendices/faq.rst index 1cf5a6407a8..039670fe064 100644 --- a/docs/docsite/rst/reference_appendices/faq.rst +++ b/docs/docsite/rst/reference_appendices/faq.rst @@ -446,7 +446,7 @@ In OpenBSD, a similar option is available in the base system called encrypt(1): encrypt -.. _commercial_support: +.. _dot_or_array_notation: Ansible supports dot notation and array notation for variables. Which notation should I use? ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ @@ -464,6 +464,38 @@ safer to use the array notation for variables. Also array notation allows for dynamic variable composition, see dynamic_variables_. + +.. _argsplat_unsafe: + +When is it unsafe to bulk-set task arguments from a variable? ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ + + +You can set all of a task's arguments from a dictionary-typed variable. This +technique can be useful in some dynamic execution scenarios. However, it +introduces a security risk. We do not recommend it, so Ansible issues a +warning when you do something like this:: + + #... + vars: + usermod_args: + name: testuser + state: present + update_password: always + tasks: + - user: '{{ usermod_args }}' + +This particular example is safe. However, constructing tasks like this is +risky because the parameters and values passed to ``usermod_args`` could +be overwritten by malicious values in the ``host facts`` on a compromised +target machine. To mitigate this risk: + +* set bulk variables at a level of precedence greater than ``host facts`` in the order of precedence found in :ref:`ansible_variable_precedence` (the example above is safe because play vars take precedence over facts) +* disable the :ref:`inject_facts_as_vars` configuration setting to prevent fact values from colliding with variables (this will also disable the original warning) + + +.. _commercial_support: + Can I get training on Ansible? ++++++++++++++++++++++++++++++ diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py index baeaeecafe0..94644981bc9 100644 --- a/lib/ansible/executor/task_executor.py +++ b/lib/ansible/executor/task_executor.py @@ -519,7 +519,11 @@ class TaskExecutor: if '_variable_params' in self._task.args: variable_params = self._task.args.pop('_variable_params') if isinstance(variable_params, dict): - raise AnsibleError("Using a variable for a task's 'args' is not allowed as it is unsafe, facts can come from untrusted sources.") + if C.INJECT_FACTS_AS_VARS: + display.warning("Using a variable for a task's 'args' is unsafe in some situations " + "(see https://docs.ansible.com/ansible/devel/reference_appendices/faq.html#argsplat-unsafe)") + variable_params.update(self._task.args) + self._task.args = variable_params # get the connection and the handler for this execution if (not self._connection or