CMP 2196 update ingress operator ciphers #12220
Conversation
|
Start a new ephemeral environment with changes proposed in this pull request: ocp4 (from CTF) Environment (using Fedora as testing environment) Fedora Testing Environment Oracle Linux 8 Environment |
|
🤖 A k8s content image for this PR is available at: Click here to see how to deploy itIf you alread have Compliance Operator deployed: Otherwise deploy the content and operator together by checking out ComplianceAsCode/compliance-operator and: |
Yes - good catch. I can update. |
applications/openshift/kubelet/kubelet_configure_tls_cipher_suites_ingresscontroller/rule.yml
Outdated
Show resolved
Hide resolved
rhmdnd
force-pushed
the
CMP-2196-update-ingress-operator-ciphers
branch
from
5ac8611
to
dc7c6ba
Compare
|
This datastream diff is auto generated by the check Click here to see the full diffNew content has different text for rule 'xccdf_org.ssgproject.content_rule_kubelet_configure_tls_cipher_suites_ingresscontroller'.
--- xccdf_org.ssgproject.content_rule_kubelet_configure_tls_cipher_suites_ingresscontroller
+++ xccdf_org.ssgproject.content_rule_kubelet_configure_tls_cipher_suites_ingresscontroller
@@ -10,7 +10,7 @@
Therefore, you need to use a tool that can query the OCP API, retrieve the /apis/operator.openshift.io/v1/namespaces/openshift-ingress-operator/ingresscontrollers/default API endpoint to the local /apis/operator.openshift.io/v1/namespaces/openshift-ingress-operator/ingresscontrollers/default file.
[reference]:
-4.2.13
+4.2.12
[rationale]:
TLS ciphers have had a number of known vulnerabilities and weaknesses,
OCIL for rule 'xccdf_org.ssgproject.content_rule_kubelet_configure_tls_cipher_suites_ingresscontroller' differs.
--- ocil:ssg-kubelet_configure_tls_cipher_suites_ingresscontroller_ocil:questionnaire:1
+++ ocil:ssg-kubelet_configure_tls_cipher_suites_ingresscontroller_ocil:questionnaire:1
@@ -1,4 +1,4 @@
Run the following command on the kubelet nodes(s):
-oc -n openshift-ingress-operator patch ingresscontroller/default --type merge -p '{"spec":{"tlsSecurityProfile":{"type":"Custom","custom":{"ciphers":["ECDHE-ECDSA-AES128-GCM-SHA256","ECDHE-RSA-AES128-GCM-SHA256","ECDHE-RSA-AES256-GCM-SHA384"],"minTLSVersion":"VersionTLS12"} } } }'
- Is it the case that TLS cipher suite configuration is not configured?
+oc -n openshift-ingress-operator patch ingresscontroller/default --type merge -p '{"spec":{"tlsSecurityProfile":{"type":"Custom","custom":{"ciphers":["ECDHE-ECDSA-AES128-GCM-SHA256","ECDHE-ECDSA-CHACHA20-POLY1305","ECDHE-ECDSA-AES256-GCM-SHA384","TLS_CHACHA20_POLY1305_SHA256","TLS_AES_128_GCM_SHA256","TLS_AES_256_GCM_SHA384","ECDHE-RSA-AES128-GCM-SHA256","ECDHE-RSA-AES256-GCM-SHA384","ECDHE-RSA-CHACHA20-POLY1305"],"minTLSVersion":"VersionTLS12"} } } }'
+ Is it the case that Ingress controller TLS cipher suite configuration is incomplete or possibly insecure?
kubernetes remediation for rule 'xccdf_org.ssgproject.content_rule_kubelet_configure_tls_cipher_suites_ingresscontroller' differs.
--- xccdf_org.ssgproject.content_rule_kubelet_configure_tls_cipher_suites_ingresscontroller
+++ xccdf_org.ssgproject.content_rule_kubelet_configure_tls_cipher_suites_ingresscontroller
@@ -10,6 +10,12 @@
ciphers:
- ECDHE-ECDSA-AES128-GCM-SHA256
- ECDHE-RSA-AES128-GCM-SHA256
+ - ECDHE-ECDSA-CHACHA20-POLY1305
- ECDHE-RSA-AES256-GCM-SHA384
+ - ECDHE-RSA-CHACHA20-POLY1305
+ - ECDHE-ECDSA-AES256-GCM-SHA384
+ - TLS_AES_128_GCM_SHA256
+ - TLS_AES_256_GCM_SHA384
+ - TLS_CHACHA20_POLY1305_SHA256
minTLSVersion: VersionTLS12
type: Custom |
|
/hold for test |
|
@rhmdnd According to https://access.redhat.com/articles/5348961, TLS_AES_128_CCM_SHA256 is also a new cipher suite new in TLS 1.3. Could you please double check we need to add it or not? Thanks. |
|
@rhmdnd Another issue is: seems the autoremediation cr will be in error status and will always fail due to permission issue: Details seen from below: |
Interesting - we proposed a separate permission fix to the operator and it landed recently. ComplianceAsCode/compliance-operator#558 Were you able to recreate with that fix? |
No. Sorry. Let me use master branch to verify it again. |
|
@rhmdnd I tried catalogsource ghcr.io/complianceascode/compliance-operator-catalog:558 and ghcr.io/complianceascode/compliance-operator-catalog:573, I still got the same error. Could you please help to double check? Thanks. |
Strange - I wonder if the catalog source it's updating. I tried with commit ff6c7de96c1a7f38ac37243640703858d035b7cc on https://github.com/ComplianceAsCode/compliance-operator using I was able to use the following binding: apiVersion: compliance.openshift.io/v1alpha1
kind: ScanSettingBinding
metadata:
name: upstream-cis
namespace: openshift-compliance
profiles:
- name: upstream-ocp4-cis
kind: Profile
apiGroup: compliance.openshift.io/v1alpha1
- name: upstream-ocp4-cis-node
kind: Profile
apiGroup: compliance.openshift.io/v1alpha1
settingsRef:
name: default
kind: ScanSetting
apiGroup: compliance.openshift.io/v1alpha1And then I applied the remediation through the Compliance Operator by setting I was also able to verify this with the deploy technique mentioned in the comment above, using ff6c7de96c1a7f38ac37243640703858d035b7cc from ComplianceAsCode/compliance-operator. Do either of those techniques work for you? |
|
Sorry. Seems the PR image issue. |
|
Then #12220 (comment) this comment will be the only question left. |
rhmdnd
force-pushed
the
CMP-2196-update-ingress-operator-ciphers
branch
from
2186746
to
b370c11
Compare
Good catch - fixed. |
|
/test |
|
@rhmdnd: The
Use
In response to this:
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. |
|
/test 4.12-e2e-aws-ocp4-cis |
I can remove it. I found it in https://ciphersuite.info/cs/TLS_AES_128_CCM_SHA256/ which says it's secure, not recommended. We can add it to the rule later if needed, but the other updates we've made are useful on their own and align better than what we have today. |
rhmdnd
force-pushed
the
CMP-2196-update-ingress-operator-ciphers
branch
from
b370c11
to
3f447d3
Compare
|
/test 4.12-e2e-aws-ocp4-cis |
|
Still debugging some of the issues with the remediation. Looks like the e2e tests are pointing out a legitimate issue. |
The following ciphers are all supported with TLS v1.3, but we weren't checking for them in the OpenShift ingress controller configuration: - TLS_AES_128_GCM_SHA256 - TLS_AES_256_GCM_SHA384 - TLS_CHACHA20_POLY1305_SHA256 This commit updates the regular expression in the rule to check for those ciphers so the check doesn't fail if OpenShift is using them. It also add some formatting to the rule so it's consistent with other TLS-related rules, like for the API server. The following ciphers were listed in the "old" profile, or insecure, which should only be used as a last resort for server TLS configuration: - AES128-GCM-SHA256 - AES256-GCM-SHA384 - DHE-RSA-AES128-GCM-SHA256 - DHE-RSA-AES256-GCM-SHA384 This commit removes them from the ingress controller rule so that it fails if a cluster is using these ciphers. References: - https://wiki.mozilla.org/Security/Server_Side_TLS - https://docs.openssl.org/1.1.1/man1/ciphers/
Since we're updating the recommended OCIL, we can also update the remediation shipped with the content so that it matches. This will allow users to apply a remediation that updates their TLS ciphers so their either Recommended or Secure. This commit has a dependency on a permission change to the operator cluster role so that it can actually apply the remediation at runtime: ComplianceAsCode/compliance-operator#558
This rule was originally written for CIS benchmarks, but somewhere along the way it was refactored out. This could have been due to a re-indexing of the controls from the benchmark. This commit adds the rule back into the CIS profiles so that it's run with all supports CIS benchmarks. We should be able to prevent against regressions by including it to the e2e rule assertion files.
rhmdnd
force-pushed
the
CMP-2196-update-ingress-operator-ciphers
branch
from
3f447d3
to
3c24d28
Compare
|
/test 4.12-e2e-aws-ocp4-cis |
|
Code Climate has analyzed commit 3c24d28 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 59.4% (0.0% change). View more on Code Climate. |
|
/test 4.12-e2e-aws-ocp4-cis Underlying cluster issues unrelated to this change. |
applications/openshift/kubelet/kubelet_configure_tls_cipher_suites_ingresscontroller/rule.yml
Show resolved
Hide resolved
|
The remediations applied through the test suite does cause the cluster operators to update. Checking to see if waiting for all operator to come back as stable affects this PR. |
|
Verified with two scenarios: by default rule pass and by default rule fail: % oc -n openshift-ingress-operator patch ingresscontroller/default --type merge -p '{"spec":{"tlsSecurityProfile":{"type":"Modern"}}}'
% oc -n openshift-ingress-operator patch ingresscontroller/default --type merge -p '{"spec":{"tlsSecurityProfile":{"type":"Intermediate"}}}'
|
|
I am now suspecting this is failing because some manual remediations |
|
The rule is passing when using |
|
/test 4.15-e2e-aws-ocp4-cis |
|
/test 4.12-e2e-aws-ocp4-cis |
|
/test e2e-aws-ocp4-cis |
|
/test 4.13-e2e-aws-ocp4-cis Cluster degraded during install |
|
/lgtm |
We recently incorporated a new rule into the CIS profile that checks ingress controller TLS configs: ComplianceAsCode#12220 We added it to the CIS profile, but didn't update the assertions in the moderate or high profiles, which is causing periodic CI to fail. This commit adds the assertion to the moderate and high test files so we're checking it in subsequent CI runs.
We recently incorporated a new rule into the CIS profile that checks ingress controller TLS configs: ComplianceAsCode#12220 We added it to the CIS profile, but didn't update the assertions in the moderate or high profiles, which is causing periodic CI to fail. This commit adds the assertion to the moderate and high test files so we're checking it in subsequent CI runs.
We recently incorporated a new rule into the CIS profile that checks ingress controller TLS configs: ComplianceAsCode#12220 We added it to the CIS profile, but didn't update the assertions in the moderate or high profiles, which is causing periodic CI to fail. This commit adds the assertion to the moderate and high test files so we're checking it in subsequent CI runs.
The following ciphers are all supported with TLS v1.3, but we weren't
checking for them in the OpenShift ingress controller configuration:
This commit updates the regular expression in the rule to check for
those ciphers so the check doesn't fail if OpenShift is using them.
It also add some formatting to the rule so it's consistent with other
TLS-related rules, like for the API server.
The following ciphers were listed in the "old" profile, or insecure, which should
only be used as a last resort for server TLS configuration:
This commit removes them from the ingress controller rule so that it
fails if a cluster is using these ciphers.
References:
Hints for reviewers - you'll need to make a tailored profile to test this rule since it's not included in any profiles by default.