Skip to content

Commit

Permalink
Clear stale sender certificates
Browse files Browse the repository at this point in the history
Co-authored-by: Scott Nonnenberg <scott@signal.org>
  • Loading branch information
automated-signal and scottnonnenberg-signal committed Sep 27, 2021
1 parent 5a98304 commit 8bf3fb3
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 14 deletions.
7 changes: 6 additions & 1 deletion ts/background.ts
Expand Up @@ -754,6 +754,11 @@ export async function startApp(): Promise<void> {
});
}

if (window.isBeforeVersion(lastVersion, 'v5.18.0')) {
await window.storage.remove('senderCertificate');
await window.storage.remove('senderCertificateNoE164');
}

// This one should always be last - it could restart the app
if (window.isBeforeVersion(lastVersion, 'v1.15.0-beta.5')) {
await deleteAllLogs();
Expand Down Expand Up @@ -3355,7 +3360,7 @@ export async function startApp(): Promise<void> {
);

try {
log.info('unlinkAndDisconnect: removing configuration');
log.info(`unlinkAndDisconnect: removing configuration, mode ${mode}`);
await window.textsecure.storage.protocol.removeAllConfiguration(mode);

// This was already done in the database with removeAllConfiguration; this does it
Expand Down
18 changes: 18 additions & 0 deletions ts/services/senderCertificate.ts
Expand Up @@ -76,6 +76,24 @@ export class SenderCertificateService {
return this.fetchCertificate(mode);
}

// This is intended to be called when our credentials have been deleted, so any fetches
// made until this function is complete would fail anyway.
async clear(): Promise<void> {
log.info(
'Sender certificate service: Clearing in-progress fetches and ' +
'deleting cached certificates'
);
await Promise.all(this.fetchPromises.values());

const { storage } = this;
assert(
storage,
'Sender certificate service method was called before it was initialized'
);
await storage.remove('senderCertificate');
await storage.remove('senderCertificateNoE164');
}

private getStoredCertificate(
mode: SenderCertificateMode
): undefined | SerializedCertificateType {
Expand Down
35 changes: 32 additions & 3 deletions ts/test-electron/services/senderCertificate_test.ts
Expand Up @@ -22,6 +22,7 @@ describe('SenderCertificateService', () => {
const FIFTEEN_MINUTES = 15 * durations.MINUTE;

let fakeValidCertificate: SenderCertificate;
let fakeValidEncodedCertificate: string;
let fakeValidCertificateExpiry: number;
let fakeServer: any;
let fakeNavigator: { onLine: boolean };
Expand All @@ -47,12 +48,13 @@ describe('SenderCertificateService', () => {
fakeValidCertificate.certificate = SenderCertificate.Certificate.encode(
certificate
).finish();
fakeValidEncodedCertificate = Bytes.toBase64(
SenderCertificate.encode(fakeValidCertificate).finish()
);

fakeServer = {
getSenderCertificate: sinon.stub().resolves({
certificate: Bytes.toBase64(
SenderCertificate.encode(fakeValidCertificate).finish()
),
certificate: fakeValidEncodedCertificate,
}),
};

Expand Down Expand Up @@ -225,5 +227,32 @@ describe('SenderCertificateService', () => {

assert.isUndefined(await service.get(SenderCertificateMode.WithE164));
});

it('clear waits for any outstanding requests then erases storage', async () => {
let count = 0;

fakeServer = {
getSenderCertificate: sinon.spy(async () => {
await new Promise(resolve => setTimeout(resolve, 500));

count += 1;
return {
certificate: fakeValidEncodedCertificate,
};
}),
};

const service = initializeTestService();

service.get(SenderCertificateMode.WithE164);
service.get(SenderCertificateMode.WithoutE164);

await service.clear();

assert.equal(count, 2);

assert.isUndefined(fakeStorage.get('senderCertificate'));
assert.isUndefined(fakeStorage.get('senderCertificateNoE164'));
});
});
});
21 changes: 11 additions & 10 deletions ts/textsecure/AccountManager.ts
Expand Up @@ -18,6 +18,8 @@ import ProvisioningCipher from './ProvisioningCipher';
import { IncomingWebSocketRequest } from './WebsocketResources';
import createTaskWithTimeout from './TaskWithTimeout';
import * as Bytes from '../Bytes';
import { RemoveAllConfiguration } from '../types/RemoveAllConfiguration';
import { senderCertificateService } from '../services/senderCertificate';
import {
deriveAccessKey,
generateRegistrationId,
Expand Down Expand Up @@ -520,33 +522,32 @@ export default class AccountManager extends EventTarget {
if (uuidChanged || numberChanged) {
if (uuidChanged) {
log.warn(
'New uuid is different from old uuid; deleting all previous data'
'createAccount: New uuid is different from old uuid; deleting all previous data'
);
}
if (numberChanged) {
log.warn(
'New number is different from old number; deleting all previous data'
'createAccount: New number is different from old number; deleting all previous data'
);
}

try {
await storage.protocol.removeAllData();
log.info('Successfully deleted previous data');
log.info('createAccount: Successfully deleted previous data');
} catch (error) {
log.error(
'Something went wrong deleting data from previous number',
error && error.stack ? error.stack : error
);
}
} else {
log.info('createAccount: Erasing configuration (soft)');
await storage.protocol.removeAllConfiguration(
RemoveAllConfiguration.Soft
);
}

await Promise.all([
storage.user.removeCredentials(),
storage.remove('regionCode'),
storage.remove('userAgent'),
storage.remove('profileKey'),
storage.remove('read-receipt-setting'),
]);
await senderCertificateService.clear();

if (previousUuid) {
await Promise.all([
Expand Down

0 comments on commit 8bf3fb3

Please sign in to comment.