diff --git a/package.json b/package.json index 83424d69849..ce154d87b1f 100644 --- a/package.json +++ b/package.json @@ -195,7 +195,7 @@ "@electron/fuses": "1.5.0", "@formatjs/intl": "2.6.7", "@mixer/parallel-prettier": "2.0.3", - "@signalapp/mock-server": "4.1.1", + "@signalapp/mock-server": "4.1.2", "@storybook/addon-a11y": "6.5.6", "@storybook/addon-actions": "6.5.6", "@storybook/addon-controls": "6.5.6", diff --git a/ts/services/storageRecordOps.ts b/ts/services/storageRecordOps.ts index 42c6e22cb32..f38f8cb1e81 100644 --- a/ts/services/storageRecordOps.ts +++ b/ts/services/storageRecordOps.ts @@ -49,6 +49,9 @@ import { normalizeServiceId, normalizePni, ServiceIdKind, + isUntaggedPniString, + toUntaggedPni, + toTaggedPni, } from '../types/ServiceId'; import { normalizeAci } from '../util/normalizeAci'; import * as Stickers from '../types/Stickers'; @@ -171,7 +174,7 @@ export async function toContactRecord( } const pni = conversation.getPni(); if (pni && RemoteConfig.isEnabled('desktop.pnp')) { - contactRecord.pni = pni; + contactRecord.pni = toUntaggedPni(pni); } const profileKey = conversation.get('profileKey'); if (profileKey) { @@ -972,9 +975,14 @@ export async function mergeContactRecord( aci: originalContactRecord.aci ? normalizeAci(originalContactRecord.aci, 'ContactRecord.aci') : undefined, - pni: originalContactRecord.pni - ? normalizePni(originalContactRecord.pni, 'ContactRecord.pni') - : undefined, + pni: + originalContactRecord.pni && + isUntaggedPniString(originalContactRecord.pni) + ? normalizePni( + toTaggedPni(originalContactRecord.pni), + 'ContactRecord.pni' + ) + : undefined, }; const isPniSupported = RemoteConfig.isEnabled('desktop.pnp'); diff --git a/ts/sql/migrations/960-untag-pni.ts b/ts/sql/migrations/960-untag-pni.ts new file mode 100644 index 00000000000..7f65fb4cd0e --- /dev/null +++ b/ts/sql/migrations/960-untag-pni.ts @@ -0,0 +1,265 @@ +// Copyright 2023 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import type { Database } from '@signalapp/better-sqlite3'; + +import type { LoggerType } from '../../types/Logging'; +import type { + ServiceIdString, + AciString, + PniString, +} from '../../types/ServiceId'; +import { normalizePni } from '../../types/ServiceId'; +import { normalizeAci } from '../../util/normalizeAci'; +import type { JSONWithUnknownFields } from '../../types/Util'; + +export const version = 960; + +export function updateToSchemaVersion960( + currentVersion: number, + db: Database, + logger: LoggerType +): void { + if (currentVersion >= 960) { + return; + } + + db.transaction(() => { + const ourServiceIds = migratePni(db, logger); + if (!ourServiceIds) { + logger.info('updateToSchemaVersion960: not running, pni is normalized'); + return; + } + + // Migrate JSON fields + db.prepare( + ` + UPDATE conversations + SET json = json_set(json, '$.pni', $pni) + WHERE serviceId IS $aci + ` + ).run({ + aci: ourServiceIds.aci, + pni: ourServiceIds.pni, + }); + + migratePreKeys(db, 'preKeys', ourServiceIds, logger); + migratePreKeys(db, 'signedPreKeys', ourServiceIds, logger); + migratePreKeys(db, 'kyberPreKeys', ourServiceIds, logger); + + db.pragma('user_version = 960'); + })(); + + logger.info('updateToSchemaVersion960: success!'); +} + +// +// migratePni checks if `pni` needs normalization: +// +// * If yes - return legacy and updated pni +// * It no - return undefined +// + +type OurServiceIds = Readonly<{ + aci: AciString; + legacyPni: string; + pni: PniString; +}>; + +function migratePni( + db: Database, + logger: LoggerType +): OurServiceIds | undefined { + // Get our ACI and PNI + const uuidIdJson = db + .prepare( + ` + SELECT json + FROM items + WHERE id IS 'uuid_id' + ` + ) + .pluck() + .get(); + const pniJson = db + .prepare( + ` + SELECT json + FROM items + WHERE id IS 'pni' + ` + ) + .pluck() + .get(); + + let aci: string | undefined; + try { + [aci] = JSON.parse(uuidIdJson).value.split('.', 2); + } catch (error) { + if (uuidIdJson) { + logger.warn( + 'updateToSchemaVersion960: failed to parse uuid_id item', + error + ); + } else { + logger.info('updateToSchemaVersion960: Our ACI not found'); + } + } + if (!aci) { + return undefined; + } + + let legacyPni: string | undefined; + try { + legacyPni = JSON.parse(pniJson).value; + } catch (error) { + if (pniJson) { + logger.warn('updateToSchemaVersion960: failed to parse pni item', error); + } else { + logger.info('updateToSchemaVersion960: Our PNI not found'); + } + } + if (!legacyPni) { + return undefined; + } + + const pni = prefixPni(legacyPni, 'pni', logger); + if (!pni || pni === legacyPni) { + return undefined; + } + + const maps: Array<{ id: string; json: string }> = db + .prepare( + ` + SELECT id, json + FROM items + WHERE id IN ('identityKeyMap', 'registrationIdMap'); + ` + ) + .all(); + + const updateStmt = db.prepare( + 'UPDATE items SET json = $json WHERE id IS $id' + ); + + updateStmt.run({ + id: 'pni', + json: JSON.stringify({ id: 'pni', value: pni }), + }); + + for (const { id, json } of maps) { + try { + const data: { id: string; value: Record } = + JSON.parse(json); + + const pniValue = data.value[legacyPni]; + if (pniValue) { + delete data.value[legacyPni]; + data.value[pni] = pniValue; + } + + updateStmt.run({ id, json: JSON.stringify(data) }); + } catch (error) { + logger.warn( + `updateToSchemaVersion960: failed to parse ${id} item`, + error + ); + } + } + return { + aci: normalizeAci(aci, 'uuid_id', logger), + pni, + legacyPni, + }; +} + +// migratePreKeys does the following: +// +// 1. Update `ourServiceId` to prefixed PNI +// 2. Update `id` to use new `ourServiceId` value +// (the schema is `${ourServiceId}:${keyId}`) +// + +function migratePreKeys( + db: Database, + table: string, + { legacyPni, pni }: OurServiceIds, + logger: LoggerType +): void { + const preKeys = db + .prepare(`SELECT id, json FROM ${table} WHERE ourServiceId IS $legacyPni`) + .all({ legacyPni }); + + const updateStmt = db.prepare(` + UPDATE ${table} + SET id = $newId, json = $newJson + WHERE id = $id + `); + + logger.info(`updateToSchemaVersion960: updating ${preKeys.length} ${table}`); + for (const { id, json } of preKeys) { + const match = id.match(/^(.*):(.*)$/); + if (!match) { + logger.warn(`updateToSchemaVersion960: invalid ${table} id ${id}`); + continue; + } + + let legacyData: JSONWithUnknownFields>; + try { + legacyData = JSON.parse(json); + } catch (error) { + logger.warn( + `updateToSchemaVersion960: failed to parse ${table} ${id}`, + error + ); + continue; + } + + const [, ourServiceId, keyId] = match; + if (ourServiceId !== legacyPni) { + logger.warn( + 'updateToSchemaVersion960: unexpected ourServiceId', + ourServiceId, + legacyPni + ); + continue; + } + + const newId = `${pni}:${keyId}`; + + const newData: JSONWithUnknownFields<{ + id: string; + ourServiceId: ServiceIdString; + }> = { + ...legacyData, + id: newId, + ourServiceId: pni, + }; + + updateStmt.run({ + id, + newId, + newJson: JSON.stringify(newData), + }); + } +} + +// +// Various utility methods below. +// + +function prefixPni( + legacyPni: string | null | undefined, + context: string, + logger: LoggerType +): PniString | undefined { + if (legacyPni == null) { + return undefined; + } + + if (legacyPni.toLowerCase().startsWith('pni:')) { + return normalizePni(legacyPni, context, logger); + } + + return normalizePni(`PNI:${legacyPni}`, context, logger); +} diff --git a/ts/sql/migrations/index.ts b/ts/sql/migrations/index.ts index 8a047421199..f2d0b12849b 100644 --- a/ts/sql/migrations/index.ts +++ b/ts/sql/migrations/index.ts @@ -70,10 +70,11 @@ import updateToSchemaVersion91 from './91-clean-keys'; import { updateToSchemaVersion920 } from './920-clean-more-keys'; import { updateToSchemaVersion930 } from './930-fts5-secure-delete'; import { updateToSchemaVersion940 } from './940-fts5-revert'; +import { updateToSchemaVersion950 } from './950-fts5-secure-delete'; import { version as MAX_VERSION, - updateToSchemaVersion950, -} from './950-fts5-secure-delete'; + updateToSchemaVersion960, +} from './960-untag-pni'; function updateToSchemaVersion1( currentVersion: number, @@ -2011,6 +2012,7 @@ export const SCHEMA_VERSIONS = [ updateToSchemaVersion930, updateToSchemaVersion940, updateToSchemaVersion950, + updateToSchemaVersion960, ]; export class DBVersionFromFutureError extends Error { diff --git a/ts/test-mock/pnp/merge_test.ts b/ts/test-mock/pnp/merge_test.ts index d2f713e682a..4f87c70dd21 100644 --- a/ts/test-mock/pnp/merge_test.ts +++ b/ts/test-mock/pnp/merge_test.ts @@ -9,6 +9,7 @@ import Long from 'long'; import * as durations from '../../util/durations'; import { uuidToBytes } from '../../util/uuidToBytes'; +import { toUntaggedPni } from '../../types/ServiceId'; import { MY_STORY_ID } from '../../types/Stories'; import { Bootstrap } from '../bootstrap'; import type { App } from '../bootstrap'; @@ -392,7 +393,7 @@ describe('pnp/merge', function needsName() { aciContacts += 1; assert.strictEqual(pni, ''); assert.strictEqual(serviceE164, ''); - } else if (pni === pniContact.device.pni) { + } else if (pni === toUntaggedPni(pniContact.device.pni)) { pniContacts += 1; assert.strictEqual(aci, ''); assert.strictEqual(serviceE164, pniContact.device.number); @@ -401,7 +402,10 @@ describe('pnp/merge', function needsName() { assert.strictEqual(aciContacts, 1); assert.strictEqual(pniContacts, 1); - assert.strictEqual(removed[0].contact?.pni, pniContact.device.pni); + assert.strictEqual( + removed[0].contact?.pni, + toUntaggedPni(pniContact.device.pni) + ); assert.strictEqual(removed[0].contact?.aci, pniContact.device.aci); // Pin PNI so that it appears in the left pane diff --git a/ts/test-mock/pnp/pni_change_test.ts b/ts/test-mock/pnp/pni_change_test.ts index 137c14b555d..2fd6e5095d4 100644 --- a/ts/test-mock/pnp/pni_change_test.ts +++ b/ts/test-mock/pnp/pni_change_test.ts @@ -7,7 +7,7 @@ import type { PrimaryDevice } from '@signalapp/mock-server'; import createDebug from 'debug'; import * as durations from '../../util/durations'; -import { generatePni } from '../../types/ServiceId'; +import { generatePni, toUntaggedPni } from '../../types/ServiceId'; import { Bootstrap } from '../bootstrap'; import type { App } from '../bootstrap'; @@ -48,7 +48,7 @@ describe('pnp/PNI Change', function needsName() { whitelisted: true, serviceE164: contactA.device.number, identityKey: contactA.getPublicKey(ServiceIdKind.PNI).serialize(), - pni: contactA.device.getServiceIdByKind(ServiceIdKind.PNI), + pni: toUntaggedPni(contactA.device.pni), givenName: 'ContactA', }, ServiceIdKind.PNI @@ -132,8 +132,7 @@ describe('pnp/PNI Change', function needsName() { state .removeRecord( item => - item.record.contact?.pni === - contactA.device.getServiceIdByKind(ServiceIdKind.PNI) + item.record.contact?.pni === toUntaggedPni(contactA.device.pni) ) .addContact( contactA, @@ -141,8 +140,7 @@ describe('pnp/PNI Change', function needsName() { identityState: Proto.ContactRecord.IdentityState.DEFAULT, whitelisted: true, serviceE164: contactA.device.number, - aci: updatedPni, - pni: updatedPni, + pni: toUntaggedPni(updatedPni), identityKey: contactA.getPublicKey(ServiceIdKind.PNI).serialize(), }, ServiceIdKind.PNI @@ -232,8 +230,7 @@ describe('pnp/PNI Change', function needsName() { state .removeRecord( item => - item.record.contact?.pni === - contactA.device.getServiceIdByKind(ServiceIdKind.PNI) + item.record.contact?.pni === toUntaggedPni(contactA.device.pni) ) .addContact( contactB, @@ -241,7 +238,7 @@ describe('pnp/PNI Change', function needsName() { identityState: Proto.ContactRecord.IdentityState.DEFAULT, whitelisted: true, serviceE164: contactA.device.number, - pni: contactB.device.getServiceIdByKind(ServiceIdKind.PNI), + pni: toUntaggedPni(contactB.device.pni), // Key change - different identity key identityKey: contactB.publicKey.serialize(), @@ -336,8 +333,7 @@ describe('pnp/PNI Change', function needsName() { state .removeRecord( item => - item.record.contact?.pni === - contactA.device.getServiceIdByKind(ServiceIdKind.PNI) + item.record.contact?.pni === toUntaggedPni(contactA.device.pni) ) .addContact( contactB, @@ -345,7 +341,7 @@ describe('pnp/PNI Change', function needsName() { identityState: Proto.ContactRecord.IdentityState.DEFAULT, whitelisted: true, serviceE164: contactA.device.number, - pni: contactB.device.getServiceIdByKind(ServiceIdKind.PNI), + pni: toUntaggedPni(contactB.device.pni), // Note: No identityKey key provided here! }, @@ -470,8 +466,7 @@ describe('pnp/PNI Change', function needsName() { state .removeRecord( item => - item.record.contact?.pni === - contactA.device.getServiceIdByKind(ServiceIdKind.PNI) + item.record.contact?.pni === toUntaggedPni(contactA.device.pni) ) .addContact( contactB, @@ -479,7 +474,7 @@ describe('pnp/PNI Change', function needsName() { identityState: Proto.ContactRecord.IdentityState.DEFAULT, whitelisted: true, serviceE164: contactA.device.number, - pni: contactB.device.getServiceIdByKind(ServiceIdKind.PNI), + pni: toUntaggedPni(contactB.device.pni), // Note: No identityKey key provided here! }, @@ -503,8 +498,7 @@ describe('pnp/PNI Change', function needsName() { state .removeRecord( item => - item.record.contact?.pni === - contactB.device.getServiceIdByKind(ServiceIdKind.PNI) + item.record.contact?.pni === toUntaggedPni(contactB.device.pni) ) .addContact( contactB, @@ -512,7 +506,7 @@ describe('pnp/PNI Change', function needsName() { identityState: Proto.ContactRecord.IdentityState.DEFAULT, whitelisted: true, serviceE164: contactA.device.number, - pni: contactA.device.getServiceIdByKind(ServiceIdKind.PNI), + pni: toUntaggedPni(contactA.device.pni), }, ServiceIdKind.PNI ) diff --git a/ts/test-mock/pnp/pni_signature_test.ts b/ts/test-mock/pnp/pni_signature_test.ts index 88d166598a6..22a22d3c194 100644 --- a/ts/test-mock/pnp/pni_signature_test.ts +++ b/ts/test-mock/pnp/pni_signature_test.ts @@ -15,6 +15,7 @@ import createDebug from 'debug'; import * as durations from '../../util/durations'; import { uuidToBytes } from '../../util/uuidToBytes'; import { MY_STORY_ID } from '../../types/Stories'; +import { isUntaggedPniString, toTaggedPni } from '../../types/ServiceId'; import { Bootstrap } from '../bootstrap'; import type { App } from '../bootstrap'; @@ -345,17 +346,22 @@ describe('pnp/PNI Signature', function needsName() { after: state, }); - const pni = newState.getContact(pniContact, ServiceIdKind.PNI); - const aci = newState.getContact(pniContact, ServiceIdKind.ACI); + const pniRecord = newState.getContact(pniContact, ServiceIdKind.PNI); + const aciRecord = newState.getContact(pniContact, ServiceIdKind.ACI); assert.strictEqual( - aci, - pni, + aciRecord, + pniRecord, 'ACI Contact must be the same as PNI Contact storage service' ); - assert(aci, 'ACI Contact must be in storage service'); + assert(aciRecord, 'ACI Contact must be in storage service'); - assert.strictEqual(aci?.aci, pniContact.device.aci); - assert.strictEqual(aci?.pni, pniContact.device.pni); + assert.strictEqual(aciRecord?.aci, pniContact.device.aci); + assert.strictEqual( + aciRecord?.pni && + isUntaggedPniString(aciRecord?.pni) && + toTaggedPni(aciRecord?.pni), + pniContact.device.pni + ); // Two outgoing, one incoming const messages = window.locator('.module-message__text'); diff --git a/ts/test-mock/pnp/pni_unlink_test.ts b/ts/test-mock/pnp/pni_unlink_test.ts index 5330cd628bd..1fcebe0ec89 100644 --- a/ts/test-mock/pnp/pni_unlink_test.ts +++ b/ts/test-mock/pnp/pni_unlink_test.ts @@ -12,7 +12,7 @@ import { import createDebug from 'debug'; import * as durations from '../../util/durations'; -import { generatePni } from '../../types/ServiceId'; +import { generatePni, toUntaggedPni } from '../../types/ServiceId'; import { Bootstrap } from '../bootstrap'; import type { App } from '../bootstrap'; @@ -92,7 +92,10 @@ describe('pnp/PNI DecryptionError unlink', function needsName() { pniChangeNumber, }, }, - { timestamp: bootstrap.getTimestamp(), updatedPni: generatePni() } + { + timestamp: bootstrap.getTimestamp(), + updatedPni: toUntaggedPni(generatePni()), + } ) ); sendPromises.push( @@ -103,7 +106,10 @@ describe('pnp/PNI DecryptionError unlink', function needsName() { pniChangeNumber, }, }, - { timestamp: bootstrap.getTimestamp(), updatedPni: desktop.pni } + { + timestamp: bootstrap.getTimestamp(), + updatedPni: toUntaggedPni(desktop.pni), + } ) ); diff --git a/ts/test-mock/rate-limit/viewed_test.ts b/ts/test-mock/rate-limit/viewed_test.ts index c6c05c370f1..f41efe0ec28 100644 --- a/ts/test-mock/rate-limit/viewed_test.ts +++ b/ts/test-mock/rate-limit/viewed_test.ts @@ -9,6 +9,7 @@ import * as durations from '../../util/durations'; import { Bootstrap } from '../bootstrap'; import type { App } from '../bootstrap'; import { ReceiptType } from '../../types/Receipt'; +import { toUntaggedPni } from '../../types/ServiceId'; export const debug = createDebug('mock:test:challenge:receipts'); @@ -49,7 +50,7 @@ describe('challenge/receipts', function challengeReceiptsTest() { whitelisted: true, serviceE164: contact.device.number, identityKey: contact.getPublicKey(ServiceIdKind.PNI).serialize(), - pni: contact.device.getServiceIdByKind(ServiceIdKind.PNI), + pni: toUntaggedPni(contact.device.pni), givenName: 'Jamie', }, ServiceIdKind.PNI diff --git a/ts/test-node/sql/migration_960_test.ts b/ts/test-node/sql/migration_960_test.ts new file mode 100644 index 00000000000..cf5acd5fd33 --- /dev/null +++ b/ts/test-node/sql/migration_960_test.ts @@ -0,0 +1,175 @@ +// Copyright 2023 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { assert } from 'chai'; +import type { Database } from '@signalapp/better-sqlite3'; +import SQL from '@signalapp/better-sqlite3'; +import { v4 as generateGuid } from 'uuid'; + +import { updateToVersion, insertData, getTableData } from './helpers'; + +const CONVO_ID = generateGuid(); +const OUR_ACI = generateGuid(); +const OUR_UNPREFIXED_PNI = generateGuid(); +const OUR_PREFIXED_PNI = `PNI:${OUR_UNPREFIXED_PNI}`; + +describe('SQL/updateToSchemaVersion960', () => { + let db: Database; + + beforeEach(() => { + db = new SQL(':memory:'); + updateToVersion(db, 950); + + insertData(db, 'items', [ + { + id: 'uuid_id', + json: { + id: 'uuid_id', + value: `${OUR_ACI}.1`, + }, + }, + { + id: 'pni', + json: { + id: 'pni', + value: OUR_UNPREFIXED_PNI, + }, + }, + ]); + }); + + afterEach(() => { + db.close(); + }); + + it('should migrate our conversation', () => { + insertData(db, 'conversations', [ + { + id: CONVO_ID, + type: 'direct', + serviceId: OUR_ACI, + json: { + id: CONVO_ID, + serviceId: OUR_ACI, + pni: OUR_UNPREFIXED_PNI, + }, + }, + ]); + updateToVersion(db, 960); + assert.deepStrictEqual(getTableData(db, 'conversations'), [ + { + id: CONVO_ID, + type: 'direct', + serviceId: OUR_ACI, + json: { + id: CONVO_ID, + serviceId: OUR_ACI, + pni: OUR_PREFIXED_PNI, + }, + }, + ]); + }); + + it('should migrate items', () => { + insertData(db, 'items', [ + { + id: 'registrationIdMap', + json: { + id: 'registrationIdMap', + value: { + [OUR_ACI]: 123, + [OUR_UNPREFIXED_PNI]: 456, + }, + }, + }, + { + id: 'identityKeyMap', + json: { + id: 'identityKeyMap', + value: { + [OUR_ACI]: {}, + [OUR_UNPREFIXED_PNI]: {}, + }, + }, + }, + ]); + updateToVersion(db, 960); + assert.deepStrictEqual(getTableData(db, 'items'), [ + { + id: 'uuid_id', + json: { + id: 'uuid_id', + value: `${OUR_ACI}.1`, + }, + }, + { + id: 'pni', + json: { + id: 'pni', + value: OUR_PREFIXED_PNI, + }, + }, + { + id: 'registrationIdMap', + json: { + id: 'registrationIdMap', + value: { + [OUR_ACI]: 123, + [OUR_PREFIXED_PNI]: 456, + }, + }, + }, + { + id: 'identityKeyMap', + json: { + id: 'identityKeyMap', + value: { + [OUR_ACI]: {}, + [OUR_PREFIXED_PNI]: {}, + }, + }, + }, + ]); + }); + + for (const table of ['preKeys', 'signedPreKeys', 'kyberPreKeys']) { + // eslint-disable-next-line no-loop-func + it(`should migrate ${table}`, () => { + insertData(db, table, [ + { + id: `${OUR_ACI}:123`, + json: { + id: `${OUR_ACI}:123`, + ourServiceId: OUR_ACI, + }, + }, + { + id: `${OUR_UNPREFIXED_PNI}:456`, + json: { + id: `${OUR_UNPREFIXED_PNI}:456`, + ourServiceId: OUR_UNPREFIXED_PNI, + }, + }, + ]); + updateToVersion(db, 960); + assert.deepStrictEqual(getTableData(db, table), [ + { + id: `${OUR_ACI}:123`, + json: { + id: `${OUR_ACI}:123`, + ourServiceId: OUR_ACI, + }, + ourServiceId: OUR_ACI, + }, + { + id: `${OUR_PREFIXED_PNI}:456`, + json: { + id: `${OUR_PREFIXED_PNI}:456`, + ourServiceId: OUR_PREFIXED_PNI, + }, + ourServiceId: OUR_PREFIXED_PNI, + }, + ]); + }); + } +}); diff --git a/ts/textsecure/AccountManager.ts b/ts/textsecure/AccountManager.ts index 877d62edea8..9d2541de9dd 100644 --- a/ts/textsecure/AccountManager.ts +++ b/ts/textsecure/AccountManager.ts @@ -401,7 +401,7 @@ export default class AccountManager extends EventTarget { !provisionMessage.pniKeyPair || !provisionMessage.profileKey || !provisionMessage.aci || - !isUntaggedPniString(provisionMessage.pni) + !isUntaggedPniString(provisionMessage.untaggedPni) ) { throw new Error( 'AccountManager.registerSecondDevice: Provision message was missing key data' @@ -410,7 +410,7 @@ export default class AccountManager extends EventTarget { const ourAci = normalizeAci(provisionMessage.aci, 'provisionMessage.aci'); const ourPni = normalizePni( - toTaggedPni(provisionMessage.pni), + toTaggedPni(provisionMessage.untaggedPni), 'provisionMessage.pni' ); diff --git a/ts/textsecure/MessageReceiver.ts b/ts/textsecure/MessageReceiver.ts index 84da8be8882..9b35315e73d 100644 --- a/ts/textsecure/MessageReceiver.ts +++ b/ts/textsecure/MessageReceiver.ts @@ -59,8 +59,10 @@ import { normalizeServiceId, normalizePni, isPniString, + isUntaggedPniString, isServiceIdString, fromPniObject, + toTaggedPni, } from '../types/ServiceId'; import { normalizeAci } from '../util/normalizeAci'; import { isAciString } from '../util/isAciString'; @@ -421,12 +423,13 @@ export default class MessageReceiver 'MessageReceiver.handleRequest.destinationServiceId' ) : ourAci, - updatedPni: decoded.updatedPni - ? normalizePni( - decoded.updatedPni, - 'MessageReceiver.handleRequest.updatedPni' - ) - : undefined, + updatedPni: + decoded.updatedPni && isUntaggedPniString(decoded.updatedPni) + ? normalizePni( + toTaggedPni(decoded.updatedPni), + 'MessageReceiver.handleRequest.updatedPni' + ) + : undefined, timestamp: decoded.timestamp?.toNumber(), content: dropNull(decoded.content), serverGuid: decoded.serverGuid, @@ -878,8 +881,11 @@ export default class MessageReceiver decoded.destinationServiceId || item.destinationServiceId || ourAci, 'CachedEnvelope.destinationServiceId' ), - updatedPni: decoded.updatedPni - ? normalizePni(decoded.updatedPni, 'CachedEnvelope.updatedPni') + updatedPni: isUntaggedPniString(decoded.updatedPni) + ? normalizePni( + toTaggedPni(decoded.updatedPni), + 'CachedEnvelope.updatedPni' + ) : undefined, timestamp: decoded.timestamp?.toNumber(), content: dropNull(decoded.content), diff --git a/ts/textsecure/ProvisioningCipher.ts b/ts/textsecure/ProvisioningCipher.ts index 717b9b6344b..78f5d29d8d1 100644 --- a/ts/textsecure/ProvisioningCipher.ts +++ b/ts/textsecure/ProvisioningCipher.ts @@ -12,17 +12,14 @@ import { } from '../Crypto'; import { calculateAgreement, createKeyPair, generateKeyPair } from '../Curve'; import { SignalService as Proto } from '../protobuf'; -import type { PniString, AciString } from '../types/ServiceId'; -import { normalizePni } from '../types/ServiceId'; -import { normalizeAci } from '../util/normalizeAci'; import { strictAssert } from '../util/assert'; type ProvisionDecryptResult = { aciKeyPair: KeyPairType; pniKeyPair?: KeyPairType; number?: string; - aci?: AciString; - pni?: PniString; + aci?: string; + untaggedPni?: string; provisioningCode?: string; userAgent?: string; readReceipts?: boolean; @@ -75,13 +72,14 @@ class ProvisioningCipherInner { const { aci, pni } = provisionMessage; strictAssert(aci, 'Missing aci in provisioning message'); + strictAssert(pni, 'Missing pni in provisioning message'); const ret: ProvisionDecryptResult = { aciKeyPair, pniKeyPair, number: provisionMessage.number, - aci: normalizeAci(aci, 'ProvisionMessage.aci'), - pni: pni ? normalizePni(pni, 'ProvisionMessage.pni') : undefined, + aci, + untaggedPni: pni, provisioningCode: provisionMessage.provisioningCode, userAgent: provisionMessage.userAgent, readReceipts: provisionMessage.readReceipts, diff --git a/ts/types/ServiceId.ts b/ts/types/ServiceId.ts index 488b2f7223b..74cff7b681b 100644 --- a/ts/types/ServiceId.ts +++ b/ts/types/ServiceId.ts @@ -49,6 +49,10 @@ export function toTaggedPni(untagged: UntaggedPniString): PniString { return `PNI:${untagged}` as PniString; } +export function toUntaggedPni(pni: PniString): UntaggedPniString { + return pni.replace(/^PNI:/i, '') as UntaggedPniString; +} + export function normalizeServiceId( rawServiceId: string, context: string, @@ -106,10 +110,9 @@ export function normalizePni( } const result = rawPni.toLowerCase().replace(/^pni:/, 'PNI:'); - if (!isPniString(result)) { logger.warn( - `Normalizing invalid serviceId: ${rawPni} to ${result} in context "${context}"` + `Normalizing invalid pni: ${rawPni} to ${result} in context "${context}"` ); // Cast anyway we don't want to throw here diff --git a/ts/util/normalizeAci.ts b/ts/util/normalizeAci.ts index 11aa96f5859..280861a1cae 100644 --- a/ts/util/normalizeAci.ts +++ b/ts/util/normalizeAci.ts @@ -31,7 +31,7 @@ export function normalizeAci( if (!isAciString(result)) { logger.warn( - `Normalizing invalid serviceId: ${rawAci} to ${result} in context "${context}"` + `Normalizing invalid aci: ${rawAci} to ${result} in context "${context}"` ); // Cast anyway we don't want to throw here diff --git a/yarn.lock b/yarn.lock index d45c82444b2..bb9f4b590ba 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3379,10 +3379,10 @@ node-gyp-build "^4.2.3" uuid "^8.3.0" -"@signalapp/mock-server@4.1.1": - version "4.1.1" - resolved "https://registry.yarnpkg.com/@signalapp/mock-server/-/mock-server-4.1.1.tgz#563a31a30cbefcb6c443a8fe7c77d9f20d3920db" - integrity sha512-u+8BJK3Nl1Daw/I1J5ki4LtB99NvwSCUassEcTllWQppSg0wU0nxOwlDedMseyUvIhtUIePu2/nmysT1E3jRiw== +"@signalapp/mock-server@4.1.2": + version "4.1.2" + resolved "https://registry.yarnpkg.com/@signalapp/mock-server/-/mock-server-4.1.2.tgz#3f497d4cc5cc6613d2a860173ee1d9cee24ce9cd" + integrity sha512-vOFJ8bVQdhII6ZGc34wurxJZ9roeoq4ch0VeorImcyavL5p7d9VbNwpWyOA/VAlfTaUgaiXegVmzK3t52lCQTw== dependencies: "@signalapp/libsignal-client" "^0.30.2" debug "^4.3.2"