Skip to content
This repository has been archived by the owner on Sep 4, 2024. It is now read-only.

ArconnectSigner should sign with saltLength 32 #75

Closed
jim-toth opened this issue Aug 8, 2023 · 1 comment · Fixed by #77
Closed

ArconnectSigner should sign with saltLength 32 #75

jim-toth opened this issue Aug 8, 2023 · 1 comment · Fixed by #77

Comments

@jim-toth
Copy link

jim-toth commented Aug 8, 2023

https://github.com/Bundlr-Network/arbundles/blob/master/src/signing/chains/arconnectSigner.ts#L31

tl;dr saltLength should be 32

This issue isn't obvious unless you try to sign "large" files. For example, attempting to sign a bundle of varying sized files (ranging from several kb to ~17mb), all data items except the 17mb were able to be verified. The 17mb file got a signature (i.e. the dummy id was replaced), but failed data item verification due to invalid signature. Changing saltLength to 32 fixes this. To reiterate, you have to test with a "large enough" file as saltLength 0 signed "small files" will have verifiable signatures.

One thing to note is that any non-zero saltLength means signing the same data twice in a row will result in different signatures, although both will be valid and verifiable. We have used saltLength 32 on art by city for ~1.5 years with no issue, creators have uploaded larger files than the example above.

Why doesn't saltLength 0 work for big files? I'm not sure, but I know that's what it is supposed to be 😄
I probably referenced arconnect or arweavejs internals to discover this.

Edit: here ya go https://github.com/ArweaveTeam/arweave-js/blob/997677dfe2ba722ac6abc3e075a8dd00313e4a6d/src/common/lib/crypto/webcrypto-driver.ts#L56

@JesseTheRobot
Copy link
Member

@jim-toth thanks for the bug report!
This is a weird issue for sure, I'll release a fix for this shortly.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants