-
Notifications
You must be signed in to change notification settings - Fork 82
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.
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
Supporting CtOption::map
for non-Default
types?
#63
Comments
Hmm, regardless of the |
One thing that might work to relax the |
If #94 were addressed which would unlock using combinators like Unconditionally applying the closure to It's also problematic to use |
`ConstantTimeSelect` is intended as a replacement to `ConditionallySelectable`, which is preserved but deprecated. It replaces the previous `Copy` bound with a bound on a new `ConstantTimeClone` marker trait, which allows the trait to be impl'd for heap-allocated types. No existing impls of `ConditionallySelectable` have been removed, however a blanket impl of `ConstantTimeSelect` for `T: ConditionallySelectable` has been added, allowing the two traits to interoperate and for `ConstantTimeSelect` to work on all types which currently impl `ConditionallySelectable`. `ConstantTimeClone` likewise has a blanket impl for all types which impl `Copy`. `CtOption`'s combinator methods have been changed to bound on `ConstantTimeSelect` which unlocks using them with heap-allocated types, which otherwise is a major limitation. In theory these changes are all backwards compatible due to the blanket impl, which should allow all types which previously worked to continue to do so. Closes dalek-cryptography#63, dalek-cryptography#94
`ConstantTimeSelect` is intended as a replacement to `ConditionallySelectable`, which is preserved but deprecated. It replaces the previous `Copy` bound with a bound on a new `ConstantTimeClone` marker trait, which allows the trait to be impl'd for heap-allocated types. No existing impls of `ConditionallySelectable` have been removed, however a blanket impl of `ConstantTimeSelect` for `T: ConditionallySelectable` has been added, allowing the two traits to interoperate and for `ConstantTimeSelect` to work on all types which currently impl `ConditionallySelectable`. `ConstantTimeClone` likewise has a blanket impl for all types which impl `Copy`. `CtOption`'s combinator methods have been changed to bound on `ConstantTimeSelect` which unlocks using them with heap-allocated types, which otherwise is a major limitation. In theory these changes are all backwards compatible due to the blanket impl, which should allow all types which previously worked to continue to do so. Closes dalek-cryptography#63, dalek-cryptography#94
I have proposed removing this bound in #118, although I worry it's a breaking change. An non-breaking alternative would be to add new methods which do the same thing but don't have the bound. |
I'm going to add my voice here that |
This commit contains a minimal set of proposed breaking changes, as an alternative to trying to make additive changes which may be "subtly" breaking: - Add `ConstantTimeClone` trait and use it as `ConditionallySelectable`'s supertrait bound (alternative to dalek-cryptography#118): this makes it possible to implement `ConditionallySelectable` on heap-allocated types (fixes dalek-cryptography#94) - Remove `Default` bound on `CtOption::map` and `CtOption::and_then`: allows using these methods with types that don't impl `Default` and avoids potential timing sidechannels if there might be timing variability in `Default` types (fixes dalek-cryptography#63)
For now,
CtOption::map
requires aDefault
type.This is especially problematic for types which require some constant-time validation of the input in their public constructors, say with the following API.
In that case, exposing a
Default
forT
would allow the caller to bypassfrom_bytes
validation by creating aDefault
.Could there be a variant of
map
with an explicitly provided dummy value? Same question forCtOption::and_then
.The text was updated successfully, but these errors were encountered: