signature_derive: Custom derive support for Signer/Verifier by tarcieri · Pull Request #18 · RustCrypto/signatures

tarcieri · 2019-06-06T22:57:28Z

We've explored several different possible trait bounds for a blanket impl of Signer for DigestSigner (and likewise for Verifier).

Unfortunately, the approach we netted out at in #12 does not permit anything but the blanket impl, and had to be replaced (with something I think is suboptimal and messy) in #16.

It's definitely still worth exploring if there's a way to define the bounds of the default impl that works, however there is one approach we haven't explored yet: a procedural macro.

This approach feels like a bit of a hack, but gets us to where we originally wanted to be with the blanket impl API-wise. The impl it derives uses the same approach and bounds as the blanket impl did: the Digest to use is sourced from an associated type of the DigestSignature trait, which means the digest to use is both automatic and ensures the type deriving the trait supports the expected Digest for the given signature.

The implementation uses synstructure which simplifies both handling of generics and testing the output of the proc macro.

Additionally, it includes a complete integration test of the derived code which ensures it works as expected.

It's gated under a cargo feature and disabled-by-default. This should make it unobtrusive for downstream crates which don't need its functionality.

tarcieri · 2019-06-06T22:58:25Z

I will try integrating this change into signatory and see how it pans out when used with real-world Signer/Verifier impls.

We've explored several different possible trait bounds for a blanket impl of `Signer` for `DigestSigner` (and likewise for `Verifier`). Unfortunately, the approach we netted out at in #12 does not permit anything but the blanket impl, and was removed in #16. It's definitely still worth exploring if there's a way to define the bounds of the default impl that works, however there is one approach we haven't explored yet: a procedural macro. This approach feels like a bit of a hack, but gets us to where we originally wanted to be with the blanket impl API-wise. The impl it derives uses the same approach and bounds as the blanket impl did: the `Digest` to use is sourced from an associated type of the `DigestSignature` trait, which means the digest to use is both automatic and ensures the type deriving the trait supports the expected `Digest` for the given signature. The implementation uses `synstructure` which simplifies both handling of generics and testing the output of the proc macro. Additionally, it includes a complete integration test of the derived code which ensures it works as expected. It's gated under a cargo feature and disabled-by-default. This should make it unobtrusive for downstream crates which don't need its functionality.

tarcieri · 2019-06-07T00:17:06Z

I've managed to update Signatory using these changes, and I really like the result.

I'd like to ship this in signature crate v0.2.0, and then ship a release of signatory which is based on this crate.

We can evaluate whether signature_derive can be eliminated by changing the API to better leverage the type system in signature v0.3 (or later).

The `DigestSignature` trait was originally used as the marker trait used by a blanket impl of `Signer` for `DigestSigner` and `Verifier` for `DigestVerifier` (#12, #16). Unfortunately, this approach turned out to be too inflexible to cover real-world use cases encountered and was replaced with a proc macro (#18) The `DigestSignature` trait is leftover from this, and is no longer needed. This commit removes it.

The `DigestSignature` trait was originally used as the marker trait used by a blanket impl of `Signer` for `DigestSigner` and `Verifier` for `DigestVerifier` (#12, #16). Unfortunately, this approach turned out to be too inflexible to cover real-world use cases encountered and was replaced with a proc macro (#18) The `DigestSignature` trait is leftover from this, and is no longer needed. This commit replaces it with a `#[digest(...)]` custom derive attribute which can be used to specify the digest which should be used when computing a signature.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. This reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: we failed this test: > Test #18: signature with a repeated hint (Invalid) The issue was a regression from #895 which changed a comparison operator in the wrong direction. This has been corrected. On ML-DSA-44 (only, for some reason) we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exception for that for now, and all the other tests pass.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. On ML-DSA-44, this reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: > Test #18: signature with a repeated hint (Invalid) Likewise, on ML-DSA-44 only we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exceptions for those two for now, and all the other tests pass. I opened #1183 as a tracking issue for these verification failures.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. On ML-DSA-44, this reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: > Test #18: signature with a repeated hint (Invalid) Likewise, on ML-DSA-44 only we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exceptions for those two for now, and all the other tests pass. I opened #1183 as a tracking issue for these verification failures.

tarcieri · 2019-06-06T22:57:28Z

We've explored several different possible trait bounds for a blanket impl of Signer for DigestSigner (and likewise for Verifier).

Unfortunately, the approach we netted out at in #12 does not permit anything but the blanket impl, and had to be replaced (with something I think is suboptimal and messy) in #16.

It's definitely still worth exploring if there's a way to define the bounds of the default impl that works, however there is one approach we haven't explored yet: a procedural macro.

This approach feels like a bit of a hack, but gets us to where we originally wanted to be with the blanket impl API-wise. The impl it derives uses the same approach and bounds as the blanket impl did: the Digest to use is sourced from an associated type of the DigestSignature trait, which means the digest to use is both automatic and ensures the type deriving the trait supports the expected Digest for the given signature.

The implementation uses synstructure which simplifies both handling of generics and testing the output of the proc macro.

Additionally, it includes a complete integration test of the derived code which ensures it works as expected.

It's gated under a cargo feature and disabled-by-default. This should make it unobtrusive for downstream crates which don't need its functionality.

tarcieri · 2019-06-06T22:58:25Z

I will try integrating this change into signatory and see how it pans out when used with real-world Signer/Verifier impls.

We've explored several different possible trait bounds for a blanket impl of `Signer` for `DigestSigner` (and likewise for `Verifier`). Unfortunately, the approach we netted out at in #12 does not permit anything but the blanket impl, and was removed in #16. It's definitely still worth exploring if there's a way to define the bounds of the default impl that works, however there is one approach we haven't explored yet: a procedural macro. This approach feels like a bit of a hack, but gets us to where we originally wanted to be with the blanket impl API-wise. The impl it derives uses the same approach and bounds as the blanket impl did: the `Digest` to use is sourced from an associated type of the `DigestSignature` trait, which means the digest to use is both automatic and ensures the type deriving the trait supports the expected `Digest` for the given signature. The implementation uses `synstructure` which simplifies both handling of generics and testing the output of the proc macro. Additionally, it includes a complete integration test of the derived code which ensures it works as expected. It's gated under a cargo feature and disabled-by-default. This should make it unobtrusive for downstream crates which don't need its functionality.

tarcieri · 2019-06-07T00:17:06Z

I've managed to update Signatory using these changes, and I really like the result.

I'd like to ship this in signature crate v0.2.0, and then ship a release of signatory which is based on this crate.

We can evaluate whether signature_derive can be eliminated by changing the API to better leverage the type system in signature v0.3 (or later).

The `DigestSignature` trait was originally used as the marker trait used by a blanket impl of `Signer` for `DigestSigner` and `Verifier` for `DigestVerifier` (#12, #16). Unfortunately, this approach turned out to be too inflexible to cover real-world use cases encountered and was replaced with a proc macro (#18) The `DigestSignature` trait is leftover from this, and is no longer needed. This commit removes it.

The `DigestSignature` trait was originally used as the marker trait used by a blanket impl of `Signer` for `DigestSigner` and `Verifier` for `DigestVerifier` (#12, #16). Unfortunately, this approach turned out to be too inflexible to cover real-world use cases encountered and was replaced with a proc macro (#18) The `DigestSignature` trait is leftover from this, and is no longer needed. This commit replaces it with a `#[digest(...)]` custom derive attribute which can be used to specify the digest which should be used when computing a signature.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. This reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: we failed this test: > Test #18: signature with a repeated hint (Invalid) The issue was a regression from #895 which changed a comparison operator in the wrong direction. This has been corrected. On ML-DSA-44 (only, for some reason) we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exception for that for now, and all the other tests pass.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. On ML-DSA-44, this reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: > Test #18: signature with a repeated hint (Invalid) Likewise, on ML-DSA-44 only we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exceptions for those two for now, and all the other tests pass. I opened #1183 as a tracking issue for these verification failures.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. On ML-DSA-44, this reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: > Test #18: signature with a repeated hint (Invalid) Likewise, on ML-DSA-44 only we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exceptions for those two for now, and all the other tests pass. I opened #1183 as a tracking issue for these verification failures.

tarcieri · 2019-06-06T22:57:28Z

We've explored several different possible trait bounds for a blanket impl of Signer for DigestSigner (and likewise for Verifier).

Unfortunately, the approach we netted out at in #12 does not permit anything but the blanket impl, and had to be replaced (with something I think is suboptimal and messy) in #16.

It's definitely still worth exploring if there's a way to define the bounds of the default impl that works, however there is one approach we haven't explored yet: a procedural macro.

This approach feels like a bit of a hack, but gets us to where we originally wanted to be with the blanket impl API-wise. The impl it derives uses the same approach and bounds as the blanket impl did: the Digest to use is sourced from an associated type of the DigestSignature trait, which means the digest to use is both automatic and ensures the type deriving the trait supports the expected Digest for the given signature.

The implementation uses synstructure which simplifies both handling of generics and testing the output of the proc macro.

Additionally, it includes a complete integration test of the derived code which ensures it works as expected.

It's gated under a cargo feature and disabled-by-default. This should make it unobtrusive for downstream crates which don't need its functionality.

tarcieri · 2019-06-06T22:58:25Z

I will try integrating this change into signatory and see how it pans out when used with real-world Signer/Verifier impls.

We've explored several different possible trait bounds for a blanket impl of `Signer` for `DigestSigner` (and likewise for `Verifier`). Unfortunately, the approach we netted out at in #12 does not permit anything but the blanket impl, and was removed in #16. It's definitely still worth exploring if there's a way to define the bounds of the default impl that works, however there is one approach we haven't explored yet: a procedural macro. This approach feels like a bit of a hack, but gets us to where we originally wanted to be with the blanket impl API-wise. The impl it derives uses the same approach and bounds as the blanket impl did: the `Digest` to use is sourced from an associated type of the `DigestSignature` trait, which means the digest to use is both automatic and ensures the type deriving the trait supports the expected `Digest` for the given signature. The implementation uses `synstructure` which simplifies both handling of generics and testing the output of the proc macro. Additionally, it includes a complete integration test of the derived code which ensures it works as expected. It's gated under a cargo feature and disabled-by-default. This should make it unobtrusive for downstream crates which don't need its functionality.

tarcieri · 2019-06-07T00:17:06Z

I've managed to update Signatory using these changes, and I really like the result.

I'd like to ship this in signature crate v0.2.0, and then ship a release of signatory which is based on this crate.

We can evaluate whether signature_derive can be eliminated by changing the API to better leverage the type system in signature v0.3 (or later).

The `DigestSignature` trait was originally used as the marker trait used by a blanket impl of `Signer` for `DigestSigner` and `Verifier` for `DigestVerifier` (#12, #16). Unfortunately, this approach turned out to be too inflexible to cover real-world use cases encountered and was replaced with a proc macro (#18) The `DigestSignature` trait is leftover from this, and is no longer needed. This commit removes it.

The `DigestSignature` trait was originally used as the marker trait used by a blanket impl of `Signer` for `DigestSigner` and `Verifier` for `DigestVerifier` (#12, #16). Unfortunately, this approach turned out to be too inflexible to cover real-world use cases encountered and was replaced with a proc macro (#18) The `DigestSignature` trait is leftover from this, and is no longer needed. This commit replaces it with a `#[digest(...)]` custom derive attribute which can be used to specify the digest which should be used when computing a signature.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. This reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: we failed this test: > Test #18: signature with a repeated hint (Invalid) The issue was a regression from #895 which changed a comparison operator in the wrong direction. This has been corrected. On ML-DSA-44 (only, for some reason) we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exception for that for now, and all the other tests pass.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. On ML-DSA-44, this reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: > Test #18: signature with a repeated hint (Invalid) Likewise, on ML-DSA-44 only we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exceptions for those two for now, and all the other tests pass. I opened #1183 as a tracking issue for these verification failures.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. On ML-DSA-44, this reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: > Test #18: signature with a repeated hint (Invalid) Likewise, on ML-DSA-44 only we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exceptions for those two for now, and all the other tests pass. I opened #1183 as a tracking issue for these verification failures.

tarcieri · 2019-06-06T22:57:28Z

We've explored several different possible trait bounds for a blanket impl of Signer for DigestSigner (and likewise for Verifier).

Unfortunately, the approach we netted out at in #12 does not permit anything but the blanket impl, and had to be replaced (with something I think is suboptimal and messy) in #16.

It's definitely still worth exploring if there's a way to define the bounds of the default impl that works, however there is one approach we haven't explored yet: a procedural macro.

This approach feels like a bit of a hack, but gets us to where we originally wanted to be with the blanket impl API-wise. The impl it derives uses the same approach and bounds as the blanket impl did: the Digest to use is sourced from an associated type of the DigestSignature trait, which means the digest to use is both automatic and ensures the type deriving the trait supports the expected Digest for the given signature.

The implementation uses synstructure which simplifies both handling of generics and testing the output of the proc macro.

Additionally, it includes a complete integration test of the derived code which ensures it works as expected.

It's gated under a cargo feature and disabled-by-default. This should make it unobtrusive for downstream crates which don't need its functionality.

tarcieri · 2019-06-06T22:58:25Z

I will try integrating this change into signatory and see how it pans out when used with real-world Signer/Verifier impls.

We've explored several different possible trait bounds for a blanket impl of `Signer` for `DigestSigner` (and likewise for `Verifier`). Unfortunately, the approach we netted out at in #12 does not permit anything but the blanket impl, and was removed in #16. It's definitely still worth exploring if there's a way to define the bounds of the default impl that works, however there is one approach we haven't explored yet: a procedural macro. This approach feels like a bit of a hack, but gets us to where we originally wanted to be with the blanket impl API-wise. The impl it derives uses the same approach and bounds as the blanket impl did: the `Digest` to use is sourced from an associated type of the `DigestSignature` trait, which means the digest to use is both automatic and ensures the type deriving the trait supports the expected `Digest` for the given signature. The implementation uses `synstructure` which simplifies both handling of generics and testing the output of the proc macro. Additionally, it includes a complete integration test of the derived code which ensures it works as expected. It's gated under a cargo feature and disabled-by-default. This should make it unobtrusive for downstream crates which don't need its functionality.

tarcieri · 2019-06-07T00:17:06Z

I've managed to update Signatory using these changes, and I really like the result.

I'd like to ship this in signature crate v0.2.0, and then ship a release of signatory which is based on this crate.

We can evaluate whether signature_derive can be eliminated by changing the API to better leverage the type system in signature v0.3 (or later).

The `DigestSignature` trait was originally used as the marker trait used by a blanket impl of `Signer` for `DigestSigner` and `Verifier` for `DigestVerifier` (#12, #16). Unfortunately, this approach turned out to be too inflexible to cover real-world use cases encountered and was replaced with a proc macro (#18) The `DigestSignature` trait is leftover from this, and is no longer needed. This commit removes it.

The `DigestSignature` trait was originally used as the marker trait used by a blanket impl of `Signer` for `DigestSigner` and `Verifier` for `DigestVerifier` (#12, #16). Unfortunately, this approach turned out to be too inflexible to cover real-world use cases encountered and was replaced with a proc macro (#18) The `DigestSignature` trait is leftover from this, and is no longer needed. This commit replaces it with a `#[digest(...)]` custom derive attribute which can be used to specify the digest which should be used when computing a signature.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. This reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: we failed this test: > Test #18: signature with a repeated hint (Invalid) The issue was a regression from #895 which changed a comparison operator in the wrong direction. This has been corrected. On ML-DSA-44 (only, for some reason) we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exception for that for now, and all the other tests pass.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. On ML-DSA-44, this reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: > Test #18: signature with a repeated hint (Invalid) Likewise, on ML-DSA-44 only we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exceptions for those two for now, and all the other tests pass. I opened #1183 as a tracking issue for these verification failures.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. On ML-DSA-44, this reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: > Test #18: signature with a repeated hint (Invalid) Likewise, on ML-DSA-44 only we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exceptions for those two for now, and all the other tests pass. I opened #1183 as a tracking issue for these verification failures.

tarcieri · 2019-06-06T22:57:28Z

We've explored several different possible trait bounds for a blanket impl of Signer for DigestSigner (and likewise for Verifier).

Unfortunately, the approach we netted out at in #12 does not permit anything but the blanket impl, and had to be replaced (with something I think is suboptimal and messy) in #16.

It's definitely still worth exploring if there's a way to define the bounds of the default impl that works, however there is one approach we haven't explored yet: a procedural macro.

This approach feels like a bit of a hack, but gets us to where we originally wanted to be with the blanket impl API-wise. The impl it derives uses the same approach and bounds as the blanket impl did: the Digest to use is sourced from an associated type of the DigestSignature trait, which means the digest to use is both automatic and ensures the type deriving the trait supports the expected Digest for the given signature.

The implementation uses synstructure which simplifies both handling of generics and testing the output of the proc macro.

Additionally, it includes a complete integration test of the derived code which ensures it works as expected.

It's gated under a cargo feature and disabled-by-default. This should make it unobtrusive for downstream crates which don't need its functionality.

tarcieri · 2019-06-06T22:58:25Z

I will try integrating this change into signatory and see how it pans out when used with real-world Signer/Verifier impls.

We've explored several different possible trait bounds for a blanket impl of `Signer` for `DigestSigner` (and likewise for `Verifier`). Unfortunately, the approach we netted out at in #12 does not permit anything but the blanket impl, and was removed in #16. It's definitely still worth exploring if there's a way to define the bounds of the default impl that works, however there is one approach we haven't explored yet: a procedural macro. This approach feels like a bit of a hack, but gets us to where we originally wanted to be with the blanket impl API-wise. The impl it derives uses the same approach and bounds as the blanket impl did: the `Digest` to use is sourced from an associated type of the `DigestSignature` trait, which means the digest to use is both automatic and ensures the type deriving the trait supports the expected `Digest` for the given signature. The implementation uses `synstructure` which simplifies both handling of generics and testing the output of the proc macro. Additionally, it includes a complete integration test of the derived code which ensures it works as expected. It's gated under a cargo feature and disabled-by-default. This should make it unobtrusive for downstream crates which don't need its functionality.

tarcieri · 2019-06-07T00:17:06Z

I've managed to update Signatory using these changes, and I really like the result.

I'd like to ship this in signature crate v0.2.0, and then ship a release of signatory which is based on this crate.

We can evaluate whether signature_derive can be eliminated by changing the API to better leverage the type system in signature v0.3 (or later).

The `DigestSignature` trait was originally used as the marker trait used by a blanket impl of `Signer` for `DigestSigner` and `Verifier` for `DigestVerifier` (#12, #16). Unfortunately, this approach turned out to be too inflexible to cover real-world use cases encountered and was replaced with a proc macro (#18) The `DigestSignature` trait is leftover from this, and is no longer needed. This commit removes it.

The `DigestSignature` trait was originally used as the marker trait used by a blanket impl of `Signer` for `DigestSigner` and `Verifier` for `DigestVerifier` (#12, #16). Unfortunately, this approach turned out to be too inflexible to cover real-world use cases encountered and was replaced with a proc macro (#18) The `DigestSignature` trait is leftover from this, and is no longer needed. This commit replaces it with a `#[digest(...)]` custom derive attribute which can be used to specify the digest which should be used when computing a signature.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. This reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: we failed this test: > Test #18: signature with a repeated hint (Invalid) The issue was a regression from #895 which changed a comparison operator in the wrong direction. This has been corrected. On ML-DSA-44 (only, for some reason) we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exception for that for now, and all the other tests pass.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. On ML-DSA-44, this reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: > Test #18: signature with a repeated hint (Invalid) Likewise, on ML-DSA-44 only we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exceptions for those two for now, and all the other tests pass. I opened #1183 as a tracking issue for these verification failures.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. On ML-DSA-44, this reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: > Test #18: signature with a repeated hint (Invalid) Likewise, on ML-DSA-44 only we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exceptions for those two for now, and all the other tests pass. I opened #1183 as a tracking issue for these verification failures.

tarcieri · 2019-06-06T22:57:28Z

We've explored several different possible trait bounds for a blanket impl of Signer for DigestSigner (and likewise for Verifier).

Unfortunately, the approach we netted out at in #12 does not permit anything but the blanket impl, and had to be replaced (with something I think is suboptimal and messy) in #16.

It's definitely still worth exploring if there's a way to define the bounds of the default impl that works, however there is one approach we haven't explored yet: a procedural macro.

This approach feels like a bit of a hack, but gets us to where we originally wanted to be with the blanket impl API-wise. The impl it derives uses the same approach and bounds as the blanket impl did: the Digest to use is sourced from an associated type of the DigestSignature trait, which means the digest to use is both automatic and ensures the type deriving the trait supports the expected Digest for the given signature.

The implementation uses synstructure which simplifies both handling of generics and testing the output of the proc macro.

Additionally, it includes a complete integration test of the derived code which ensures it works as expected.

It's gated under a cargo feature and disabled-by-default. This should make it unobtrusive for downstream crates which don't need its functionality.

tarcieri · 2019-06-06T22:58:25Z

I will try integrating this change into signatory and see how it pans out when used with real-world Signer/Verifier impls.

We've explored several different possible trait bounds for a blanket impl of `Signer` for `DigestSigner` (and likewise for `Verifier`). Unfortunately, the approach we netted out at in #12 does not permit anything but the blanket impl, and was removed in #16. It's definitely still worth exploring if there's a way to define the bounds of the default impl that works, however there is one approach we haven't explored yet: a procedural macro. This approach feels like a bit of a hack, but gets us to where we originally wanted to be with the blanket impl API-wise. The impl it derives uses the same approach and bounds as the blanket impl did: the `Digest` to use is sourced from an associated type of the `DigestSignature` trait, which means the digest to use is both automatic and ensures the type deriving the trait supports the expected `Digest` for the given signature. The implementation uses `synstructure` which simplifies both handling of generics and testing the output of the proc macro. Additionally, it includes a complete integration test of the derived code which ensures it works as expected. It's gated under a cargo feature and disabled-by-default. This should make it unobtrusive for downstream crates which don't need its functionality.

tarcieri · 2019-06-07T00:17:06Z

I've managed to update Signatory using these changes, and I really like the result.

I'd like to ship this in signature crate v0.2.0, and then ship a release of signatory which is based on this crate.

We can evaluate whether signature_derive can be eliminated by changing the API to better leverage the type system in signature v0.3 (or later).

The `DigestSignature` trait was originally used as the marker trait used by a blanket impl of `Signer` for `DigestSigner` and `Verifier` for `DigestVerifier` (#12, #16). Unfortunately, this approach turned out to be too inflexible to cover real-world use cases encountered and was replaced with a proc macro (#18) The `DigestSignature` trait is leftover from this, and is no longer needed. This commit removes it.

The `DigestSignature` trait was originally used as the marker trait used by a blanket impl of `Signer` for `DigestSigner` and `Verifier` for `DigestVerifier` (#12, #16). Unfortunately, this approach turned out to be too inflexible to cover real-world use cases encountered and was replaced with a proc macro (#18) The `DigestSignature` trait is leftover from this, and is no longer needed. This commit replaces it with a `#[digest(...)]` custom derive attribute which can be used to specify the digest which should be used when computing a signature.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. This reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: we failed this test: > Test #18: signature with a repeated hint (Invalid) The issue was a regression from #895 which changed a comparison operator in the wrong direction. This has been corrected. On ML-DSA-44 (only, for some reason) we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exception for that for now, and all the other tests pass.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. On ML-DSA-44, this reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: > Test #18: signature with a repeated hint (Invalid) Likewise, on ML-DSA-44 only we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exceptions for those two for now, and all the other tests pass. I opened #1183 as a tracking issue for these verification failures.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. On ML-DSA-44, this reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: > Test #18: signature with a repeated hint (Invalid) Likewise, on ML-DSA-44 only we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exceptions for those two for now, and all the other tests pass. I opened #1183 as a tracking issue for these verification failures.

tarcieri · 2019-06-06T22:57:28Z

We've explored several different possible trait bounds for a blanket impl of Signer for DigestSigner (and likewise for Verifier).

Unfortunately, the approach we netted out at in #12 does not permit anything but the blanket impl, and had to be replaced (with something I think is suboptimal and messy) in #16.

It's definitely still worth exploring if there's a way to define the bounds of the default impl that works, however there is one approach we haven't explored yet: a procedural macro.

This approach feels like a bit of a hack, but gets us to where we originally wanted to be with the blanket impl API-wise. The impl it derives uses the same approach and bounds as the blanket impl did: the Digest to use is sourced from an associated type of the DigestSignature trait, which means the digest to use is both automatic and ensures the type deriving the trait supports the expected Digest for the given signature.

The implementation uses synstructure which simplifies both handling of generics and testing the output of the proc macro.

Additionally, it includes a complete integration test of the derived code which ensures it works as expected.

It's gated under a cargo feature and disabled-by-default. This should make it unobtrusive for downstream crates which don't need its functionality.

tarcieri · 2019-06-06T22:58:25Z

I will try integrating this change into signatory and see how it pans out when used with real-world Signer/Verifier impls.

We've explored several different possible trait bounds for a blanket impl of `Signer` for `DigestSigner` (and likewise for `Verifier`). Unfortunately, the approach we netted out at in #12 does not permit anything but the blanket impl, and was removed in #16. It's definitely still worth exploring if there's a way to define the bounds of the default impl that works, however there is one approach we haven't explored yet: a procedural macro. This approach feels like a bit of a hack, but gets us to where we originally wanted to be with the blanket impl API-wise. The impl it derives uses the same approach and bounds as the blanket impl did: the `Digest` to use is sourced from an associated type of the `DigestSignature` trait, which means the digest to use is both automatic and ensures the type deriving the trait supports the expected `Digest` for the given signature. The implementation uses `synstructure` which simplifies both handling of generics and testing the output of the proc macro. Additionally, it includes a complete integration test of the derived code which ensures it works as expected. It's gated under a cargo feature and disabled-by-default. This should make it unobtrusive for downstream crates which don't need its functionality.

tarcieri · 2019-06-07T00:17:06Z

I've managed to update Signatory using these changes, and I really like the result.

I'd like to ship this in signature crate v0.2.0, and then ship a release of signatory which is based on this crate.

We can evaluate whether signature_derive can be eliminated by changing the API to better leverage the type system in signature v0.3 (or later).

The `DigestSignature` trait was originally used as the marker trait used by a blanket impl of `Signer` for `DigestSigner` and `Verifier` for `DigestVerifier` (#12, #16). Unfortunately, this approach turned out to be too inflexible to cover real-world use cases encountered and was replaced with a proc macro (#18) The `DigestSignature` trait is leftover from this, and is no longer needed. This commit removes it.

The `DigestSignature` trait was originally used as the marker trait used by a blanket impl of `Signer` for `DigestSigner` and `Verifier` for `DigestVerifier` (#12, #16). Unfortunately, this approach turned out to be too inflexible to cover real-world use cases encountered and was replaced with a proc macro (#18) The `DigestSignature` trait is leftover from this, and is no longer needed. This commit replaces it with a `#[digest(...)]` custom derive attribute which can be used to specify the digest which should be used when computing a signature.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. This reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: we failed this test: > Test #18: signature with a repeated hint (Invalid) The issue was a regression from #895 which changed a comparison operator in the wrong direction. This has been corrected. On ML-DSA-44 (only, for some reason) we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exception for that for now, and all the other tests pass.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. On ML-DSA-44, this reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: > Test #18: signature with a repeated hint (Invalid) Likewise, on ML-DSA-44 only we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exceptions for those two for now, and all the other tests pass. I opened #1183 as a tracking issue for these verification failures.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. On ML-DSA-44, this reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: > Test #18: signature with a repeated hint (Invalid) Likewise, on ML-DSA-44 only we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exceptions for those two for now, and all the other tests pass. I opened #1183 as a tracking issue for these verification failures.

tarcieri · 2019-06-06T22:57:28Z

We've explored several different possible trait bounds for a blanket impl of Signer for DigestSigner (and likewise for Verifier).

Unfortunately, the approach we netted out at in #12 does not permit anything but the blanket impl, and had to be replaced (with something I think is suboptimal and messy) in #16.

It's definitely still worth exploring if there's a way to define the bounds of the default impl that works, however there is one approach we haven't explored yet: a procedural macro.

This approach feels like a bit of a hack, but gets us to where we originally wanted to be with the blanket impl API-wise. The impl it derives uses the same approach and bounds as the blanket impl did: the Digest to use is sourced from an associated type of the DigestSignature trait, which means the digest to use is both automatic and ensures the type deriving the trait supports the expected Digest for the given signature.

The implementation uses synstructure which simplifies both handling of generics and testing the output of the proc macro.

Additionally, it includes a complete integration test of the derived code which ensures it works as expected.

It's gated under a cargo feature and disabled-by-default. This should make it unobtrusive for downstream crates which don't need its functionality.

tarcieri · 2019-06-06T22:58:25Z

I will try integrating this change into signatory and see how it pans out when used with real-world Signer/Verifier impls.

We've explored several different possible trait bounds for a blanket impl of `Signer` for `DigestSigner` (and likewise for `Verifier`). Unfortunately, the approach we netted out at in #12 does not permit anything but the blanket impl, and was removed in #16. It's definitely still worth exploring if there's a way to define the bounds of the default impl that works, however there is one approach we haven't explored yet: a procedural macro. This approach feels like a bit of a hack, but gets us to where we originally wanted to be with the blanket impl API-wise. The impl it derives uses the same approach and bounds as the blanket impl did: the `Digest` to use is sourced from an associated type of the `DigestSignature` trait, which means the digest to use is both automatic and ensures the type deriving the trait supports the expected `Digest` for the given signature. The implementation uses `synstructure` which simplifies both handling of generics and testing the output of the proc macro. Additionally, it includes a complete integration test of the derived code which ensures it works as expected. It's gated under a cargo feature and disabled-by-default. This should make it unobtrusive for downstream crates which don't need its functionality.

tarcieri · 2019-06-07T00:17:06Z

I've managed to update Signatory using these changes, and I really like the result.

I'd like to ship this in signature crate v0.2.0, and then ship a release of signatory which is based on this crate.

We can evaluate whether signature_derive can be eliminated by changing the API to better leverage the type system in signature v0.3 (or later).

The `DigestSignature` trait was originally used as the marker trait used by a blanket impl of `Signer` for `DigestSigner` and `Verifier` for `DigestVerifier` (#12, #16). Unfortunately, this approach turned out to be too inflexible to cover real-world use cases encountered and was replaced with a proc macro (#18) The `DigestSignature` trait is leftover from this, and is no longer needed. This commit removes it.

The `DigestSignature` trait was originally used as the marker trait used by a blanket impl of `Signer` for `DigestSigner` and `Verifier` for `DigestVerifier` (#12, #16). Unfortunately, this approach turned out to be too inflexible to cover real-world use cases encountered and was replaced with a proc macro (#18) The `DigestSignature` trait is leftover from this, and is no longer needed. This commit replaces it with a `#[digest(...)]` custom derive attribute which can be used to specify the digest which should be used when computing a signature.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. This reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: we failed this test: > Test #18: signature with a repeated hint (Invalid) The issue was a regression from #895 which changed a comparison operator in the wrong direction. This has been corrected. On ML-DSA-44 (only, for some reason) we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exception for that for now, and all the other tests pass.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. On ML-DSA-44, this reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: > Test #18: signature with a repeated hint (Invalid) Likewise, on ML-DSA-44 only we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exceptions for those two for now, and all the other tests pass. I opened #1183 as a tracking issue for these verification failures.

@orenyomtov

Adds Wycheproof verification tests for ML-DSA-44/65/87. On ML-DSA-44, this reproduced an issue previously reported as GHSA-5x2r-hc65-25f9 by @orenyomtov: > Test #18: signature with a repeated hint (Invalid) Likewise, on ML-DSA-44 only we are also failing this case: > Test #68: public key with t1 component set to zero (Valid) I have added an exceptions for those two for now, and all the other tests pass. I opened #1183 as a tracking issue for these verification failures.

tarcieri force-pushed the signature_derive branch from 6edecb6 to 559fe47Compare June 6, 2019 22:57

tarcieri force-pushed the signature_derive branch from 559fe47 to 338dbb1Compare June 7, 2019 00:07

tarcieri merged commit 6bc013a into masterJun 7, 2019

tarcieri deleted the signature_derive branch June 7, 2019 00:17

tarcieri mentioned this pull request Jun 7, 2019
signature v0.2.0 #19
Merged

tarcieri mentioned this pull request Oct 9, 2019
signature-crate: Replace DigestSignature with derive attr #27
Merged

tarcieri mentioned this pull request Mar 17, 2020
signature: tracking issue for derive(Signer) and derive(Verifier) stabilization RustCrypto/traits#93
Closed

This was referenced Jan 26, 2026
ml-dsa: add Wycheproof verification tests #1176
Merged
ML-DSA-44 Wycheproof verification test failures #1183
Closed

tarcieri force-pushed the signature_derive branch from 6edecb6 to 559fe47Compare June 6, 2019 22:57

tarcieri force-pushed the signature_derive branch from 559fe47 to 338dbb1Compare June 7, 2019 00:07

tarcieri merged commit 6bc013a into masterJun 7, 2019

tarcieri deleted the signature_derive branch June 7, 2019 00:17

tarcieri mentioned this pull request Jun 7, 2019
signature v0.2.0 #19
Merged

tarcieri mentioned this pull request Oct 9, 2019
signature-crate: Replace DigestSignature with derive attr #27
Merged

tarcieri mentioned this pull request Mar 17, 2020
signature: tracking issue for derive(Signer) and derive(Verifier) stabilization RustCrypto/traits#93
Closed

This was referenced Jan 26, 2026
ml-dsa: add Wycheproof verification tests #1176
Merged
ML-DSA-44 Wycheproof verification test failures #1183
Closed

tarcieri force-pushed the signature_derive branch from 6edecb6 to 559fe47Compare June 6, 2019 22:57

tarcieri force-pushed the signature_derive branch from 559fe47 to 338dbb1Compare June 7, 2019 00:07

tarcieri merged commit 6bc013a into masterJun 7, 2019

tarcieri deleted the signature_derive branch June 7, 2019 00:17

tarcieri mentioned this pull request Jun 7, 2019
signature v0.2.0 #19
Merged

tarcieri mentioned this pull request Oct 9, 2019
signature-crate: Replace DigestSignature with derive attr #27
Merged

tarcieri mentioned this pull request Mar 17, 2020
signature: tracking issue for derive(Signer) and derive(Verifier) stabilization RustCrypto/traits#93
Closed

This was referenced Jan 26, 2026
ml-dsa: add Wycheproof verification tests #1176
Merged
ML-DSA-44 Wycheproof verification test failures #1183
Closed

tarcieri force-pushed the signature_derive branch from 6edecb6 to 559fe47Compare June 6, 2019 22:57

tarcieri force-pushed the signature_derive branch from 559fe47 to 338dbb1Compare June 7, 2019 00:07

tarcieri merged commit 6bc013a into masterJun 7, 2019

tarcieri deleted the signature_derive branch June 7, 2019 00:17

tarcieri mentioned this pull request Jun 7, 2019
signature v0.2.0 #19
Merged

tarcieri mentioned this pull request Oct 9, 2019
signature-crate: Replace DigestSignature with derive attr #27
Merged

tarcieri mentioned this pull request Mar 17, 2020
signature: tracking issue for derive(Signer) and derive(Verifier) stabilization RustCrypto/traits#93
Closed

This was referenced Jan 26, 2026
ml-dsa: add Wycheproof verification tests #1176
Merged
ML-DSA-44 Wycheproof verification test failures #1183
Closed

tarcieri force-pushed the signature_derive branch from 6edecb6 to 559fe47Compare June 6, 2019 22:57

tarcieri force-pushed the signature_derive branch from 559fe47 to 338dbb1Compare June 7, 2019 00:07

tarcieri merged commit 6bc013a into masterJun 7, 2019

tarcieri deleted the signature_derive branch June 7, 2019 00:17

tarcieri mentioned this pull request Jun 7, 2019
signature v0.2.0 #19
Merged

tarcieri mentioned this pull request Oct 9, 2019
signature-crate: Replace DigestSignature with derive attr #27
Merged

tarcieri mentioned this pull request Mar 17, 2020
signature: tracking issue for derive(Signer) and derive(Verifier) stabilization RustCrypto/traits#93
Closed

This was referenced Jan 26, 2026
ml-dsa: add Wycheproof verification tests #1176
Merged
ML-DSA-44 Wycheproof verification test failures #1183
Closed

tarcieri force-pushed the signature_derive branch from 6edecb6 to 559fe47Compare June 6, 2019 22:57

tarcieri force-pushed the signature_derive branch from 559fe47 to 338dbb1Compare June 7, 2019 00:07

tarcieri merged commit 6bc013a into masterJun 7, 2019

tarcieri deleted the signature_derive branch June 7, 2019 00:17

tarcieri mentioned this pull request Jun 7, 2019
signature v0.2.0 #19
Merged

tarcieri mentioned this pull request Oct 9, 2019
signature-crate: Replace DigestSignature with derive attr #27
Merged

tarcieri mentioned this pull request Mar 17, 2020
signature: tracking issue for derive(Signer) and derive(Verifier) stabilization RustCrypto/traits#93
Closed

This was referenced Jan 26, 2026
ml-dsa: add Wycheproof verification tests #1176
Merged
ML-DSA-44 Wycheproof verification test failures #1183
Closed

tarcieri force-pushed the signature_derive branch from 6edecb6 to 559fe47Compare June 6, 2019 22:57

tarcieri force-pushed the signature_derive branch from 559fe47 to 338dbb1Compare June 7, 2019 00:07

tarcieri merged commit 6bc013a into masterJun 7, 2019

tarcieri deleted the signature_derive branch June 7, 2019 00:17

tarcieri mentioned this pull request Jun 7, 2019
signature v0.2.0 #19
Merged

tarcieri mentioned this pull request Oct 9, 2019
signature-crate: Replace DigestSignature with derive attr #27
Merged

tarcieri mentioned this pull request Mar 17, 2020
signature: tracking issue for derive(Signer) and derive(Verifier) stabilization RustCrypto/traits#93
Closed

This was referenced Jan 26, 2026
ml-dsa: add Wycheproof verification tests #1176
Merged
ML-DSA-44 Wycheproof verification test failures #1183
Closed

tarcieri force-pushed the signature_derive branch from 6edecb6 to 559fe47Compare June 6, 2019 22:57

tarcieri force-pushed the signature_derive branch from 559fe47 to 338dbb1Compare June 7, 2019 00:07

tarcieri merged commit 6bc013a into masterJun 7, 2019

tarcieri deleted the signature_derive branch June 7, 2019 00:17

tarcieri mentioned this pull request Jun 7, 2019
signature v0.2.0 #19
Merged

tarcieri mentioned this pull request Oct 9, 2019
signature-crate: Replace DigestSignature with derive attr #27
Merged

tarcieri mentioned this pull request Mar 17, 2020
signature: tracking issue for derive(Signer) and derive(Verifier) stabilization RustCrypto/traits#93
Closed

This was referenced Jan 26, 2026
ml-dsa: add Wycheproof verification tests #1176
Merged
ML-DSA-44 Wycheproof verification test failures #1183
Closed

Uh oh!

Conversation

tarcieri commented Jun 6, 2019

Uh oh!

tarcieri commented Jun 6, 2019

Uh oh!

tarcieri commented Jun 7, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

tarcieri commented Jun 6, 2019

Uh oh!

tarcieri commented Jun 6, 2019

Uh oh!

tarcieri commented Jun 7, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

tarcieri commented Jun 6, 2019

Uh oh!

tarcieri commented Jun 6, 2019

Uh oh!

tarcieri commented Jun 7, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

tarcieri commented Jun 6, 2019

Uh oh!

tarcieri commented Jun 6, 2019

Uh oh!

tarcieri commented Jun 7, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

tarcieri commented Jun 6, 2019

Uh oh!

tarcieri commented Jun 6, 2019

Uh oh!

tarcieri commented Jun 7, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant