Move to uuids for untrusted conversations needing verification

This commit is contained in:
Scott Nonnenberg 2022-05-31 12:46:56 -07:00 committed by GitHub
parent d3f9b656dd
commit d446aa9e6b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 138 additions and 101 deletions

View File

@ -330,7 +330,7 @@ export class ConversationJobQueue extends JobQueue<ConversationQueueJobData> {
}
}
} catch (error: unknown) {
const untrustedConversationIds: Array<string> = [];
const untrustedUuids: Array<string> = [];
const processError = (toProcess: unknown) => {
if (toProcess instanceof OutgoingIdentityKeyError) {
@ -339,7 +339,14 @@ export class ConversationJobQueue extends JobQueue<ConversationQueueJobData> {
'private'
);
strictAssert(failedConversation, 'Conversation should be created');
untrustedConversationIds.push(failedConversation.id);
const uuid = failedConversation.get('uuid');
if (!uuid) {
log.error(
`failedConversation: Conversation ${failedConversation.idForLogging()} missing UUID!`
);
return;
}
untrustedUuids.push(uuid);
} else if (toProcess instanceof SendMessageChallengeError) {
window.Signal.challengeHandler.register(
{
@ -358,14 +365,14 @@ export class ConversationJobQueue extends JobQueue<ConversationQueueJobData> {
(error.errors || []).forEach(processError);
}
if (untrustedConversationIds.length) {
if (untrustedUuids.length) {
log.error(
`Send failed because ${untrustedConversationIds.length} conversation(s) were untrusted. Adding to verification list.`
`Send failed because ${untrustedUuids.length} conversation(s) were untrusted. Adding to verification list.`
);
window.reduxActions.conversations.conversationStoppedByMissingVerification(
{
conversationId: conversation.id,
untrustedConversationIds,
untrustedUuids,
}
);
}

View File

@ -1,20 +0,0 @@
// Copyright 2022 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
import { isNotNil } from '../../util/isNotNil';
export function getUntrustedConversationIds(
recipients: ReadonlyArray<string>
): Array<string> {
return recipients
.map(recipient => {
const recipientConversation = window.ConversationController.getOrCreate(
recipient,
'private'
);
return recipientConversation.isUntrusted()
? recipientConversation.id
: null;
})
.filter(isNotNil);
}

View File

@ -0,0 +1,32 @@
// Copyright 2022 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
import { isNotNil } from '../../util/isNotNil';
import * as log from '../../logging/log';
export function getUntrustedConversationUuids(
recipients: ReadonlyArray<string>
): Array<string> {
return recipients
.map(recipient => {
const recipientConversation = window.ConversationController.getOrCreate(
recipient,
'private'
);
if (!recipientConversation.isUntrusted()) {
return null;
}
const uuid = recipientConversation.get('uuid');
if (!uuid) {
log.warn(
`getUntrustedConversationUuids: Conversation ${recipientConversation.idForLogging()} had no UUID`
);
return null;
}
return uuid;
})
.filter(isNotNil);
}

View File

@ -23,7 +23,7 @@ import type {
ConversationQueueJobBundle,
DeleteForEveryoneJobData,
} from '../conversationJobQueue';
import { getUntrustedConversationIds } from './getUntrustedConversationIds';
import { getUntrustedConversationUuids } from './getUntrustedConversationUuids';
import { handleMessageSend } from '../../util/handleMessageSend';
import { isConversationAccepted } from '../../util/isConversationAccepted';
import { isConversationUnregistered } from '../../util/isConversationUnregistered';
@ -79,14 +79,14 @@ export async function sendDeleteForEveryone(
? getRecipients(deletedForEveryoneSendStatus)
: recipientsFromJob;
const untrustedConversationIds = getUntrustedConversationIds(recipients);
if (untrustedConversationIds.length) {
const untrustedUuids = getUntrustedConversationUuids(recipients);
if (untrustedUuids.length) {
window.reduxActions.conversations.conversationStoppedByMissingVerification({
conversationId: conversation.id,
untrustedConversationIds,
untrustedUuids,
});
throw new Error(
`Delete for everyone blocked because ${untrustedConversationIds.length} conversation(s) were untrusted. Failing this attempt.`
`Delete for everyone blocked because ${untrustedUuids.length} conversation(s) were untrusted. Failing this attempt.`
);
}

View File

@ -44,9 +44,14 @@ export async function sendDirectExpirationTimerUpdate(
}
if (conversation.isUntrusted()) {
const uuid = conversation
.getCheckedUuid(
'Expiration timer send blocked: untrusted and missing uuid!'
)
.toString();
window.reduxActions.conversations.conversationStoppedByMissingVerification({
conversationId: conversation.id,
untrustedConversationIds: [conversation.id],
untrustedUuids: [uuid],
});
throw new Error(
'Expiration timer send blocked because conversation is untrusted. Failing this attempt.'

View File

@ -19,7 +19,7 @@ import type {
GroupUpdateJobData,
ConversationQueueJobBundle,
} from '../conversationJobQueue';
import { getUntrustedConversationIds } from './getUntrustedConversationIds';
import { getUntrustedConversationUuids } from './getUntrustedConversationUuids';
// Note: because we don't have a recipient map, if some sends fail, we will resend this
// message to folks that got it on the first go-round. This is okay, because receivers
@ -53,14 +53,14 @@ export async function sendGroupUpdate(
const { groupChangeBase64, recipients, revision } = data;
const untrustedConversationIds = getUntrustedConversationIds(recipients);
if (untrustedConversationIds.length) {
const untrustedUuids = getUntrustedConversationUuids(recipients);
if (untrustedUuids.length) {
window.reduxActions.conversations.conversationStoppedByMissingVerification({
conversationId: conversation.id,
untrustedConversationIds,
untrustedUuids,
});
throw new Error(
`Group update blocked because ${untrustedConversationIds.length} conversation(s) were untrusted. Failing this attempt.`
`Group update blocked because ${untrustedUuids.length} conversation(s) were untrusted. Failing this attempt.`
);
}

View File

@ -108,21 +108,22 @@ export async function sendNormalMessage(
const {
allRecipientIdentifiers,
recipientIdentifiersWithoutMe,
untrustedConversationIds,
untrustedUuids,
} = getMessageRecipients({
log,
message,
conversation,
});
if (untrustedConversationIds.length) {
if (untrustedUuids.length) {
window.reduxActions.conversations.conversationStoppedByMissingVerification(
{
conversationId: conversation.id,
untrustedConversationIds,
untrustedUuids,
}
);
throw new Error(
`Message ${messageId} sending blocked because ${untrustedConversationIds.length} conversation(s) were untrusted. Failing this attempt.`
`Message ${messageId} sending blocked because ${untrustedUuids.length} conversation(s) were untrusted. Failing this attempt.`
);
}
@ -326,19 +327,21 @@ export async function sendNormalMessage(
}
function getMessageRecipients({
log,
conversation,
message,
}: Readonly<{
log: LoggerType;
conversation: ConversationModel;
message: MessageModel;
}>): {
allRecipientIdentifiers: Array<string>;
recipientIdentifiersWithoutMe: Array<string>;
untrustedConversationIds: Array<string>;
untrustedUuids: Array<string>;
} {
const allRecipientIdentifiers: Array<string> = [];
const recipientIdentifiersWithoutMe: Array<string> = [];
const untrustedConversationIds: Array<string> = [];
const untrustedUuids: Array<string> = [];
const currentConversationRecipients = conversation.getMemberConversationIds();
@ -365,7 +368,14 @@ function getMessageRecipients({
}
if (recipient.isUntrusted()) {
untrustedConversationIds.push(recipientConversationId);
const uuid = recipient.get('uuid');
if (!uuid) {
log.error(
`sendNormalMessage/getMessageRecipients: Untrusted conversation ${recipient.idForLogging()} missing UUID.`
);
return;
}
untrustedUuids.push(uuid);
return;
}
if (recipient.isUnregistered()) {
@ -387,7 +397,7 @@ function getMessageRecipients({
return {
allRecipientIdentifiers,
recipientIdentifiersWithoutMe,
untrustedConversationIds,
untrustedUuids,
};
}

View File

@ -34,6 +34,7 @@ import type {
} from '../conversationJobQueue';
import { isConversationAccepted } from '../../util/isConversationAccepted';
import { isConversationUnregistered } from '../../util/isConversationUnregistered';
import type { LoggerType } from '../../types/Logging';
export async function sendReaction(
conversation: ConversationModel,
@ -106,18 +107,18 @@ export async function sendReaction(
const {
allRecipientIdentifiers,
recipientIdentifiersWithoutMe,
untrustedConversationIds,
} = getRecipients(pendingReaction, conversation);
untrustedUuids,
} = getRecipients(log, pendingReaction, conversation);
if (untrustedConversationIds.length) {
if (untrustedUuids.length) {
window.reduxActions.conversations.conversationStoppedByMissingVerification(
{
conversationId: conversation.id,
untrustedConversationIds,
untrustedUuids,
}
);
throw new Error(
`Reaction for message ${messageId} sending blocked because ${untrustedConversationIds.length} conversation(s) were untrusted. Failing this attempt.`
`Reaction for message ${messageId} sending blocked because ${untrustedUuids.length} conversation(s) were untrusted. Failing this attempt.`
);
}
@ -382,16 +383,17 @@ const setReactions = (
};
function getRecipients(
log: LoggerType,
reaction: Readonly<MessageReactionType>,
conversation: ConversationModel
): {
allRecipientIdentifiers: Array<string>;
recipientIdentifiersWithoutMe: Array<string>;
untrustedConversationIds: Array<string>;
untrustedUuids: Array<string>;
} {
const allRecipientIdentifiers: Array<string> = [];
const recipientIdentifiersWithoutMe: Array<string> = [];
const untrustedConversationIds: Array<string> = [];
const untrustedUuids: Array<string> = [];
const currentConversationRecipients = conversation.getMemberConversationIds();
@ -412,11 +414,18 @@ function getRecipients(
}
if (recipient.isUntrusted()) {
untrustedConversationIds.push(recipientIdentifier);
const uuid = recipient.get('uuid');
if (!uuid) {
log.error(
`sendReaction/getRecipients: Untrusted conversation ${recipient.idForLogging()} missing UUID.`
);
continue;
}
untrustedUuids.push(uuid);
continue;
}
if (recipient.isUnregistered()) {
untrustedConversationIds.push(recipientIdentifier);
untrustedUuids.push(recipientIdentifier);
continue;
}
@ -429,7 +438,7 @@ function getRecipients(
return {
allRecipientIdentifiers,
recipientIdentifiersWithoutMe,
untrustedConversationIds,
untrustedUuids,
};
}

View File

@ -56,7 +56,7 @@ import { ContactSpoofingType } from '../../util/contactSpoofing';
import { writeProfile } from '../../services/writeProfile';
import { writeUsername } from '../../services/writeUsername';
import {
getConversationIdsStoppingSend,
getConversationUuidsStoppingSend,
getConversationIdsStoppedForVerification,
getMe,
getUsernameSaveState,
@ -280,7 +280,7 @@ type ComposerGroupCreationState = {
export type ConversationVerificationData =
| {
type: ConversationVerificationState.PendingVerification;
conversationsNeedingVerification: ReadonlyArray<string>;
uuidsNeedingVerification: ReadonlyArray<string>;
}
| {
type: ConversationVerificationState.VerificationCancelled;
@ -535,7 +535,7 @@ type ConversationStoppedByMissingVerificationActionType = {
type: typeof CONVERSATION_STOPPED_BY_MISSING_VERIFICATION;
payload: {
conversationId: string;
untrustedConversationIds: ReadonlyArray<string>;
untrustedUuids: ReadonlyArray<string>;
};
};
export type MessageChangedActionType = {
@ -1280,19 +1280,22 @@ function verifyConversationsStoppingSend(): ThunkAction<
> {
return async (dispatch, getState) => {
const state = getState();
const conversationIdsStoppingSend = getConversationIdsStoppingSend(state);
const uuidsStoppingSend = getConversationUuidsStoppingSend(state);
const conversationIdsBlocked =
getConversationIdsStoppedForVerification(state);
log.info(
`verifyConversationsStoppingSend: Starting with ${conversationIdsBlocked.length} blocked ` +
`conversations and ${conversationIdsStoppingSend.length} conversations to verify.`
`conversations and ${uuidsStoppingSend.length} conversations to verify.`
);
// Mark conversations as approved/verified as appropriate
const promises: Array<Promise<unknown>> = [];
conversationIdsStoppingSend.forEach(async conversationId => {
const conversation = window.ConversationController.get(conversationId);
uuidsStoppingSend.forEach(async uuid => {
const conversation = window.ConversationController.get(uuid);
if (!conversation) {
log.warn(
`verifyConversationsStoppingSend: Cannot verify missing converastion for uuid ${uuid}`
);
return;
}
@ -1512,19 +1515,17 @@ function selectMessage(
function conversationStoppedByMissingVerification(payload: {
conversationId: string;
untrustedConversationIds: ReadonlyArray<string>;
untrustedUuids: ReadonlyArray<string>;
}): ConversationStoppedByMissingVerificationActionType {
// Fetching profiles to ensure that we have their latest identity key in storage
const profileFetchQueue = new PQueue({
concurrency: 3,
});
payload.untrustedConversationIds.forEach(untrustedConversationId => {
const conversation = window.ConversationController.get(
untrustedConversationId
);
payload.untrustedUuids.forEach(uuid => {
const conversation = window.ConversationController.get(uuid);
if (!conversation) {
log.error(
`conversationStoppedByMissingVerification: conversationId ${untrustedConversationId} not found!`
`conversationStoppedByMissingVerification: uuid ${uuid} not found!`
);
return;
}
@ -2440,7 +2441,7 @@ export function reducer(
};
}
if (action.type === CONVERSATION_STOPPED_BY_MISSING_VERIFICATION) {
const { conversationId, untrustedConversationIds } = action.payload;
const { conversationId, untrustedUuids } = action.payload;
const { verificationDataByConversation } = state;
const existingPendingState = getOwn(
@ -2459,16 +2460,16 @@ export function reducer(
...verificationDataByConversation,
[conversationId]: {
type: ConversationVerificationState.PendingVerification as const,
conversationsNeedingVerification: untrustedConversationIds,
uuidsNeedingVerification: untrustedUuids,
},
},
};
}
const conversationsNeedingVerification: ReadonlyArray<string> = Array.from(
const uuidsNeedingVerification: ReadonlyArray<string> = Array.from(
new Set([
...existingPendingState.conversationsNeedingVerification,
...untrustedConversationIds,
...existingPendingState.uuidsNeedingVerification,
...untrustedUuids,
])
);
@ -2478,7 +2479,7 @@ export function reducer(
...verificationDataByConversation,
[conversationId]: {
type: ConversationVerificationState.PendingVerification as const,
conversationsNeedingVerification,
uuidsNeedingVerification,
},
},
};

View File

@ -1011,13 +1011,13 @@ export const getConversationsStoppedForVerification = createSelector(
}
);
export const getConversationIdsStoppingSend = createSelector(
export const getConversationUuidsStoppingSend = createSelector(
getConversationVerificationData,
(pendingData): Array<string> => {
const result = new Set<string>();
Object.values(pendingData).forEach(item => {
if (item.type === ConversationVerificationState.PendingVerification) {
item.conversationsNeedingVerification.forEach(conversationId => {
item.uuidsNeedingVerification.forEach(conversationId => {
result.add(conversationId);
});
}
@ -1027,20 +1027,13 @@ export const getConversationIdsStoppingSend = createSelector(
);
export const getConversationsStoppingSend = createSelector(
getConversationByIdSelector,
getConversationIdsStoppingSend,
getConversationSelector,
getConversationUuidsStoppingSend,
(
conversationSelector: (id: string) => undefined | ConversationType,
conversationIds: ReadonlyArray<string>
conversationSelector: GetConversationByIdType,
uuids: ReadonlyArray<string>
): Array<ConversationType> => {
const conversations = conversationIds
.map(conversationId => conversationSelector(conversationId))
.filter(isNotNil);
if (conversationIds.length !== conversations.length) {
log.warn(
`getConversationsStoppingSend: Started with ${conversationIds.length} items, ended up with ${conversations.length}.`
);
}
const conversations = uuids.map(uuid => conversationSelector(uuid));
return sortByTitle(conversations);
}
);

View File

@ -27,7 +27,7 @@ import {
getComposeSelectedContacts,
getContactNameColorSelector,
getConversationByIdSelector,
getConversationIdsStoppingSend,
getConversationUuidsStoppingSend,
getConversationIdsStoppedForVerification,
getConversationsByTitleSelector,
getConversationSelector,
@ -311,17 +311,17 @@ describe('both/state/selectors/conversations', () => {
verificationDataByConversation: {
'convo a': {
type: ConversationVerificationState.PendingVerification as const,
conversationsNeedingVerification: ['abc'],
uuidsNeedingVerification: ['abc'],
},
'convo b': {
type: ConversationVerificationState.PendingVerification as const,
conversationsNeedingVerification: ['def', 'abc'],
uuidsNeedingVerification: ['def', 'abc'],
},
},
},
};
assert.sameDeepMembers(getConversationIdsStoppingSend(state), [
assert.sameDeepMembers(getConversationUuidsStoppingSend(state), [
'abc',
'def',
]);
@ -354,11 +354,11 @@ describe('both/state/selectors/conversations', () => {
verificationDataByConversation: {
'convo a': {
type: ConversationVerificationState.PendingVerification as const,
conversationsNeedingVerification: ['abc'],
uuidsNeedingVerification: ['abc'],
},
'convo b': {
type: ConversationVerificationState.PendingVerification as const,
conversationsNeedingVerification: ['def', 'abc'],
uuidsNeedingVerification: ['def', 'abc'],
},
},
},

View File

@ -798,28 +798,28 @@ describe('both/state/ducks/conversations', () => {
getEmptyState(),
conversationStoppedByMissingVerification({
conversationId: 'convo A',
untrustedConversationIds: ['convo 1'],
untrustedUuids: ['convo 1'],
})
);
const second = reducer(
first,
conversationStoppedByMissingVerification({
conversationId: 'convo A',
untrustedConversationIds: ['convo 2'],
untrustedUuids: ['convo 2'],
})
);
const third = reducer(
second,
conversationStoppedByMissingVerification({
conversationId: 'convo A',
untrustedConversationIds: ['convo 1', 'convo 3'],
untrustedUuids: ['convo 1', 'convo 3'],
})
);
assert.deepStrictEqual(third.verificationDataByConversation, {
'convo A': {
type: ConversationVerificationState.PendingVerification,
conversationsNeedingVerification: ['convo 1', 'convo 2', 'convo 3'],
uuidsNeedingVerification: ['convo 1', 'convo 2', 'convo 3'],
},
});
});
@ -838,14 +838,14 @@ describe('both/state/ducks/conversations', () => {
state,
conversationStoppedByMissingVerification({
conversationId: 'convo A',
untrustedConversationIds: ['convo 1', 'convo 2'],
untrustedUuids: ['convo 1', 'convo 2'],
})
);
assert.deepStrictEqual(actual.verificationDataByConversation, {
'convo A': {
type: ConversationVerificationState.PendingVerification,
conversationsNeedingVerification: ['convo 1', 'convo 2'],
uuidsNeedingVerification: ['convo 1', 'convo 2'],
},
});
});
@ -877,7 +877,7 @@ describe('both/state/ducks/conversations', () => {
verificationDataByConversation: {
'convo A': {
type: ConversationVerificationState.PendingVerification,
conversationsNeedingVerification: ['convo 1', 'convo 2'],
uuidsNeedingVerification: ['convo 1', 'convo 2'],
},
},
};
@ -971,7 +971,7 @@ describe('both/state/ducks/conversations', () => {
verificationDataByConversation: {
'convo A': {
type: ConversationVerificationState.PendingVerification,
conversationsNeedingVerification: ['convo 1', 'convo 2'],
uuidsNeedingVerification: ['convo 1', 'convo 2'],
},
},
};