From 91f50c028fe3801e448d8ce36c3f401c5a56dd36 Mon Sep 17 00:00:00 2001 From: Scott Nonnenberg Date: Wed, 12 Jul 2017 14:53:56 -0700 Subject: [PATCH] Unify processVerifiedMessage with Java implementation This removes our support for the New Key/DEFAULT case, which iOS will sync to us. Why? Because it ensures that in out of date scenarios, we don't lose the higher-security state we were in previously. FREEBIE --- js/signal_protocol_store.js | 110 +++++++++++++++++++++++++++--------- test/storage_test.js | 36 ++++++------ 2 files changed, 100 insertions(+), 46 deletions(-) diff --git a/js/signal_protocol_store.js b/js/signal_protocol_store.js index 6bb9580fc7..b54c1bb262 100644 --- a/js/signal_protocol_store.js +++ b/js/signal_protocol_store.js @@ -580,13 +580,20 @@ }); }, // Resolves to true if a new identity key was saved - processVerifiedMessage: function(identifier, verifiedStatus, publicKey) { + processContactSyncVerificationState: function(identifier, verifiedStatus, publicKey) { + if (verifiedStatus === VerifiedStatus.UNVERIFIED) { + return this.processUnverifiedMessage(identifier, verifiedStatus, publicKey); + } else { + return this.processVerifiedMessage(identifier, verifiedStatus, publicKey); + } + }, + // This function encapsulates the non-Java behavior, since the mobile apps don't + // currently receive contact syncs and therefore will see a verify sync with + // UNVERIFIED status + processUnverifiedMessage: function(identifier, verifiedStatus, publicKey) { if (identifier === null || identifier === undefined) { throw new Error("Tried to set verified for undefined/null key"); } - if (!validateVerifiedStatus(verifiedStatus)) { - throw new Error("Invalid verified status"); - } if (publicKey !== undefined && !(publicKey instanceof ArrayBuffer)) { throw new Error("Invalid public key"); } @@ -600,36 +607,16 @@ isEqual = equalArrayBuffers(publicKey, identityRecord.get('publicKey')); } }).always(function() { - // Because new keys always start as DEFAULT, we don't need to create new record here - if (!isPresent && verifiedStatus === VerifiedStatus.DEFAULT) { - console.log('No existing record for default status'); - return resolve(); - } - - // If we had a key before and it's the same, and we're not changing to - // VERIFIED, then it's a simple update of the verified flag. - if (isPresent && isEqual - && identityRecord.get('verified') !== verifiedStatus - && verifiedStatus !== VerifiedStatus.VERIFIED) { + if (isPresent + && isEqual + && identityRecord.get('verified') !== VerifiedStatus.UNVERIFIED) { return textsecure.storage.protocol.setVerified( identifier, verifiedStatus, publicKey ).then(resolve, reject); } - // We need to create a new record in three cases: - // 1. We had no key previously (checks above ensure that this is - // either VERIFIED/UNVERIFIED) - // 2. We had a key before, but we got a new key - // (no matter the VERIFIED state) - // 3. It's the same key, but we weren't VERIFIED before and are now - // (checks above handle the situation when 'state != VERIFIED') - if (!isPresent - || (isPresent && !isEqual) - || (isPresent - && identityRecord.get('verified') !== verifiedStatus - && verifiedStatus === VerifiedStatus.VERIFIED)) { - + if (!isPresent || !isEqual) { return textsecure.storage.protocol.saveIdentityWithAttributes(identifier, { publicKey : publicKey, verified : verifiedStatus, @@ -657,6 +644,73 @@ }.bind(this)); }.bind(this)); }, + // This matches the Java method as of + // https://github.com/WhisperSystems/Signal-Android/blob/d0bb68e1378f689e4d10ac6a46014164992ca4e4/src/org/thoughtcrime/securesms/util/IdentityUtil.java#L188 + processVerifiedMessage: function(identifier, verifiedStatus, publicKey) { + if (identifier === null || identifier === undefined) { + throw new Error("Tried to set verified for undefined/null key"); + } + if (!validateVerifiedStatus(verifiedStatus)) { + throw new Error("Invalid verified status"); + } + if (publicKey !== undefined && !(publicKey instanceof ArrayBuffer)) { + throw new Error("Invalid public key"); + } + return new Promise(function(resolve, reject) { + var identityRecord = new IdentityRecord({id: identifier}); + var isPresent = false; + var isEqual = false; + identityRecord.fetch().then(function() { + isPresent = true; + if (publicKey) { + isEqual = equalArrayBuffers(publicKey, identityRecord.get('publicKey')); + } + }).always(function() { + if (!isPresent && verifiedStatus === VerifiedStatus.DEFAULT) { + console.log('No existing record for default status'); + return resolve(); + } + + if (isPresent && isEqual + && identityRecord.get('verified') !== VerifiedStatus.DEFAULT + && verifiedStatus === VerifiedStatus.DEFAULT) { + + return textsecure.storage.protocol.setVerified( + identifier, verifiedStatus, publicKey + ).then(resolve, reject); + } + + if (verifiedStatus === VerifiedStatus.VERIFIED + && (!isPresent + || (isPresent && !isEqual) + || (isPresent && identityRecord.get('verified') !== VerifiedStatus.VERIFIED))) { + + return textsecure.storage.protocol.saveIdentityWithAttributes(identifier, { + publicKey : publicKey, + verified : verifiedStatus, + firstUse : false, + timestamp : Date.now(), + nonblockingApproval : true + }).then(function() { + if (isPresent && !isEqual) { + this.trigger('keychange', identifier); + return this.archiveAllSessions(identifier).then(function() { + // true signifies that we overwrote a previous key with a new one + return resolve(true); + }, reject); + } + + return resolve(); + }.bind(this), reject); + } + + // We get here if we got a new key and the status is DEFAULT. If the + // message is out of date, we don't want to lose whatever more-secure + // state we had before. + return resolve(); + }.bind(this)); + }.bind(this)); + }, isUntrusted: function(identifier) { if (identifier === null || identifier === undefined) { throw new Error("Tried to set verified for undefined/null key"); diff --git a/test/storage_test.js b/test/storage_test.js index e3d8a1b441..bef917f0b9 100644 --- a/test/storage_test.js +++ b/test/storage_test.js @@ -355,7 +355,7 @@ describe("SignalProtocolStore", function() { }); }); }); - describe('processVerifiedMessage', function() { + describe('processContactSyncVerificationState', function() { var record; var newIdentity = libsignal.crypto.getRandomBytes(33); var keychangeTriggered; @@ -387,12 +387,12 @@ describe("SignalProtocolStore", function() { }); it ('does nothing', function() { - return store.processVerifiedMessage( + return store.processContactSyncVerificationState( identifier, store.VerifiedStatus.DEFAULT, newIdentity ).then(fetchRecord).then(function() { // fetchRecord resolved so there is a record. // Bad. - throw new Error("processVerifiedMessage should not save new records"); + throw new Error("processContactSyncVerificationState should not save new records"); }, function() { assert.strictEqual(keychangeTriggered, 0); }); @@ -412,13 +412,13 @@ describe("SignalProtocolStore", function() { return wrapDeferred(record.save()); }); - it ('saves the new identity and marks it DEFAULT', function() { - return store.processVerifiedMessage( + it ('does not save the new identity (because this is a less secure state)', function() { + return store.processContactSyncVerificationState( identifier, store.VerifiedStatus.DEFAULT, newIdentity ).then(fetchRecord).then(function() { - assert.strictEqual(record.get('verified'), store.VerifiedStatus.DEFAULT); - assertEqualArrayBuffers(record.get('publicKey'), newIdentity); - assert.strictEqual(keychangeTriggered, 1); + assert.strictEqual(record.get('verified'), store.VerifiedStatus.VERIFIED); + assertEqualArrayBuffers(record.get('publicKey'), testKey.pubKey); + assert.strictEqual(keychangeTriggered, 0); }); }); }); @@ -436,7 +436,7 @@ describe("SignalProtocolStore", function() { }); it ('updates the verified status', function() { - return store.processVerifiedMessage( + return store.processContactSyncVerificationState( identifier, store.VerifiedStatus.DEFAULT, testKey.pubKey ).then(fetchRecord).then(function() { assert.strictEqual(record.get('verified'), store.VerifiedStatus.DEFAULT); @@ -459,7 +459,7 @@ describe("SignalProtocolStore", function() { }); it ('does not hang', function() { - return store.processVerifiedMessage( + return store.processContactSyncVerificationState( identifier, store.VerifiedStatus.DEFAULT, testKey.pubKey ).then(fetchRecord).then(function() { assert.strictEqual(keychangeTriggered, 0); @@ -476,7 +476,7 @@ describe("SignalProtocolStore", function() { }); it ('saves the new identity and marks it verified', function() { - return store.processVerifiedMessage( + return store.processContactSyncVerificationState( identifier, store.VerifiedStatus.UNVERIFIED, newIdentity ).then(fetchRecord).then(function() { assert.strictEqual(record.get('verified'), store.VerifiedStatus.UNVERIFIED); @@ -500,7 +500,7 @@ describe("SignalProtocolStore", function() { }); it ('saves the new identity and marks it UNVERIFIED', function() { - return store.processVerifiedMessage( + return store.processContactSyncVerificationState( identifier, store.VerifiedStatus.UNVERIFIED, newIdentity ).then(fetchRecord).then(function() { assert.strictEqual(record.get('verified'), store.VerifiedStatus.UNVERIFIED); @@ -523,7 +523,7 @@ describe("SignalProtocolStore", function() { }); it ('updates the verified status', function() { - return store.processVerifiedMessage( + return store.processContactSyncVerificationState( identifier, store.VerifiedStatus.UNVERIFIED, testKey.pubKey ).then(fetchRecord).then(function() { assert.strictEqual(record.get('verified'), store.VerifiedStatus.UNVERIFIED); @@ -546,7 +546,7 @@ describe("SignalProtocolStore", function() { }); it ('does not hang', function() { - return store.processVerifiedMessage( + return store.processContactSyncVerificationState( identifier, store.VerifiedStatus.UNVERIFIED, testKey.pubKey ).then(fetchRecord).then(function() { assert.strictEqual(keychangeTriggered, 0); @@ -565,7 +565,7 @@ describe("SignalProtocolStore", function() { }); it ('saves the new identity and marks it verified', function() { - return store.processVerifiedMessage( + return store.processContactSyncVerificationState( identifier, store.VerifiedStatus.VERIFIED, newIdentity ).then(fetchRecord).then(function() { assert.strictEqual(record.get('verified'), store.VerifiedStatus.VERIFIED); @@ -589,7 +589,7 @@ describe("SignalProtocolStore", function() { }); it ('saves the new identity and marks it VERIFIED', function() { - return store.processVerifiedMessage( + return store.processContactSyncVerificationState( identifier, store.VerifiedStatus.VERIFIED, newIdentity ).then(fetchRecord).then(function() { assert.strictEqual(record.get('verified'), store.VerifiedStatus.VERIFIED); @@ -612,7 +612,7 @@ describe("SignalProtocolStore", function() { }); it ('saves the identity and marks it verified', function() { - return store.processVerifiedMessage( + return store.processContactSyncVerificationState( identifier, store.VerifiedStatus.VERIFIED, testKey.pubKey ).then(fetchRecord).then(function() { assert.strictEqual(record.get('verified'), store.VerifiedStatus.VERIFIED); @@ -635,7 +635,7 @@ describe("SignalProtocolStore", function() { }); it ('does not hang', function() { - return store.processVerifiedMessage( + return store.processContactSyncVerificationState( identifier, store.VerifiedStatus.VERIFIED, testKey.pubKey ).then(fetchRecord).then(function() { assert.strictEqual(keychangeTriggered, 0);