fix: support RSA and fix ed25519 key encoding in EncodePrivateKey#148
Merged
fix: support RSA and fix ed25519 key encoding in EncodePrivateKey#148
Conversation
EncodePrivateKey only handled *ecdsa.PrivateKey and ed25519.PrivateKey, rejecting RSA keys with "unsupported key type". Since RSA keys also use x509.MarshalPKCS8PrivateKey with the same PKCS#8 block type, add *rsa.PrivateKey to the existing case list. Adds table-driven tests covering RSA-2048, RSA-4096, ECDSA P-256, Ed25519, and unsupported type with roundtrip verification through DecodePEMPrivateKey. Signed-off-by: Nelson Parente <nelson_parente@live.com.pt>
nelson-parente
force-pushed
the
fix/pem-encode-rsa-key
branch
from
3f6ea43 to
bf1cd3c
Compare
JoshVanL
requested changes
Mar 18, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the crypto/pem helper to support encoding RSA private keys to PEM (PKCS#8), and adds unit tests to validate encode/decode roundtrips across supported key types.
Changes:
- Add
*rsa.PrivateKeysupport toEncodePrivateKeyby marshaling RSA keys as PKCS#8. - Add
pem_test.gocovering ECDSA, RSA, Ed25519 encode/decode roundtrips and an unsupported-type error case.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crypto/pem/pem.go | Expands EncodePrivateKey to accept *rsa.PrivateKey and marshal it as PKCS#8 PEM. |
| crypto/pem/pem_test.go | Adds tests for supported key types and error behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
- Drop RSA-4096 test case to avoid slow CI (2048 is sufficient coverage) - Replace custom containsSubstr/searchSubstr with strings.Contains - Remove unused helper functions Signed-off-by: Nelson Parente <nelson_parente@live.com.pt>
JoshVanL
approved these changes
Mar 18, 2026
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
EncodePrivateKeyincrypto/pemonly handled*ecdsa.PrivateKeyand*ed25519.PrivateKey, rejecting RSA keys with"unsupported key type". Since all three key types usex509.MarshalPKCS8PrivateKeywith the same PKCS#8 block type, this adds*rsa.PrivateKeyto the existing case list.Also fixes a pre-existing bug:
*ed25519.PrivateKey(pointer) should beed25519.PrivateKey(value type, sinceed25519.PrivateKeyis[]byteanded25519.GenerateKeyreturns a value). The pointer form never matched at runtime.Changes
crypto/pem/pem.go*rsa.PrivateKeyto type switch, fixed25519.PrivateKeyto value typecrypto/pem/pem_test.goTest plan
DecodePEMPrivateKeySigned-off-by: Nelson Parente nelson_parente@live.com.pt