Skip to content

Commit

Permalink
[lib] Restart session when any Olm error is thrown
Browse files Browse the repository at this point in the history
Summary:
This diff restarts session when any Olm error is thrown by throwing an exception with OLM_ERROR flag set if error occured when decrypting the message.
If flag is included in the exception then the session is reset.

https://linear.app/comm/issue/ENG-9729/session-not-reset-after-getting-bad-message-mac

Test Plan:
1. Simulate BAD_MESSAGE_MAC by changing the last 3 characters of the message passed to session.decrypt() in the code
2. Verify in the logs that BAD_MESSAGE_MAC is thrown and session is reset.

Reviewers: kamil, tomek, ashoat

Reviewed By: kamil, ashoat

Subscribers: ashoat, tomek

Differential Revision: https://phab.comm.dev/D13757
  • Loading branch information
graszka22 committed Oct 24, 2024
1 parent b0cce8b commit 4723c3b
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 16 deletions.
4 changes: 2 additions & 2 deletions lib/tunnelbroker/use-peer-to-peer-message-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import { getContentSigningKey } from '../utils/crypto-utils.js';
import { getMessageForException } from '../utils/errors.js';
import {
hasHigherDeviceID,
OLM_SESSION_ERROR_PREFIX,
OLM_ERROR_FLAG,
olmSessionErrors,
} from '../utils/olm-utils.js';
import { getClientMessageIDFromTunnelbrokerMessageID } from '../utils/peer-to-peer-communication-utils.js';
Expand Down Expand Up @@ -314,7 +314,7 @@ function usePeerToPeerMessageHandler(): (
);

if (
!e.message?.includes(OLM_SESSION_ERROR_PREFIX) &&
!e.message?.includes(OLM_ERROR_FLAG) &&
!e.message?.includes(olmSessionErrors.sessionDoesNotExist)
) {
throw e;
Expand Down
5 changes: 5 additions & 0 deletions lib/utils/olm-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@ function retrieveAccountKeysSet(account: OlmAccount): AccountKeysSet {
}

export const OLM_SESSION_ERROR_PREFIX = 'OLM_';

// this constant has to match olmErrorFlag constant
// in native/cpp/CommonCpp/CryptoTools/Session.cpp
export const OLM_ERROR_FLAG = 'OLM_ERROR';

const olmSessionErrors = Object.freeze({
// Two clients send the session request to each other at the same time,
// we choose which session to keep based on `deviceID`.
Expand Down
8 changes: 6 additions & 2 deletions native/cpp/CommonCpp/CryptoTools/Session.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
namespace comm {
namespace crypto {

// this constant has to match OLM_ERROR_FLAG constant in
// lib/utils/olm-utils.js
static const std::string olmErrorFlag = "OLM_ERROR";

OlmSession *Session::getOlmSession() {
return reinterpret_cast<OlmSession *>(this->olmSessionBuffer.data());
}
Expand Down Expand Up @@ -174,8 +178,8 @@ std::string Session::decrypt(EncryptedData &encryptedData) {
decryptedMessage.size());
if (decryptedSize == -1) {
throw std::runtime_error{
"error decrypt => " + std::string{::olm_session_last_error(session)} +
". Hash: " +
"error decrypt => " + olmErrorFlag + " " +
std::string{::olm_session_last_error(session)} + ". Hash: " +
std::string{messageHashBuffer.begin(), messageHashBuffer.end()}};
}
return std::string{(char *)decryptedMessage.data(), decryptedSize};
Expand Down
40 changes: 28 additions & 12 deletions web/shared-worker/worker/worker-crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
shouldForgetPrekey,
shouldRotatePrekey,
olmSessionErrors,
OLM_ERROR_FLAG,
} from 'lib/utils/olm-utils.js';

import { getIdentityClient } from './identity-client.js';
Expand Down Expand Up @@ -611,10 +612,15 @@ const olmAPI: OlmAPI = {
throw new Error(olmSessionErrors.invalidSessionVersion);
}

const result = olmSession.session.decrypt(
encryptedData.messageType,
encryptedData.message,
);
let result;
try {
result = olmSession.session.decrypt(
encryptedData.messageType,
encryptedData.message,
);
} catch (e) {
throw new Error(`error decrypt => ${OLM_ERROR_FLAG} ` + e.message);
}

await persistCryptoStore();

Expand Down Expand Up @@ -642,10 +648,15 @@ const olmAPI: OlmAPI = {
throw new Error(olmSessionErrors.invalidSessionVersion);
}

const result = olmSession.session.decrypt(
encryptedData.messageType,
encryptedData.message,
);
let result;
try {
result = olmSession.session.decrypt(
encryptedData.messageType,
encryptedData.message,
);
} catch (e) {
throw new Error(`error decrypt => ${OLM_ERROR_FLAG} ` + e.message);
}

const sqliteQueryExecutor = getSQLiteQueryExecutor();
const dbModule = getDBModule();
Expand Down Expand Up @@ -703,10 +714,15 @@ const olmAPI: OlmAPI = {
);

contentAccount.remove_one_time_keys(session);
const initialEncryptedMessage = session.decrypt(
initialEncryptedData.messageType,
initialEncryptedData.message,
);
let initialEncryptedMessage;
try {
initialEncryptedMessage = session.decrypt(
initialEncryptedData.messageType,
initialEncryptedData.message,
);
} catch (e) {
throw new Error(`error decrypt => ${OLM_ERROR_FLAG} ` + e.message);
}

contentSessions[contentIdentityKeys.ed25519] = {
session,
Expand Down

0 comments on commit 4723c3b

Please sign in to comment.