-
Notifications
You must be signed in to change notification settings - Fork 748
[RFE-6859] Configurable cipher suites #13866
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
[RFE-6859] Configurable cipher suites #13866
Conversation
Hi @sluetze. 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 Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
ba1ccd4
to
9b1071a
Compare
type: Custom | ||
custom: | ||
ciphers: [ {{ .var_apiserver_tls_cipher_suites }} ] | ||
minTLSVersion: VersionTLS12 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should parameterize this, too? I'm thinking about cases where someone might want to force the cluster to use PQC-safe algorithms only available through TLS 1.3.
- TLS_AES_256_GCM_SHA384 | ||
- TLS_CHACHA20_POLY1305_SHA256 | ||
ciphers: [ {{ .var_ingresscontroller_tls_cipher_suites }} ] | ||
minTLSVersion: VersionTLS12 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar comment here... Or, if we should consider a master switch for all min TLS versions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a sound, reasonable change to make. Only curious if we also want to add a parameter for the minimum TLS version since I think that could be useful for people looking for PQC-safe algorithms.
conclusionWe cannot remove the minTLSVersion option, as it is needed to apply the remediation. From my understanding the minTLSVersion should not matter for the PQC usecase. If you want to ensure PQC safe ciphers, then just provide them in the Cipher list and no others. You do not need to set minTLSVersion here, as you will not have any TLS1.2 ciphers that you offer. In my understanding the that being said, I can create variables for the minTLSVersion, but for this I would also:
rationale: risk: renaming the rules of course might break tailoredprofiles which exclude or include these rules. long versionOh no a rabbit-hole :( There are separate rules for minTLSVersion
the kubelet_configure_tls_min_version and tls_version_masters_workers check the same configuration but on different ways as far as I understand it. So they are redundant. I wanted to understand the behavior of remediations which target the same part of the config and how they would interact. So I tested this with kubelet and ingresscontroller. removing minTLSVersionthis seems to not be a good option.
kubeletI created a tailoredprofile upon the code of this branch, which checks, that only TLS1.3 is enabled AND only TLS1.3 Ciphers are used. kind: TailoredProfile
apiVersion: compliance.openshift.io/v1alpha1
metadata:
name: conflict-tailoredprofile
namespace: openshift-compliance
spec:
description: Example of a tailoredProfile that extends OCP4 FedRAMP Moderate
extends: upstream-ocp4-bsi-node
setValues:
- name: upstream-ocp4-var-kubelet-tls-cipher-suites
rationale: KUBELET 2024-01-BSI-TR-02102-2
value: '"TLS_AES_128_GCM_SHA256","TLS_AES_256_GCM_SHA384"'
- name: upstream-ocp4-var-kubelet-tls-cipher-suites-regex
rationale: KUBELET 2024-01-BSI-TR-02102-2
value: ^(TLS_AES_128_GCM_SHA256|TLS_AES_256_GCM_SHA384)$
- name: upstream-ocp4-var-kubelet-tls-min-version
rationale: CONFLICT TESTING
value: VersionTLS13
- name: upstream-ocp4-var-kubelet-tls-min-version-regex
rationale: CONFLICT TESTING
value: ^(?!VersionTLS10|VersionTLS11|VersionTLS12)
title: conflict test
---
apiVersion: compliance.openshift.io/v1alpha1
kind: ScanSettingBinding
metadata:
name: conflict-profile
namespace: openshift-compliance
profiles:
- apiGroup: compliance.openshift.io/v1alpha1
kind: TailoredProfile
name: conflict-tailoredprofile
settingsRef:
apiGroup: compliance.openshift.io/v1alpha1
kind: ScanSetting
name: default The result were of course FAILS with the following remediations (x2 for compute and control): spec:
apply: false
current:
object:
apiVersion: machineconfiguration.openshift.io/v1
kind: KubeletConfig
spec:
kubeletConfig:
tlsCipherSuites:
- TLS_AES_128_GCM_SHA256
- TLS_AES_256_GCM_SHA384 spec:
apply: false
current:
object:
apiVersion: machineconfiguration.openshift.io/v1
kind: KubeletConfig
spec:
kubeletConfig:
tlsMinVersion: VersionTLS13 I applied both of them and they are compatible with each other. In this case the fields are two separate fields in the config. Ingresscontrollerfor Ingresscontroller I tried to solve it with the TLSProfiles. so I modified the remediations in the code like: ---
# platform = multi_platform_ocp
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: default
namespace: openshift-ingress-operator
spec:
tlsSecurityProfile:
custom:
minTLSVersion: VersionTLS13
type: Custom
---
# platform = multi_platform_ocp
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: default
namespace: openshift-ingress-operator
spec:
tlsSecurityProfile:
custom:
ciphers: [ {{ .var_ingresscontroller_tls_cipher_suites }} ]
type: Custom and hardcoded the tlsminversion in create tailoredprofile which demands TLS1.3 and special ciphers kind: TailoredProfile
apiVersion: compliance.openshift.io/v1alpha1
metadata:
name: ingress-tailoredprofile
namespace: openshift-compliance
spec:
description: Example of a tailoredProfile that extends OCP4 FedRAMP Moderate
extends: upstream-ocp4-bsi
enableRules:
- name: upstream-ocp4-tls-version-check-router
rationale: need to test this!
setValues:
- name: upstream-ocp4-var-ingresscontroller-tls-cipher-suites-regex
rationale: KUBELET 2024-01-BSI-TR-02102-2
value: ^(TLS_AES_128_GCM_SHA256|TLS_AES_256_GCM_SHA384)$
- name: upstream-ocp4-var-ingresscontroller-tls-cipher-suites
rationale: KUBELET 2024-01-BSI-TR-02102-2
value: '"TLS_AES_128_GCM_SHA256","TLS_AES_256_GCM_SHA384"'
title: conflict test
---
apiVersion: compliance.openshift.io/v1alpha1
kind: ScanSettingBinding
metadata:
name: ingress-profile
namespace: openshift-compliance
profiles:
- apiGroup: compliance.openshift.io/v1alpha1
kind: TailoredProfile
name: ingress-tailoredprofile
settingsRef:
apiGroup: compliance.openshift.io/v1alpha1
kind: ScanSetting
name: default this resulted in the following remediations spec:
apply: false
current:
object:
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: default
namespace: openshift-ingress-operator
spec:
tlsSecurityProfile:
custom:
ciphers:
- TLS_AES_128_GCM_SHA256
- TLS_AES_256_GCM_SHA384
type: Custom
---
spec:
apply: false
current:
object:
apiVersion: operator.openshift.io/v1
kind: IngressController
metadata:
name: default
namespace: openshift-ingress-operator
spec:
tlsSecurityProfile:
custom:
minTLSVersion: VersionTLS13
type: Custom apply both remediations with the result in the default ingresscontroller tlsSecurityProfile:
custom:
ciphers:
- TLS_AES_128_GCM_SHA256
- TLS_AES_256_GCM_SHA384
minTLSVersion: VersionTLS13
type: Custom obviously concurrent remediations work, as long as they do not overlap. (I did not test overlapping ones, but cannot think of good results). |
/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.19-e2e-aws-ocp4-pci-dss |
/test 4.19-e2e-aws-ocp4-bsi |
/test 4.12-e2e-aws-ocp4-pci-dss Testing against some older clusters to see what those rules do with potentially different defaults. |
@sluetze: The following test failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
Description:
This PR adds the ability to configure the cipher suites for the different components in OCP
It also adds remediations / makes them configurable and fixes some minor rendering issues in the texts.
Rationale:
Customers have to comply to different Security Standards
PCI-DSS, CIS, BSI see different Ciphers as secure
There is change in what is considered secure across the time, thus these ciphers might change
Customers will need time to move from one cipher-suite-set to another
Fixes https://issues.redhat.com/browse/RFE-6859
Review Hints:
Some of the fixes need openSSL Notation, while the checks check in IANA notation. This table should help mapping them
etcd:
apiserver:
oc get apiserver cluster -oyaml
does not provide the same output asoc get configmap config -n openshift-kube-apiserver -ojson | jq -r '.data["config.yaml"]' | jq '.servingInfo'
. The format is different and also DHE-RSA-AES128-GCM-SHA256 and DHE-RSA-AES256-GCM-SHA384 are rejectedingresscontroller:
This are the result tables, of my manual tests:
While the changes to the BSI profile could be a separate PR, I think it is easier to use it directly in testing, as the pipeline will get triggered.
Examples of manually configured Tailored Profiles do look like that: