Keep profileKey in situations where it didn't work; still fail over

This commit is contained in:
automated-signal 2024-12-18 14:40:09 -06:00 committed by GitHub
parent 3f217cb36d
commit de9eb4db6e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 55 additions and 95 deletions

View file

@ -2341,6 +2341,9 @@ export class ConversationModel extends window.Backbone
async addMessageRequestResponseEventMessage(
event: MessageRequestResponseEvent
): Promise<void> {
const idForLogging = getConversationIdForLogging(this.attributes);
log.info(`addMessageRequestResponseEventMessage/${idForLogging}: ${event}`);
const timestamp = Date.now();
const lastMessageTimestamp =
// Fallback to `timestamp` since `lastMessageReceivedAtMs` is new
@ -5012,7 +5015,7 @@ export class ConversationModel extends window.Backbone
}
async setProfileKey(
profileKey: string | undefined,
profileKey: string,
{
viaStorageServiceSync = false,
reason,
@ -5022,6 +5025,10 @@ export class ConversationModel extends window.Backbone
profileKey == null || profileKey.length > 0,
'setProfileKey: Profile key cannot be an empty string'
);
if (profileKey === undefined) {
log.warn('setProfileKey: Refusing to set an undefined profileKey');
return false;
}
const oldProfileKey = this.get('profileKey');
@ -5071,7 +5078,7 @@ export class ConversationModel extends window.Backbone
// 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) {
if (!viaStorageServiceSync) {
this.captureChange('profileKey');
}
@ -5145,10 +5152,8 @@ export class ConversationModel extends window.Backbone
const profileKeyVersion = deriveProfileKeyVersion(profileKey, serviceId);
if (!profileKeyVersion) {
log.warn(
'deriveProfileKeyVersion: Failed to derive profile key version, ' +
'clearing profile key.'
'deriveProfileKeyVersion: Failed to derive profile key version, return nothing.'
);
void this.setProfileKey(undefined, { reason: 'deriveProfileKeyVersion' });
return;
}
@ -5183,32 +5188,6 @@ export class ConversationModel extends window.Backbone
await DataWriter.updateConversation(this.attributes);
}
async removeLastProfile(
oldValue: ConversationLastProfileType | undefined
): Promise<void> {
// Atomic updates only
if (this.get('lastProfile') !== oldValue) {
return;
}
log.warn(
'ConversationModel.removeLastProfile: called for',
this.idForLogging()
);
this.set({
lastProfile: undefined,
// We don't have any knowledge of profile anymore. Drop all associated
// data.
about: undefined,
aboutEmoji: undefined,
profileAvatar: undefined,
});
await DataWriter.updateConversation(this.attributes);
}
hasMember(serviceId: ServiceIdString): boolean {
const members = this.getMembers();

View file

@ -292,11 +292,13 @@ async function buildProfileFetchOptions({
lastProfile,
clientZkProfileCipher,
groupId,
options,
}: {
conversation: ConversationModel;
lastProfile: ConversationLastProfileType | null;
clientZkProfileCipher: ClientZkProfileOperations;
groupId: string | null;
options: { ignoreProfileKey: boolean; ignoreGroupSendToken: boolean };
}): Promise<ProfileFetchOptions.Auth | ProfileFetchOptions.Unauth> {
const logId = `buildGetProfileOptions(${conversation.idForLogging()})`;
@ -310,12 +312,12 @@ async function buildProfileFetchOptions({
const accessKey = conversation.get('accessKey');
const serviceId = conversation.getCheckedServiceId('getProfile');
if (profileKey) {
strictAssert(
profileKeyVersion != null && accessKey != null,
`${logId}: profileKeyVersion and accessKey are derived from profileKey`
);
if (
profileKey &&
profileKeyVersion &&
accessKey &&
!options.ignoreProfileKey
) {
if (!conversation.hasProfileKeyCredentialExpired()) {
log.info(`${logId}: using unexpired profile key credential`);
return {
@ -351,15 +353,12 @@ async function buildProfileFetchOptions({
};
}
strictAssert(
accessKey == null,
`${logId}: accessKey have to be absent because there is no profileKey`
);
// If we have a `lastProfile`, try getting the versioned profile with auth.
// Note: We can't try the group send token here because the versioned profile
// can't be decrypted without an up to date profile key.
// If we're ignoring profileKey, try getting the versioned profile with lastProfile.
// Note: No access key, since this is almost guaranteed not to be their current profile.
// Also, we can't try the group send token here because the versioned profile can't be
// decrypted without an up to date profile key.
if (
options.ignoreProfileKey &&
lastProfile != null &&
lastProfile.profileKey != null &&
lastProfile.profileKeyVersion != null
@ -379,7 +378,7 @@ async function buildProfileFetchOptions({
}
// Fallback to group send tokens for unversioned profiles
if (groupId != null) {
if (groupId != null && !options.ignoreGroupSendToken) {
log.info(`${logId}: fetching group endorsements`);
let result = await maybeCreateGroupSendEndorsementState(groupId, false);
@ -456,7 +455,14 @@ function getFetchOptionsLabel(
async function doGetProfile(
c: ConversationModel,
groupId: string | null
groupId: string | null,
{
ignoreProfileKey,
ignoreGroupSendToken,
}: {
ignoreProfileKey: boolean;
ignoreGroupSendToken: boolean;
} = { ignoreProfileKey: false, ignoreGroupSendToken: false }
): Promise<void> {
const logId = groupId
? `getProfile(${c.idForLogging()} in groupv2(${groupId}))`
@ -482,10 +488,8 @@ async function doGetProfile(
const serviceId = c.getCheckedServiceId('getProfile');
// Step #: Grab the profile key and version we last were successful decrypting with
// `lastProfile` is saved at the end of `doGetProfile` after successfully decrypting.
// `lastProfile` is used in case the `profileKey` was cleared because of a 401/403.
// `lastProfile` is cleared when we get a 404 fetching a profile.
// Step #: Grab the profile key and version we last were successful decrypting with.
// We'll use it in case we've failed to fetch with our `profileKey`.
const lastProfile = c.get('lastProfile');
// Step #: Build the request options we will use for fetching and decrypting the profile
@ -494,6 +498,10 @@ async function doGetProfile(
lastProfile: lastProfile ?? null,
clientZkProfileCipher,
groupId,
options: {
ignoreProfileKey,
ignoreGroupSendToken,
},
});
const { request } = options;
@ -523,43 +531,30 @@ async function doGetProfile(
// Fallback from failed unauth (access key) request
if (request.accessKey != null) {
log.warn(
`${logId}: Got ${error.code} when using access key, removing profileKey and retrying`
`${logId}: Got ${error.code} when using access key, failing over to lastProfile`
);
await c.setProfileKey(undefined, {
reason: 'doGetProfile/accessKey/401+403',
});
// Retry fetch using last known profileKeyVersion or fetch
// unversioned profile.
return doGetProfile(c, groupId);
// Record that the accessKey we have in the conversation is invalid
const sealedSender = c.get('sealedSender');
if (sealedSender !== SEALED_SENDER.DISABLED) {
c.set('sealedSender', SEALED_SENDER.DISABLED);
}
// Retry fetch using last known profileKey or fetch unversioned profile.
return doGetProfile(c, groupId, {
ignoreProfileKey: true,
ignoreGroupSendToken,
});
}
// Fallback from failed unauth (group send token) request
if (request.groupSendToken != null) {
log.warn(`${logId}: Got ${error.code} when using group send token`);
return doGetProfile(c, null);
}
}
// Step #: Record if the accessKey we have in the conversation is valid
const sealedSender = c.get('sealedSender');
if (
sealedSender === SEALED_SENDER.ENABLED ||
sealedSender === SEALED_SENDER.UNRESTRICTED
) {
if (!isMe(c.attributes)) {
log.warn(
`${logId}: Got ${error.code} when using accessKey, removing profileKey`
);
await c.setProfileKey(undefined, {
reason: 'doGetProfile/accessKey/401+403',
return doGetProfile(c, null, {
ignoreProfileKey,
ignoreGroupSendToken: true,
});
}
} else if (sealedSender === SEALED_SENDER.UNKNOWN) {
log.warn(
`${logId}: Got ${error.code} fetching profile, setting sealedSender = DISABLED`
);
c.set('sealedSender', SEALED_SENDER.DISABLED);
}
// TODO: Is it safe to ignore these errors?
@ -571,10 +566,8 @@ async function doGetProfile(
log.info(`${logId}: Profile not found`);
c.set('profileLastFetchedAt', Date.now());
// Note: Writes to DB:
await c.removeLastProfile(lastProfile);
if (!isVersioned) {
if (!isVersioned || ignoreProfileKey) {
log.info(`${logId}: Marking conversation unregistered`);
c.setUnregistered();
}

View file

@ -107,19 +107,7 @@ function processError(error: unknown): void {
'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`
);
void conversation.setProfileKey(undefined, {
reason: 'handleMessageSend/processError',
});
}
if (conversation.get('sealedSender') === SEALED_SENDER.UNKNOWN) {
if (conversation.get('sealedSender') !== SEALED_SENDER.DISABLED) {
log.warn(
`handleMessageSend: Got 401/403 for ${conversation.idForLogging()}, setting sealedSender = DISABLED`
);