protocol/crypto: fix CBC decrypting in binding

Fixes: #1061
mscdex · Sep 3, 2021 · 56fd3de · 56fd3de
1 parent 918eb6d
commit 56fd3de
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 46 deletions.
diff --git a/lib/protocol/crypto.js b/lib/protocol/crypto.js
@@ -1334,7 +1334,7 @@ class GenericDecipherBinding {
           this._len = need = readUInt32BE(this._block, 0);
         } else {
           // Decrypt first block to get packet length
-          this._instance.decryptBlock(this._block, this.inSeqno);
+          this._instance.decryptBlock(this._block);
           this._len = readUInt32BE(this._block, 0);
           need = 4 + this._len - this._block.length;
         }
@@ -1346,35 +1346,15 @@ class GenericDecipherBinding {
         }
 
         if (!this._macETM) {
-          const pktStart = (this._block.length - 4);
-          const startP = p - pktStart;
-          let endP;
-          if (p >= pktStart && (endP = startP + this._len) <= dataLen) {
-            // The entire packet exists within the current chunk, with the
-            // first block already decrypted
-            if (startP === 0 && endP === dataLen) {
-              this._packet = data;
-              this._pktLen = this._len;
-            } else {
-              this._packet = new FastBuffer(
-                data.buffer,
-                data.byteOffset + startP,
-                this._len
-              );
-              this._pktLen = this._len;
-            }
-            p = endP;
-          } else {
-            this._pktLen = pktStart;
-            if (this._pktLen) {
-              this._packet = Buffer.allocUnsafe(this._len);
-              this._packet.set(
-                new Uint8Array(this._block.buffer,
-                               this._block.byteOffset + 4,
-                               this._pktLen),
-                0
-              );
-            }
+          this._pktLen = (this._block.length - 4);
+          if (this._pktLen) {
+            this._packet = Buffer.allocUnsafe(this._len);
+            this._packet.set(
+              new Uint8Array(this._block.buffer,
+                             this._block.byteOffset + 4,
+                             this._pktLen),
+              0
+            );
           }
         }
 

diff --git a/lib/protocol/crypto/src/binding.cc b/lib/protocol/crypto/src/binding.cc
@@ -1687,19 +1687,11 @@ class GenericDecipher : public ObjectWrap {
     return r;
   }
 
-  ErrorType decrypt_block(unsigned char* data,
-                          uint32_t data_len,
-                          uint32_t seqno) {
+  ErrorType decrypt_block(unsigned char* data, uint32_t data_len) {
     ErrorType r = kErrNone;
 
     int outlen;
 
-    uint8_t seqbuf[4] = {0};
-    ((uint8_t*)(seqbuf))[0] = (seqno >> 24) & 0xff;
-    ((uint8_t*)(seqbuf))[1] = (seqno >> 16) & 0xff;
-    ((uint8_t*)(seqbuf))[2] = (seqno >> 8) & 0xff;
-    ((uint8_t*)(seqbuf))[3] = seqno & 0xff;
-
     if (!is_stream_ && data_len != block_size_) {
       r = kErrBadBlockLen;
       goto out;
@@ -1749,7 +1741,7 @@ class GenericDecipher : public ObjectWrap {
         unsigned int outlen = hmac_len_;
         if (HMAC_Init_ex(ctx_hmac_, nullptr, 0, nullptr, nullptr) != 1
             || HMAC_Update(ctx_hmac_, seqbuf, sizeof(seqbuf)) != 1
-            || HMAC_Update(ctx_hmac_, first_block, first_block_len) != 1
+            || HMAC_Update(ctx_hmac_, first_block, 4) != 1
             || HMAC_Update(ctx_hmac_, packet, packet_len) != 1
             || HMAC_Final(ctx_hmac_, calc_mac, &outlen) != 1) {
           r = kErrOpenSSL;
@@ -1804,7 +1796,7 @@ class GenericDecipher : public ObjectWrap {
         unsigned int outlen = hmac_len_;
         if (HMAC_Init_ex(ctx_hmac_, nullptr, 0, nullptr, nullptr) != 1
             || HMAC_Update(ctx_hmac_, seqbuf, sizeof(seqbuf)) != 1
-            || HMAC_Update(ctx_hmac_, first_block, first_block_len) != 1
+            || HMAC_Update(ctx_hmac_, first_block, 4) != 1
             || HMAC_Update(ctx_hmac_, packet, packet_len) != 1
             || HMAC_Final(ctx_hmac_, calc_mac, &outlen) != 1) {
           r = kErrOpenSSL;
@@ -1908,13 +1900,9 @@ class GenericDecipher : public ObjectWrap {
     if (!Buffer::HasInstance(info[0]))
       return Nan::ThrowTypeError("Missing/Invalid block");
 
-    if (!info[1]->IsUint32())
-      return Nan::ThrowTypeError("Missing/Invalid sequence number");
-
     ErrorType r = obj->decrypt_block(
       reinterpret_cast<unsigned char*>(Buffer::Data(info[0])),
-      Buffer::Length(info[0]),
-      Nan::To<uint32_t>(info[1]).FromJust()
+      Buffer::Length(info[0])
     );
     switch (r) {
       case kErrNone:
@@ -1969,6 +1957,8 @@ class GenericDecipher : public ObjectWrap {
         return Nan::ThrowError("Failed to completely decrypt packet");
       case kErrBadHMACLen:
         return Nan::ThrowError("Unexpected HMAC length");
+      case kErrInvalidMAC:
+        return Nan::ThrowError("Invalid MAC");
       case kErrOpenSSL: {
         char msg_buf[128] = {0};
         ERR_error_string_n(ERR_get_error(), msg_buf, sizeof(msg_buf));