aead is non deterministic w/ certain parameters about bcrypto HOT 5 CLOSED

The padding is specified here: https://tools.ietf.org/html/rfc7539#section-2.8.1

I believe it's implemented properly. If there is a problem here, it probably has something to do with the lazy computation of the aad padding (the C code is something like a port of a port of a port of my original bip151 code, so it could probably use some maintenance).

Anyway, code to reproduce this would be very helpful.

from bcrypto.

Apologies for my delayed response with the test. I cleaned up the tests I was running last night and compiled them into this file here: https://github.com/kilpatty/hsd/blob/brontide-cipher-test/test/brontide-test.js#L19

I think your implementation of the padding is correct, just something is lingering around to make the next tag returned from encryption different, even if it's a different object in JS with the same parameters.

I can dive into this more tonight and see if I can figure out what is causing this exactly.

from bcrypto.

Did some more digging on this today, I couldn't find anything in your implementation of AEAD that would cause this issue.

I think what might be happening is an issue with Buffer reuse in Node - when I reuse the const buffer HELLO, this issue occurs. When I instead create two new buffers, then it works as intended.

Attempting to reprint HELLO to string after the first encryption attempt shows that it no longer prints to it's normal text. I don't think this is actually a big issue because A. I didn't notice any corruption occurring if we replace HELLO with EMPTY, and B. we aren't sharing buffers that many places in the Brontide code.

I'm going to close this now on the basis that I think it's entirely on that Buffer reuse issue. I'm not sure if that's something we should investigate further, as I think it's just isolated to tests at the moment.

from bcrypto.

@kilpatty, ah, I should have mentioned: all of the stream ciphers in bcrypto encrypt the buffer in-place. This isn't typical of js apis, but it's done for extra speed. This might be the cause of the issue. Maybe this feature should be documented more clearly (?).

from bcrypto.

Ah my mistake -> not sure how I didn't see that in my testing earlier. Noted for the future, happy to work on some docs for that.

from bcrypto.