Skip to content

Commit

Permalink
Unify processVerifiedMessage with Java implementation
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@scottnonnenberg
scottnonnenberg committed Aug 4, 2017
1 parent 5bba6d3 commit 91f50c0
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 39 deletions.
96 changes: 75 additions & 21 deletions js/signal_protocol_store.js
View file
Expand Up @@ -580,6 +580,72 @@
});
},
// Resolves to true if a new identity key was saved
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 (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
&& isEqual
&& identityRecord.get('verified') !== VerifiedStatus.UNVERIFIED) {

return textsecure.storage.protocol.setVerified(
identifier, verifiedStatus, publicKey
).then(resolve, reject);
}

if (!isPresent || !isEqual) {
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);
}

// The situation which could get us here is:
// 1. had a previous key
// 2. new key is the same
// 3. desired new status is same as what we had before
return resolve();
}.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");
Expand All @@ -600,35 +666,24 @@
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) {
&& identityRecord.get('verified') !== VerifiedStatus.DEFAULT
&& verifiedStatus === VerifiedStatus.DEFAULT) {

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 (verifiedStatus === VerifiedStatus.VERIFIED
&& (!isPresent
|| (isPresent && !isEqual)
|| (isPresent && identityRecord.get('verified') !== VerifiedStatus.VERIFIED))) {

return textsecure.storage.protocol.saveIdentityWithAttributes(identifier, {
publicKey : publicKey,
Expand All @@ -649,10 +704,9 @@
}.bind(this), reject);
}

// The situation which could get us here is:
// 1. had a previous key
// 2. new key is the same
// 3. desired new status is same as what we had before
// 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));
Expand Down
36 changes: 18 additions & 18 deletions test/storage_test.js
View file
Expand Up @@ -355,7 +355,7 @@ describe("SignalProtocolStore", function() {
});
});
});
describe('processVerifiedMessage', function() {
describe('processContactSyncVerificationState', function() {
var record;
var newIdentity = libsignal.crypto.getRandomBytes(33);
var keychangeTriggered;
Expand Down Expand Up @@ -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);
});
Expand All @@ -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);
});
});
});
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down

0 comments on commit 91f50c0

Please sign in to comment.