This repository was archived by the owner on Nov 24, 2025. It is now read-only.
CLOSED - Fix add sslkeys endpoint to always use the input certificate#3417
Closed
rawlinp wants to merge 1 commit intoapache:masterfrom
Closed
CLOSED - Fix add sslkeys endpoint to always use the input certificate#3417rawlinp wants to merge 1 commit intoapache:masterfrom
rawlinp wants to merge 1 commit intoapache:masterfrom
Conversation
Sometimes, the Certificate.Verify method will remove elements from a valid certificate chain. Instead of using the output of that method, always use the user input certificate and return a warning if the input certificate does not match the output of the Certificate.Verify method. Fixes apache#3398
rawlinp
added
bug
Contributor
|
Refer to this link for build results (access rights to CI server needed): |
Contributor
Author
|
This PR might be getting rolled into #3382 since that PR touches the same code. |
ghost
mentioned this pull request
rawlinp
pushed a commit
that referenced
this pull request
Apr 3, 2019
…ps (AddSSLKeys Endpoint) (#3382) * Fix add sslkeys endpoint to always use the input certificate Sometimes, the Certificate.Verify method will remove elements from a valid certificate chain. Instead of using the output of that method, always use the user input certificate and return a warning if the input certificate does not match the output of the Certificate.Verify method. Fixes #3398 (cherry picked from commit dde742e) * Combine PR #3382 with PR #3417 and add ECDSA support for DNS delivery services * Update unit tests to fail faster * Update comments related to RSA/ECDSA key mismatch verification * Make decoding ECDSA PEM blocks more readable * Add Unit tests for ECDSA privateKey decoding with and without param pem block * Rename Unit test methods. Add unit test for encrypted ECDSA private key * Add unit test to verify DSA signed x509 certificates are rejected * Gofmt * Fix ECDSA unit tests Add ECDSA mismatched cert/key unit test Update error messages to be more meaningful * Add more unit tests related to critical x509 cert/key validation Missing RSA keyEncipherment unit test Missing ECDSA digitalSignature unit test Missing serverAuth extendedKeyUsage (x509v3 only) unit test * Add self-signed x509v1 server certificate unit test. * Remove redundant error message header. * Revert change, gofmt * Use getDSType() to permit ECDSA keys for only DNS* DS types. * Update tc.DSType.IsDNS() method to only include DNS DS types. * Use updated tc.DSType.isDNS() method to determine if DS is a DNS type. * Check errors returned from rsa/ecdsa private key decoding methods Minor changes to error message formatting and string comparison Re-sort go import statements
Contributor
Author
|
This has been addressed in #3382 |
mitchell852
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue is fixed by this PR? If not related to an existing issue, what does this PR do?
Sometimes, the Certificate.Verify method will remove elements from a
valid certificate chain. Instead of using the output of that method,
always use the user input certificate and return a warning if the input
certificate does not match the output of the Certificate.Verify method.
Fixes #3398
Which TC components are affected by this PR?
What is the best way to verify this PR? Please include manual steps or automated tests.
(If no tests are part of this PR, please provide explanation as to why no tests are included.)
Includes changes to the unit tests, but there is currently an unrelated unit test failure in statuses_test.go which should be addressed in a separate PR:
Manual verification can be done by adding an sslkey to a delivery service that would normally get mangled by the API and checking that the stored cert matches the input cert.
If you don't have a cert like that on hand, you can still check by adding a non-self-signed cert and verifying that the stored cert matches the input cert.
Check all that apply