Use untagged pnis in storage service

2023-09-28 01:14:55 +02:00 · 2023-09-28 01:14:55 +02:00 · eb7942dd1e
commit eb7942dd1e
parent 283ef57779
16 changed files with 530 additions and 62 deletions
--- 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",
--- 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');
--- a/ts/sql/migrations/960-untag-pni.ts
+++ 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<string, unknown> } =
+        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<Record<string, unknown>>;
+    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);
+}
--- 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 {
--- 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
--- 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
          )
--- 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');
--- 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),
+        }
      )
    );

--- 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
--- a/ts/test-node/sql/migration_960_test.ts
+++ 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,
+        },
+      ]);
+    });
+  }
+});
--- 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'
      );

--- 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),
--- 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,
--- 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
--- 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
--- 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"