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.

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

Add "raw digest" signature marker trait and sign/verify traits #10

Closed
wants to merge 1 commit into from

Conversation

tarcieri
Copy link
Member

@tarcieri tarcieri commented Mar 26, 2019

Introduces the notion of a "raw digest" signature algorithm, i.e. any algorithm where signatures are always computed as S(H(m))) where:

  • S: signature algorithm
  • H: hash (a.k.a. digest) function
  • m: message

Notably this does not hold true for Ed25519, which hashes the input message twice in an effort to remain secure even in the event of collisions in the underlying hash function.

However, it supports a separate IUF mode Ed25519ph, which is trivial for any Ed25519 implementation to implement (it hashes the message in advance, then performs a regular Ed25519 signature albeit with a domain separation tweak).

For cases like this, where the IUF mode is deliberately domain separated from the normal mode, it would be nice to avoid a blanket impl which assumes all signature algorithms are composed as S(H(m)), so signers can, if they so desire, impl both traits and support both modes using a single underlying type (e.g. KeyPair for signers and PublicKey for verifiers), as the keys used for either algorithm are identical and both modes can be used safely under the same key due to domain separation.

To accomplish this, this commit adds a RawDigestSignature marker trait to be applied at the algorithm level to the corresponding signature trait.

For signature types marked as RawDigestSignature, it's possible to impl a corresponding SignRawDigest trait for which a blanket impl of Sign exists which hashes the message with the corresponding Digest algorithm.

Additionally, SignDigest is blanket impl'd for all SignRawDigest types.

Introduces the notion of a "raw digest" signature algorithm, i.e.
any algorithm where signatures are always computed as `S(H(m)))` where:

- `S`: signature algorithm
- `H`: hash (a.k.a. digest) function
- `m`: message

Notably this does not hold true for Ed25519, which hashes the input
message twice in an effort to remain secure even in the event of
collisions in the underlying hash function.

However, it supports a separate IUF mode Ed25519ph, which is trivial
for any Ed25519 implementation to implement (it hashes the message
in advance, then performs a regular Ed25519 signature albeit with a
domain separation tweak).

For cases like this, where the IUF mode is deliberately domain separated
from the normal mode, it would be nice to avoid a blanket impl which
assumes all signature algorithms are composed as `S(H(m))`.

To accomplish this, this commit adds a `RawDigestSignature` marker trait
to be applied at the algorithm level to the corresponding signature
trait.

For signature types marked as `RawDigestSignature`, it's possible to
`impl` a corresponding `SignRawDigest` trait for which a blanket `impl`
of `Sign` exists which hashes the message with the corresponding
`Digest` algorithm.

Additionally, `SignDigest` is blanket impl'd for all `SignRawDigest`
types.
@tarcieri
Copy link
Member Author

@newpavlov I think this accomplishes what you were trying to do, which was to wrap a non-object-trait ala 80f518a with an object safe one, and use the former for the blanket impls so they could be constrained by the associated type.

I think this approach solves everything elegantly with simple trait composition, has clearer concepts it uses in the trait bounds, and best of all hoists the relevant marker trait up to the signature algorithm level so it can be solved once per algorithm.

@tarcieri
Copy link
Member Author

If there aren't any objections, I'd like to merge this

@newpavlov
Copy link
Member

Sorry for the delay! I wanted to think more carefully about this PR on this weekend. I have some questions, which I think we can discuss in IRC.

@tarcieri
Copy link
Member Author

@newpavlov sounds good

where
D: Digest,
S: Signature + RawDigestSignature,
T: SignRawDigest<S, Digest = D>,
Copy link
Member Author

Choose a reason for hiding this comment

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

@newpavlov here is where the Digest associated type from SignRawDigest is used. It's needed to constrain the blanket impl.

@tarcieri
Copy link
Member Author

@newpavlov I'll admit this still feels a bit weird. Let me briefly spell out the concerns I'm trying to juggle here:

  1. Allow Ed25519 crates to impl both Sign and SignDigest (and corresponding verify traits) on the same type which use slightly different algorithms (Ed25519/Ed25519ph) to accommodate an IUF mode
  2. Allow crates to electively impl SignDigest for multiple Digest algorithms on the same type
  3. Make it easy for crates to pick a canonical digest, and in the process receive a blanket impl of Sign which uses Digest to hash the message then does sign_digest

Alternative 1: proc macros

One alternative I can think of to the approach in this PR is to add signature_derive crate with a proc macro for deriving Sign (or Signer or whatever we end up calling it) for types which impl SignDigest. Something like:

  • #[derive(Sign(digest=Sha256))]

This is quite flexible and keeps the public facing API simple (just Sign/SignDigest), but resorting to a proc macro for this feels a bit dirty and of course they're something of a maintenance burden.

Alternative 2: move associated type to the signature trait

Another is to move the Digest associated type to the RawDigestSignature trait itself, and constrain the blanket impl with that.

It means that SignDigest types would only receive the blanket impl for the "preferred" digest algorithm, but that seems ok to me.

@tarcieri
Copy link
Member Author

Actually I like alternative 2 so much I'm going to close this and try again 😉

@tarcieri tarcieri closed this Mar 29, 2019
@tarcieri tarcieri deleted the raw-digest-blanket-impl branch March 29, 2019 18:43
@newpavlov
Copy link
Member

Allow Ed25519 crates to impl both Sign and SignDigest (and corresponding verify traits) on the same type which use slightly different algorithms

I would expect two different types for two different signature algorithms, even if they are jsut a slightly different.

Make it easy for crates to pick a canonical digest

I think it shouldn't be done on traits side. A better approach will be to use default type parameters, something like this:

struct SignAlg<D: Digest<Output=U32> = Sha256> { .. }

@tarcieri
Copy link
Member Author

tarcieri commented Mar 29, 2019

I would expect two different types for two different signature algorithms, even if they are jsut a slightly different.

Presently ed25519_dalek::Keypair has a sign and sign_prehashed methods. I think it really makes sense to make it possible to support this, rather than making a redundant Keypair type just for Ed25519ph.

The same also goes for supporting multiple digest algorithms per type. Then the type can be generic around the digest function.

I think it shouldn't be done on traits side. A better approach will be to use default type parameters, something like this

Using a trait means the problem can be solved once at an algorithm level (e.g. in the ecdsa crate), rather than every crate that implements it having to make it a generic parameter of their struct.

It also means the blanket impl "Just Works" on signature algorithms that are constructed as S(H(m)) without impacting algorithms that don't have this property. It's something the implementer of the digest::Signer and digest::Verifier traits doesn't even have to think about.

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