profileKey: Check length of incoming values, clear on failed send/fetch

This commit is contained in:
Scott Nonnenberg 2022-02-22 12:34:57 -08:00 committed by GitHub
parent b96c7e90fe
commit b33b5d2a30
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
18 changed files with 284 additions and 56 deletions

View file

@ -2590,7 +2590,7 @@ export async function startApp(): Promise<void> {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const conversation = window.ConversationController.get(detailsId)!;
if (details.profileKey) {
if (details.profileKey && details.profileKey.length > 0) {
const profileKey = Bytes.toBase64(details.profileKey);
conversation.setProfileKey(profileKey);
}

View file

@ -2730,7 +2730,11 @@ async function updateGroup(
'private'
);
if (member.profileKey && !contact.get('profileKey')) {
if (
member.profileKey &&
member.profileKey.length > 0 &&
!contact.get('profileKey')
) {
contactsWithoutProfileKey.push(contact);
contact.setProfileKey(member.profileKey);
}

View file

@ -7,6 +7,7 @@ import { getSendOptions } from '../../util/getSendOptions';
import {
isDirectConversation,
isGroupV2,
isMe,
} from '../../util/whatTypeOfConversation';
import { SignalService as Proto } from '../../protobuf';
import {
@ -22,6 +23,9 @@ import type {
DeleteForEveryoneJobData,
} from '../conversationJobQueue';
import { getUntrustedConversationIds } from './getUntrustedConversationIds';
import { handleMessageSend } from '../../util/handleMessageSend';
import { isConversationAccepted } from '../../util/isConversationAccepted';
import { isConversationUnregistered } from '../../util/isConversationUnregistered';
// 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 a delete
@ -78,7 +82,48 @@ export async function sendDeleteForEveryone(
const sendOptions = await getSendOptions(conversation.attributes);
try {
if (isDirectConversation(conversation.attributes)) {
if (isMe(conversation.attributes)) {
const proto = await window.textsecure.messaging.getContentMessage({
deletedForEveryoneTimestamp: targetTimestamp,
profileKey,
recipients: conversation.getRecipients(),
timestamp,
});
if (!proto.dataMessage) {
log.error(
"ContentMessage proto didn't have a data message; cancelling job."
);
return;
}
await handleMessageSend(
window.textsecure.messaging.sendSyncMessage({
encodedDataMessage: Proto.DataMessage.encode(
proto.dataMessage
).finish(),
destination: conversation.get('e164'),
destinationUuid: conversation.get('uuid'),
expirationStartTimestamp: null,
options: sendOptions,
timestamp,
}),
{ messageIds, sendType }
);
} else if (isDirectConversation(conversation.attributes)) {
if (!isConversationAccepted(conversation.attributes)) {
log.info(
`conversation ${conversation.idForLogging()} is not accepted; refusing to send`
);
return;
}
if (isConversationUnregistered(conversation.attributes)) {
log.info(
`conversation ${conversation.idForLogging()} is unregistered; refusing to send`
);
return;
}
await wrapWithSyncMessageSend({
conversation,
logId,

View file

@ -16,6 +16,9 @@ import type {
ExpirationTimerUpdateJobData,
ConversationQueueJobBundle,
} from '../conversationJobQueue';
import { handleMessageSend } from '../../util/handleMessageSend';
import { isConversationAccepted } from '../../util/isConversationAccepted';
import { isConversationUnregistered } from '../../util/isConversationUnregistered';
export async function sendDirectExpirationTimerUpdate(
conversation: ConversationModel,
@ -86,17 +89,33 @@ export async function sendDirectExpirationTimerUpdate(
try {
if (isMe(conversation.attributes)) {
await window.textsecure.messaging.sendSyncMessage({
encodedDataMessage: Proto.DataMessage.encode(
proto.dataMessage
).finish(),
destination: conversation.get('e164'),
destinationUuid: conversation.get('uuid'),
expirationStartTimestamp: null,
options: sendOptions,
timestamp,
});
await handleMessageSend(
window.textsecure.messaging.sendSyncMessage({
encodedDataMessage: Proto.DataMessage.encode(
proto.dataMessage
).finish(),
destination: conversation.get('e164'),
destinationUuid: conversation.get('uuid'),
expirationStartTimestamp: null,
options: sendOptions,
timestamp,
}),
{ messageIds: [], sendType }
);
} else if (isDirectConversation(conversation.attributes)) {
if (!isConversationAccepted(conversation.attributes)) {
log.info(
`conversation ${conversation.idForLogging()} is not accepted; refusing to send`
);
return;
}
if (isConversationUnregistered(conversation.attributes)) {
log.info(
`conversation ${conversation.idForLogging()} is unregistered; refusing to send`
);
return;
}
await wrapWithSyncMessageSend({
conversation,
logId,

View file

@ -29,6 +29,8 @@ import type {
import { handleMultipleSendErrors } from './handleMultipleSendErrors';
import { ourProfileKeyService } from '../../services/ourProfileKey';
import { isConversationUnregistered } from '../../util/isConversationUnregistered';
import { isConversationAccepted } from '../../util/isConversationAccepted';
export async function sendNormalMessage(
conversation: ConversationModel,
@ -209,6 +211,25 @@ export async function sendNormalMessage(
})
);
} else {
if (!isConversationAccepted(conversation.attributes)) {
log.info(
`conversation ${conversation.idForLogging()} is not accepted; refusing to send`
);
markMessageFailed(message, [
new Error('Message request was not accepted'),
]);
return;
}
if (isConversationUnregistered(conversation.attributes)) {
log.info(
`conversation ${conversation.idForLogging()} is unregistered; refusing to send`
);
markMessageFailed(message, [
new Error('Contact no longer has a Signal account'),
]);
return;
}
log.info('sending direct message');
innerPromise = window.textsecure.messaging.sendMessageToIdentifier({
identifier: recipientIdentifiersWithoutMe[0],

View file

@ -24,6 +24,8 @@ import type {
import type { CallbackResultType } from '../../textsecure/Types.d';
import { getUntrustedConversationIds } from './getUntrustedConversationIds';
import { areAllErrorsUnregistered } from './areAllErrorsUnregistered';
import { isConversationAccepted } from '../../util/isConversationAccepted';
import { isConversationUnregistered } from '../../util/isConversationUnregistered';
// Note: because we don't have a recipient map, we will resend this message to folks that
// got it on the first go-round, if some sends fail. This is okay, because a recipient
@ -83,6 +85,19 @@ export async function sendProfileKey(
}
if (isDirectConversation(conversation.attributes)) {
if (!isConversationAccepted(conversation.attributes)) {
log.info(
`conversation ${conversation.idForLogging()} is not accepted; refusing to send`
);
return;
}
if (isConversationUnregistered(conversation.attributes)) {
log.info(
`conversation ${conversation.idForLogging()} is unregistered; refusing to send`
);
return;
}
const proto = await window.textsecure.messaging.getContentMessage({
flags: Proto.DataMessage.Flags.PROFILE_KEY_UPDATE,
profileKey,

View file

@ -31,6 +31,8 @@ import type {
ConversationQueueJobBundle,
ReactionJobData,
} from '../conversationJobQueue';
import { isConversationAccepted } from '../../util/isConversationAccepted';
import { isConversationUnregistered } from '../../util/isConversationUnregistered';
export async function sendReaction(
conversation: ConversationModel,
@ -180,6 +182,21 @@ export async function sendReaction(
let promise: Promise<CallbackResultType>;
if (isDirectConversation(conversation.attributes)) {
if (!isConversationAccepted(conversation.attributes)) {
log.info(
`conversation ${conversation.idForLogging()} is not accepted; refusing to send`
);
markReactionFailed(message, pendingReaction);
return;
}
if (isConversationUnregistered(conversation.attributes)) {
log.info(
`conversation ${conversation.idForLogging()} is unregistered; refusing to send`
);
markReactionFailed(message, pendingReaction);
return;
}
log.info('sending direct reaction message');
promise = window.textsecure.messaging.sendMessageToIdentifier({
identifier: recipientIdentifiersWithoutMe[0],

View file

@ -34,11 +34,11 @@ export function initializeAllJobQueues({
deliveryReceiptsJobQueue.streamJobs();
readReceiptsJobQueue.streamJobs();
viewedReceiptsJobQueue.streamJobs();
viewOnceOpenJobQueue.streamJobs();
// Syncs to ourselves
readSyncJobQueue.streamJobs();
viewSyncJobQueue.streamJobs();
viewOnceOpenJobQueue.streamJobs();
// Other queues
removeStorageKeyJobQueue.streamJobs();

View file

@ -20,6 +20,8 @@ import {
handleMultipleSendErrors,
maybeExpandErrors,
} from './helpers/handleMultipleSendErrors';
import { isConversationUnregistered } from '../util/isConversationUnregistered';
import { isConversationAccepted } from '../util/isConversationAccepted';
const MAX_RETRY_TIME = DAY;
const MAX_PARALLEL_JOBS = 5;
@ -76,6 +78,19 @@ export class SingleProtoJobQueue extends JobQueue<SingleProtoJobData> {
);
}
if (!isConversationAccepted(conversation.attributes)) {
log.info(
`conversation ${conversation.idForLogging()} is not accepted; refusing to send`
);
return;
}
if (isConversationUnregistered(conversation.attributes)) {
log.info(
`conversation ${conversation.idForLogging()} is unregistered; refusing to send`
);
return;
}
const proto = Proto.Content.decode(Bytes.fromBase64(protoBase64));
const options = await getSendOptions(conversation.attributes, {
syncMessage: isSyncMessage,

View file

@ -4624,7 +4624,7 @@ export class ConversationModel extends window.Backbone
}
async setProfileKey(
profileKey: string,
profileKey: string | undefined,
{ viaStorageServiceSync = false } = {}
): Promise<void> {
// profileKey is a string so we can compare it directly
@ -4643,7 +4643,10 @@ export class ConversationModel extends window.Backbone
sealedSender: SEALED_SENDER.UNKNOWN,
});
if (!viaStorageServiceSync) {
// If our profile key was cleared above, we don't tell our linked devices about it.
// We want linked devices to tell us what it should be, instead of telling them to
// erase their local value.
if (!viaStorageServiceSync && profileKey) {
this.captureChange('profileKey');
}
@ -4686,6 +4689,12 @@ export class ConversationModel extends window.Backbone
profileKey,
uuid
);
if (!profileKeyVersion) {
log.warn(
'deriveProfileKeyVersionIfNeeded: Failed to derive profile key version, clearing profile key.'
);
this.setProfileKey(undefined);
}
this.set({ profileKeyVersion });
}

View file

@ -19,6 +19,7 @@ import {
repeat,
zipObject,
} from '../util/iterables';
import type { SentEventData } from '../textsecure/messageReceiverEvents';
import { isNotNil } from '../util/isNotNil';
import { isNormalNumber } from '../util/isNormalNumber';
import { strictAssert } from '../util/assert';
@ -72,6 +73,7 @@ import { markRead, markViewed } from '../services/MessageUpdater';
import { isMessageUnread } from '../util/isMessageUnread';
import {
isDirectConversation,
isGroup,
isGroupV1,
isGroupV2,
isMe,
@ -1376,7 +1378,9 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
break;
}
case 'UnregisteredUserError':
shouldSaveError = false;
if (conversation && isGroup(conversation.attributes)) {
shouldSaveError = false;
}
// If we just found out that we couldn't send to a user because they are no
// longer registered, we will update our unregistered flag. In groups we
// will not event try to send to them for 6 hours. And we will never try
@ -2171,11 +2175,11 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
}
}
handleDataMessage(
async handleDataMessage(
initialMessage: ProcessedDataMessage,
confirm: () => void,
options: { data?: typeof window.WhatIsThis } = {}
): WhatIsThis {
options: { data?: SentEventData } = {}
): Promise<void> {
const { data } = options;
// This function is called from the background script in a few scenarios:
@ -2198,7 +2202,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const conversation = window.ConversationController.get(conversationId)!;
return conversation.queueJob('handleDataMessage', async () => {
await conversation.queueJob('handleDataMessage', async () => {
log.info(
`Starting handleDataMessage for message ${message.idForLogging()} in conversation ${conversation.idForLogging()}`
);
@ -2243,7 +2247,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
};
const unidentifiedStatus: Array<ProcessedUnidentifiedDeliveryStatus> =
Array.isArray(data.unidentifiedStatus)
data && Array.isArray(data.unidentifiedStatus)
? data.unidentifiedStatus
: [];
@ -2265,9 +2269,10 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
return;
}
const updatedAt: number = isNormalNumber(data.timestamp)
? data.timestamp
: Date.now();
const updatedAt: number =
data && isNormalNumber(data.timestamp)
? data.timestamp
: Date.now();
const previousSendState = getOwn(
sendStateByConversationId,
@ -2747,7 +2752,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
}
if (dataMessage.profileKey) {
const profileKey = dataMessage.profileKey.toString('base64');
const { profileKey } = dataMessage;
if (
source === window.textsecure.storage.user.getNumber() ||
sourceUuid ===

View file

@ -1,4 +1,4 @@
// Copyright 2020-2021 Signal Messenger, LLC
// Copyright 2020-2022 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
import { isEqual, isNumber } from 'lodash';
@ -796,7 +796,7 @@ export async function mergeContactRecord(
'private'
);
if (contactRecord.profileKey) {
if (contactRecord.profileKey && contactRecord.profileKey.length > 0) {
await conversation.setProfileKey(Bytes.toBase64(contactRecord.profileKey), {
viaStorageServiceSync: true,
});
@ -1102,7 +1102,7 @@ export async function mergeAccountRecord(
storageVersion,
});
if (accountRecord.profileKey) {
if (accountRecord.profileKey && accountRecord.profileKey.length > 0) {
await conversation.setProfileKey(Bytes.toBase64(accountRecord.profileKey));
}

View file

@ -1,4 +1,4 @@
// Copyright 2020-2021 Signal Messenger, LLC
// Copyright 2020-2022 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
/* eslint-disable no-bitwise */
@ -1824,7 +1824,10 @@ export default class MessageReceiver
}
if (msg.flags && msg.flags & Proto.DataMessage.Flags.PROFILE_KEY_UPDATE) {
strictAssert(msg.profileKey, 'PROFILE_KEY_UPDATE without profileKey');
strictAssert(
msg.profileKey && msg.profileKey.length > 0,
'PROFILE_KEY_UPDATE without profileKey'
);
const ev = new ProfileKeyUpdateEvent(
{

View file

@ -1,4 +1,4 @@
// Copyright 2020-2021 Signal Messenger, LLC
// Copyright 2020-2022 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
import Long from 'long';
@ -263,9 +263,10 @@ export async function processDataMessage(
groupV2: processGroupV2Context(message.groupV2),
flags: message.flags ?? 0,
expireTimer: message.expireTimer ?? 0,
profileKey: message.profileKey
? Bytes.toBase64(message.profileKey)
: undefined,
profileKey:
message.profileKey && message.profileKey.length > 0
? Bytes.toBase64(message.profileKey)
: undefined,
timestamp,
quote: processQuote(message.quote),
contact: processContact(message.contact),

View file

@ -1,4 +1,4 @@
// Copyright 2020-2021 Signal Messenger, LLC
// Copyright 2020-2022 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
import type { ProfileKeyCredentialRequestContext } from '@signalapp/signal-client/zkgroup';
@ -244,12 +244,27 @@ export async function getProfile(
}
} catch (error) {
switch (error?.code) {
case 401:
case 403:
throw error;
if (
c.get('sealedSender') === SEALED_SENDER.ENABLED ||
c.get('sealedSender') === SEALED_SENDER.UNRESTRICTED
) {
log.warn(
`getProfile: Got 401/403 when using accessKey for ${c.idForLogging()}, removing profileKey`
);
c.setProfileKey(undefined);
}
if (c.get('sealedSender') === SEALED_SENDER.UNKNOWN) {
log.warn(
`getProfile: Got 401/403 when using accessKey for ${c.idForLogging()}, setting sealedSender = DISABLED`
);
c.set('sealedSender', SEALED_SENDER.DISABLED);
}
return;
case 404:
log.warn(
`getProfile failure: failed to find a profile for ${c.idForLogging()}`,
error && error.stack ? error.stack : error
log.info(
`getProfile: failed to find a profile for ${c.idForLogging()}`
);
c.setUnregistered();
return;

View file

@ -6,15 +6,15 @@ import { isNumber } from 'lodash';
import type { CallbackResultType } from '../textsecure/Types.d';
import dataInterface from '../sql/Client';
import * as log from '../logging/log';
import {
OutgoingMessageError,
SendMessageNetworkError,
SendMessageProtoError,
UnregisteredUserError,
} from '../textsecure/Errors';
import { SEALED_SENDER } from '../types/SealedSender';
const { insertSentProto } = dataInterface;
export const SEALED_SENDER = {
UNKNOWN: 0,
ENABLED: 1,
DISABLED: 2,
UNRESTRICTED: 3,
};
const { insertSentProto, updateConversation } = dataInterface;
export const sendTypesEnum = z.enum([
'blockSyncRequest',
@ -72,6 +72,53 @@ export function shouldSaveProto(sendType: SendTypesType): boolean {
return true;
}
function processError(error: unknown): void {
if (
error instanceof OutgoingMessageError ||
error instanceof SendMessageNetworkError
) {
const conversation = window.ConversationController.getOrCreate(
error.identifier,
'private'
);
if (error.code === 401 || error.code === 403) {
if (
conversation.get('sealedSender') === SEALED_SENDER.ENABLED ||
conversation.get('sealedSender') === SEALED_SENDER.UNRESTRICTED
) {
log.warn(
`handleMessageSend: Got 401/403 for ${conversation.idForLogging()}, removing profile key`
);
conversation.setProfileKey(undefined);
}
if (conversation.get('sealedSender') === SEALED_SENDER.UNKNOWN) {
log.warn(
`handleMessageSend: Got 401/403 for ${conversation.idForLogging()}, setting sealedSender = DISABLED`
);
conversation.set('sealedSender', SEALED_SENDER.DISABLED);
updateConversation(conversation.attributes);
}
}
if (error.code === 404) {
log.warn(
`handleMessageSend: Got 404 for ${conversation.idForLogging()}, marking unregistered.`
);
conversation.setUnregistered();
}
}
if (error instanceof UnregisteredUserError) {
const conversation = window.ConversationController.getOrCreate(
error.identifier,
'private'
);
log.warn(
`handleMessageSend: Got 404 for ${conversation.idForLogging()}, marking unregistered.`
);
conversation.setUnregistered();
}
}
export async function handleMessageSend(
promise: Promise<CallbackResultType>,
options: {
@ -91,12 +138,17 @@ export async function handleMessageSend(
return result;
} catch (err) {
if (err) {
processError(err);
if (err instanceof SendMessageProtoError) {
await handleMessageSendResult(
err.failoverIdentifiers,
err.unidentifiedDeliveries
);
err.errors?.forEach(processError);
}
throw err;
}
}

View file

@ -1,4 +1,4 @@
// Copyright 2021 Signal Messenger, LLC
// Copyright 2021-2022 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
import { chunk } from 'lodash';
@ -8,6 +8,7 @@ import { ReceiptType } from '../types/Receipt';
import { getSendOptions } from './getSendOptions';
import { handleMessageSend } from './handleMessageSend';
import { isConversationAccepted } from './isConversationAccepted';
import { isConversationUnregistered } from './isConversationUnregistered';
import { map } from './iterables';
import { missingCaseError } from './missingCaseError';
@ -91,6 +92,15 @@ export async function sendReceipts({
}
if (!isConversationAccepted(sender.attributes)) {
log.info(
`conversation ${sender.idForLogging()} is not accepted; refusing to send`
);
return;
}
if (isConversationUnregistered(sender.attributes)) {
log.info(
`conversation ${sender.idForLogging()} is unregistered; refusing to send`
);
return;
}

View file

@ -1,4 +1,4 @@
// Copyright 2021 Signal Messenger, LLC
// Copyright 2021-2022 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
import { differenceWith, omit, partition } from 'lodash';
@ -46,11 +46,8 @@ import type {
SenderKeyInfoType,
} from '../model-types.d';
import type { SendTypesType } from './handleMessageSend';
import {
handleMessageSend,
SEALED_SENDER,
shouldSaveProto,
} from './handleMessageSend';
import { handleMessageSend, shouldSaveProto } from './handleMessageSend';
import { SEALED_SENDER } from '../types/SealedSender';
import { parseIntOrThrow } from './parseIntOrThrow';
import {
multiRecipient200ResponseSchema,