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 signing #464

Merged
merged 9 commits into from
Aug 15, 2023
Merged

Add signing #464

merged 9 commits into from
Aug 15, 2023

Conversation

marsella
Copy link

@marsella marsella commented Aug 9, 2023

Closes #425.

This adds the non-interactive signing protocol and a happy test for it.

It's quite large so I think it would benefit from review from both @gatoWololo and @hridambasu , just to have more eyes on it.

I tried to avoid adding too much miscellaneous clean-up, but I did make the following 2 changes:

  • I removed some redundancy around protocol executions, especially with session IDs; lots of the methods on various ProtocolParticipants take sid as a parameter, but each protocol participant also holds a session ID. We're at risk for bugs if they're different, so I reduced a bit of the usage of parameter-SIDs. This could be done more thoroughly across the library; I mainly fixed an actual bug in keygen tests where the messages had different SID from the participant, and simplified the implementation so it couldn't happen again.
  • There was some unnecessary separation of methods in the test_utils module that I found more confusing than helpful, so I consolidated into the two functions we actually use. No functional changes here.

From Aug 9:

I'm trying to add the complete signing protocol to the repo, but I'm getting some errors where signing generates an invalid signature. Here's what I've figured out:

  • Sometimes it works correctly
  • If a given (record, message) pair fails, changing the message will often pass
  • If I use the presign records to reconstruct the signing mask k and build the signature as if it were non-distributed signing, I get the same signature as the distributed algorithm (but it also fails).

This kind of reads to me like the signing algorithm is correct, but I'm generating presign records wrong, although I'm not sure how wrong they can be because even a "failing" record will work for some messages. I'm feeling pretty stuck on this.

This is a huge PR (as promised) and there's a lot to wade through. I hardcoded a seed that causes the tests to fail for reference.

@marsella marsella marked this pull request as ready for review August 10, 2023 17:42
src/presign/record.rs Show resolved Hide resolved
src/sign/participant.rs Show resolved Hide resolved
src/participant.rs Show resolved Hide resolved
Copy link

@hridambasu hridambasu left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@marsella marsella changed the title WIP - add signing Add signing Aug 14, 2023
Copy link

@gatoWololo gatoWololo left a comment

Choose a reason for hiding this comment

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

Looks good! Couple of minor questions.

@marsella marsella merged commit 0d8978e into main Aug 15, 2023
2 checks passed
@marsella marsella deleted the 425-signing-part-2 branch August 15, 2023 15:35
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.

Implement the non-interactive signing protocol
3 participants