Skip to content

feat(x25519-dalek): add PKCS#8 and PEM private key support #847

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
Open
imlk0 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat(x25519-dalek): add PKCS#8 and PEM private key support #847

imlk0 wants to merge 2 commits into from

Conversation

@imlk0
Copy link

@imlk0 imlk0 commented Nov 11, 2025

Introduce support for encoding and decoding X25519 private keys using the PKCS#8 standard (both DER and PEM formats). This enables interoperability with OpenSSL, TLS stacks, and other crypto systems.

Changes:

  • Add pkcs8 crate as optional dependency
  • Implement EncodePrivateKey and TryFrom<PrivateKeyInfoRef>
  • Handle nested OCTET STRING structure required by RFC
  • Add tests for DER/PEM encoding round-trips
  • Wire up pem feature behind feature gate

@imlk0
Copy link
Author

imlk0 commented Nov 11, 2025

Hi @tarcieri. This is a implementation for #846. I've written tests, however it seems the current CI setup doesn't seem to test all feature combinations (especially pkcs8 and pem). As a result, these new tests may not be triggered in the CI pipeline. Do you have any advice on how to proceed? Any advice would be welcome!

@imlk0
feat(x25519-dalek): add PKCS#8 and PEM private key support
9aca24a 
Introduce support for encoding and decoding X25519 private keys
using the PKCS#8 standard (both DER and PEM formats). This enables
interoperability with OpenSSL, TLS stacks, and other crypto systems.

Changes:
- Add `pkcs8` crate as optional dependency
- Implement `EncodePrivateKey` and `TryFrom<PrivateKeyInfoRef>`
- Handle nested OCTET STRING structure required by RFC
- Add tests for DER/PEM encoding round-trips
- Wire up `pem` feature behind feature gate

Signed-off-by: Kun Lai <laikun@linux.alibaba.com>
@imlk0
fix(x25519-dalek): make clippy happy
b531fa5 
Signed-off-by: Kun Lai <laikun@linux.alibaba.com>
@imlk0
Copy link
Author

imlk0 commented Nov 11, 2025

Just pushed a fix to keep Clippy happy. Thanks for catching that! @tarcieri

@rozbb
Copy link
Contributor

rozbb commented Dec 2, 2025

Thank you! Just two things:

  1. Can you explain how you generated the PEM/DER test vectors and put them in a comment?
  2. Maybe relatedly, why precisely is the public_key field in a PrivateKeyRef struct encoded as an ASN.1 BitStringRef? Why not an octet string, for example? Is this a standard somewhere?

@tarcieri
Copy link
Contributor

tarcieri commented Dec 2, 2025

It’s standard PKCS#8, more specifically https://datatracker.ietf.org/doc/html/rfc5958

@rozbb
Copy link
Contributor

rozbb commented Dec 2, 2025 •
edited
Loading

Ah, I see the answer to question 2 is that it's specified in RFC 8410 §7. Could you write that in a comment somewhere?

It looks like the PEM is not from the RFC, though. Is that right? Could you change the PEM to the PEM found in the RFC? Also make the DER equal to the translated PEM (and write the command you used for the translation)?

@tarcieri
Copy link
Contributor

tarcieri commented Dec 27, 2025

@imlk0 can you fix the conflicts?

pinkforest
pinkforest reviewed Jan 25, 2026
View reviewed changes
x25519-dalek/src/x25519.rs

#[cfg(all(feature = "alloc", feature = "pkcs8"))]
impl EncodePrivateKey for EphemeralSecret {
fn to_pkcs8_der(&self) -> Result<SecretDocument, pkcs8::Error> {
Copy link
Contributor

@pinkforest pinkforest Jan 25, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

This should not be exported out as it violates perfect forward secrecy.

We made a point not to add any impls that allows serialization out.

pinkforest
pinkforest reviewed Jan 25, 2026
View reviewed changes
x25519-dalek/src/x25519.rs
impl TryFrom<PrivateKeyInfoRef<'_>> for EphemeralSecret {
type Error = pkcs8::Error;

fn try_from(private_key: PrivateKeyInfoRef<'_>) -> Result<Self, pkcs8::Error> {
Copy link
Contributor

@pinkforest pinkforest Jan 25, 2026

Choose a reason for hiding this comment

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

EphemeralSecret is supposed to be random generated always.

This should not be extended to it.

@pinkforest
Copy link
Contributor

pinkforest commented Jan 25, 2026 •
edited
Loading

EphemeralSecret purpose is to ensure no serialization out to guarantee perfect forward secrecy.

Adding any type of serialization to it will break this property and also breaks the property of ensuring randomized keys for it.

It's fine for StaticSecret but it shouldn't be for Ephemeral.

See documentation for EphemeralSecret:

This type is identical to the StaticSecret type, except that the EphemeralSecret::diffie_hellman method consumes and then wipes the secret key, and there are no serialization methods defined. This means that EphemeralSecrets can only be generated from fresh randomness where the compiler statically checks that the resulting secret is used at most once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tarcieri tarcieri tarcieri approved these changes

+1 more reviewer

@pinkforest pinkforest pinkforest left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@imlk0 @rozbb @tarcieri @pinkforest