fix: handle EC parameters before TLS private keys#9338
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 886f3e1be0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9338 +/- ##
==========================================
+ Coverage 75.55% 75.60% +0.04%
==========================================
Files 252 252
Lines 41742 41755 +13
==========================================
+ Hits 31539 31568 +29
+ Misses 8072 8058 -14
+ Partials 2131 2129 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: Jules Dutel <jules.dutel@numberly.com>
Signed-off-by: Jules Dutel <jules.dutel@numberly.com>
74f33fc to
5a71558
Compare
|
Overall, this looks good. Hi @jvlxz could you please fix DCO check by signing off the commits and resolve the merge conflicts? Thanks! |
| // firstSupportedPrivateKeyBlock returns the first private key PEM block supported by the parsers below: | ||
| // PKCS1, PKCS8, or EC. If none is found, it returns the first block so the caller can report its unsupported format. | ||
| func firstSupportedPrivateKeyBlock(data []byte) *pem.Block { |
There was a problem hiding this comment.
Nit, for clarity:
| // firstSupportedPrivateKeyBlock returns the first private key PEM block supported by the parsers below: | |
| // PKCS1, PKCS8, or EC. If none is found, it returns the first block so the caller can report its unsupported format. | |
| func firstSupportedPrivateKeyBlock(data []byte) *pem.Block { | |
| // privateKeyBlock returns the private key PEM block from the given data, skipping | |
| // any preceding non-key blocks such as the "EC PARAMETERS" block that tools like | |
| // OpenSSL emit ahead of an "EC PRIVATE KEY". | |
| // | |
| // If no supported private key block is found, it falls back to returning the first | |
| // block of any type (or nil if the data contains no PEM blocks at all) so the caller | |
| // can report a meaningful error about the unexpected block type rather than a generic | |
| // decode failure. | |
| func privateKeyBlock(data []byte) *pem.Block { |
Signed-off-by: Jules Dutel <jules.dutel@gmail.com>
b635ce8 to
1138cd6
Compare
Co-authored-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: jvlxz <jules.dutel@gmail.com> Signed-off-by: Jules Dutel <jules.dutel@gmail.com>
1138cd6 to
98af8a3
Compare
# Conflicts: # internal/gatewayapi/tls.go
What type of PR is this?
fix
What this PR does / why we need it:
This PR fixes TLS Secret parsing when an ECDSA private key contains an
EC PARAMETERSPEM block before the actual private key block.Some tools, including OpenSSL workflows, can produce
tls.keydata shaped like:Envoy Gateway previously decoded only the first PEM block from
tls.key. When that first block wasEC PARAMETERS, the Secret was rejected becauseEC PARAMETERSis not a private key format. That caused the Gateway listener to report an invalid certificate reference even though the same Secret also contained a validEC PRIVATE KEYblock.The fix changes TLS key parsing to scan through PEM blocks and use the first supported private key block:
PRIVATE KEY,RSA PRIVATE KEY, orEC PRIVATE KEY. If no supported private key block exists, Envoy Gateway still returns the existing validation error path using the first decoded PEM block type, so invalid Secrets continue to fail clearly.This keeps compatibility with existing PKCS1, PKCS8, and EC private keys while accepting EC keys that include leading EC parameters.
Tests:
go test ./internal/gatewayapitls.keystarts withEC PARAMETERSWhich issue(s) this PR fixes:
Fixes #
Release Notes: No