Skip to content

Parameterize SSH client configuration paths via product properties (preserve defaults)#14449

Open
Smouhoune wants to merge 5 commits intoComplianceAsCode:masterfrom
Smouhoune:feat/ssh-client-path-overrides
Open

Parameterize SSH client configuration paths via product properties (preserve defaults)#14449
Smouhoune wants to merge 5 commits intoComplianceAsCode:masterfrom
Smouhoune:feat/ssh-client-path-overrides

Conversation

@Smouhoune
Copy link

Description:

  - Add product-overridable SSH client path properties:
    - ssh_client_main_config_file
    - ssh_client_config_dir
  - Add backward-compatible defaults in core product property resolution:
    - ssh_client_main_config_file=/etc/ssh/ssh_config
    - ssh_client_config_dir=/etc/ssh/ssh_config.d
  - Replace hardcoded SSH client paths in the following rules and their checks/remediations:
    - ssh_client_rekey_limit
    - ssh_client_use_approved_ciphers_ordered_stig
    - ssh_use_approved_macs_ordered_stig
    - harden_ssh_client_crypto_policy

  #### Rationale:

  - These rules currently hardcode SSH client config paths (/etc/ssh/ssh_config*), which prevents clean reuse on products that place OpenSSH client configuration elsewhere.
  - This PR makes path handling product-driven while preserving existing behavior for current products through explicit defaults.
  - Rule intent and security logic are unchanged; only path parameterization was introduced.

  #### Review Hints:

  - Suggested review order:
    1. core(ssg): add product-overridable SSH client path properties
    2. rules(ssh_client): replace hardcoded ssh client paths with product properties
  - Scope is intentionally limited to SSH client path parameterization in the four rules above.
  - No rule was removed/disabled.
  - Backward compatibility:
    - Products without overrides continue to use /etc/ssh/ssh_config and /etc/ssh/ssh_config.d.
  - Local validation performed:
    - ./build_product --datastream-only rhel9
    - ./build_product --datastream-only ubuntu2204
    - ctest -R 'validate-ssg-ubuntu2204-ds.xml' --output-on-failure

@openshift-ci
Copy link

openshift-ci bot commented Feb 24, 2026

Hi @Smouhoune. Thanks for your PR.

I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

… path

Avoid Jinja whitespace trimming that concatenated a comment and the file assignment in harden_ssh_client_crypto_policy Bash remediation. This keeps 'file=...' on its own line and fixes shellcheck SC2154 in generated fixes.
@Smouhoune Smouhoune force-pushed the feat/ssh-client-path-overrides branch from a875c67 to 1ef0ca0 Compare February 24, 2026 22:05
@Smouhoune Smouhoune requested review from a team and matusmarhefka as code owners February 24, 2026 22:05
@github-actions
Copy link

github-actions bot commented Feb 25, 2026

This datastream diff is auto generated by the check Compare DS/Generate Diff

Click here to see the full diff
bash remediation for rule 'xccdf_org.ssgproject.content_rule_configure_custom_crypto_policy_cis' differs.
--- xccdf_org.ssgproject.content_rule_configure_custom_crypto_policy_cis
+++ xccdf_org.ssgproject.content_rule_configure_custom_crypto_policy_cis
@@ -1,30 +1,28 @@
-
-expected_crypto_policy="DEFAULT:NO-SHA1"
 
 
-expected_crypto_policy="${expected_crypto_policy}:NO-SSHCBC"
+
 cat << 'EOF' > /etc/crypto-policies/policies/modules/NO-SSHCBC.pmod
 cipher@SSH = -*-CBC
 EOF
 
-expected_crypto_policy="${expected_crypto_policy}:NO-SSHWEAKCIPHERS"
+
 cat << 'EOF' > /etc/crypto-policies/policies/modules/NO-SSHWEAKCIPHERS.pmod
 cipher@SSH = -3DES-CBC -AES-128-CBC -AES-192-CBC -AES-256-CBC -CHACHA20-POLY1305
 EOF
 
-expected_crypto_policy="${expected_crypto_policy}:NO-SSHWEAKMACS"
+
 cat << 'EOF' > /etc/crypto-policies/policies/modules/NO-SSHWEAKMACS.pmod
 mac@SSH = -HMAC-MD5* -UMAC-64* -UMAC-128*
 EOF
 
-expected_crypto_policy="${expected_crypto_policy}:NO-WEAKMAC"
+
 cat << 'EOF' > /etc/crypto-policies/policies/modules/NO-WEAKMAC.pmod
 mac = -*-128*
 EOF
 
 
 current_crypto_policy=$(update-crypto-policies --show)
-
+expected_crypto_policy="DEFAULT:NO-SHA1:NO-SSHCBC:NO-SSHWEAKCIPHERS:NO-SSHWEAKMACS:NO-WEAKMAC"
 if [[ "$current_crypto_policy" != "$expected_crypto_policy" ]] ; then
     update-crypto-policies --set "$expected_crypto_policy"
 fi

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_configure_custom_crypto_policy_cis' differs.
--- xccdf_org.ssgproject.content_rule_configure_custom_crypto_policy_cis
+++ xccdf_org.ssgproject.content_rule_configure_custom_crypto_policy_cis
@@ -1,16 +1,3 @@
-- name: Implement Custom Crypto Policy Modules for CIS Benchmark - Set the base crypto
-    policy
-  ansible.builtin.set_fact:
-    expected_crypto_policy: DEFAULT:NO-SHA1
-  tags:
-  - CCE-86707-7
-  - configure_custom_crypto_policy_cis
-  - configure_strategy
-  - low_complexity
-  - low_disruption
-  - medium_severity
-  - reboot_required
-
 - name: Implement Custom Crypto Policy Modules for CIS Benchmark - Create custom crypto
     policy module NO-SSHCBC
   ansible.builtin.lineinfile:
@@ -21,19 +8,6 @@
     line: cipher@SSH = -*-CBC
     create: true
     regexp: cipher@SSH
-  tags:
-  - CCE-86707-7
-  - configure_custom_crypto_policy_cis
-  - configure_strategy
-  - low_complexity
-  - low_disruption
-  - medium_severity
-  - reboot_required
-
-- name: Implement Custom Crypto Policy Modules for CIS Benchmark - Update the expected
-    policy
-  ansible.builtin.set_fact:
-    expected_crypto_policy: '{{ expected_crypto_policy + '':NO-SSHCBC'' }}'
   tags:
   - CCE-86707-7
   - configure_custom_crypto_policy_cis
@@ -62,19 +36,6 @@
   - medium_severity
   - reboot_required
 
-- name: Implement Custom Crypto Policy Modules for CIS Benchmark - Update the expected
-    policy
-  ansible.builtin.set_fact:
-    expected_crypto_policy: '{{ expected_crypto_policy + '':NO-SSHWEAKCIPHERS'' }}'
-  tags:
-  - CCE-86707-7
-  - configure_custom_crypto_policy_cis
-  - configure_strategy
-  - low_complexity
-  - low_disruption
-  - medium_severity
-  - reboot_required
-
 - name: Implement Custom Crypto Policy Modules for CIS Benchmark - Create custom crypto
     policy module NO-SSHWEAKMACS
   ansible.builtin.lineinfile:
@@ -85,19 +46,6 @@
     line: mac@SSH = -HMAC-MD5* -UMAC-64* -UMAC-128*
     create: true
     regexp: mac@SSH
-  tags:
-  - CCE-86707-7
-  - configure_custom_crypto_policy_cis
-  - configure_strategy
-  - low_complexity
-  - low_disruption
-  - medium_severity
-  - reboot_required
-
-- name: Implement Custom Crypto Policy Modules for CIS Benchmark - Update the expected
-    policy
-  ansible.builtin.set_fact:
-    expected_crypto_policy: '{{ expected_crypto_policy + '':NO-SSHWEAKMACS'' }}'
   tags:
   - CCE-86707-7
   - configure_custom_crypto_policy_cis
@@ -126,19 +74,6 @@
   - medium_severity
   - reboot_required
 
-- name: Implement Custom Crypto Policy Modules for CIS Benchmark - Update the expected
-    policy
-  ansible.builtin.set_fact:
-    expected_crypto_policy: '{{ expected_crypto_policy + '':NO-WEAKMAC'' }}'
-  tags:
-  - CCE-86707-7
-  - configure_custom_crypto_policy_cis
-  - configure_strategy
-  - low_complexity
-  - low_disruption
-  - medium_severity
-  - reboot_required
-
 - name: Implement Custom Crypto Policy Modules for CIS Benchmark - Check current crypto
     policy
   ansible.builtin.command: update-crypto-policies --show
@@ -156,9 +91,8 @@
   - reboot_required
 
 - name: Implement Custom Crypto Policy Modules for CIS Benchmark - Update crypto-policies
-  ansible.builtin.command: update-crypto-policies --set {{ expected_crypto_policy
-    }}
-  when: current_crypto_policy.stdout.strip() != expected_crypto_policy
+  ansible.builtin.command: update-crypto-policies --set DEFAULT:NO-SHA1:NO-SSHCBC:NO-SSHWEAKCIPHERS:NO-SSHWEAKMACS:NO-WEAKMAC
+  when: current_crypto_policy.stdout.strip() != "DEFAULT:NO-SHA1:NO-SSHCBC:NO-SSHWEAKCIPHERS:NO-SSHWEAKMACS:NO-WEAKMAC"
   tags:
   - CCE-86707-7
   - configure_custom_crypto_policy_cis

New content has different text for rule 'xccdf_org.ssgproject.content_rule_harden_ssh_client_crypto_policy'.
--- xccdf_org.ssgproject.content_rule_harden_ssh_client_crypto_policy
+++ xccdf_org.ssgproject.content_rule_harden_ssh_client_crypto_policy
@@ -4,7 +4,7 @@
 
 [description]:
 Crypto Policies are means of enforcing certain cryptographic settings for selected applications including OpenSSH client.
-To override the system wide crypto policy for Openssh client, place a file in the /etc/ssh/ssh_config.d/ so that it is loaded before the 05-redhat.conf. In this case it is file named 02-ospp.conf containing parameters which need to be changed with respect to the crypto policy.
+To override the system wide crypto policy for Openssh client, place a file in the /etc/ssh/ssh_config.d directory so that it is loaded before the 05-redhat.conf. In this case it is the /etc/ssh/ssh_config.d/02-ospp.conf file containing parameters which need to be changed with respect to the crypto policy.
 This rule checks if the file exists and if it contains required parameters and values which modify the Crypto Policy.
 During the parsing process, as soon as Openssh client parses some configuration option and its value, it remembers it and ignores any subsequent overrides. The customization mechanism provided by crypto policies appends eventual customizations at the end of the system wide crypto policy. Therefore, if the crypto policy customization overrides some parameter which is already configured in the system wide crypto policy, the SSH client will not honor that customized parameter.
 

bash remediation for rule 'xccdf_org.ssgproject.content_rule_harden_ssh_client_crypto_policy' differs.
--- xccdf_org.ssgproject.content_rule_harden_ssh_client_crypto_policy
+++ xccdf_org.ssgproject.content_rule_harden_ssh_client_crypto_policy
@@ -1,5 +1,6 @@
 
 #the file starts with 02 so that it is loaded before the 05-redhat.conf which activates configuration provided by system vide crypto policy
+
 file="/etc/ssh/ssh_config.d/02-ospp.conf"
 echo -e "Match final all\n\
 RekeyLimit 512M 1h\n\

New content has different text for rule 'xccdf_org.ssgproject.content_rule_gnome_gdm_disable_xdmcp'.
--- xccdf_org.ssgproject.content_rule_gnome_gdm_disable_xdmcp
+++ xccdf_org.ssgproject.content_rule_gnome_gdm_disable_xdmcp
@@ -3,7 +3,9 @@
 Disable XDMCP in GDM
 
 [description]:
-XDMCP is an unencrypted protocol, and therefore, presents a security risk.
+XDMCP is an unencrypted protocol, and therefore, presents a security risk, see e.g.
+XDMCP Gnome docs.
+
 To disable XDMCP support in Gnome, set Enable to false under the [xdmcp] configuration section in /etc/gdm/custom.conf. For example:
 
 [xdmcp]

bash remediation for rule 'xccdf_org.ssgproject.content_rule_ensure_redhat_gpgkey_installed' differs.
--- xccdf_org.ssgproject.content_rule_ensure_redhat_gpgkey_installed
+++ xccdf_org.ssgproject.content_rule_ensure_redhat_gpgkey_installed
@@ -20,9 +20,11 @@
   # No CRC error, safe to proceed
   if [ "${GPG_RESULT}" -eq "0" ]
   then
-  # If $REDHAT_RELEASE_KEY file doesn't contain any keys with unknown fingerprint, import it
 
-    echo "${GPG_OUT[*]}" | grep -vE "${REDHAT_RELEASE_FINGERPRINT}|${REDHAT_AUXILIARY_FINGERPRINT}" || rpm --import "${REDHAT_RELEASE_KEY}"
+    echo "${GPG_OUT[*]}" | grep -vE "${REDHAT_RELEASE_FINGERPRINT}|${REDHAT_AUXILIARY_FINGERPRINT}" || {
 
+      # If $REDHAT_RELEASE_KEY file doesn't contain any keys with unknown fingerprint, import it
+      rpm --import "${REDHAT_RELEASE_KEY}"
+    }
   fi
 fi

bash remediation for rule 'xccdf_org.ssgproject.content_rule_ssh_client_rekey_limit' differs.
--- xccdf_org.ssgproject.content_rule_ssh_client_rekey_limit
+++ xccdf_org.ssgproject.content_rule_ssh_client_rekey_limit
@@ -1,8 +1,6 @@
 
 var_ssh_client_rekey_limit_size=''
 var_ssh_client_rekey_limit_time=''
-
-
 main_config="/etc/ssh/ssh_config"
 include_directory="/etc/ssh/ssh_config.d"
 

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_ssh_client_rekey_limit' differs.
--- xccdf_org.ssgproject.content_rule_ssh_client_rekey_limit
+++ xccdf_org.ssgproject.content_rule_ssh_client_rekey_limit
@@ -26,7 +26,7 @@
 
 - name: Collect all include config files for ssh client which configure RekeyLimit
   ansible.builtin.find:
-    paths: /etc/ssh/ssh_config.d/
+    paths: /etc/ssh/ssh_config.d
     contains: ^[\s]*RekeyLimit.*$
     patterns: '*.config'
   register: ssh_config_include_files

@Smouhoune Smouhoune force-pushed the feat/ssh-client-path-overrides branch 2 times, most recently from 98d79c0 to 01bbd1d Compare February 25, 2026 20:16
Update product stability references for ssh client path properties and fix Jinja whitespace trimming in the Ubuntu bash remediation template.\n\nThe template change preserves the newline between variable assignments in the generated shell script and avoids shellcheck failures.
@Smouhoune Smouhoune force-pushed the feat/ssh-client-path-overrides branch from 01bbd1d to 27a8c9c Compare February 25, 2026 20:17
@Smouhoune
Copy link
Author

hi @Mab879
Could you please also review this PR as well when you have some time? ? It follows on from #14445 that you already approved.

@Mab879 Mab879 self-assigned this Feb 26, 2026
@Smouhoune
Copy link
Author

Hi @mrkanon Could you please also review this PR as well when you have some time? ? It follows on from #14445 that you already approved.

Copy link
Member

@Mab879 Mab879 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor formatting.

{{%- if product == 'ubuntu2404' %}}
{{%- set ssh_approved_ciphers = "aes256-gcm@openssh.com,aes128-gcm@openssh.com,aes256-ctr,aes128-ctr" %}}
{{%- endif %}}
{{% set sshc_main_config = ssh_client_main_config_file %}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{{% set sshc_main_config = ssh_client_main_config_file %}}
{{%- set sshc_main_config = ssh_client_main_config_file -%}}

@@ -1,10 +1,12 @@
documentation_complete: true
{{% set sshc_main_config = ssh_client_main_config_file %}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{{% set sshc_main_config = ssh_client_main_config_file %}}
{{%- set sshc_main_config = ssh_client_main_config_file -%}}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -1,10 +1,12 @@
documentation_complete: true
{{% set sshc_main_config = ssh_client_main_config_file %}}
{{% set sshc_config_dir = ssh_client_config_dir %}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{{% set sshc_config_dir = ssh_client_config_dir %}}
{{%- set sshc_config_dir = ssh_client_config_dir -%}}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

# platform = multi_platform_ubuntu

ssh_approved_ciphers="aes256-gcm@openssh.com,aes128-gcm@openssh.com,aes256-ctr,aes128-ctr"
{{% set sshc_cipher_list_config = ssh_client_config_dir ~ "/00-cipher-list.conf" %}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{{% set sshc_cipher_list_config = ssh_client_config_dir ~ "/00-cipher-list.conf" %}}
{{%- set sshc_cipher_list_config = ssh_client_config_dir ~ "/00-cipher-list.conf" -%}}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using {{%- ... -%}} there changes the rendered shell script structure (newlines are trimmed), which concatenates adjacent shell statements. WhenI applied it, CI started failing

@Mab879 Mab879 modified the milestone: 0.1.81 Feb 27, 2026
…e metadata

Apply whitespace-trim Jinja delimiters in SSH client rule YAML metadata where it is formatting-only and does not affect rendered remediation script behavior.

Changes:

- linux_os/guide/services/ssh/ssh_client/ssh_client_use_approved_ciphers_ordered_stig/rule.yml

- linux_os/guide/services/ssh/ssh_client/ssh_use_approved_macs_ordered_stig/rule.yml

No functional changes intended; this is a style/alignment update per review feedback.
@Smouhoune Smouhoune force-pushed the feat/ssh-client-path-overrides branch from 77b4c76 to 5892fd6 Compare February 27, 2026 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ok-to-test Used by openshift-ci bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants