security: use quote with command, shell and validate with variable #245
Conversation
|
Changed test to use flagfile instead of user. |
This avoids issues if file names are not safepaths.
Skip quotation only if variable is checked. Add test suit to excercise some quote use cases.
Ensure systemd.unit contents is robust. This disables possibility to
have something that needs to be quoted there. But as ansible lacks
proper way to quote systemd unit files (see man systemd.syntax, rules
are not shell rules), it is better to fail such configs. If you are
trying to do that, you are doing it wrong anyway or have malicious
intent.
Also ensure similar issue with sysctl.conf.
Issue can be seen with `tests_hostkeys_unsafe_path.yml`, when adding
following to role params:
sshd_install_service: true
sshd_config_file: "{{ ansible_facts.env.TMPDIR }}/sshd.d/foo.conf"
sshd_binary: "{{ ansible_facts.env.TMPDIR }}/sshd"
__sshd_runtime_directory: "{{ ansible_facts.env.TMPDIR }}/run"
|
Decided to split test backup / restore from main commit and some commit message fixes. Also changed test as testing with |
|
The centos 8 fails reproducibly in your new test for some reason. Can you have a look for the reason? I see it fails on |
|
Please change the title to |
|
ping - can this be rebased? |
ping ping |
mattwillsher
added
enhancement
bug
|
Any views on what to do with this? There is outstanding comment about the part of a path that gets quoted. If this is a significant improvement we can pick it up. In my mind, which quote will help avoid injection exploits Ansible 'should not' be getting run with unchecked, unsanitised variables, though maybe I'm missing something in my understanding. |
Yes, this is a good security hardening patch. Yes, users of the role should not be running Ansible with unchecked, unsanitised variables, but I suppose it could happen. |
|
Do either of you wants to take it into new PR as this one is stale, or should I do that? |
|
If you have the time @Jakuje it would be appreciated otherwise I'll pick it up when I have a bit of time |
|
Rebased and moved to #298 -- will address the CI issues if there will be some. Closing this one. |
Enhancement:
Use
quotealways when usingcommandplugin,shellplugin, validate content as command to execute or system to configure.Reject bad content for
systemd.unitandsysctlconfig.Reason:
This improves security and robustness. At least TMPDIR can be defined outside in various ways.
This follows example in
tests/tests_all_options.ymlwherepkg_mgris quoted.When using
commandplugin, it could be possible to useargvfunctionality and avoid quoting.Initial reason was single quotes in
'{{ sshd_test_hostkey.path }}/rsa_key'intasks/install.ymlbut not intasks/install_config.ymlandtasks/install_namespace.yml.Result:
Included test passes. Care should be taken if test fails. It might leave
BADFLAGuser or related files behind.Issue Tracker Tickets (Jira or BZ if any): -