Skip to content

Commit

Permalink
Don't promote archived sessions, more logging on error
Browse files Browse the repository at this point in the history
  • Loading branch information
@scottnonnenberg-signal
scottnonnenberg-signal authored Dec 9, 2020
1 parent 1098e59 commit 5369950
Show file tree
Hide file tree
Showing 7 changed files with 161 additions and 48 deletions.
48 changes: 42 additions & 6 deletions js/signal_protocol_store.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@

try {
const id = await normalizeEncodedAddress(encodedAddress);
window.log.info('loadSession', { encodedAddress, id });
const session = this.sessions[id];

if (session) {
Expand All @@ -365,6 +366,7 @@

try {
const id = await normalizeEncodedAddress(encodedAddress);
window.log.info('storeSession', { encodedAddress, id });

const data = {
id,
Expand Down Expand Up @@ -393,18 +395,34 @@
const sessions = allSessions.filter(
session => session.conversationId === id
);
const openSessions = await Promise.all(
sessions.map(async session => {
const sessionCipher = new libsignal.SessionCipher(
textsecure.storage.protocol,
session.id
);

return _.pluck(sessions, 'deviceId');
} catch (e) {
const hasOpenSession = await sessionCipher.hasOpenSession();
if (hasOpenSession) {
return session;
}

return undefined;
})
);

return openSessions.filter(Boolean).map(item => item.deviceId);
} catch (error) {
window.log.error(
`could not get device ids for identifier ${identifier}`
`could not get device ids for identifier ${identifier}`,
error && error.stack ? error.stack : error
);
}

return [];
},
async removeSession(encodedAddress) {
window.log.info('deleting session for ', encodedAddress);
window.log.info('removeSession: deleting session for', encodedAddress);
try {
const id = await normalizeEncodedAddress(encodedAddress);
delete this.sessions[id];
Expand All @@ -418,6 +436,8 @@
throw new Error('Tried to remove sessions for undefined/null number');
}

window.log.info('removeAllSessions: deleting sessions for', identifier);

const id = ConversationController.getConversationId(identifier);

const allSessions = Object.values(this.sessions);
Expand All @@ -432,6 +452,11 @@
await window.Signal.Data.removeSessionsByConversation(identifier);
},
async archiveSiblingSessions(identifier) {
window.log.info(
'archiveSiblingSessions: archiving sibling sessions for',
identifier
);

const address = libsignal.SignalProtocolAddress.fromString(identifier);

const deviceIds = await this.getDeviceIds(address.getName());
Expand All @@ -443,7 +468,10 @@
address.getName(),
deviceId
);
window.log.info('closing session for', sibling.toString());
window.log.info(
'archiveSiblingSessions: closing session for',
sibling.toString()
);
const sessionCipher = new libsignal.SessionCipher(
textsecure.storage.protocol,
sibling
Expand All @@ -453,6 +481,11 @@
);
},
async archiveAllSessions(identifier) {
window.log.info(
'archiveAllSessions: archiving all sessions for',
identifier
);

const deviceIds = await this.getDeviceIds(identifier);

await Promise.all(
Expand All @@ -461,7 +494,10 @@
identifier,
deviceId
);
window.log.info('closing session for', address.toString());
window.log.info(
'archiveAllSessions: closing session for',
address.toString()
);
const sessionCipher = new libsignal.SessionCipher(
textsecure.storage.protocol,
address
Expand Down
76 changes: 59 additions & 17 deletions libtextsecure/libsignal-protocol.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24840,7 +24840,14 @@ SessionCipher.prototype = {
if (sessionList.length === 0) {
var error = errors[0];
if (!error) {
error = new Error('decryptWithSessionList: list is empty, but no errors in array');
error = new Error('decryptWithSessionList: list is empty, but no errors in array');
}
if (errors.length > 1) {
errors.forEach((item, index) => {
var stackString = error && error.stack ? error.stack : error;
var extraString = error && error.extra ? JSON.stringify(error.extra) : '';
console.error(`decryptWithSessionList: Error at index ${index}: ${extraString} ${stackString}`);
});
}
return Promise.reject(error);
}
Expand All @@ -24865,11 +24872,17 @@ SessionCipher.prototype = {
if (!record) {
throw new Error("No record for device " + address);
}

// Only used for printing out debug information when errors happen
var messageProto = buffer.slice(1, buffer.byteLength - 8);
var message = Internal.protobuf.WhisperMessage.decode(messageProto);
var byEphemeralKey = record.getSessionByRemoteEphemeralKey(util.toString(message.ephemeralKey));

var errors = [];
return this.decryptWithSessionList(buffer, record.getSessions(), errors).then(function(result) {
return this.getRecord(address).then(function(record) {
var openSession = record.getOpenSession();
if (!openSession || result.session.indexInfo.baseKey !== openSession.indexInfo.baseKey) {
if (!openSession) {
record.archiveCurrentState();
record.promoteState(result.session);
}
Expand All @@ -24889,7 +24902,36 @@ SessionCipher.prototype = {
});
}.bind(this));
}.bind(this));
}.bind(this));
}.bind(this)).catch(function(error) {
try {
error.extra = error.extra || {};
error.extra.foundMatchingSession = Boolean(byEphemeralKey);

if (byEphemeralKey) {
var receivingChainInfo = {};
var entries = Object.entries(byEphemeralKey);

entries.forEach(([key, item]) => {
if (item && item.chainType === Internal.ChainType.RECEIVING && item.chainKey) {
var hexKey = dcodeIO.ByteBuffer.wrap(key, 'binary').toString('hex');
receivingChainInfo[hexKey] = {
counter: item.chainKey.counter,
key: item.chainKey.key ? dcodeIO.ByteBuffer.wrap(item.chainKey.key, 'binary').toString('hex') : null,
};
}
});

error.extra.receivingChainInfo = receivingChainInfo;
}
} catch (innerError) {
console.error(
'decryptWhisperMessage: Problem collecting extra information:',
innerError && innerError.stack ? innerError.stack : innerError
);
}

throw error;
});
}.bind(this));
}.bind(this));
},
Expand Down Expand Up @@ -24946,7 +24988,12 @@ SessionCipher.prototype = {
var remoteEphemeralKey = message.ephemeralKey.toArrayBuffer();

if (session === undefined) {
return Promise.reject(new Error("No session found to decrypt message from " + this.remoteAddress.toString()));
var error = new Error('No session found to decrypt message from ' + this.remoteAddress.toString());
error.extra = {
messageCounter: message.counter,
ratchetKey: message.ephemeralKey.toString('hex'),
};
return Promise.reject(error);
}
if (session.indexInfo.closed != -1) {
console.log('decrypting message for closed session');
Expand All @@ -24961,7 +25008,7 @@ SessionCipher.prototype = {
return this.fillMessageKeys(chain, message.counter).then(function() {
var messageKey = chain.messageKeys[message.counter];
if (messageKey === undefined) {
var e = new Error("Message key not found. The counter was repeated or the key was not filled.");
var e = new Error(`Message key not found. Counter ${message.counter} was repeated or the key was not filled.`);
e.name = 'MessageCounterError';
throw e;
}
Expand All @@ -24979,24 +25026,19 @@ SessionCipher.prototype = {
macInput[33*2] = (3 << 4) | 3;
macInput.set(new Uint8Array(messageProto), 33*2 + 1);

return Internal.verifyMAC(macInput.buffer, keys[1], mac, 8).catch(function(error) {
function logArrayBuffer(name, arrayBuffer) {
console.log('Bad MAC: ' + name + ' - truthy: ' + Boolean(arrayBuffer) +', length: ' + (arrayBuffer ? arrayBuffer.byteLength : 'NaN'));
}

logArrayBuffer('ourPubKey', ourPubKey);
logArrayBuffer('remoteIdentityKey', remoteIdentityKey);
logArrayBuffer('messageProto', messageProto);
logArrayBuffer('mac', mac);

throw error;
});
return Internal.verifyMAC(macInput.buffer, keys[1], mac, 8);
}.bind(this)).then(function() {
return Internal.crypto.decrypt(keys[0], message.ciphertext.toArrayBuffer(), keys[2].slice(0, 16));
});
}.bind(this)).then(function(plaintext) {
delete session.pendingPreKey;
return plaintext;
}).catch(function(error) {
error.extra = {
messageCounter: message.counter,
ratchetKey: message.ephemeralKey.toString('hex'),
};
throw error;
});
},
fillMessageKeys: function(chain, counter) {
Expand Down
32 changes: 27 additions & 5 deletions test/storage_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -975,17 +975,39 @@ describe('SignalProtocolStore', () => {
});
describe('getDeviceIds', () => {
it('returns deviceIds for a number', async () => {
const testRecord = 'an opaque string';
const devices = [1, 2, 3, 10].map(deviceId => {
const openRecord = JSON.stringify({
version: 'v1',
sessions: {
ephemeralKey: {
registrationId: 25,
indexInfo: {
closed: -1,
},
},
},
});
const openDevices = [1, 2, 3, 10].map(deviceId => {
return [number, deviceId].join('.');
});

await Promise.all(
devices.map(async encodedNumber => {
await store.storeSession(encodedNumber, testRecord + encodedNumber);
openDevices.map(async encodedNumber => {
await store.storeSession(encodedNumber, openRecord);
})
);

const closedRecord = JSON.stringify({
version: 'v1',
sessions: {
ephemeralKey: {
registrationId: 24,
indexInfo: {
closed: Date.now(),
},
},
},
});
await store.storeSession([number, 11].join('.'), closedRecord);

const deviceIds = await store.getDeviceIds(number);
assert.sameMembers(deviceIds, [1, 2, 3, 10]);
});
Expand Down
9 changes: 7 additions & 2 deletions ts/textsecure/MessageReceiver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,7 @@ class MessageReceiverInner extends EventTarget {
return promise.catch(error => {
window.log.error(
`queueDecryptedEnvelope error handling envelope ${id}:`,
error && error.extra ? JSON.stringify(error.extra) : '',
error && error.stack ? error.stack : error
);
});
Expand All @@ -796,6 +797,7 @@ class MessageReceiverInner extends EventTarget {
'queueEnvelope error handling envelope',
this.getEnvelopeId(envelope),
':',
error && error.extra ? JSON.stringify(error.extra) : '',
error && error.stack ? error.stack : error,
];
if (error.warn) {
Expand Down Expand Up @@ -2044,8 +2046,11 @@ class MessageReceiverInner extends EventTarget {
address
);

window.log.info('deleting sessions for', address.toString());
return sessionCipher.deleteAllSessionsForDevice();
window.log.info(
'handleEndSession: closing sessions for',
address.toString()
);
return sessionCipher.closeOpenSessionForDevice();
})
);
}
Expand Down
21 changes: 13 additions & 8 deletions ts/textsecure/OutgoingMessage.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -589,14 +589,19 @@ export default class OutgoingMessage {
identifier: string,
deviceIdsToRemove: Array<number>
): Promise<void> {
let promise = Promise.resolve();
for (const j in deviceIdsToRemove) {
promise = promise.then(async () => {
const encodedAddress = `${identifier}.${deviceIdsToRemove[j]}`;
return window.textsecure.storage.protocol.removeSession(encodedAddress);
});
}
return promise;
await Promise.all(
deviceIdsToRemove.map(async deviceId => {
const address = new window.libsignal.SignalProtocolAddress(
identifier,
deviceId
);
const sessionCipher = new window.libsignal.SessionCipher(
window.textsecure.storage.protocol,
address
);
await sessionCipher.closeOpenSessionForDevice();
})
);
}

async sendToIdentifier(providedIdentifier: string): Promise<void> {
Expand Down
Loading

0 comments on commit 5369950

Please sign in to comment.