Skip to content

Commit

Permalink
Simplify signed prekey handling - always save for 30 days, always sav…
Browse files Browse the repository at this point in the history 
…e five
  • Loading branch information
@scottnonnenberg-signal
scottnonnenberg-signal committed Aug 3, 2021
1 parent 402dda0 commit a78d30c
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 126 deletions.
137 changes: 76 additions & 61 deletions libtextsecure/test/account_manager_test.js → ...lectron/textsecure/AccountManager_test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,47 +1,68 @@
// Copyright 2017-2020 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only

import { assert } from 'chai';

import { getRandomBytes } from '../../Crypto';
import AccountManager from '../../textsecure/AccountManager';
import { OuterSignedPrekeyType } from '../../textsecure/Types.d';

/* eslint-disable @typescript-eslint/no-explicit-any */

describe('AccountManager', () => {
let accountManager;
let accountManager: AccountManager;

beforeEach(() => {
accountManager = new window.textsecure.AccountManager();
const server: any = {};
accountManager = new AccountManager(server);
});

describe('#cleanSignedPreKeys', () => {
let originalProtocolStorage;
let signedPreKeys;
let originalGetIdentityKeyPair: any;
let originalLoadSignedPreKeys: any;
let originalRemoveSignedPreKey: any;
let signedPreKeys: Array<OuterSignedPrekeyType>;
const DAY = 1000 * 60 * 60 * 24;

const pubKey = getRandomBytes(33);
const privKey = getRandomBytes(32);

beforeEach(async () => {
const identityKey = window.Signal.Curve.generateKeyPair();

originalProtocolStorage = window.textsecure.storage.protocol;
window.textsecure.storage.protocol = {
getIdentityKeyPair() {
return identityKey;
},
loadSignedPreKeys() {
return Promise.resolve(signedPreKeys);
},
};
originalGetIdentityKeyPair =
window.textsecure.storage.protocol.getIdentityKeyPair;
originalLoadSignedPreKeys =
window.textsecure.storage.protocol.loadSignedPreKeys;
originalRemoveSignedPreKey =
window.textsecure.storage.protocol.removeSignedPreKey;

window.textsecure.storage.protocol.getIdentityKeyPair = async () =>
identityKey;
window.textsecure.storage.protocol.loadSignedPreKeys = async () =>
signedPreKeys;
});
afterEach(() => {
window.textsecure.storage.protocol = originalProtocolStorage;
window.textsecure.storage.protocol.getIdentityKeyPair = originalGetIdentityKeyPair;
window.textsecure.storage.protocol.loadSignedPreKeys = originalLoadSignedPreKeys;
window.textsecure.storage.protocol.removeSignedPreKey = originalRemoveSignedPreKey;
});

describe('encrypted device name', () => {
it('roundtrips', async () => {
const deviceName = 'v2.5.0 on Ubunto 20.04';
const encrypted = await accountManager.encryptDeviceName(deviceName);
if (!encrypted) {
throw new Error('failed to encrypt!');
}
assert.strictEqual(typeof encrypted, 'string');
const decrypted = await accountManager.decryptDeviceName(encrypted);

assert.strictEqual(decrypted, deviceName);
});

it('handles null deviceName', async () => {
const encrypted = await accountManager.encryptDeviceName(null);
it('handles falsey deviceName', async () => {
const encrypted = await accountManager.encryptDeviceName('');
assert.strictEqual(encrypted, null);
});
});
Expand All @@ -53,90 +74,80 @@ describe('AccountManager', () => {
keyId: 1,
created_at: now - DAY * 32,
confirmed: true,
pubKey,
privKey,
},
{
keyId: 2,
created_at: now - DAY * 34,
confirmed: true,
pubKey,
privKey,
},
{
keyId: 3,
created_at: now - DAY * 38,
confirmed: true,
pubKey,
privKey,
},
];

// should be no calls to store.removeSignedPreKey, would cause crash
return accountManager.cleanSignedPreKeys();
});

it('eliminates confirmed keys over a month old, if more than three', async () => {
it('eliminates oldest keys, even if recent key is unconfirmed', async () => {
const now = Date.now();
signedPreKeys = [
{
keyId: 1,
created_at: now - DAY * 32,
confirmed: true,
pubKey,
privKey,
},
{
keyId: 2,
created_at: now - DAY * 31,
confirmed: true,
confirmed: false,
pubKey,
privKey,
},
{
keyId: 3,
created_at: now - DAY * 24,
confirmed: true,
pubKey,
privKey,
},
{
// Oldest, should be dropped
keyId: 4,
created_at: now - DAY * 38,
confirmed: true,
pubKey,
privKey,
},
{
keyId: 5,
created_at: now - DAY,
confirmed: true,
},
];

let count = 0;
window.textsecure.storage.protocol.removeSignedPreKey = keyId => {
if (keyId !== 1 && keyId !== 4) {
throw new Error(`Wrong keys were eliminated! ${keyId}`);
}

count += 1;
};

await accountManager.cleanSignedPreKeys();
assert.strictEqual(count, 2);
});

it('keeps at least three unconfirmed keys if no confirmed', async () => {
const now = Date.now();
signedPreKeys = [
{
keyId: 1,
created_at: now - DAY * 32,
},
{
keyId: 2,
created_at: now - DAY * 44,
},
{
keyId: 3,
created_at: now - DAY * 36,
pubKey,
privKey,
},
{
keyId: 4,
created_at: now - DAY * 20,
keyId: 6,
created_at: now - DAY * 5,
confirmed: true,
pubKey,
privKey,
},
];

let count = 0;
window.textsecure.storage.protocol.removeSignedPreKey = keyId => {
if (keyId !== 2) {
window.textsecure.storage.protocol.removeSignedPreKey = async keyId => {
if (keyId !== 4) {
throw new Error(`Wrong keys were eliminated! ${keyId}`);
}

Expand All @@ -147,40 +158,44 @@ describe('AccountManager', () => {
assert.strictEqual(count, 1);
});

it('if some confirmed keys, keeps unconfirmed to addd up to three total', async () => {
it('Removes no keys if less than five', async () => {
const now = Date.now();
signedPreKeys = [
{
keyId: 1,
created_at: now - DAY * 32,
confirmed: true,
pubKey,
privKey,
},
{
keyId: 2,
created_at: now - DAY * 44,
confirmed: true,
pubKey,
privKey,
},
{
keyId: 3,
created_at: now - DAY * 36,
confirmed: false,
pubKey,
privKey,
},
{
keyId: 4,
created_at: now - DAY * 20,
confirmed: false,
pubKey,
privKey,
},
];

let count = 0;
window.textsecure.storage.protocol.removeSignedPreKey = keyId => {
if (keyId !== 3) {
throw new Error(`Wrong keys were eliminated! ${keyId}`);
}

count += 1;
window.textsecure.storage.protocol.removeSignedPreKey = async () => {
throw new Error('None should be removed!');
};

await accountManager.cleanSignedPreKeys();
assert.strictEqual(count, 1);
});
});
});
111 changes: 46 additions & 65 deletions ts/textsecure/AccountManager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,10 @@ import { assert } from '../util/assert';
import { getProvisioningUrl } from '../util/getProvisioningUrl';
import { SignalService as Proto } from '../protobuf';

const ARCHIVE_AGE = 30 * 24 * 60 * 60 * 1000;
const PREKEY_ROTATION_AGE = 24 * 60 * 60 * 1000;
const DAY = 24 * 60 * 60 * 1000;
const MINIMUM_SIGNED_PREKEYS = 5;
const ARCHIVE_AGE = 30 * DAY;
const PREKEY_ROTATION_AGE = DAY;
const PROFILE_KEY_LENGTH = 32;
const SIGNED_KEY_GEN_BATCH_SIZE = 100;

Expand Down Expand Up @@ -321,13 +323,14 @@ export default class AccountManager extends EventTarget {
const existingKeys = await store.loadSignedPreKeys();
existingKeys.sort((a, b) => (b.created_at || 0) - (a.created_at || 0));
const confirmedKeys = existingKeys.filter(key => key.confirmed);
const mostRecent = confirmedKeys[0];

if (
confirmedKeys.length >= 3 &&
isMoreRecentThan(confirmedKeys[0].created_at, PREKEY_ROTATION_AGE)
confirmedKeys.length >= 2 ||
isMoreRecentThan(mostRecent?.created_at || 0, PREKEY_ROTATION_AGE)
) {
window.log.warn(
'rotateSignedPreKey: 3+ confirmed keys, most recent is less than a day old. Cancelling rotation.'
`rotateSignedPreKey: ${confirmedKeys.length} confirmed keys, most recent was created ${mostRecent?.created_at}. Cancelling rotation.`
);
return;
}
Expand Down Expand Up @@ -411,71 +414,49 @@ export default class AccountManager extends EventTarget {
}

async cleanSignedPreKeys() {
const MINIMUM_KEYS = 3;
const store = window.textsecure.storage.protocol;
return store.loadSignedPreKeys().then(async allKeys => {
allKeys.sort((a, b) => (b.created_at || 0) - (a.created_at || 0));
const confirmed = allKeys.filter(key => key.confirmed);
const unconfirmed = allKeys.filter(key => !key.confirmed);

const recent = allKeys[0] ? allKeys[0].keyId : 'none';
const recentConfirmed = confirmed[0] ? confirmed[0].keyId : 'none';
window.log.info(`Most recent signed key: ${recent}`);
window.log.info(`Most recent confirmed signed key: ${recentConfirmed}`);
window.log.info(
'Total signed key count:',
allKeys.length,
'-',
confirmed.length,
'confirmed'
);

let confirmedCount = confirmed.length;
const allKeys = await store.loadSignedPreKeys();
allKeys.sort((a, b) => (b.created_at || 0) - (a.created_at || 0));
const confirmed = allKeys.filter(key => key.confirmed);
const unconfirmed = allKeys.filter(key => !key.confirmed);

// Keep MINIMUM_KEYS confirmed keys, then drop if older than a week
await Promise.all(
confirmed.map(async (key, index) => {
if (index < MINIMUM_KEYS) {
return;
}
const createdAt = key.created_at || 0;

if (isOlderThan(createdAt, ARCHIVE_AGE)) {
window.log.info(
'Removing confirmed signed prekey:',
key.keyId,
'with timestamp:',
new Date(createdAt).toJSON()
);
await store.removeSignedPreKey(key.keyId);
confirmedCount -= 1;
}
})
);

const stillNeeded = MINIMUM_KEYS - confirmedCount;
const recent = allKeys[0] ? allKeys[0].keyId : 'none';
const recentConfirmed = confirmed[0] ? confirmed[0].keyId : 'none';
const recentUnconfirmed = unconfirmed[0] ? unconfirmed[0].keyId : 'none';
window.log.info(`cleanSignedPreKeys: Most recent signed key: ${recent}`);
window.log.info(
`cleanSignedPreKeys: Most recent confirmed signed key: ${recentConfirmed}`
);
window.log.info(
`cleanSignedPreKeys: Most recent unconfirmed signed key: ${recentUnconfirmed}`
);
window.log.info(
'cleanSignedPreKeys: Total signed key count:',
allKeys.length,
'-',
confirmed.length,
'confirmed'
);

// If we still don't have enough total keys, we keep as many unconfirmed
// keys as necessary. If not necessary, and over a week old, we drop.
await Promise.all(
unconfirmed.map(async (key, index) => {
if (index < stillNeeded) {
return;
}
// Keep MINIMUM_SIGNED_PREKEYS keys, then drop if older than ARCHIVE_AGE
await Promise.all(
allKeys.map(async (key, index) => {
if (index < MINIMUM_SIGNED_PREKEYS) {
return;
}
const createdAt = key.created_at || 0;

const createdAt = key.created_at || 0;
if (isOlderThan(createdAt, ARCHIVE_AGE)) {
window.log.info(
'Removing unconfirmed signed prekey:',
key.keyId,
'with timestamp:',
new Date(createdAt).toJSON()
);
await store.removeSignedPreKey(key.keyId);
}
})
);
});
if (isOlderThan(createdAt, ARCHIVE_AGE)) {
const timestamp = new Date(createdAt).toJSON();
const confirmedText = key.confirmed ? ' (confirmed)' : '';
window.log.info(
`Removing signed prekey: ${key.keyId} with timestamp ${timestamp}${confirmedText}`
);
await store.removeSignedPreKey(key.keyId);
}
})
);
}

async createAccount(
Expand Down

0 comments on commit a78d30c

Please sign in to comment.