Skip to content

Add HMACSHA512#4

Open
corrideat wants to merge 1 commit intomainfrom
hmacsha512
Open

Add HMACSHA512#4
corrideat wants to merge 1 commit intomainfrom
hmacsha512

Conversation

@corrideat
Copy link
Copy Markdown
Member

@corrideat corrideat commented Mar 8, 2026

This PR is part of the ephemeral messages series and adds a new key type, HMACSHA512, along with corresponding verifySignature and sign implementations.

Instead of having the app have low-level crypto code for implementing deletion tokens, we can use a new key type, which also has the advantage that deletion tokens can be handled as any other key (for example, for key rotations).

As for why choose SHA-512 and not some other algorithm (SHA-256, Blake, SHA-3 or a myriad of others), the main reason is that it's what tweetnacl provides as tweetnacl.hash.

@corrideat corrideat requested a review from taoeffect March 8, 2026 22:35
@corrideat corrideat self-assigned this Mar 8, 2026
@taoeffect
Copy link
Copy Markdown
Member

As for why choose SHA-512 and not some other algorithm (SHA-256, Blake, SHA-3 or a myriad of others), the main reason is that it's what tweetnacl provides as tweetnacl.hash.

So, that doesn't seem like a complete answer to that question. Sure, tweetnacl may use it, but we also are already using blake2 heavily in Shelter Protocol. So why should we add additional code to the codebase, thereby increasing the bundle size, and adding additional cognitive load through an extra decision added ("blake2 vs sha512?")?

@corrideat
Copy link
Copy Markdown
Member Author

Sure, tweetnacl may use it, but we also are already using blake2 heavily in Shelter Protocol.

Mostly for things that are identifiers.

So why should we add additional code to the codebase, thereby increasing the bundle size, and adding additional cognitive load through an extra decision added ("blake2 vs sha512?")?

This looks like it's referring to two different things.

  1. So why should we add additional code to the codebase, thereby increasing the bundle size

This part doesn't seem to deal with the choice of hash function.

The reason for this is to be able to use these primitives for generating tokens, and that they can be handled like regular Shelter code.

We do use blake2 in a few places in the codebase with ad-hoc constructions, but that's something that we probably should avoid, or at least not add more places of ad-hoc crypto. That makes things difficult to audit, difficult to write and difficult to maintain.

Now, the purpose of this code in particular (HMAC primitives) is to make computing tokens (see the ephemeral messages doc):

H = SHA-256(concat("token", user-secret_A, CONTRACT, KV-key = EXAMPLE_KEY_123))

Instead of having the app developer have to call SHA-256 (or blake2, or whatever), they can use this primitive, and instead of user-secret_A being an app-managed token that needs actions for updating, setting, etc., it can be a Chelonia key.

This would become something like:

H = HMAC(key: user-secret_A, value: concat("token", CONTRACT, KV-key = EXAMPLE_KEY_123))

and adding additional cognitive load through an extra decision added ("blake2 vs sha512?")?

The goal of this again is to reduce the cognitive load by just using @chelonia/crypto.

HMAC can be applied to any hash function. However, besides it being what tweetnacl provides, HMAC-SHA512 is much more common and easier to find an implementation for, as well as test vectors, etc. Since in this usage it doesn't really matter which function we go with, I'd go with the one tweetnacl provides.

@taoeffect
Copy link
Copy Markdown
Member

taoeffect commented Mar 9, 2026

The goal of this again is to reduce the cognitive load by just using @chelonia/crypto.

But we are not doing that. We are not "just using @chelonia/crypto" ourselves. We are using blake2 everywhere we use a hash function.

Therefore it would make sense to me to continue to rely on blake2 for all of our hash/HMAC needs.

This PR says, "No, let us use an additional hash function for this specific scenario".

Again - why? Why not just continue to use blake2? We could add that to @chelonia/crypto, right?

@corrideat
Copy link
Copy Markdown
Member Author

We are using blake2 everywhere we use a hash function.

Yes, but mostly that's been for generating identifiers. And, fortunately, not in a lot of places (I say fortunately because there shouldn't be much need for raw hashes in the app, or that should be the goal)

Therefore it would make sense to me to continue to rely on blake2 for all of our hash/HMAC need
This PR says, "No, let us use an additional hash function for this specific scenario".

That's not entirely true.

We use:

  • SHA-256 for scrypt (via scrypt-async)
  • SHA-512 literally on every SPMessage. The signing algorithm is called EDWARDS25519SHA512BATCH.

If you feel strongly about this, I can switch it up to Blake. But it wouldn't be my first choice because:

  • HMAC-SHA512 is ubiquitous and HMAC-Blake or HMAC-any-other-hash is seldom used.
  • We already rely on SHA-512. Using it for HMACs would make sense

@taoeffect
Copy link
Copy Markdown
Member

Thanks, that's what I was looking for. I "feel strongly" about decisions being well-thought out and having reasons behind them. Your latest comment helps me understand those reasons better, thanks for taking the time to explain. I think we can go with this. 👍

@corrideat
Copy link
Copy Markdown
Member Author

Awesome. In any case, let's not make a release of this just yet.

I'll try actually using it and see if it simplifies code the way I thought it would.

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.

2 participants