Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add sigtimestamp to all content message #69

Merged
merged 3 commits into from
Feb 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@
"fs-extra": "9.0.0",
"glob": "10.3.10",
"image-type": "^4.1.0",
"libsession_util_nodejs": "https://github.com/session-foundation/libsession-util-nodejs/releases/download/v0.4.16/libsession_util_nodejs-v0.4.16.tar.gz",
"libsession_util_nodejs": "https://github.com/session-foundation/libsession-util-nodejs/releases/download/v0.4.17/libsession_util_nodejs-v0.4.17.tar.gz",
"libsodium-wrappers-sumo": "^0.7.9",
"linkify-it": "^4.0.1",
"lodash": "^4.17.21",
Expand Down
5 changes: 3 additions & 2 deletions protos/SignalService.proto
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ message MessageRequestResponse {


message Content {
reserved 7, 11;
reserved "configurationMessage", "sharedConfigMessage";
reserved 7, 11, 14;
reserved "configurationMessage", "sharedConfigMessage", "lastDisappearingMessageChangeTimestamp";

enum ExpirationType {
UNKNOWN = 0;
Expand All @@ -60,6 +60,7 @@ message Content {
optional MessageRequestResponse messageRequestResponse = 10;
optional ExpirationType expirationType = 12;
optional uint32 expirationTimer = 13;
optional uint64 sigTimestamp = 15;
}

message KeyPair {
Expand Down
21 changes: 21 additions & 0 deletions ts/receiver/common.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { toNumber } from 'lodash';
import { EnvelopePlus } from './types';
import type { SignalService } from '../protobuf';
import { DURATION } from '../session/constants';

export function getEnvelopeId(envelope: EnvelopePlus) {
if (envelope.source) {
Expand All @@ -8,3 +10,22 @@ export function getEnvelopeId(envelope: EnvelopePlus) {

return envelope.id;
}

export function shouldProcessContentMessage(
envelope: Pick<EnvelopePlus, 'timestamp'>,
content: Pick<SignalService.Content, 'sigTimestamp'>,
isCommunity: boolean
) {
// FIXME: drop this case once the change has been out in the wild long enough
if (!content.sigTimestamp || !toNumber(content.sigTimestamp)) {
// legacy client
return true;
}
const envelopeTimestamp = toNumber(envelope.timestamp);
const contentTimestamp = toNumber(content.sigTimestamp);
if (!isCommunity) {
return envelopeTimestamp === contentTimestamp;
}
// we want to process a community message and allow a window of 6 hours
return Math.abs(envelopeTimestamp - contentTimestamp) <= 6 * DURATION.HOURS;
}
8 changes: 8 additions & 0 deletions ts/receiver/contentMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import { handleCallMessage } from './callMessage';
import { getAllCachedECKeyPair, sentAtMoreRecentThanWrapper } from './closedGroups';
import { ECKeyPair } from './keypairs';
import { CONVERSATION_PRIORITIES, ConversationTypeEnum } from '../models/types';
import { shouldProcessContentMessage } from './common';

export async function handleSwarmContentMessage(
envelope: EnvelopePlus,
Expand Down Expand Up @@ -480,6 +481,13 @@ export async function innerHandleSwarmContentMessage({
window.log.info('innerHandleSwarmContentMessage');

const content = SignalService.Content.decode(new Uint8Array(contentDecrypted));
if (!shouldProcessContentMessage(envelope, content, false)) {
window.log.info(
`innerHandleSwarmContentMessage: dropping invalid content message ${envelope.timestamp}`
);
await IncomingMessageCache.removeFromCache(envelope);
return;
}

/**
* senderIdentity is set ONLY if that message is a closed group message.
Expand Down
9 changes: 9 additions & 0 deletions ts/receiver/opengroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { fromBase64ToArray } from '../session/utils/String';
import { cleanIncomingDataMessage, messageHasVisibleContent } from './dataMessage';
import { handleMessageJob, toRegularMessage } from './queuedJob';
import { OpenGroupRequestCommonType } from '../data/types';
import { shouldProcessContentMessage } from './common';

export const handleOpenGroupV4Message = async (
message: OpenGroupMessageV4,
Expand Down Expand Up @@ -52,6 +53,14 @@ const handleOpenGroupMessage = async (

const decodedContent = SignalService.Content.decode(dataUint);

if (!shouldProcessContentMessage({ timestamp: sentTimestamp }, decodedContent, true)) {
window?.log?.info(
'sogs message: shouldProcessContentMessage is false for message sentAt:',
sentTimestamp
);
return;
}

const conversationId = getOpenGroupV2ConversationId(serverUrl, roomId);
if (!conversationId) {
window?.log?.error('We cannot handle a message without a conversationId');
Expand Down
10 changes: 8 additions & 2 deletions ts/session/apis/open_group_api/sogsv3/sogsApiV3.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import { sogsRollingDeletions } from './sogsRollingDeletions';
import { processMessagesUsingCache } from './sogsV3MutationCache';
import { OpenGroupRequestCommonType } from '../../../../data/types';
import { ConversationTypeEnum } from '../../../../models/types';
import { shouldProcessContentMessage } from '../../../../receiver/common';

/**
* Get the convo matching those criteria and make sure it is an opengroup convo, or return null.
Expand Down Expand Up @@ -405,7 +406,13 @@ async function handleInboxOutboxMessages(
id: v4(),
type: SignalService.Envelope.Type.SESSION_MESSAGE, // this is not right, but we forward an already decrypted envelope so we don't care
};

const contentDecrypted = SignalService.Content.decode(content);
if (!shouldProcessContentMessage(builtEnvelope, contentDecrypted, true)) {
window.log.warn(
`received inbox/outbox message that did not pass the shouldProcessContentMessage test envelopeTs: ${builtEnvelope.timestamp}`
);
continue;
}
if (isOutbox) {
/**
* Handling outbox messages needs to skip some of the pipeline.
Expand All @@ -414,7 +421,6 @@ async function handleInboxOutboxMessages(
* We will need this to send new message to that user from our second device.
*/
const recipient = inboxOutboxItem.recipient;
const contentDecrypted = SignalService.Content.decode(content);

// if we already know this user's unblinded pubkey, store the blinded message we sent to that blinded recipient under
// the unblinded conversation instead (as we would have merge the blinded one with the other )
Expand Down
20 changes: 19 additions & 1 deletion ts/session/messages/outgoing/ContentMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,32 @@ import { SignalService } from '../../../protobuf';
import { TTL_DEFAULT } from '../../constants';
import { Message } from './Message';

type InstanceFields<T> = {
// eslint-disable-next-line @typescript-eslint/ban-types
[K in keyof T as T[K] extends Function ? never : K]: T[K];
};

type ContentFields = Partial<Omit<InstanceFields<SignalService.Content>, 'sigTimestamp'>>;

export abstract class ContentMessage extends Message {
public plainTextBuffer(): Uint8Array {
return SignalService.Content.encode(this.contentProto()).finish();
const contentProto = this.contentProto();
if (!contentProto.sigTimestamp) {
throw new Error('trying to build a ContentMessage without a sig timestamp is unsupported');
}
return SignalService.Content.encode(contentProto).finish();
}

public ttl(): number {
return TTL_DEFAULT.CONTENT_MESSAGE;
}

public makeContentProto<T extends ContentFields>(extra: T) {
return new SignalService.Content({
...extra,
sigTimestamp: this.createAtNetworkTimestamp,
});
}

public abstract contentProto(): SignalService.Content;
}
2 changes: 1 addition & 1 deletion ts/session/messages/outgoing/ExpirableMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export class ExpirableMessage extends ContentMessage {
}

public contentProto(): SignalService.Content {
return new SignalService.Content({
return super.makeContentProto({
// TODO legacy messages support will be removed in a future release
expirationType:
this.expirationType === 'deleteAfterSend'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export class ExpirationTimerUpdateMessage extends DataMessage {
}

public contentProto(): SignalService.Content {
// TODO: I am pretty sure we don't need this anymore (super.contentProto does the same in DataMessage)
return new SignalService.Content({
...super.contentProto(),
dataMessage: this.dataProto(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ export class MessageRequestResponse extends ContentMessage {
}

public contentProto(): SignalService.Content {
return new SignalService.Content({
messageRequestResponse: this.messageRequestResponseProto(),
});
return super.makeContentProto({ messageRequestResponse: this.messageRequestResponseProto() });
}

public messageRequestResponseProto(): SignalService.MessageRequestResponse {
Expand Down
4 changes: 1 addition & 3 deletions ts/session/messages/outgoing/controlMessage/TypingMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@ export class TypingMessage extends ContentMessage {
}

public contentProto(): SignalService.Content {
return new SignalService.Content({
typingMessage: this.typingProto(),
});
return super.makeContentProto({ typingMessage: this.typingProto() });
}

protected typingProto(): SignalService.TypingMessage {
Expand Down
4 changes: 1 addition & 3 deletions ts/session/messages/outgoing/controlMessage/UnsendMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ export class UnsendMessage extends ContentMessage {
}

public contentProto(): SignalService.Content {
return new SignalService.Content({
unsendMessage: this.unsendProto(),
});
return super.makeContentProto({ unsendMessage: this.unsendProto() });
}

public unsendProto(): SignalService.Unsend {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ export class ReadReceiptMessage extends ContentMessage {
}

public contentProto(): SignalService.Content {
return new SignalService.Content({
receiptMessage: this.receiptProto(),
});
return super.makeContentProto({ receiptMessage: this.receiptProto() });
}

protected receiptProto(): SignalService.ReceiptMessage {
Expand Down
4 changes: 2 additions & 2 deletions ts/session/sending/MessageSentHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ async function handleSwarmMessageSentSuccess(
}
} catch (e) {
window.log.info(
'failed to decode content (excpected except if message was for a 1o1 as we need it to send the sync message'
'failed to decode content (expected except if message was for a 1o1 as we need it to send the sync message'
);
}
} else if (shouldMarkMessageAsSynced) {
Expand Down Expand Up @@ -185,7 +185,7 @@ async function handleSwarmMessageSentFailure(
expirationStartTimestamp: undefined,
});
window.log.warn(
`[handleSwarmMessageSentFailure] Stopping a message from disppearing until we retry the send operation. messageId: ${fetchedMessage.get(
`[handleSwarmMessageSentFailure] Stopping a message from disappearing until we retry the send operation. messageId: ${fetchedMessage.get(
'id'
)}`
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
import { describe } from 'mocha';
import Sinon from 'sinon';
import Long from 'long';
import { expect } from 'chai';
import { TestUtils } from '../../../../test-utils';
import { shouldProcessContentMessage } from '../../../../../receiver/common';
import { DURATION } from '../../../../../session/constants';

describe('shouldProcessContentMessage', () => {
let envelopeTs: number;
beforeEach(() => {
TestUtils.stubWindowLog();
envelopeTs = Math.floor(Date.now() + Math.random() * 1000000);
});

afterEach(() => {
Sinon.restore();
});

describe('not a community', () => {
const isCommunity = false;
describe('with sig timestamp', () => {
it('if timestamps match: return true', async () => {
expect(
shouldProcessContentMessage(
{ timestamp: envelopeTs },
{ sigTimestamp: envelopeTs },
isCommunity
)
).to.eq(true);
});
it('if timestamps do not match: return false', async () => {
expect(
shouldProcessContentMessage(
{ timestamp: envelopeTs },
{ sigTimestamp: envelopeTs + 2 },
isCommunity
)
).to.eq(false);
});
});
describe('without sig timestamp', () => {
it('if timestamps match or not: return true', async () => {
expect(
shouldProcessContentMessage(
{ timestamp: envelopeTs },
{ sigTimestamp: undefined as any },
isCommunity
)
).to.eq(true);
expect(
shouldProcessContentMessage({ timestamp: envelopeTs }, { sigTimestamp: 0 }, isCommunity)
).to.eq(true);
expect(
shouldProcessContentMessage(
{ timestamp: envelopeTs },
{ sigTimestamp: Long.fromNumber(0) as any },
isCommunity
)
).to.eq(true);
});
});
});

describe('a community', () => {
const isCommunity = true;
describe('with sig timestamp', () => {
it('if timestamps roughly match: return true', async () => {
expect(
shouldProcessContentMessage(
{ timestamp: envelopeTs },
{ sigTimestamp: envelopeTs },
isCommunity
),
'exact match'
).to.eq(true);
expect(
shouldProcessContentMessage(
{ timestamp: envelopeTs },
{ sigTimestamp: envelopeTs + 6 * DURATION.HOURS - 1 },
isCommunity
),
'just below 6h of diff (positive)'
).to.eq(true);
expect(
shouldProcessContentMessage(
{ timestamp: envelopeTs },
{ sigTimestamp: envelopeTs - 6 * DURATION.HOURS + 1 },
isCommunity
),
'just below 6h of diff (negative)'
).to.eq(true);
});
it('if timestamps do not roughly match: return false', async () => {
expect(
shouldProcessContentMessage(
{ timestamp: envelopeTs },
{ sigTimestamp: envelopeTs + 6 * DURATION.HOURS + 1 },
isCommunity
),
'just above 6h of diff'
).to.eq(false);
expect(
shouldProcessContentMessage(
{ timestamp: envelopeTs },
{ sigTimestamp: envelopeTs - 6 * DURATION.HOURS - 1 },
isCommunity
),
'just above 6h of diff'
).to.eq(false);
});
});
describe('without sig timestamp', () => {
it('if timestamps match or not: return true', async () => {
expect(
shouldProcessContentMessage(
{ timestamp: envelopeTs },
{ sigTimestamp: undefined as any },
isCommunity
),
'sigTimestamp undefined'
).to.eq(true);
expect(
shouldProcessContentMessage({ timestamp: envelopeTs }, { sigTimestamp: 0 }, isCommunity),
'sigTimestamp 0 as number'
).to.eq(true);
expect(
shouldProcessContentMessage(
{ timestamp: envelopeTs },
{ sigTimestamp: Long.fromNumber(0) as any },
isCommunity
),
'sigTimestamp 0 as Long'
).to.eq(true);
});
});
});
});
6 changes: 3 additions & 3 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4944,9 +4944,9 @@ levn@~0.3.0:
prelude-ls "~1.1.2"
type-check "~0.3.2"

"libsession_util_nodejs@https://github.com/session-foundation/libsession-util-nodejs/releases/download/v0.4.16/libsession_util_nodejs-v0.4.16.tar.gz":
version "0.4.16"
resolved "https://github.com/session-foundation/libsession-util-nodejs/releases/download/v0.4.16/libsession_util_nodejs-v0.4.16.tar.gz#253d4d02388b5bfb41f24c88fae5061b137ca615"
"libsession_util_nodejs@https://github.com/session-foundation/libsession-util-nodejs/releases/download/v0.4.17/libsession_util_nodejs-v0.4.17.tar.gz":
version "0.4.17"
resolved "https://github.com/session-foundation/libsession-util-nodejs/releases/download/v0.4.17/libsession_util_nodejs-v0.4.17.tar.gz#d31d7d2e1d1534c872dc64e0040d6ce533f11ffb"
dependencies:
cmake-js "7.2.1"
node-addon-api "^6.1.0"
Expand Down
Loading