Encrypted UDP messages #39

telamon · 2024-10-15T23:45:47Z

In order to send private unordered messages (live-streams or updates / UDP-style)

I've lifted udx-stream's await send(buffer), trySend(buffer) and on('message', onmessage) API
as an encrypted variant.

I'm not sure about:

_onmessage(buffer) when handshake is pending, the buffer is pushed onto the eventqueue. Do I need to copy the message to free/detach it from udx's receive buffers?
in send and trySend what happens if buffer exceeds MTU?
in _onmessage(buffer) and _boxMessage(buffer) when handshake is awaited, what happens if handshake fails?

mafintosh · 2024-10-16T08:32:09Z

index.js

+    // initialize secretbox for unordered messages
+    this._boxSecret = b4a.allocUnsafe(32)
+    sodium.crypto_generichash(this._boxSecret, NS_BOX, handshakeHash)
+    this._boxNonce = b4a.allocUnsafe(sodium.crypto_secretbox_NONCEBYTES)


use allocUnsafeSlow for these long lived ones to avoid

mafintosh · 2024-10-16T08:33:19Z

index.js

@@ -500,6 +509,38 @@ module.exports = class NoiseSecretStream extends Duplex {
    cb(null)
  }

+  async _boxMessage (buffer) {
+    if (!this._boxSecret) await new Promise(resolve => this.once('handshake', resolve))


dont rely on events for this, i think we have a promise in the constructor you can await, otherwise lets add

actually see comment below about moving this part out

mafintosh · 2024-10-16T08:34:46Z

index.js

+    this._boxSecret = b4a.allocUnsafe(32)
+    sodium.crypto_generichash(this._boxSecret, NS_BOX, handshakeHash)
+    this._boxNonce = b4a.allocUnsafe(sodium.crypto_secretbox_NONCEBYTES)
+    sodium.randombytes_buf(this._boxNonce)


@chm-diederichs do we wanna use something more fancy here or is this ok you think? eats 24 bytes, but i am guessing the minimal we can do is 8 bytes (uint64) so mb this is worth it for simplicity

yeh we could just fix the first 24* bytes to be hash(NS_NONCE, handshakeHash) and then append the uint64 bytes, not much complexity in that so probably seems worth it?

*edit: first 16 bytes

you mean eat 32 bytes instead? would like to keep this as small as possible but obvs a trade off with complexity, the udp package is in practice capped at ~1kb so less metadata is better

i mean we do:

this._boxNonce = b4a.alloc(NONCEBYTES) this._activeNonce = this._boxNonce.subarray(NONCEBYTES - 8) sodium.crypto_generichash(this._boxNonce, NS_NONCE, handshakeHash) this._activeNonce.fill(0) // ... box message const envelope = b4a.alloc(8 + buffer.length + MACBYTES) // save 16 bytes envelope.set(this._activeNonce) sodium_increment(this._activeNonce)

that gives unique nonces per session and uint64_max messages (prob need to bound check the nonce as well..)

ah yes, ok, i was thinking it was just line in hc where we blind it, but this is also fine.

chiming in, hmm.. truncating the transmitted nonce to uint64 is good in 1K constraints.
But i get a feeling that if both ends use the same secret + nonce-interval we'll end up encrypting different messages with same nonce.

Maybe also replace _boxSecret with?:

this._boxEncrypt = b4a.allocUnsafeSlow(32) this._boxDecrypt = b4a.allocUnsafeSlow(32) sodium.crypto_generichash(this._boxEncrypt, this.isInitiator ? NS_BOX_INITIATOR : NS_BOX_RESPONDER, handshakeHash) sodium.crypto_generichash(this._boxDecrypt, this.isInitiator ? NS_BOX_RESPONDER : NS_BOX_INITIATOR, handshakeHash)

Honestly, i'm not sure what to do on a triggered bounds check, like attempt to reroll a new nonce on both sides?
If that operation was painless/safe we could further compress transmitted nonce to uint32

Good point, the initiator/responder nonces should be different. I think the easiest way is to just hash in this.publicKey as well, eg. sodium.crypto_generichash_batch(this._boxSecret, [NS_NONCE, this.publicKey], handshakeHash)

As for the bounds check, throwing an error is fine as we won't ever hit this in practice

mafintosh · 2024-10-16T08:36:27Z

index.js

+
+  async send (buffer) {
+    if (!this.rawStream?.send) return // udx-stream expected
+    const message = await this._boxMessage(buffer)


i would move the handshake wait here instead of in box message, ie

if (!opened) await whateverOpensIt

mafintosh · 2024-10-16T08:36:42Z

index.js

+
+  trySend (buffer) {
+    if (!this.rawStream?.trySend) return // udx-stream expected
+    this._boxMessage(buffer)


similar to above, if not opened, would just drop the message here, simpler

mafintosh · 2024-10-16T08:36:53Z

index.js

+  trySend (buffer) {
+    if (!this.rawStream?.trySend) return // udx-stream expected
+    this._boxMessage(buffer)
+      .then(message => this.rawStream.send(message))


should call trySend on rawStream

mafintosh · 2024-10-16T08:39:41Z

index.js

+  }
+
+  async _onmessage (buffer) {
+    if (!this._boxSecret) await new Promise(resolve => this.once('handshake', resolve))


would just drop it if the handshake isnt done and have this be sync (its 1 RT in practice so requires intense reordering to not be the case and even then its udp so we can do stuff like that)

Yes that simplifies things, i had a note somewhere that messages should be buffered. But as you say, it's UDP. Critical messages should go through write/data not send/message

mafintosh · 2024-10-16T08:40:34Z

test.js

+  const stream1 = u.createStream(1)
+  const stream2 = u.createStream(2)
+  stream1.connect(socket1, stream2.id, socket2.address().port, '127.0.0.1')
+  stream2.connect(socket2, stream1.id, socket1.address().port, '127.0.0.1')


there is a test handler you can import from udx also that does this for you if you want (makeStreamPair i think it is in test/helpers)

Yup found it, but i dosen't seem like the test/ folder is included in the npm-distribution. can't import

chm-diederichs · 2024-10-16T09:12:29Z

index.js

@@ -376,6 +378,12 @@ module.exports = class NoiseSecretStream extends Duplex {
    this.remotePublicKey = remotePublicKey
    this.handshakeHash = handshakeHash

+    // initialize secretbox for unordered messages
+    this._boxSecret = b4a.allocUnsafe(32)


One tiny nit, it's easier to review crypto if all the sodium calls are grouped like so:

this._boxSecret = b4a.alloc(..) this._boxNonce = b4a.alloc(..) sodium.crypto_generichash(this._boxSecret...) sodium.randombytes_buf(this._boxNonce)

in this case, it's quite straightforward so a newline before allocing the nonce would also work

- reduced nonce-overhead to 8bytes - reimplement api as sync - added nonce bounds check - fixed typo - documented message drop conditions

chm-diederichs · 2024-10-16T17:35:41Z

index.js

+    sodium.crypto_secretbox_easy(ciphertext, buffer, nonce, this._boxSecret)
+    envelope.set(prefix)
+
+    sodium.sodium_increment(prefix)


maybe move this to an _increment method that does the bounds check as well?

ahh nevermind actually, it's nicer as it is 🥳

mafintosh · 2024-10-17T09:24:28Z

index.js

+
+    sodium.crypto_generichash(this._boxSecret, NS_BOX, handshakeHash)
+    sodium.crypto_generichash(this._boxNonceEncrypt, b4a.concat([NS_BOX, publicKey]), handshakeHash)
+    sodium.crypto_generichash(this._boxNonceDecrypt, b4a.concat([NS_BOX, remotePublicKey]), handshakeHash)


normally we just use two namesapces for this, ie _INITIATOR and _RESPONDER and then use the isInitiator state to reflect that (this also works tho, but just divergence froom that pattern)

actually you dont need to store these, the rest of the nonce can just be 0 bytes - thats perfectly fine

mafintosh · 2024-10-17T10:03:05Z

index.js

+    sodium.crypto_generichash(this._boxNonceEncrypt, b4a.concat([NS_BOX, publicKey]), handshakeHash)
+    sodium.crypto_generichash(this._boxNonceDecrypt, b4a.concat([NS_BOX, remotePublicKey]), handshakeHash)
+    b4a.fill(this._boxNonceDecrypt, 0, 0, 8) // zerofill first 8 bytes
+    b4a.fill(this._boxNonceEncrypt, 0, 0, 8)


instead of resetting this to 0, should be just save the initial buffer and use that for the overflow check instead? less 0000 bits over the wire

cc @chm-diederichs on this also

mafintosh · 2024-10-17T10:05:15Z

index.js

+    this._boxNonceEncrypt = b4a.allocUnsafeSlow(sodium.crypto_secretbox_NONCEBYTES)
+    this._boxNonceDecrypt = b4a.allocUnsafeSlow(sodium.crypto_secretbox_NONCEBYTES)
+
+    sodium.crypto_generichash(this._boxSecret, NS_BOX, handshakeHash)


we should have two secret keys, one for each side, use the flow below to generate two

- separated boxSecret into encrypt and decrypt key - random initial 8B nonce + bound check - added test for >MTU transmissions

chm-diederichs · 2024-10-18T08:32:57Z

index.js

+    const inputs = [b4a.concat([NS_INITIATOR, NS_BOX]), b4a.concat([NS_RESPONDER, NS_BOX])]
+    if (!this.isInitiator) inputs.reverse()
+
+    sodium.crypto_generichash(this._boxSecret.subarray(0, 32), inputs[0], handshakeHash)


if you use crypto_generichash_batch here you don't have to concat the inputs, just pass the array directly

oh nice, yes fixing

mafintosh · 2024-10-18T08:35:37Z

index.js

+    this._boxSeq = b4a.allocUnsafeSlow(16) // increment<8>, initial<8>
+
+    const inputs = [b4a.concat([NS_INITIATOR, NS_BOX]), b4a.concat([NS_RESPONDER, NS_BOX])]
+    if (!this.isInitiator) inputs.reverse()


const inputs = this.isInitiator ? [...] : [...]

instead

mafintosh · 2024-10-18T08:37:11Z

index.js

@@ -388,6 +393,20 @@ module.exports = class NoiseSecretStream extends Duplex {
    this._rawStream.write(buf)
  }

+  _setupSecretBox (handshakeHash) {
+    this._boxSecret = b4a.allocUnsafeSlow(64) // encrypt<32>, decrypt<32>
+    this._boxSeq = b4a.allocUnsafeSlow(16) // increment<8>, initial<8>


alloc both in one go, and just store the subarrays

mafintosh · 2024-10-18T08:38:39Z

index.js

+    const nonce = envelope.subarray(0, NB)
+    const ciphertext = envelope.subarray(8)
+
+    nonce.set(this.remotePublicKey.subarray(0, NB)) // pad suffix


just pad with zeros is fine

index.js

- renamed _boxSecret & _boxSeq => _sendState - renamed prefix|increment => counter - zero padded nonces - used hash_batch instead of concats

telamon · 2024-10-18T11:52:33Z

index.js

@@ -379,6 +380,9 @@ module.exports = class NoiseSecretStream extends Duplex {
    const id = buf.subarray(3, 3 + 32)
    streamId(handshakeHash, this.isInitiator, id)

+    // initialize secretbox state for unordered messages
+    this._setupSecretSend(handshakeHash)


self-nitpick, this function doesn't technically have to be a class member anymore:

this._sendState = initializeSend(handshakeHash)

but the current version is probably easier on the eyes.

mafintosh · 2024-10-21T10:06:00Z

index.js

+    const counter = this._sendState.subarray(64, 72)
+    sodium.sodium_increment(counter)
+    if (b4a.equals(counter, this._sendState.subarray(72))) {
+      this.destroy(new Error('udp send nonce exchausted'))


missing a return after this line

good catch..

index.js

test.js

index.js

Exposed udx-stream.send/onmessage API with secretbox encryption

8351cae

mafintosh reviewed Oct 16, 2024

View reviewed changes

chm-diederichs reviewed Oct 16, 2024

View reviewed changes

Fixed nonce size and removed async calls

dd90ba9

- reduced nonce-overhead to 8bytes - reimplement api as sync - added nonce bounds check - fixed typo - documented message drop conditions

chm-diederichs reviewed Oct 16, 2024

View reviewed changes

mafintosh reviewed Oct 17, 2024

View reviewed changes

Redesigned secrets and nonce

b09b865

- separated boxSecret into encrypt and decrypt key - random initial 8B nonce + bound check - added test for >MTU transmissions

chm-diederichs reviewed Oct 18, 2024

View reviewed changes

mafintosh reviewed Oct 18, 2024

View reviewed changes

index.js Show resolved Hide resolved

telamon added 2 commits October 18, 2024 11:50

Tweaked nonce exhausted case, removed fragmentation test

7fbbfcf

Refactored state and names

c8df337

- renamed _boxSecret & _boxSeq => _sendState - renamed prefix|increment => counter - zero padded nonces - used hash_batch instead of concats

telamon commented Oct 18, 2024

View reviewed changes

telamon self-assigned this Oct 21, 2024

telamon merged commit 9a26004 into main Oct 21, 2024
4 checks passed

mafintosh reviewed Oct 21, 2024

View reviewed changes

index.js Show resolved Hide resolved

mafintosh reviewed Oct 21, 2024

View reviewed changes

index.js Show resolved Hide resolved

mafintosh reviewed Oct 21, 2024

View reviewed changes

test.js Show resolved Hide resolved

mafintosh reviewed Oct 21, 2024

View reviewed changes

index.js Show resolved Hide resolved

Encrypted UDP messages #39

Encrypted UDP messages #39

Conversation

telamon commented Oct 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chm-diederichs Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chm-diederichs Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chm-diederichs Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chm-diederichs Oct 16, 2024 •

edited

Loading

chm-diederichs Oct 16, 2024 •

edited

Loading

chm-diederichs Oct 16, 2024 •

edited

Loading