-
Notifications
You must be signed in to change notification settings - Fork 118
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
[release-4.15] OCPBUGS-49392: Block Upgrades for CA-Signed Certs Using SHA1 #641
base: release-4.15
Are you sure you want to change the base?
Conversation
Skipping CI for Draft Pull Request. |
@gcs278: This pull request references Jira Issue OCPBUGS-45290, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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 openshift-eng/jira-lifecycle-plugin repository. |
6710e76
to
9e9d1fb
Compare
@gcs278: This pull request references Jira Issue OCPBUGS-45290, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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 openshift-eng/jira-lifecycle-plugin repository. |
9e9d1fb
to
027c83e
Compare
027c83e
to
bc6739c
Compare
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
95000e6
to
558fcce
Compare
@gcs278: No Jira issue is referenced in the title of this pull request. 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 openshift-eng/jira-lifecycle-plugin repository. |
@gcs278: This pull request references Jira Issue OCPBUGS-49392, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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 openshift-eng/jira-lifecycle-plugin repository. |
/assign |
Previously, upgrades from 4.15 to 4.16 were only blocked for leaf certs using SHA1. However, in 4.16, any SHA1 cert that is CA-signed (not self-signed) is unsupported. As a result, we were incorrectly allowing upgrades for SHA1 intermediate certificates, while blocking upgrades for self-signed SHA1 leaf certificates. This update refactors the upgrade validation plugin to address these issues by checking all certificates (server, CA, destinationCA) for SHA1 while excluding self-signed certificates from the validation.
558fcce
to
41db25b
Compare
@Miciah made me realize that we shouldn't be adding DSA SHA1 to our upgrade blocker for the router because DSA is already rejected by the lack of key parse logic for DSA. Thread with more details. Since DSA SHA1 is already rejected in 4.15, we shouldn't be blocking upgrades for it as that may cause unnecessary for users that have lingering rejected DSA SHA1 routes (side note: I realize the extended validation does indeed come before the upgrade validation, so DSA SHA1 routes wouldn't block upgrades anyways, but better to be more precise and less confusing I suppose). |
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.
switch cert.SignatureAlgorithm { | ||
case x509.SHA1WithRSA, x509.ECDSAWithSHA1: | ||
sha1UnsupportedMsg := "OpenShift 4.16 does not support CA-signed certificates using SHA1 signature algorithms. This route " + | ||
"will be rejected in OpenShift 4.16. To maintain functionality in OpenShift 4.16, generate a new certificate " + |
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.
Should there be a suggesting ref link on how to generate a new cert maybe?
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.
Yea it's a good thought. My personal opinion, I think generating a cert is common procedure widely known enough to omit a specific link to reference. I don't disagree it might be useful to someone, but I think not useful enough to make the error more wordy.
�[36mINFO�[0m[2025-01-30T22:28:10Z] Running step e2e-aws-serial-openshift-e2e-test. It took 30 minutes to acquire an AWS Lease. /test e2e-aws-serial |
@gcs278: all tests passed! 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. |
/jira refresh |
@Miciah: This pull request references Jira Issue OCPBUGS-49392, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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 openshift-eng/jira-lifecycle-plugin repository. |
Previously, upgrades from 4.15 to 4.16 were only blocked for leaf certs using SHA1. However, in 4.16, any SHA1 cert that is CA-signed (not self-signed) is unsupported. As a result, we were incorrectly allowing upgrades for SHA1 intermediate certificates, while blocking upgrades for self-signed SHA1 leaf certificates.
This update refactors the upgrade validation plugin to address these issues by checking all certificates (server, CA, destinationCA) for SHA1 while excluding self-signed certificates from the validation.