Skip to content

fix: add preimage tags for domain separation of signatures#160

Open
cfm wants to merge 2 commits intomainfrom
159-message-tags
Open

fix: add preimage tags for domain separation of signatures#160
cfm wants to merge 2 commits intomainfrom
159-message-tags

Conversation

@cfm
Copy link
Member

@cfm cfm commented Jan 23, 2026

The manuscript does not specify domain-separating preimage tags. The Tamarin models, helpfully, do, so here we adopt those values.

@cfm cfm added this to the v0.3 milestone Jan 23, 2026
@cfm cfm requested a review from Copilot January 23, 2026 20:11
@cfm cfm added this to SecureDrop Jan 23, 2026
@cfm cfm moved this to In Progress in SecureDrop Jan 23, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the protocol specification to explicitly domain-separate signed messages using fixed message tags (matching the Tamarin model values).

Changes:

  • Updates the SIG building block to sign/verify tag ∥ m instead of just m.
  • Adds concrete domain-separation tags to signature inputs throughout the setup and key-fetch verification flows.
  • Updates the signature verification checks in the “Sender fetches keys” step to include the new tags.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@cfm cfm force-pushed the 159-message-tags branch 2 times, most recently from d63165f to 98dcdc2 Compare January 23, 2026 20:21
@cfm cfm moved this from In Progress to Ready For Review in SecureDrop Jan 23, 2026
@cfm cfm marked this pull request as ready for review January 23, 2026 20:22
@cfm cfm requested review from a team as code owners January 23, 2026 20:22
@cfm
Copy link
Member Author

cfm commented Jan 23, 2026

I believe this also addresses #53 by preventing type confusion between signed keys returned by the server.

@rocodes
Copy link
Contributor

rocodes commented Feb 2, 2026

Just because I think there's potential for message/$message$ confusion, at least for my brain: This PR addresses binding signatures (eg selfsig over journalist long-term keys, selfsig over journalist one-time keys, newsroom sig over journalist signing key) to the pubkey_bytes ($message$) they are signing, and it doesn't have anything to do with plaintext info between parties (which are also called "messages" but which use Hpke open() and seal(), which under the hood implement their own message tag computations/checks).

Even though it's true that signed_pubkey_bytes is a "message", it might be friendly to our future selves if we try to disambiguate this a bit in our terminology when we describe this step, so that it's highly clear what it's for.

About the comment on #53: I'd say the primary mitigation was the 0.2 change in which the clue/hint dh key have no relation to anything used in message encryption. (and in 0.3, where all pubkeys that are involved encryption/decryption are either signed, enclosed in authenc ciphertext, or committed to in authenc ciphertext). But I agree that separating keys by context has to be specified.

@cfm cfm moved this from Ready For Review to Under Review in SecureDrop Feb 2, 2026
@cfm cfm assigned cfm and unassigned rocodes Feb 3, 2026
@cfm cfm changed the title fix: add message tags for domain separation of signatures fix: add preimage tags for domain separation of signatures Feb 3, 2026
@cfm
Copy link
Member Author

cfm commented Feb 3, 2026

That's a good disambiguation. In e46a6d5 I've suggested (and documented) s/message/preimage/g specifically for the signature scheme. Let me know what you think.

@cfm cfm requested a review from rocodes February 3, 2026 20:36
@cfm cfm assigned rocodes and unassigned cfm Feb 3, 2026
@cfm cfm changed the base branch from b1e4d41-sync to main February 4, 2026 19:55
cfm added 2 commits February 4, 2026 13:37
The manuscript does not specify domain-separating message tags.  The
Tamarin models, helpfully, do, so here we adopt those values.
@cfm cfm force-pushed the 159-message-tags branch from e46a6d5 to 0abdd69 Compare February 4, 2026 21:37
@cfm
Copy link
Member Author

cfm commented Feb 4, 2026

Rebased from main after #156.

@nathandyer nathandyer moved this from Under Review to Blocked or Waiting in SecureDrop Feb 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Blocked or Waiting

Development

Successfully merging this pull request may close these issues.

signatures do not include message tags Vulnerability found: Key Replacement on Source Submission

3 participants