ad_integration - leaks credentials when in check_mode

Resolves:rhbz#2233183
ad_integration - leaks credentials when in check_mode

(cherry picked from commit 86eefbad8d)
This commit is contained in:
Rich Megginson 2023-08-22 07:16:41 -06:00
parent 958fb35d1f
commit 44302dfdc5
3 changed files with 152 additions and 17 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

@ -15,6 +15,7 @@ Changelog
- [ha_cluster - Add possibility to load SBD watchdog kernel modules](https://bugzilla.redhat.com/show_bug.cgi?id=2190478)
- [ha_cluster - cluster and quorum can have distinct passwords](https://bugzilla.redhat.com/show_bug.cgi?id=2216485)
- [ha_cluster - support for resource and operation defaults](https://bugzilla.redhat.com/show_bug.cgi?id=2190483)
- [kdump - support auto_reset_crashkernel, dracut_args, deprecate /etc/sysconfig/kdump](https://bugzilla.redhat.com/show_bug.cgi?id=2211272)
- [keylime_server - system role for managing keylime servers](https://bugzilla.redhat.com/show_bug.cgi?id=2224387)
- [network - Support configuring auto-dns setting](https://bugzilla.redhat.com/show_bug.cgi?id=2211273)
- [network - Support no-aaaa DNS option](https://bugzilla.redhat.com/show_bug.cgi?id=2218595)
@ -32,25 +33,25 @@ Changelog
### Bug Fixes
- [ALL - facts being gathered unnecessarily](https://bugzilla.redhat.com/show_bug.cgi?id=2223036)
- [ad_integration - leaks credentials when in check_mode](https://bugzilla.redhat.com/show_bug.cgi?id=2233183)
- [certificate - rhel-system-roles.certificate does not re-issue after updating key_size](https://bugzilla.redhat.com/show_bug.cgi?id=2186057)
#- [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)
- [firewall - fix: reload on resetting to defaults](https://bugzilla.redhat.com/show_bug.cgi?id=2224648)
- [firewall - Check mode fails with replacing previous rules](https://issues.redhat.com/browse/RHEL-899)
- [firewall - Check mode fails when creating new firewall service](https://bugzilla.redhat.com/show_bug.cgi?id=2222433)
#- [firewall - Ansible RHEL firewall system role not idempotent when configuring the interface using the role in rhel9](https://issues.redhat.com/browse/RHEL-885)
- [firewall - Ansible RHEL firewall system role not idempotent when configuring the interface using the role in rhel9](https://issues.redhat.com/browse/RHEL-918)
- [firewall - Don't install python(3)-firewall it's a dependency of firewalld](https://bugzilla.redhat.com/show_bug.cgi?id=2216521)
#- [firewall - fix: files: overwrite firewalld.conf on previous replaced](https://issues.redhat.com/browse/RHEL-1495)
- [kdump - support auto_reset_crashkernel, dracut_args, deprecate /etc/sysconfig/kdump](https://bugzilla.redhat.com/show_bug.cgi?id=2211272)
#- [kdump - use failure_action instead of default on EL9 and later](https://issues.redhat.com/browse/RHEL-906)
#- [kdump - "Write new authorized_keys if needed" task idempotency issues](https://bugzilla.redhat.com/show_bug.cgi?id=2232241)
#- [kdump - system role fails if kdump_ssh_user doesn't have a .ssh/authorized_keys file in home directory](https://bugzilla.redhat.com/show_bug.cgi?id=2232231)
#- [kdump - fix: ensure .ssh directory exists for kdump_ssh_user on kdump_ssh_server](https://issues.redhat.com/browse/RHEL-1397)
#- [kdump - fix: Ensure authorized_keys management works with multiple hosts](https://issues.redhat.com/browse/RHEL-1499)
- [firewall - fix: files: overwrite firewalld.conf on previous replaced](https://issues.redhat.com/browse/RHEL-1496)
- [kdump - use failure_action instead of default on EL9 and later](https://issues.redhat.com/browse/RHEL-907)
- [kdump - role: "Write new authorized_keys if needed" task idempotency issues](https://bugzilla.redhat.com/show_bug.cgi?id=2232391)
- [kdump - system role fails if kdump_ssh_user doesn't have a .ssh/authorized_keys file in home directory](https://bugzilla.redhat.com/show_bug.cgi?id=2232392)
- [kdump - fix: ensure .ssh directory exists for kdump_ssh_user on kdump_ssh_server](https://issues.redhat.com/browse/RHEL-1398)
- [kdump - fix: Ensure authorized_keys management works with multiple hosts](https://issues.redhat.com/browse/RHEL-1500)
- [podman - Podman system role: Unable to use podman_registries_conf to set unqualified-search-registries](https://bugzilla.redhat.com/show_bug.cgi?id=2226077)
- [rhc - system role does not apply Insights tags](https://bugzilla.redhat.com/show_bug.cgi?id=2209441)
- [storage - Cannot set chunk size for RAID: Unsupported parameters for (blivet) module: pools.raid_chunk_size](https://bugzilla.redhat.com/show_bug.cgi?id=2193057)
- [storage - RAID volume pre cleanup - remove existing data from member disks as needed before creation](https://bugzilla.redhat.com/show_bug.cgi?id=2224094)
- [storage - Storage: mounted devices that are in use cannot be resized](https://bugzilla.redhat.com/show_bug.cgi?id=2168738)
#- [storage - fix: use stat.pw_name, stat.gr_name instead of owner, group](https://issues.redhat.com/browse/RHEL-1497)
- [storage - fix: use stat.pw_name, stat.gr_name instead of owner, group](https://issues.redhat.com/browse/RHEL-1498)
- [tlog - use the proxy provider - the files provider is deprecated in sssd](https://bugzilla.redhat.com/show_bug.cgi?id=2191702)
[1.21.1] - 2023-03-16

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,13 +673,14 @@ 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#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
#- Resolves RHEL-1499 : kdump - fix: Ensure authorized_keys management works with multiple hosts
- Resolves:rhbz#2233183 : ad_integration - red hat "rhel system role" ad_integration leaks credentials when in check_mode
- Resolves:rhbz#2232391 : kdump - role: "Write new authorized_keys if needed" task idempotency issues
- Resolves:rhbz#2232392 : kdump - system role fails if kdump_ssh_user doesn't have a .ssh/authorized_keys file in home directory
- Resolves RHEL-1398 : kdump - fix: ensure .ssh directory exists for kdump_ssh_user on kdump_ssh_server
- Resolves RHEL-1500 : kdump - fix: Ensure authorized_keys management works with multiple hosts
- Resolves:rhbz#2224648 : firewall - fix: reload on resetting to defaults
#- Resolves RHEL-1495 : firewall - fix: files: overwrite firewalld.conf on previous replaced
#- Resolves RHEL-1497 : storage - fix: use stat.pw_name, stat.gr_name instead of owner, group
- Resolves RHEL-1496 : firewall - fix: files: overwrite firewalld.conf on previous replaced
- Resolves RHEL-1498 : storage - fix: use stat.pw_name, stat.gr_name instead of owner, group
sshd README remove upstream only docs
first RC for 1.22.0 rhel 8.9 and 9.3
fix firewall reload test gather facts