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
NE-472:Add tlsv1.3 support #617
Conversation
Simplify the logic in validateTLSSecurityProfile so that filterTLS13Ciphers is no longer needed, and delete the superfluous function definition. * pkg/operator/controller/ingress/controller.go (validateTLSSecurityProfile): Simplify TLSv1.3 ciphers check. (filterTLS13Ciphers): Delete function.
tests are working fine
Openssl to the external IP with tlsv1.3 ciphers https://github.com/openshift/api/blob/master/config/v1/types_tlssecurityprofile.go#L254-L262 A)
B)
C)
|
@Miciah PTAL |
/retest |
2 similar comments
/retest |
/retest |
164dc07
to
44e858d
Compare
/retest |
aecdf9e
to
6f4949e
Compare
test/e2e/operator_test.go
Outdated
sort.Strings(actualCiphers) | ||
sort.Strings(expectedCiphers) | ||
|
||
if !reflect.DeepEqual(actualCiphers, expectedCiphers) && !reflect.DeepEqual(intermediateProfileSpec.MinTLSVersion, ic.Status.TLSProfile.MinTLSVersion) { |
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.
if !reflect.DeepEqual(actualCiphers, expectedCiphers) && !reflect.DeepEqual(intermediateProfileSpec.MinTLSVersion, ic.Status.TLSProfile.MinTLSVersion) { | |
if !reflect.DeepEqual(actualCiphers, expectedCiphers) || !reflect.DeepEqual(intermediateProfileSpec.MinTLSVersion, ic.Status.TLSProfile.MinTLSVersion) { |
OpenShift 4.6 is built using UBI 8, which has OpenSSL 1.1.1, which supports TLSv1.3, so allow specifying spec.tlsSecurityProfile.type: Modern or spec.tlsSecurityProfile.custom.minTLSVersion: VersionTLS13. * pkg/operator/controller/ingress/controller.go tlsProfileSpecForSecurityProfile): Allow use of the "Modern" profile. (validTLSVersions): Add configv1.VersionTLS13. (validateTLSSecurityProfile): Verify that some non-TLSv1.3 cipher is specified if minTLSVersion is a version below TLSv1.3, and verify that some TLSv1.3 cipher is specified if minTLSVersion is TLSv1.3. * pkg/operator/controller/ingress/controller_test.go (TestTLSProfileSpecForSecurityProfile) (TestTLSProfileSpecForIngressController): Verify that the "Modern" profile is used if it is specified. * pkg/operator/controller/ingress/deployment.go (desiredRouterDeployment): Allow TLSv1.3. Set ROUTER_CIPHERS based on the non-TLSv1.3 ciphers in the ingresscontroller's TLS profile, and set ROUTER_CIPHERSUITES based on the TLSv1.3 ciphers. (inferTLSProfileSpecFromDeployment): Allow TLSv1.3. * pkg/operator/controller/ingress/deployment_test.go (TestDesiredRouterDeployment): Verify that SSL_MIN_VERSION is set to TLSv1.3 if TLSv1.3 was specified on the ingresscontroller. Verify that ROUTER_CIPHERSUITES is set if the TLS profile specifies any TLSv1.3 ciphers, and verify that ROUTER_CIPHERSUITES has TLSv1.3 ciphers and ROUTER_CIPHERS has other ciphers. (TestInferTLSProfileSpecFromDeployment): Add test cases for SSL_MIN_VERSION=TLSv1.2 and for SSL_MIN_VERSION=TLSv1.3.
@sgreene570 can you please review ? |
/test e2e-aws-operator |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: miheer, sgreene570 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest Please review the full test history for this PR and help us cut down flakes. |
5 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
28 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
Delete filterTLS13Ciphers and simplify
Simplify the logic in validateTLSSecurityProfile so that filterTLS13Ciphers
is no longer needed, and delete the superfluous function definition.
(validateTLSSecurityProfile): Simplify TLSv1.3 ciphers check.
(filterTLS13Ciphers): Delete function.
Allow TLSv1.3 and the "Modern" TLS profile
OpenShift 4.6 is built using UBI 8, which has OpenSSL 1.1.1, which supports
TLSv1.3, so allow specifying spec.tlsSecurityProfile.type: Modern or
spec.tlsSecurityProfile.custom.minTLSVersion: VersionTLS13.
tlsProfileSpecForSecurityProfile): Allow use of the "Modern" profile.
(validTLSVersions): Add configv1.VersionTLS13.
(validateTLSSecurityProfile): Verify that some non-TLSv1.3 cipher is
specified if minTLSVersion is a version below TLSv1.3, and verify that some
TLSv1.3 cipher is specified if minTLSVersion is TLSv1.3.
(TestTLSProfileSpecForSecurityProfile)
(TestTLSProfileSpecForIngressController): Verify that the "Modern" profile
is used if it is specified.
Allow TLSv1.3. Set ROUTER_CIPHERS based on the non-TLSv1.3 ciphers in the
ingresscontroller's TLS profile, and set ROUTER_CIPHERSUITES based on the
TLSv1.3 ciphers.
(inferTLSProfileSpecFromDeployment): Allow TLSv1.3.
(TestDesiredRouterDeployment): Verify that SSL_MIN_VERSION is set to
TLSv1.3 if TLSv1.3 was specified on the ingresscontroller. Verify that
ROUTER_CIPHERSUITES is set if the TLS profile specifies any TLSv1.3
ciphers, and verify that ROUTER_CIPHERSUITES has TLSv1.3 ciphers and
ROUTER_CIPHERS has other ciphers.
(TestInferTLSProfileSpecFromDeployment): Add test cases for
SSL_MIN_VERSION=TLSv1.2 and for SSL_MIN_VERSION=TLSv1.3.