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

Spec & code disagree on construction of "H" #31

Open
snej opened this issue Feb 8, 2022 · 0 comments · May be fixed by dominictarr/secret-handshake-paper#11
Open

Spec & code disagree on construction of "H" #31

snej opened this issue Feb 8, 2022 · 0 comments · May be fixed by dominictarr/secret-handshake-paper#11

Comments

@snej
Copy link

snej commented Feb 8, 2022

I'm implementing SHS in C++ by following the spec/paper, but I'm also looking at the JS code here and the C code in shs1-c. I just noticed a discrepancy:

  • The spec defines: H = Ap | sign{A}(K|Bp|hash(a · b)) (p.11)
  • But the code here and in shs1.c concatenates A's public key (Ap) at the end:

state.local.hello = Buffer.concat([sig, state.local.publicKey])

Obviously one is not better than the other. But as the latter is what's actually in use, it would be a good idea to update the actual spec to match. (And a footnote could be added to mention the already-known discrepancy in the server challenge, i.e. #7.)

ahdinosaur added a commit to ahdinosaur/secret-handshake-paper that referenced this issue Nov 30, 2023
The paper defines: `H = Ap | sign{A}(K|Bp|hash(a · b))` (p.11)

The reference implementation concatenates A's public key (Ap) at the _end_.

This changes the paper to match the reference implementation.

Fixes auditdrivencrypto/secret-handshake#31
ahdinosaur added a commit to ahdinosaur/secret-handshake-paper that referenced this issue Nov 30, 2023
The paper defines: `H = Ap | sign{A}(K|Bp|hash(a · b))` (p.11)

The reference implementation concatenates A's public key (Ap) at the _end_.

This changes the paper to match the reference implementation.

Fixes auditdrivencrypto/secret-handshake#31
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 a pull request may close this issue.

1 participant