Skip to content
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

signature: Keypair design problems #1124

Closed
tarcieri opened this issue Sep 19, 2022 · 3 comments
Closed

signature: Keypair design problems #1124

tarcieri opened this issue Sep 19, 2022 · 3 comments
Labels
signature Digital signature crate

Comments

@tarcieri
Copy link
Member

The Keypair crate probably should've received more design work prior to its initial release. This is a tracking and discussion issue for problems around the design.

#1107 relaxed the trait bounds in a mostly-compatible way. However, it remains generic around S: Signature in a way which doesn't actually use the S type parameter, which is a bit odd.

The Keypair trait mandates an AsRef<Self::VerifyingKey> bound. This bound is a bit odd in that it's Self-referential which limits generic impls (e.g. it's not possible to impl AsRef generically). It also makes it difficult to define newtypes for keys, as the inner keypair types might use a verifying key type which isn't wrapped in the newtype, and can only be promoted to a reference of the newtype using an unsafe cast. See RustCrypto/RSA#190 as an example.

A possible alternative would allow computing the verifying key from a signing key without actually storing it in the same struct, e.g. using a type VerifyingKey: for<'a> From<&'a Self> bound rather than AsRef<Self::VerifyingKey> bound.
@tarcieri tarcieri added the signature Digital signature crate label Sep 19, 2022
@tarcieri tarcieri mentioned this issue Sep 19, 2022
Closed
@lumag
Copy link
Contributor

lumag commented Sep 22, 2022

Yes, AsRef sounds like a more flexible solution

@tarcieri
Copy link
Member Author

tarcieri commented Sep 24, 2022 •
edited
Loading

Err, do you mean From?

It has the disadvantage that if the key is already known, it will make a needless copy/clone, but in cases where it needs to be computed it's more flexible. Of course, as Keypair has already shipped, we can provide both (possibly with a blanket impl to unite them)

That said, I'm not sure what to name the trait.

tarcieri added a commit that referenced this issue Oct 24, 2022
@tarcieri
[WIP] signature v2.0.0-pre
21e2f4c 
This commit contains the first breaking changes to the `signature` crate
made since its initial 1.0 stabilization. Planning for these changes
occurred in #237.

The following changes have been made:

- The `Signature` trait has been renamed to `SignatureEncoding`. The
  `Signature` bound on `*Signer` and `*Verifier` traits has been
  removed, meaning there are no longer any mandatory trait bounds on the
  signature paramters at all.

- The `AsRef<[u8]>` bound formerly found on the `Signature` crate has
  been replaced with an associated `Repr` type, inspired by the
  `group::GroupEncoding` trait. This means signature types no longer
  need to retain a serialized form, and can parse the bag-of-bytes
  representation to something more convenient, which is useful for e.g.
  batch verification.

- The `std` feature is no longer enabled by default, which matches the
  other RustCrypto/traits crates.

- The `derive-preview` and `hazmat-preview` features have been
  stabilized.

- The former `Keypair` trait has been renamed to `KeypairRef`, and the
  signature generic parameter removed. A new `Keypair` trait has been
  added which returns an owned instance of the associated
  `VerifyingKey`, and a blanket impl of `Keypair` for `KeypairRef` has
  been added. This addresses the issues described in #1124.
@tarcieri tarcieri mentioned this issue Oct 24, 2022
Merged
tarcieri added a commit that referenced this issue Oct 24, 2022
@tarcieri
[WIP] signature v2.0.0-pre
b96d650 
This commit contains the first breaking changes to the `signature` crate
made since its initial 1.0 stabilization. Planning for these changes
occurred in #237.

The following changes have been made:

- The `Signature` trait has been renamed to `SignatureEncoding`. The
  `Signature` bound on `*Signer` and `*Verifier` traits has been
  removed, meaning there are no longer any mandatory trait bounds on the
  signature paramters at all.

- The `AsRef<[u8]>` bound formerly found on the `Signature` crate has
  been replaced with an associated `Repr` type, inspired by the
  `group::GroupEncoding` trait. This means signature types no longer
  need to retain a serialized form, and can parse the bag-of-bytes
  representation to something more convenient, which is useful for e.g.
  batch verification.

- The `std` feature is no longer enabled by default, which matches the
  other RustCrypto/traits crates.

- The `derive-preview` and `hazmat-preview` features have been
  stabilized.

- The former `Keypair` trait has been renamed to `KeypairRef`, and the
  signature generic parameter removed. A new `Keypair` trait has been
  added which returns an owned instance of the associated
  `VerifyingKey`, and a blanket impl of `Keypair` for `KeypairRef` has
  been added. This addresses the issues described in #1124.
tarcieri added a commit that referenced this issue Oct 24, 2022
@tarcieri
[WIP] signature v2.0.0-pre
73e4ef5 
This commit contains the first breaking changes to the `signature` crate
made since its initial 1.0 stabilization. Planning for these changes
occurred in #237.

The following changes have been made:

- The `Signature` trait has been renamed to `SignatureEncoding`. The
  `Signature` bound on `*Signer` and `*Verifier` traits has been
  removed, meaning there are no longer any mandatory trait bounds on the
  signature paramters at all.

- The `AsRef<[u8]>` bound formerly found on the `Signature` crate has
  been replaced with an associated `Repr` type, inspired by the
  `group::GroupEncoding` trait. This means signature types no longer
  need to retain a serialized form, and can parse the bag-of-bytes
  representation to something more convenient, which is useful for e.g.
  batch verification.

- The `std` feature is no longer enabled by default, which matches the
  other RustCrypto/traits crates.

- The `derive-preview` and `hazmat-preview` features have been
  stabilized.

- The former `Keypair` trait has been renamed to `KeypairRef`, and the
  signature generic parameter removed. A new `Keypair` trait has been
  added which returns an owned instance of the associated
  `VerifyingKey`, and a blanket impl of `Keypair` for `KeypairRef` has
  been added. This addresses the issues described in #1124.
tarcieri added a commit that referenced this issue Oct 24, 2022
@tarcieri
[WIP] signature v2.0.0-pre
2c1b3bd 
This commit contains the first breaking changes to the `signature` crate
made since its initial 1.0 stabilization. Planning for these changes
occurred in #237.

The following changes have been made:

- The `Signature` trait has been renamed to `SignatureEncoding`. The
  `Signature` bound on `*Signer` and `*Verifier` traits has been
  removed, meaning there are no longer any mandatory trait bounds on the
  signature paramters at all.

- The `AsRef<[u8]>` bound formerly found on the `Signature` crate has
  been replaced with an associated `Repr` type, inspired by the
  `group::GroupEncoding` trait. This means signature types no longer
  need to retain a serialized form, and can parse the bag-of-bytes
  representation to something more convenient, which is useful for e.g.
  batch verification.

- The `std` feature is no longer enabled by default, which matches the
  other RustCrypto/traits crates.

- The `derive-preview` and `hazmat-preview` features have been
  stabilized.

- The former `Keypair` trait has been renamed to `KeypairRef`, and the
  signature generic parameter removed. A new `Keypair` trait has been
  added which returns an owned instance of the associated
  `VerifyingKey`, and a blanket impl of `Keypair` for `KeypairRef` has
  been added. This addresses the issues described in #1124.
tarcieri added a commit that referenced this issue Oct 29, 2022
@tarcieri
signature v2.0.0-pre (#1141)
36aa416 
This commit contains the first breaking changes to the `signature` crate
made since its initial 1.0 stabilization. Planning for these changes
occurred in #237.

The following changes have been made:

- The `Signature` trait has been renamed to `SignatureEncoding`. The
  `Signature` bound on `*Signer` and `*Verifier` traits has been
  removed, meaning there are no longer any mandatory trait bounds on the
  signature parameters at all.

- The `AsRef<[u8]>` bound formerly found on the `Signature` crate has
  been replaced with an associated `Repr` type, inspired by the
  `group::GroupEncoding` trait. This means signature types no longer
  need to retain a serialized form, and can parse the bag-of-bytes
  representation to something more convenient, which is useful for e.g.
  batch verification.

- The `std` feature is no longer enabled by default, which matches the
  other RustCrypto/traits crates.

- The `derive-preview` and `hazmat-preview` features have been
  stabilized.

- The former `Keypair` trait has been renamed to `KeypairRef`, and the
  signature generic parameter removed. A new `Keypair` trait has been
  added which returns an owned instance of the associated
  `VerifyingKey`, and a blanket impl of `Keypair` for `KeypairRef` has
  been added. This addresses the issues described in #1124.
@tarcieri
Copy link
Member Author

Fixed in #1141

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
signature Digital signature crate
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@tarcieri @lumag