Skip to content

kem: use TryCryptoRng for encapsulate#2049

Merged
tarcieri merged 1 commit intomasterfrom
kem/try-crypto-rng
Oct 26, 2025
Merged

kem: use TryCryptoRng for encapsulate#2049
tarcieri merged 1 commit intomasterfrom
kem/try-crypto-rng

Conversation

@tarcieri
Copy link
Member

The method is fallible anyway, so errors should be propagated rather than panicking

@tarcieri
Copy link
Member Author

Ugh, tried to do a downstream PR for this and it ended up being rather annoying. I'll leave this open for now but I'm a little confused about what to do about cases where we still need to pass a TryCryptoRng as a CryptoRng (unwrap_err was complaining about ?Sized bounds)

@newpavlov
Copy link
Member

newpavlov commented Oct 26, 2025

I'm a little confused about what to do about cases where we still need to pass a TryCryptoRng as a CryptoRng

Why do you need to do it in implementation crates? With this change you should be able to pass CryptoRng into encapsulate, but I am not sure why you need to do it in the other direction. Users may wrap RNG in UnwrapErr or UnwrapMut, but it should not be our concern.

@tarcieri
Copy link
Member Author

Because all of the downstream APIs use CryptoRng, including ones in x25519-dalek


/// Encapsulates a fresh shared secret
fn encapsulate<R: CryptoRng + ?Sized>(&self, rng: &mut R) -> Result<(EK, SS), Self::Error>;
fn encapsulate<R: TryCryptoRng + ?Sized>(&self, rng: &mut R) -> Result<(EK, SS), Self::Error>;
Copy link
Member

Choose a reason for hiding this comment

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

It may be worth to add a note to the method docs that users can pass types which implement CryptoRng into this method.

@newpavlov
Copy link
Member

newpavlov commented Oct 26, 2025

Because all of the downstream APIs use CryptoRng

Maybe you meant "upstream" here? So you want to use x25519-dalek which operates over CryptoRng to implement the KEM trait which we would like to migrate to TryCryptoRng, correct?

Have you tried to use UnwrapMut? Although, ideally, it would be better to update the upstream dependencies to support TryCryptoRng.

@tarcieri
Copy link
Member Author

tarcieri commented Oct 26, 2025

It’s for x-wing (a hybrid) as well as dhkem which wraps several ECC crates, but that’s more or less the idea.

I tried unwrap_err, I can try unwrap_mut I guess

The method is fallible anyway, so errors should be propagated rather
than panicking
@tarcieri
Copy link
Member Author

Okay, unwrap_mut worked and I have a PR to update everything to use the new API, so I'll go ahead and merge this

@tarcieri tarcieri merged commit 13941dc into master Oct 26, 2025
10 checks passed
@tarcieri tarcieri deleted the kem/try-crypto-rng branch October 26, 2025 19:18
tarcieri added a commit to RustCrypto/KEMs that referenced this pull request Oct 27, 2025
tarcieri added a commit to RustCrypto/KEMs that referenced this pull request Oct 27, 2025
tarcieri added a commit to RustCrypto/KEMs that referenced this pull request Oct 27, 2025
tarcieri added a commit that referenced this pull request Jan 24, 2026
Switches from `TryCryptoRng` back to `CryptoRng` for
`encapsulate_with_rng`.

We originally switched to #2049 with the rationale that the whole trait
was fallible anyway, so we might as well handle the RNG errors. But then
in #2216 we made the rest of the trait infallible, only using
fallibility for the RNG.

`Decapsulate` is also now fully infallible, but for cases where we need
to handle errors there's a `TryDecapsulate` trait. Prospectively we
could do the same thing here, and have a fallible `TryEncapsulate` trait
that uses `TryCryptoRng` and handles RNG errors. This PR doesn't attempt
to add one because it has some trait design issues around how we convert
RNG errors into KEM-specific error types.

Closes #2214
tarcieri added a commit that referenced this pull request Jan 24, 2026
Switches from `TryCryptoRng` back to `CryptoRng` for
`encapsulate_with_rng`.

We originally switched to #2049 with the rationale that the whole trait
was fallible anyway, so we might as well handle the RNG errors. But then
in #2216 we made the rest of the trait infallible, only using
fallibility for the RNG.

`Decapsulate` is also now fully infallible, but for cases where we need
to handle errors there's a `TryDecapsulate` trait. Prospectively we
could do the same thing here, and have a fallible `TryEncapsulate` trait
that uses `TryCryptoRng` and handles RNG errors. This PR doesn't attempt
to add one because it has some trait design issues around how we convert
RNG errors into KEM-specific error types.

Closes #2214 (and see also that issue for the problems around error type
conversions)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants