sbrl/systemquery
1
0
Fork 0
Code Issues Pull requests Releases Activity

Connection: Implement sequence numbering system to avoid replay attacks

Browse source
This commit is contained in:
Starbeamrainbowlabs 2022-02-12 01:53:31 +00:00
parent a01792d2d5
commit 3f26696056
Signed by: sbrl
GPG key ID: 1BE5172E637709C2
1 changed files with 25 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

30
src/lib/transport/Connection.mjs
View file

@ -45,6 +45,9 @@ class Connection extends EventEmitter {
this.rekey_interval = this.rekey_interval_base + crypto.randomInt(0, 15 * 60 * 1000);
this.rekey_in_progress = false;
this.sequence_count_receive = 0;
this.sequence_count_send = 0;
this.session_key = Buffer.from(secret_join, "base64");
}
@ -93,8 +96,13 @@ class Connection extends EventEmitter {
this.session_key = await rekey(this, this.session_key);
this.rekey_interval = this.rekey_interval_base + crypto.randomInt(0, 15 * 60 * 1000);
this.rekey_last = new Date();
// Also reset the sequence counters. This will help avoid integer overflow issues - however unlikely they may be
this.sequence_count_send = 0;
this.sequence_count_receive = 0;
/**
* The session key has been re-exchanged.
* This event is fired with the rekeying process
* is complete.
* @event Connection#rekey
* @type {void}
*/
@ -144,9 +152,21 @@ class Connection extends EventEmitter {
}
async handle_message(msg_text) {
// If this JSON.parse() call fails, we kill the connection 'cause the
// catch part of the above try..catch will trigger in this.handle_frame.
// This is very important, because if it didn't kill the connection
// we would no doubt be open to all sorts of unpleasant attacks
// given that we wouldn't have a chance to check the sequence number
// (see below).
const msg = JSON.parse(msg_text);
l.debug(`RECEIVE:${msg.event}`, msg.message);
l.debug(`RECEIVE ${msg.sequence}:${msg.event}`, msg.message);
if(msg.sequence !== this.sequence_count_receive) {
l.warn(`Killing connection due to invalid sequence number in received message: expected ${this.sequence_count_receive}, but got ${msg.sequence}.`);
this.destroy();
}
this.sequence_count_receive++;
if(msg.event == "rekey" && !this.rekey_in_progress) {
// Set and forget here
@ -178,12 +198,12 @@ class Connection extends EventEmitter {
/*
TODO: Consider anonymous TLS, with jpake for mututal authentication
TODO: Consider https://devdocs.io/node/crypto#crypto.createCipheriv() - which lets us use any openssl ciphers we like - e.g. ChaCha20-Poly1305
TODO: We're currently vulnerable to a replay attack. We need to mitigate this somehow - probably by maintaining a sequence number. Instead of sending the sequence number though we should instead compute a MAC that also includes the message length and a bunch of other things etc. Of course, we will also need to make sure we don't fall afoul of mac-then-encrypt, encrypt-then-mac, etc issues...
Ref https://www.rfc-editor.org/rfc/rfc4346#appendix-F.2 for the sequence counter in TLS
We do *not* need another manual MAC, as tweetnacl's secretbox uses xsalsa20-poly1305, which is an *authenticated* encryption algorithm. Thus, all we need do is prepend the plaintext with a sequence number.
Note here that we do *not* need another manual MAC to make this
authenticated, as tweetnacl's secretbox uses xsalsa20-poly1305,
which is an *authenticated* encryption algorithm.
*/
let payload = JSON.stringify({ event, message });
let payload = JSON.stringify({ event, message, sequence: this.sequence_count_send++ });
payload = encrypt_bytes(
this.session_key,
Buffer.from(payload, "utf-8")