Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[TLS] Don't set commonName and only use subjectAltNames #376

Merged
merged 1 commit into from
Oct 23, 2023
Merged

[TLS] Don't set commonName and only use subjectAltNames #376

merged 1 commit into from
Oct 23, 2023

Conversation

stuggi
Copy link
Contributor

@stuggi stuggi commented Oct 23, 2023 •
edited
Loading

commonName has a max length of 64 chars, which easy can be reached in route DNS names scheme.

From [1]

RFC 2818, published in May 2000, deprecates the use of the Common Name (CN) field in HTTPS
certificates for subject name verification. Instead, it recommends using the
 “Subject Alternative Name” extension (SAN) of the “dns name” type.

Lets not set the commonName an just use the subjectAltNames for our certs.

[1] https://cloud.redhat.com/blog/details-on-https-common-name-deprecation-in-openshift-4.10

@stuggi
[TLS] Don't set commonName and only use subjectAltNames
0ef7e5f 
commonName has a max length of 64 chars, which easy can be reached
in route DNS names scheme.

From [1]
~~~
RFC 2818, published in May 2000, deprecates the use of the Common
Name (CN) field in HTTPS certificates for subject name verification.
Instead, it recommends using the “Subject Alternative Name” extension
(SAN) of the “dns name” type.
~~~

Lets not set the commonName an just use the subjectAltNames for our
certs.

[1] https://cloud.redhat.com/blog/details-on-https-common-name-deprecation-in-openshift-4.10
@stuggi stuggi requested review from olliewalsh and Deydra71 October 23, 2023 08:54
@stuggi
Copy link
Contributor Author

stuggi commented Oct 23, 2023 •
edited
Loading

I hit the max length issue for the commonName in kuttl tests for tls public endpoints:

{"level":"error","ts":"2023-10-23T06:26:34.988Z","msg":"Reconciler error","controller":"openstackcontrolplane","controllerGroup":"core.openstack.org",
"controllerKind":"OpenStackControlPlane","OpenStackControlPlane":
{"name":"openstack-basic","namespace":"openstack-kuttl-tests"},"namespace":"openstack-kuttl-tests",
"name":"openstack-basic","reconcileID":"cdded1d9-9b56-42cb-9c26-2affdf10b0ae",
"error":"admission webhook \"webhook.cert-manager.io\" denied the request: spec.commonName: Too long: must have at most 64 bytes"...

Where the route hostname was 70chars

@stuggi stuggi requested a review from vakwetu October 23, 2023 08:57
olliewalsh
olliewalsh approved these changes Oct 23, 2023
View reviewed changes
Copy link
Contributor

@olliewalsh olliewalsh left a comment

Choose a reason for hiding this comment

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

/lgtm

@stuggi stuggi merged commit 357f2fa into openstack-k8s-operators:main Oct 23, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@olliewalsh olliewalsh olliewalsh approved these changes

@Deydra71 Deydra71 Awaiting requested review from Deydra71

@vakwetu vakwetu Awaiting requested review from vakwetu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@stuggi @olliewalsh