Skip to content

Commit

Permalink
Fix Node encryption of ArrayBuffer plaintext
Browse files Browse the repository at this point in the history 
convert plaintext to Buffer before encrypting in Node

Buffer.concat doesn’t handle ArrayBuffer (TODO explain how an
ArrayBuffer might find its way here, using the same logic as how we
ended up at the InputPlaintext type)

without fix, this test fails with

> TypeError: Cannot read properties of undefined (reading 'length')

TODO understand what was going on with encryption causing mutation and
whether that’s something that I did?

Resolves #TODO
  • Loading branch information
@lawrence-forooghian
lawrence-forooghian committed May 18, 2023
1 parent 54a9fc5 commit fa9fd2e
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 18 deletions.
7 changes: 5 additions & 2 deletions src/platform/nodejs/lib/util/crypto.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,13 @@ var Crypto = (function () {

CBCCipher.prototype.encrypt = function (plaintext, callback) {
Logger.logAction(Logger.LOG_MICRO, 'CBCCipher.encrypt()', '');
var plaintextLength = plaintext.length,
var plaintextBuffer = Platform.BufferUtils.toBuffer(plaintext);
var plaintextLength = plaintextBuffer.length,
paddedLength = getPaddedLength(plaintextLength),
iv = this.getIv();
var cipherOut = this.encryptCipher.update(Buffer.concat([plaintext, pkcs5Padding[paddedLength - plaintextLength]]));
var cipherOut = this.encryptCipher.update(
Buffer.concat([plaintextBuffer, pkcs5Padding[paddedLength - plaintextLength]])
);
var ciphertext = Buffer.concat([iv, toBuffer(cipherOut)]);
return callback(null, ciphertext);
};
Expand Down
59 changes: 43 additions & 16 deletions test/realtime/crypto.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,13 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async
var item = testData.items[i];

/* read messages from test data and decode (ie remove any base64 encoding). */
var testMessage = Message.fromEncoded(item.encoded);
var createTestMessage = function () {
return Message.fromEncoded(item.encoded);
};
var encryptedMessage = Message.fromEncoded(item.encrypted);
/* reset channel cipher, to ensure it uses the given iv */
channel.setOptions({ cipher: { key: key, iv: iv } });
fixtureTest(channel.channelOptions, testMessage, encryptedMessage, item.msgpack);
fixtureTest(channel.channelOptions, createTestMessage, encryptedMessage, item.msgpack);
}
} catch (err) {
closeAndFinish(done, realtime, err);
Expand All @@ -85,6 +87,21 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async
});
}

function testEachPlaintextVariant(createTestMessage, fixtureTest) {
fixtureTest(createTestMessage);

var testMessage = createTestMessage();
if (BufferUtils.isBuffer(testMessage.data) && !(testMessage.data instanceof ArrayBuffer)) {
var createTestMessageWithArrayBufferData = function () {
var testMessageWithArrayBufferData = createTestMessage();
testMessageWithArrayBufferData.data = BufferUtils.toArrayBuffer(testMessageWithArrayBufferData.data);
return testMessageWithArrayBufferData;
};

fixtureTest(createTestMessageWithArrayBufferData);
}
}

describe('realtime/crypto', function () {
this.timeout(60 * 1000);

Expand Down Expand Up @@ -204,11 +221,14 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async
'crypto-data-128.json',
'encrypt_message_128',
2,
function (channelOpts, testMessage, encryptedMessage) {
/* encrypt plaintext message; encode() also to handle data that is not already string or buffer */
Message.encode(testMessage, channelOpts, function () {
/* compare */
testMessageEquality(done, testMessage, encryptedMessage);
function (channelOpts, createTestMessage, encryptedMessage) {
testEachPlaintextVariant(createTestMessage, function (createTestMessage) {
var testMessage = createTestMessage();
/* encrypt plaintext message; encode() also to handle data that is not already string or buffer */
Message.encode(testMessage, channelOpts, function () {
/* compare */
testMessageEquality(done, testMessage, encryptedMessage);
});
});
}
);
Expand All @@ -220,11 +240,14 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async
'crypto-data-256.json',
'encrypt_message_256',
2,
function (channelOpts, testMessage, encryptedMessage) {
/* encrypt plaintext message; encode() also to handle data that is not already string or buffer */
Message.encode(testMessage, channelOpts, function () {
/* compare */
testMessageEquality(done, testMessage, encryptedMessage);
function (channelOpts, createTestMessage, encryptedMessage) {
testEachPlaintextVariant(createTestMessage, function (createTestMessage) {
var testMessage = createTestMessage();
/* encrypt plaintext message; encode() also to handle data that is not already string or buffer */
Message.encode(testMessage, channelOpts, function () {
/* compare */
testMessageEquality(done, testMessage, encryptedMessage);
});
});
}
);
Expand All @@ -236,7 +259,8 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async
'crypto-data-128.json',
'decrypt_message_128',
2,
function (channelOpts, testMessage, encryptedMessage) {
function (channelOpts, createTestMessage, encryptedMessage) {
var testMessage = createTestMessage();
/* decrypt encrypted message; decode() also to handle data that is not string or buffer */
Message.decode(encryptedMessage, channelOpts);
/* compare */
Expand All @@ -251,7 +275,8 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async
'crypto-data-256.json',
'decrypt_message_256',
2,
function (channelOpts, testMessage, encryptedMessage) {
function (channelOpts, createTestMessage, encryptedMessage) {
var testMessage = createTestMessage();
/* decrypt encrypted message; decode() also to handle data that is not string or buffer */
Message.decode(encryptedMessage, channelOpts);
/* compare */
Expand Down Expand Up @@ -292,7 +317,8 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async
'crypto-data-128.json',
'msgpack_128',
2,
function (channelOpts, testMessage, encryptedMessage, msgpackEncodedMessage) {
function (channelOpts, createTestMessage, encryptedMessage, msgpackEncodedMessage) {
var testMessage = createTestMessage();
Message.encode(testMessage, channelOpts, function () {
var msgpackFromEncoded = msgpack.encode(testMessage);
var msgpackFromEncrypted = msgpack.encode(encryptedMessage);
Expand Down Expand Up @@ -325,7 +351,8 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async
'crypto-data-256.json',
'msgpack_256',
2,
function (channelOpts, testMessage, encryptedMessage, msgpackEncodedMessage) {
function (channelOpts, createTestMessage, encryptedMessage, msgpackEncodedMessage) {
var testMessage = createTestMessage();
Message.encode(testMessage, channelOpts, function () {
var msgpackFromEncoded = msgpack.encode(testMessage);
var msgpackFromEncrypted = msgpack.encode(encryptedMessage);
Expand Down

0 comments on commit fa9fd2e

Please sign in to comment.