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