Skip to content

fix!: verify the challenge signature over a prefixed payload - #34

Merged
zippy merged 2 commits into
mainfrom
fix/domain-separated-challenge
Oct 13, 2026
Merged

zippy merged 2 commits into
mainfrom
fix/domain-separated-challenge

Conversation

@zippy

@zippy zippy commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

/authenticate now verifies the signature over hc-auth-challenge-v1: followed by the decoded /now payload, and refuses a signature over the bare payload.

Why

A client signs the nonce /now payload with its Holochain agent key. Signing any payload is broader scope than needed for the authentication handshake. The prefix makes the signature useless for anything except this handshake.

Breaking

A client that signs the bare payload now gets 401. There is no transition path that accepts both forms. Matching client change is in holochain/runtime-tauri (branch fix/hc-auth-domain-separated-challenge).

Changes

  • routes_client.rs: CHALLENGE_SIGNING_PREFIX, challenge_signing_bytes, and the verify call. Unit tests for the prefixed form, the bare form, a wrong length, and a wrong key.
  • examples/client.rs: signs the prefixed form.
  • README.md: states what the signature must cover.

AI disclosure

The code, tests, commit message were written by an LLM and reviewd by me. This PR was drafted by an LLM and hand edited for brevity and clarity.

@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 33be8f65-f35d-404f-8f46-22c67472c823
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@zippy
zippy requested a review from jost-s October 5, 2026 17:11
@zippy
zippy marked this pull request as ready for review October 5, 2026 17:12
@mattyg
mattyg requested a review from a team October 5, 2026 17:21

@mattyg mattyg left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand this. Why is it valuable to add a prefix hc-auth-challenge-v1 to the challenge? It seems functionally unchanged.

`/authenticate` now verifies the signature over `hc-auth-challenge-v1:`
followed by the decoded `/now` payload, so the signature is valid for this
purpose only. A signature over the bare payload is refused, so clients
must sign the prefixed form.
@zippy
zippy force-pushed the fix/domain-separated-challenge branch from 11a2c78 to dd2944f Compare October 5, 2026 17:34
@zippy

zippy commented Oct 5, 2026 •

Copy link
Copy Markdown
Member Author

I don't understand this. Why is it valuable to add a prefix hc-auth-challenge-v1 to the challenge? It seems functionally unchanged.

Because otherwise you can get the client to sign anything with their agent-key. This make what they are signing match only the purpose of this challenge. E.g. a compromised auth-server could get an agent to sign what ever it wanted (e.g. an action). This is a "security-in-depth" addition, low likelyhood.

@zippy
zippy requested a review from mattyg October 5, 2026 18:30
@mattyg

mattyg commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

Got it, so the client inserts the prefix itself before signing.

I would like to avoid having the prefix magic string manually defined in multiple projects that need to include an auth client. But also we don't want them to need to import this whole crate. Could we split out a 'client' or 'types' crate that includes the magic string and functions to sign a payload with it? Then the runtime-tauri can import it.

@jost-s jost-s left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Logic looks good

A client can now depend on hc-auth-types, which has no dependencies,
and does not need its own copy of the prefix.

challenge_signing_bytes takes a 32 byte array, so the type refuses a
challenge of any other length.
@cocogitto-bot

cocogitto-bot Bot commented Oct 8, 2026

Copy link
Copy Markdown

✔️ dd2944f...5e2204c - Conventional commits check succeeded.

zippy added a commit to holochain/runtime-tauri that referenced this pull request Oct 8, 2026
The prefix and the challenge length now come from the hc-auth-types
crate in holochain/hc-auth-server, so this repo has no copy of them.

The crate is not on crates.io. The git rev is the head of
holochain/hc-auth-server#34 and must move to the merge commit.
@zippy
zippy requested a review from jost-s October 9, 2026 01:12
zippy added a commit to holochain/runtime-tauri that referenced this pull request Oct 9, 2026
The prefix and the challenge length now come from the hc-auth-types
crate in holochain/hc-auth-server, so this repo has no copy of them.

The crate is not on crates.io. The git rev is the head of
holochain/hc-auth-server#34 and must move to the merge commit.
@zippy
zippy merged commit dfd4045 into main Oct 13, 2026
5 checks passed
@zippy
zippy deleted the fix/domain-separated-challenge branch October 13, 2026 15:04
jost-s pushed a commit to holochain/runtime-tauri that referenced this pull request Oct 15, 2026
The prefix and the challenge length now come from the hc-auth-types
crate in holochain/hc-auth-server, so this repo has no copy of them.

The crate is not on crates.io. The git rev is the head of
holochain/hc-auth-server#34 and must move to the merge commit.
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.

3 participants