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

frost-rs may be better suited using Merlin transcripts #3

Closed
kayabaNerve opened this issue May 2, 2022 · 5 comments
Closed

frost-rs may be better suited using Merlin transcripts #3

kayabaNerve opened this issue May 2, 2022 · 5 comments
Labels
improvement This could be better

Comments

@kayabaNerve
Copy link
Member

Instead of obtaining context and so on, a Merlin transcript could be passed and altered as needed. The issue is that the FROST IETF draft requires a specific serialization format for its binding factor calculation, which must be honored. This presumably means we can only use Merlin transcripts when an offset/context is specified, which may be a pain to manage (or relatively simple).

The inevitable Zcash standard for handling randomization (offsets) also should be considered, yet unfortunately can't be discussed yet as they haven't released anything nor have they answered my questions.

@kayabaNerve kayabaNerve added the feature New feature or request label May 2, 2022
@kayabaNerve
Copy link
Member Author

Not only would this limit us to a 128-bit security level, which I was hoping to not lock to, the transcripts can't be used as they would desire while maintaining the level of abstraction desired.

  // Let the algorithm provide a Merlin transcript of its variables
  let mut transcript = params.algorithm.transcript();

  // If the offset functionality provided by this library is in use, include it in the transcript.
  // Not compliant with the IETF spec which doesn't have a concept of offsets, nor does it use
  // Merlin transcripts
  if params.keys.offset.is_some() {
    let offset_transcript = transcript.unwrap_or(Transcript::new(b"FROST_offset"));
    offset_transcript.append_message(b"offset", &C::F_to_le_bytes(&params.keys.offset.unwrap()));
    transcript = Some(offset_transcript);
  }

  // If a transcript was defined, move the binding factor into it and obtain its sum
  if transcript.is_some() {
    let transcript = transcript.unwrap();
    transcript.append_message(b"binding", &b);
    b = vec![];
    b.resize(64usize, 0u8);
    transcript.challenge_bytes(&mut b);
  }

The above would be what this would generally end up as, yet within the context of Monero TX signing, we'd be unable to use it for RNG (without having StateMachine take it as an argument which means Monero TX signing as a whole would need to, which I don't care to do) nor as a way to sequentially build inputs (due to the lack of DSTs for segments). I'd rather fix the potential bug (crafted commitments + message overlapping with the offset and and context) and call it a day.

@kayabaNerve
Copy link
Member Author

While working on fixing the existing bug, I decided to move Monero's context to be tag + length + value, and realized it would be helpful to have an API for this. I was going to write a crate which had Transcript... and then realized it'd just be a custom Merlin with the same usage limitations and with a customizable security level. Given the security and widespread adoption of 128-bit security, I'm just biting this bullet.

@kayabaNerve
Copy link
Member Author

kayabaNerve commented May 3, 2022

merlin doesn't work on big endian which is the nail in the coffin for me. Writing my own simple transcript format which will theoretically still be usable with Merlin as a backend.

kayabaNerve added a commit that referenced this issue May 3, 2022
@kayabaNerve
Copy link
Member Author

Closing for bf257b3

@kayabaNerve kayabaNerve added improvement This could be better and removed feature New feature or request labels Jun 5, 2022
@kayabaNerve
Copy link
Member Author

Retroactively updating this to the new labels. This was originally an improvement, not a new feature (no added functionality), even though it ended up being the transcript crate feature.

vrx00 referenced this issue in vrx00/serai Sep 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement This could be better
Projects
None yet
Development

No branches or pull requests

1 participant