mirrors/signal-desktop
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Add pniSignatureVerified support

Browse source
This commit is contained in:
Fedor Indutny 2024-01-29 14:37:26 -08:00 committed by GitHub
parent 7dc11c1928
commit 95caf59c3c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 258 additions and 126 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
package.json
View file

@ -199,7 +199,7 @@
"@electron/notarize": "2.1.0",
"@formatjs/intl": "2.6.7",
"@mixer/parallel-prettier": "2.0.3",
"@signalapp/mock-server": "4.5.0",
"@signalapp/mock-server": "4.6.0",
"@storybook/addon-a11y": "7.4.5",
"@storybook/addon-actions": "7.4.5",
"@storybook/addon-controls": "7.4.5",

1
protos/SignalStorage.proto
View file

@ -97,6 +97,7 @@ message ContactRecord {
optional string systemFamilyName = 18;
optional string systemNickname = 19;
optional bool hidden = 20;
optional bool pniSignatureVerified = 21;
}
message GroupV1Record {

15
ts/ConversationController.ts
View file

@ -54,6 +54,7 @@ const { hasOwnProperty } = Object.prototype;
function applyChangeToConversation(
conversation: ConversationModel,
pniSignatureVerified: boolean,
suggestedChange: Partial<
Pick<ConversationAttributesType, 'serviceId' | 'e164' | 'pni'>
>
@ -93,7 +94,7 @@ function applyChangeToConversation(
conversation.updateE164(change.e164);
}
if (hasOwnProperty.call(change, 'pni')) {
conversation.updatePni(change.pni);
conversation.updatePni(change.pni, pniSignatureVerified);
}
// Note: we don't do a conversation.set here, because change is limited to these fields
@ -504,6 +505,8 @@ export class ConversationController {
: undefined;
const mergePromises: Array<Promise<void>> = [];
const pniSignatureVerified = aci != null && pni != null && fromPniSignature;
if (!aci && !e164 && !pni) {
throw new Error(
`${logId}: Need to provide at least one of: aci, e164, pni`
@ -547,7 +550,7 @@ export class ConversationController {
`conversation - ${targetConversation.idForLogging()}`
);
// Note: This line might erase a known e164 or PNI
applyChangeToConversation(targetConversation, {
applyChangeToConversation(targetConversation, pniSignatureVerified, {
[key]: value,
});
} else {
@ -611,7 +614,7 @@ export class ConversationController {
log.info(
`${logId}: Applying new value for ${unused.key} to target conversation`
);
applyChangeToConversation(targetConversation, {
applyChangeToConversation(targetConversation, pniSignatureVerified, {
[unused.key]: unused.value,
});
});
@ -654,7 +657,7 @@ export class ConversationController {
if ((key === 'pni' || key === 'e164') && match.getServiceId() === pni) {
change.serviceId = undefined;
}
applyChangeToConversation(match, change);
applyChangeToConversation(match, pniSignatureVerified, change);
// Note: The PNI check here is just to be bulletproof; if we know a
// serviceId is a PNI, then that should be put in the serviceId field
@ -662,7 +665,7 @@ export class ConversationController {
const willMerge =
!match.getServiceId() && !match.get('e164') && !match.getPni();
applyChangeToConversation(targetConversation, {
applyChangeToConversation(targetConversation, pniSignatureVerified, {
[key]: value,
});
@ -687,7 +690,7 @@ export class ConversationController {
`${logId}: Re-adding ${key} on target conversation - ` +
`${targetConversation.idForLogging()}`
);
applyChangeToConversation(targetConversation, {
applyChangeToConversation(targetConversation, pniSignatureVerified, {
[key]: value,
});
}

1
ts/model-types.d.ts vendored
View file

@ -370,6 +370,7 @@ export type ConversationAttributesType = {
// Private core info
serviceId?: ServiceIdString;
pni?: PniString;
pniSignatureVerified?: boolean;
e164?: string;
// Private other fields

13
ts/models/conversations.ts
View file

@ -814,7 +814,7 @@ export class ConversationModel extends window.Backbone
const aci = this.getServiceId();
if (e164 && pni && aci && pni !== aci) {
this.updateE164(undefined);
this.updatePni(undefined);
this.updatePni(undefined, false);
const { conversation: split } =
window.ConversationController.maybeMergeContacts({
@ -1915,7 +1915,7 @@ export class ConversationModel extends window.Backbone
window.Signal.Data.updateConversation(this.attributes);
}
updatePni(pni?: PniString): void {
updatePni(pni: PniString | undefined, pniSignatureVerified: boolean): void {
const oldValue = this.getPni();
if (pni === oldValue) {
return;
@ -1925,6 +1925,15 @@ export class ConversationModel extends window.Backbone
'pni',
pni ? normalizePni(pni, 'Conversation.updatePni') : undefined
);
const newPniSignatureVerified = pni ? pniSignatureVerified : false;
if (this.get('pniSignatureVerified') !== newPniSignatureVerified) {
log.warn(
`updatePni/${this.idForLogging()}: setting ` +
`pniSignatureVerified to ${newPniSignatureVerified}`
);
this.set('pniSignatureVerified', newPniSignatureVerified);
this.captureChange('pniSignatureVerified');
}
const pniIsPrimaryId =
!this.getServiceId() ||

4
ts/services/storageRecordOps.ts
View file

@ -176,6 +176,8 @@ export async function toContactRecord(
if (pni) {
contactRecord.pni = toUntaggedPni(pni);
}
contactRecord.pniSignatureVerified =
conversation.get('pniSignatureVerified') ?? false;
const profileKey = conversation.get('profileKey');
if (profileKey) {
contactRecord.profileKey = Bytes.fromBase64(String(profileKey));
@ -991,6 +993,7 @@ export async function mergeContactRecord(
const e164 = dropNull(contactRecord.serviceE164);
const { aci } = contactRecord;
const pni = dropNull(contactRecord.pni);
const pniSignatureVerified = contactRecord.pniSignatureVerified || false;
const serviceId = aci || pni;
// All contacts must have UUID
@ -1013,6 +1016,7 @@ export async function mergeContactRecord(
aci,
e164,
pni,
fromPniSignature: pniSignatureVerified,
reason: 'mergeContactRecord',
});

226
ts/test-mock/pnp/merge_test.ts
View file

@ -9,6 +9,7 @@ import Long from 'long';
import * as durations from '../../util/durations';
import { uuidToBytes } from '../../util/uuidToBytes';
import { generateConfigMatrix } from '../../util/generateConfigMatrix';
import { toUntaggedPni } from '../../types/ServiceId';
import { MY_STORY_ID } from '../../types/Stories';
import { Bootstrap } from '../bootstrap';
@ -98,131 +99,134 @@ describe('pnp/merge', function (this: Mocha.Suite) {
await bootstrap.teardown();
});
for (const finalContact of [ServiceIdKind.ACI, ServiceIdKind.PNI]) {
for (const withNotification of [false, true]) {
const testName =
'happens via storage service, ' +
`${withNotification ? 'with' : 'without'} notification ` +
`(${finalContact})`;
const matrix = generateConfigMatrix({
finalContact: [ServiceIdKind.ACI, ServiceIdKind.PNI],
withPNIMessage: [false, true],
pniSignatureVerified: [false, true],
});
for (const { finalContact, withPNIMessage, pniSignatureVerified } of matrix) {
const testName =
'happens via storage service, ' +
`${withPNIMessage ? 'with' : 'without'} pni message ` +
`${pniSignatureVerified ? 'with' : 'without'} pniSignatureVerified ` +
`(${finalContact})`;
// eslint-disable-next-line no-loop-func
it(testName, async () => {
const { phone } = bootstrap;
// eslint-disable-next-line no-loop-func
it(testName, async () => {
const { phone } = bootstrap;
const window = await app.getWindow();
const leftPane = window.locator('#LeftPane');
const window = await app.getWindow();
const leftPane = window.locator('#LeftPane');
debug('opening conversation with the aci contact');
debug('opening conversation with the aci contact');
await leftPane
.locator(`[data-testid="${pniContact.device.aci}"]`)
.click();
await window.locator('.module-conversation-hero').waitFor();
debug('Send message to ACI');
{
const compositionInput = await app.waitForEnabledComposer();
await compositionInput.type('Hello ACI');
await compositionInput.press('Enter');
}
debug('opening conversation with the pni contact');
await leftPane
.locator('.module-conversation-list__item--contact-or-conversation')
.first()
.click();
await window.locator('.module-conversation-hero').waitFor();
debug('Verify starting state');
{
// No messages
const messages = window.locator('.module-message__text');
assert.strictEqual(await messages.count(), 0, 'message count');
// No notifications
const notifications = window.locator('.SystemMessage');
assert.strictEqual(
await notifications.count(),
0,
'notification count'
);
}
if (withPNIMessage) {
debug('Send message to PNI');
const compositionInput = await app.waitForEnabledComposer();
await compositionInput.type('Hello PNI');
await compositionInput.press('Enter');
}
if (finalContact === ServiceIdKind.ACI) {
debug('switching back to ACI conversation');
await leftPane
.locator(`[data-testid="${pniContact.device.aci}"]`)
.click();
await window.locator('.module-conversation-hero').waitFor();
}
debug('Send message to ACI');
{
const compositionInput = await app.waitForEnabledComposer();
await compositionInput.type('Hello ACI');
await compositionInput.press('Enter');
}
debug('opening conversation with the pni contact');
await leftPane
.locator('.module-conversation-list__item--contact-or-conversation')
.first()
.click();
await window.locator('.module-conversation-hero').waitFor();
debug('Verify starting state');
{
// No messages
const messages = window.locator('.module-message__text');
assert.strictEqual(await messages.count(), 0, 'message count');
// No notifications
const notifications = window.locator('.SystemMessage');
assert.strictEqual(
await notifications.count(),
0,
'notification count'
);
}
if (withNotification) {
debug('Send message to PNI');
const compositionInput = await app.waitForEnabledComposer();
await compositionInput.type('Hello PNI');
await compositionInput.press('Enter');
}
if (finalContact === ServiceIdKind.ACI) {
debug('switching back to ACI conversation');
await leftPane
.locator(`[data-testid="${pniContact.device.aci}"]`)
.click();
await window.locator('.module-conversation-hero').waitFor();
}
debug(
'removing both contacts from storage service, adding one combined contact'
debug(
'removing both contacts from storage service, adding one combined contact'
);
{
const state = await phone.expectStorageState('consistency check');
await phone.setStorageState(
state.mergeContact(pniContact, {
identityState: Proto.ContactRecord.IdentityState.DEFAULT,
whitelisted: true,
identityKey: pniContact.publicKey.serialize(),
profileKey: pniContact.profileKey.serialize(),
pniSignatureVerified,
})
);
{
const state = await phone.expectStorageState('consistency check');
await phone.setStorageState(
state.mergeContact(pniContact, {
identityState: Proto.ContactRecord.IdentityState.DEFAULT,
whitelisted: true,
identityKey: pniContact.publicKey.serialize(),
profileKey: pniContact.profileKey.serialize(),
})
);
await phone.sendFetchStorage({
timestamp: bootstrap.getTimestamp(),
});
await app.waitForManifestVersion(state.version);
}
await phone.sendFetchStorage({
timestamp: bootstrap.getTimestamp(),
});
await app.waitForManifestVersion(state.version);
}
debug('Verify final state');
{
// Should have both PNI and ACI messages
debug('Verify final state');
{
// Should have both PNI and ACI messages
await window.locator('.module-message__text >> "Hello ACI"').waitFor();
if (withPNIMessage) {
await window
.locator('.module-message__text >> "Hello ACI"')
.locator('.module-message__text >> "Hello PNI"')
.waitFor();
if (withNotification) {
await window
.locator('.module-message__text >> "Hello PNI"')
.waitFor();
}
const messages = window.locator('.module-message__text');
assert.strictEqual(
await messages.count(),
withNotification ? 2 : 1,
'message count'
);
// One notification - the merge
const notifications = window.locator('.SystemMessage');
assert.strictEqual(
await notifications.count(),
withNotification ? 1 : 0,
'notification count'
);
if (withNotification) {
const first = await notifications.first();
assert.match(
await first.innerText(),
/Your message history with ACI Contact and their number .* has been merged./
);
}
}
});
}
const messages = window.locator('.module-message__text');
assert.strictEqual(
await messages.count(),
withPNIMessage ? 2 : 1,
'message count'
);
// One notification - the merge
const notifications = window.locator('.SystemMessage');
assert.strictEqual(
await notifications.count(),
withPNIMessage && !pniSignatureVerified ? 1 : 0,
'notification count'
);
if (withPNIMessage && !pniSignatureVerified) {
const first = await notifications.first();
assert.match(
await first.innerText(),
/Your message history with ACI Contact and their number .* has been merged./
);
}
}
});
}
it('accepts storage service contact splitting', async () => {

1
ts/test-mock/pnp/pni_signature_test.ts
View file

@ -371,6 +371,7 @@ describe('pnp/PNI Signature', function (this: Mocha.Suite) {
toTaggedPni(aciRecord?.pni),
pniContact.device.pni
);
assert.strictEqual(aciRecord?.pniSignatureVerified, true);
// Two outgoing, one incoming
const messages = window.locator('.module-message__text');

50
ts/test-node/util/generateConfigMatrix_test.ts Normal file
View file

@ -0,0 +1,50 @@
// Copyright 2024 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
import { assert } from 'chai';
import { generateConfigMatrix } from '../../util/generateConfigMatrix';
describe('generateConfigMatrix', () => {
it('generates an empty list', () => {
assert.deepStrictEqual(generateConfigMatrix({}), []);
assert.deepStrictEqual(
generateConfigMatrix({
prop1: [],
prop2: [],
}),
[]
);
});
it('generates a single-element list', () => {
assert.deepStrictEqual(
generateConfigMatrix({
prop1: ['a'],
prop2: ['b'],
}),
[
{
prop1: 'a',
prop2: 'b',
},
]
);
});
it('generates multiple permutations', () => {
assert.deepStrictEqual(
generateConfigMatrix({
prop1: ['a', 'b'],
prop2: ['c', 'd'],
}),
[
{ prop1: 'a', prop2: 'c' },
{ prop1: 'b', prop2: 'c' },
{ prop1: 'a', prop2: 'd' },
{ prop1: 'b', prop2: 'd' },
]
);
});
});

50
ts/util/generateConfigMatrix.ts Normal file
View file

@ -0,0 +1,50 @@
// Copyright 2024 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
// Only for tests. Use this to generate multiple test cases for a config matrix.
//
// Example:
//
// generateConfigMatrix({
// contacts: [20, 30],
// hasNotification: [false, true],
// });
//
// Outputs:
//
// [
// { contacts: 20, hasNotification: false },
// { contacts: 30, hasNotification: false },
// { contacts: 20, hasNotification: true },
// { contacts: 30, hasNotification: true },
// ]
export function generateConfigMatrix<Config>(combinations: {
[K in keyof Config]: ReadonlyArray<Config[K]>;
}): ReadonlyArray<Config> {
let result = [{} as Record<string, unknown>];
const entries = [...Object.entries(combinations)] as Array<
[string, ReadonlyArray<unknown>]
>;
if (
entries.length === 0 ||
entries.every(([, values]) => values.length === 0)
) {
return [];
}
for (const [key, values] of entries) {
result = values
// Make a copy of each existing result for each value
// eslint-disable-next-line no-loop-func
.map(value =>
result.map(config => ({
...config,
[key]: value,
}))
)
.flat();
}
return result as ReadonlyArray<Config>;
}

21
yarn.lock
View file

@ -3955,7 +3955,7 @@
bindings "^1.5.0"
tar "^6.1.0"
"@signalapp/libsignal-client@0.39.1", "@signalapp/libsignal-client@^0.39.1":
"@signalapp/libsignal-client@0.39.1":
version "0.39.1"
resolved "https://registry.yarnpkg.com/@signalapp/libsignal-client/-/libsignal-client-0.39.1.tgz#15b41f15c516ae3eecf8a098a9c9c7aac00444d7"
integrity sha512-Drna/0rQTa/jB475KssoBA86Da/DLdJYDznkbiFG2YD/OeWEKoDpi64bp+BIpnc2o16GnVhGLFzNvMfVkI41eQ==
@ -3964,12 +3964,21 @@
type-fest "^3.5.0"
uuid "^8.3.0"
"@signalapp/mock-server@4.5.0":
version "4.5.0"
resolved "https://registry.yarnpkg.com/@signalapp/mock-server/-/mock-server-4.5.0.tgz#532afb96a916dea17e39d5112bbc58b0135ccbe2"
integrity sha512-kcZHfipopBV6UtbEwI7O96zE0au+BLf8qDHAzCefFG28Wy7c4nsia3Me0tKBrguvgo3ySq70iuqTGrqsyzR/5g==
"@signalapp/libsignal-client@^0.39.2":
version "0.39.3"
resolved "https://registry.yarnpkg.com/@signalapp/libsignal-client/-/libsignal-client-0.39.3.tgz#f246fca94e3ac3bdfbe7dc34a70ff90a108a6b7b"
integrity sha512-5WnTyT2AYsSTZrt4vWVYXZw93bO8b7Z4STwEqTX3mUtHh6hcoaVSMC50WLuNrSsfDnqiPATO4mlGJrQ79siguA==
dependencies:
"@signalapp/libsignal-client" "^0.39.1"
node-gyp-build "^4.2.3"
type-fest "^3.5.0"
uuid "^8.3.0"
"@signalapp/mock-server@4.6.0":
version "4.6.0"
resolved "https://registry.yarnpkg.com/@signalapp/mock-server/-/mock-server-4.6.0.tgz#4530ba1cc56fe71be9137ac0434523bec1f1b163"
integrity sha512-2TobdaMERrhXpY0dbwCszNJpcp1YdLOWiDByX4XHnoJ6yyubnW00rfM6h7PzDVIgLpb6fVfHI3sLbQlubxeo5g==
dependencies:
"@signalapp/libsignal-client" "^0.39.2"
debug "^4.3.2"
long "^4.0.0"
micro "^9.3.4"