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 a verify_prehashed method to identity.rs's JWS verifiers #1420

Closed
wants to merge 4 commits into from

Conversation

UMR1352
Copy link
Contributor

@UMR1352 UMR1352 commented Oct 16, 2024

Description of change

Enable pre-hash verification for EcDSA verifier. Small refactor for JWS verifiers.

Links to any relevant issues

Fixes issue #1418

Type of change

  • Bug fix (a non-breaking change which fixes an issue)
  • Enhancement (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Fix

How the change has been tested

Ran tests

Change checklist

  • I have followed the contribution guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@UMR1352 UMR1352 added Enhancement New feature or improvement to an existing feature Rust Related to the core Rust code. Becomes part of the Rust changelog. labels Oct 16, 2024
@UMR1352 UMR1352 self-assigned this Oct 16, 2024
@UMR1352 UMR1352 added Wasm Related to Wasm bindings. Becomes part of the Wasm changelog Patch Change without affecting the API that requires a patch release. Part of "Patch" section in changelog labels Oct 16, 2024
@UMR1352
Copy link
Contributor Author

UMR1352 commented Oct 16, 2024

Unfortunately implementing pre-hash verification for for our EdDSA verifier is not as straight forward. At the moment there's only one Rust crate that allows Ed2219 pre-hash verification - i.e. ed25519-dalek but the requirements to call its validate_prehashed are way too strict.

The data to be verified must be passed to the function as a type D: Digest where Digest requires 4 more traits in order to be implemented.

@UMR1352 UMR1352 marked this pull request as ready for review October 16, 2024 09:48
@UMR1352 UMR1352 requested a review from a team as a code owner October 16, 2024 09:48
@UMR1352
Copy link
Contributor Author

UMR1352 commented Oct 31, 2024

After some discussion with the team we decided to ditch this feature as we want the JWS verifiers we provide with the library to only support verification of spec-compliant signatures (e.g. ES256 -> P256 + SHA256 and no other hashing algorithm).
We still do support the verification of custom signatures through our JwsSigner trait.

@UMR1352 UMR1352 closed this Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or improvement to an existing feature Patch Change without affecting the API that requires a patch release. Part of "Patch" section in changelog Rust Related to the core Rust code. Becomes part of the Rust changelog. Wasm Related to Wasm bindings. Becomes part of the Wasm changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants