ad_integration - leaks credentials when in check_mode

Resolves:rhbz#2223764
ad_integration - leaks credentials when in check_mode
This commit is contained in:
Rich Megginson 2023-08-22 07:16:41 -06:00
parent a0cc364663
commit 86eefbad8d
3 changed files with 135 additions and 0 deletions

View File

@ -0,0 +1,127 @@
From 1931ebccaa146bd6ee8365c664ab62d294adaa31 Mon Sep 17 00:00:00 2001
From: Rich Megginson <rmeggins@redhat.com>
Date: Fri, 18 Aug 2023 12:35:44 -0600
Subject: [PATCH] fix: use command stdin for password, and do not log password
Cause: The code was constructing the realm join command to be passed
via the shell module, including piping the password into the command,
and was showing the command, including the password, when using
check mode.
Consequence: The clear text password was available in the logs when
using check mode.
Fix: Use command with stdin for the password instead of shell. The
password is not part of the command. command with stdin is more
secure than using shell. The debug output has been changed to
show the command with the `ad_integration_join_parameters` removed,
because we cannot know if those parameters contain data which should
not be logged. Those parameters will still be passed to the actual
realm join command.
Result: The password is not logged. The role is more secure.
Signed-off-by: Rich Megginson <rmeggins@redhat.com>
---
tasks/main.yml | 57 ++++++++++++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 25 deletions(-)
diff --git a/tasks/main.yml b/tasks/main.yml
index fe2602e..265c6fe 100644
--- a/tasks/main.yml
+++ b/tasks/main.yml
@@ -3,8 +3,7 @@
- name: Ensure that mandatory variable ad_integration_realm is available
fail:
msg: Variable ad_integration_realm must be provided!
- when:
- - not ad_integration_realm
+ when: not ad_integration_realm
- name: Assume managing timesync if timesource is set
set_fact:
@@ -26,8 +25,7 @@
- name: Assume managing crypto policies if allow_rc4_crypto is set
set_fact:
ad_integration_manage_crypto_policies: true
- when:
- - ad_integration_allow_rc4_crypto | bool
+ when: ad_integration_allow_rc4_crypto | bool
- name: Ensure manage_crypt_policies is set with crypto_allow_rc4
fail:
@@ -141,41 +139,50 @@
- name: Build Command - Join to a specific Domain Controller
set_fact:
- __ad_integration_join_command: |
- set -euo pipefail
- echo {{ ad_integration_password | quote }} | realm join -U \
- {{ ad_integration_user | quote }} --membership-software \
- {{ ad_integration_membership_software | quote }} \
- {{ ad_integration_join_parameters }} \
- {{ ad_integration_join_to_dc | quote }}
+ __ad_integration_join_command: >-
+ realm join -U {{ ad_integration_user | quote }} --membership-software
+ {{ ad_integration_membership_software | quote }}
+ {{ ad_integration_join_parameters }}
+ {{ ad_integration_join_to_dc | quote }}
+ __ad_integration_debug_command: >-
+ realm join -U {{ ad_integration_user | quote }} --membership-software
+ {{ ad_integration_membership_software | quote }}
+ {{ ad_integration_join_to_dc | quote }}
no_log: true
- when:
- - ad_integration_join_to_dc is not none
+ when: ad_integration_join_to_dc is not none
- name: Build Join Command - Perform discovery-based realm join operation
set_fact:
- __ad_integration_join_command: |
- set -euo pipefail
- echo {{ ad_integration_password | quote }} | realm join -U \
- {{ ad_integration_user | quote }} --membership-software \
- {{ ad_integration_membership_software | quote }} \
- {{ ad_integration_join_parameters }} \
- {{ ad_integration_realm | quote }}
+ __ad_integration_join_command: >-
+ realm join -U {{ ad_integration_user | quote }} --membership-software
+ {{ ad_integration_membership_software | quote }}
+ {{ ad_integration_join_parameters }}
+ {{ ad_integration_realm | quote }}
+ __ad_integration_debug_command: >-
+ realm join -U {{ ad_integration_user | quote }} --membership-software
+ {{ ad_integration_membership_software | quote }}
+ {{ ad_integration_realm | quote }}
no_log: true
- when:
- - ad_integration_join_to_dc is none
+ when: ad_integration_join_to_dc is none
- name: Show the join command for debug
debug:
- msg: "Would run: '{{ __ad_integration_join_command }}'"
+ msg:
+ - >-
+ Would run the following command. Note that
+ ad_integration_join_parameters have been removed for security purposes,
+ the role will pass them to the actual realm join command when running
+ without check mode.
+ - "{{ __ad_integration_debug_command }}"
when:
- ad_integration_join_to_dc == __ad_integration_sample_dc
or ad_integration_realm == __ad_integration_sample_realm
or ansible_check_mode
- name: Run realm join command
- # noqa command-instead-of-shell
- shell: "{{ __ad_integration_join_command }}"
+ command: "{{ __ad_integration_join_command }}"
+ args:
+ stdin: "{{ ad_integration_password }}"
no_log: true
when:
- ad_integration_join_to_dc != __ad_integration_sample_dc
--
2.41.0

View File

@ -34,6 +34,7 @@ Changelog
### Bug Fixes
- [ALL - facts being gathered unnecessarily](https://bugzilla.redhat.com/show_bug.cgi?id=2223032)
- [ad_integration - leaks credentials when in check_mode](https://bugzilla.redhat.com/show_bug.cgi?id=2223764)
- [certificate - does not re-issue after updating key_size](https://bugzilla.redhat.com/show_bug.cgi?id=2224138)
- [firewall - fix: reload on resetting to defaults](https://bugzilla.redhat.com/show_bug.cgi?id=2223764)
- [firewall - Check mode fails with replacing previous rules](https://issues.redhat.com/browse/RHEL-898)

View File

@ -217,6 +217,8 @@ Source1004: vendoring-build.inc
Source995: CHANGELOG.md
Patch2201: 0001-fix-use-command-stdin-for-password-and-do-not-log-pa.patch
BuildArch: noarch
%if %{with html}
@ -329,6 +331,10 @@ if [ "$rolesdir" != "$realrolesdir" ]; then
fi
cd ..
cd %{rolename22}
%patch2201 -p1
cd ..
# vendoring build steps, if any
%include %{SOURCE1004}
@ -667,6 +673,7 @@ find %{buildroot}%{ansible_roles_dir} -mindepth 1 -maxdepth 1 | \
%changelog
* Tue Aug 15 2023 Rich Megginson <rmeggins@redhat.com> - 1.22.0-1
- Resolves:rhbz#2223764 : ad_integration - leaks credentials when in check_mode
- Resolves:rhbz#2232241 : kdump - "Write new authorized_keys if needed" task idempotency issues
- Resolves:rhbz#2232231 : kdump - system role fails if kdump_ssh_user doesn't have a .ssh/authorized_keys file in home directory
- Resolves RHEL-1397 : kdump - fix: ensure .ssh directory exists for kdump_ssh_user on kdump_ssh_server