Skip to content

Commit

Permalink
Session Refactoring to bring logic more near to specs (#590)
Browse files Browse the repository at this point in the history
* Implement MessageCounters a defined and test

This commit moves the MessageCounter class into an own file and implements the required variants as defined by specification. It also adds tests

* Move MessageCounter into Session

As by specification the message counter should be handled by the respective session, so also move it there.

* Add Message Reception State implementation and tests

To detect duplicate messages the spec defined message Reception state which tracks messages and tests.

* Add message reception states into session

The message Reception state for the incoming messages is tracked by the sessions. So we add it there. The method "updateMessageCounter" throws an DuplicateMessageError in case of a duplicate.

* Remove some messagecounter leftovers in exchangemanager

* Adjust SecureSession creation to object style

* Refactor duplicate identifier for unspecificed node id

* Adjust testing for recent changes

* Refactor calculation of next session id

The logic to find the next available session id should wrap and search free spots - in worst case it should close the oldest unused session.
Additionally this commit moves the session id finding to the respective place where it is needed to prevent taking session ids that are then not ued because of errors.

* Allow multiple UnsecureSessions as defined by specs

Unsecure Sessions are created by their initiator Node Id ... od 0 if not specified. With this change UnsecureSessions also get a close callback for cleanup reasons only.

* Close unsecure sessions after usage

* remove unneeded typecase after changes

* Update session determination

* Add Message duplication handling

* Update Exchange handling to the specs

* Update CASE handling to handle more error cases correctly

* Close exchange after retransmissions

... if close is not already in progress

* remove debug logging

* [execute-chiptests-long] prepare changelog

* enhance tests

* Changelog

* Address review feedback
  • Loading branch information
Apollon77 authored Dec 22, 2023
1 parent 34fb920 commit ec1d465
Show file tree
Hide file tree
Showing 23 changed files with 1,665 additions and 204 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ The main work (all changes without a GitHub username in brackets in the below li
## __WORK IN PROGRESS__
-->

## __WORK IN PROGRESS__
* Matter-Core functionality:
* Enhancement: Refactor Core Session management to match specification
* Enhancement: Refactor message duplication detection and handling to match specification

## 0.7.3 (2023-12-18)
* Matter-Core functionality:
* Feature: Added CASE Authenticated Tags support (initialization from NOC and validation only)
Expand Down
10 changes: 5 additions & 5 deletions packages/matter-node.js/test/fabric/FabricTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { ByteArray } from "@project-chip/matter.js/util";

import { FabricId, FabricIndex, NodeId, VendorId } from "@project-chip/matter.js/datatype";
import { Fabric } from "@project-chip/matter.js/fabric";
import { SecureSession, UNDEFINED_NODE_ID } from "@project-chip/matter.js/session";
import { SecureSession } from "@project-chip/matter.js/session";
import * as assert from "assert";
import { buildFabric } from "./FabricTestingUtil.js";

Expand Down Expand Up @@ -125,7 +125,7 @@ describe("Fabric", () => {
context: {} as any,
id: 1,
fabric: undefined,
peerNodeId: UNDEFINED_NODE_ID,
peerNodeId: NodeId.UNSPECIFIED_NODE_ID,
peerSessionId: 0x8d4b,
decryptKey: DECRYPT_KEY,
encryptKey: ENCRYPT_KEY,
Expand All @@ -139,7 +139,7 @@ describe("Fabric", () => {
context: {} as any,
id: 2,
fabric: undefined,
peerNodeId: UNDEFINED_NODE_ID,
peerNodeId: NodeId.UNSPECIFIED_NODE_ID,
peerSessionId: 0x8d4b,
decryptKey: DECRYPT_KEY,
encryptKey: ENCRYPT_KEY,
Expand Down Expand Up @@ -178,7 +178,7 @@ describe("Fabric", () => {
context: {} as any,
id: 1,
fabric: undefined,
peerNodeId: UNDEFINED_NODE_ID,
peerNodeId: NodeId.UNSPECIFIED_NODE_ID,
peerSessionId: 0x8d4b,
decryptKey: DECRYPT_KEY,
encryptKey: ENCRYPT_KEY,
Expand All @@ -192,7 +192,7 @@ describe("Fabric", () => {
context: {} as any,
id: 2,
fabric: undefined,
peerNodeId: UNDEFINED_NODE_ID,
peerNodeId: NodeId.UNSPECIFIED_NODE_ID,
peerSessionId: 0x8d4b,
decryptKey: DECRYPT_KEY,
encryptKey: ENCRYPT_KEY,
Expand Down
9 changes: 5 additions & 4 deletions packages/matter-node.js/test/session/SecureSessionTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,20 @@
*/

import { Message, MessageCodec, SessionType } from "@project-chip/matter.js/codec";
import { SecureSession, UNDEFINED_NODE_ID } from "@project-chip/matter.js/session";
import { NodeId } from "@project-chip/matter.js/datatype";
import { SecureSession } from "@project-chip/matter.js/session";
import { ByteArray } from "@project-chip/matter.js/util";
import * as assert from "assert";

const DECRYPT_KEY = ByteArray.fromHex("bacb178b2588443d5d5b1e4559e7accc");
export const DECRYPT_KEY = ByteArray.fromHex("bacb178b2588443d5d5b1e4559e7accc");
const MESSAGE_ENCRYPTED = ByteArray.fromHex(
"001d350022145300ec2b931025dada82ed67521c966d2454d131a271023be699e4e2796650f568e590fd9b65f456c720a60a0da127eaa53974c5d41d3d933ed7b58a9ce5b5cb96ad94a7762611c48774cf75458327e74c34668a45dc9943546f8a6aa1dcd40bd4b8014befb49954a097a60cbdff333ee3f2fd1f49",
);
const DECRYPTED_BYTES = ByteArray.fromHex(
"153600172403312504fcff18172402002403302404001817240200240330240401181724020024033024040218172402002403302404031817240200240328240402181724020024032824040418172403312404031818290324ff0118",
);

const ENCRYPT_KEY = ByteArray.fromHex("66951379d0a6d151cf5472cccf13f360");
export const ENCRYPT_KEY = ByteArray.fromHex("66951379d0a6d151cf5472cccf13f360");
const MESSAGE: Message = {
packetHeader: {
messageId: 12519906,
Expand Down Expand Up @@ -49,7 +50,7 @@ describe("SecureSession", () => {
context: {} as any,
id: 1,
fabric: undefined,
peerNodeId: UNDEFINED_NODE_ID,
peerNodeId: NodeId.UNSPECIFIED_NODE_ID,
peerSessionId: 0x8d4b,
decryptKey: DECRYPT_KEY,
encryptKey: ENCRYPT_KEY,
Expand Down
110 changes: 110 additions & 0 deletions packages/matter-node.js/test/session/SessionManagerTest.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/**
* @license
* Copyright 2022-2023 Project CHIP Authors
* SPDX-License-Identifier: Apache-2.0
*/

import { NodeId } from "@project-chip/matter.js/datatype";
import { SessionManager } from "@project-chip/matter.js/session";
import { StorageBackendMemory, StorageContext } from "@project-chip/matter.js/storage";
import { ByteArray } from "@project-chip/matter.js/util";
import * as assert from "assert";

const DUMMY_BYTEARRAY = new ByteArray();

describe("SessionManager", () => {
describe("getNextAvailableSessionId", () => {
let storage: StorageBackendMemory;
let storageContext: StorageContext;
let sessionManager: SessionManager<any>;

beforeEach(() => {
storage = new StorageBackendMemory();
storageContext = new StorageContext(storage, ["context"]);

sessionManager = new SessionManager({}, storageContext);
});

it("next number is increasing", async () => {
let first = await sessionManager.getNextAvailableSessionId();
if (first === 0xffff) {
// Keep test simple and just ignore the special case and let it overflow
first = await sessionManager.getNextAvailableSessionId();
}
const second = await sessionManager.getNextAvailableSessionId();
assert.strictEqual(first + 1, second);
});

it("verify that id is 1 after being 0xffff", async () => {
const first = await sessionManager.getNextAvailableSessionId();
if (first === 0xffff) {
assert.strictEqual(await sessionManager.getNextAvailableSessionId(), 1);
} else {
for (let i = first; i < 0xfffe; i++) {
// read over until one before overrun
await sessionManager.getNextAvailableSessionId();
}
assert.strictEqual(await sessionManager.getNextAvailableSessionId(), 0xffff);
assert.strictEqual(await sessionManager.getNextAvailableSessionId(), 1);
}
});

it("verify that existing session ids are skipped", async () => {
let first = await sessionManager.getNextAvailableSessionId();
if (first === 0xfffe) {
// Keep test simple and just ignore the special case and let it overflow
first = await sessionManager.getNextAvailableSessionId();
}
if (first === 0xffff) {
// Keep test simple and just ignore the special case and let it overflow
first = await sessionManager.getNextAvailableSessionId();
}
// Create a session with "next expected number"
await sessionManager.createSecureSession({
sessionId: first + 1,
fabric: undefined,
peerNodeId: NodeId.UNSPECIFIED_NODE_ID,
peerSessionId: 0x8d4b,
sharedSecret: DUMMY_BYTEARRAY,
salt: DUMMY_BYTEARRAY,
isInitiator: false,
isResumption: false,
});
assert.strictEqual(await sessionManager.getNextAvailableSessionId(), first + 2);
});

it("verify that oldest session gets closed when no more ids are available", async () => {
const first = await sessionManager.getNextAvailableSessionId();
let firstClosed = false;
await sessionManager.createSecureSession({
sessionId: first,
fabric: undefined,
peerNodeId: NodeId.UNSPECIFIED_NODE_ID,
peerSessionId: 0x8d4b,
sharedSecret: DUMMY_BYTEARRAY,
salt: DUMMY_BYTEARRAY,
isInitiator: false,
isResumption: false,
closeCallback: async () => {
firstClosed = true;
},
});
await MockTime.advance(1000);
for (let i = 0; i < 0xfffe; i++) {
const sessionId = await sessionManager.getNextAvailableSessionId();
await sessionManager.createSecureSession({
sessionId,
fabric: undefined,
peerNodeId: NodeId.UNSPECIFIED_NODE_ID,
peerSessionId: 0x8d4b,
sharedSecret: DUMMY_BYTEARRAY,
salt: DUMMY_BYTEARRAY,
isInitiator: false,
isResumption: false,
});
}
assert.strictEqual(await sessionManager.getNextAvailableSessionId(), first);
assert.strictEqual(firstClosed, true);
}).timeout(10000);
});
});
52 changes: 23 additions & 29 deletions packages/matter.js/src/MatterController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,8 @@ export class MatterController {
}

// Do PASE paring
const paseUnsecureMessageChannel = new MessageChannel(paseChannel, this.sessionManager.getUnsecureSession());
const unsecureSession = this.sessionManager.createUnsecureSession();
const paseUnsecureMessageChannel = new MessageChannel(paseChannel, unsecureSession);
const paseExchange = this.exchangeManager.initiateExchangeWithChannel(
paseUnsecureMessageChannel,
SECURE_CHANNEL_PROTOCOL_ID,
Expand All @@ -384,6 +385,8 @@ export class MatterController {
},
);

await paseUnsecureMessageChannel.close();
await unsecureSession.destroy();
return new MessageChannel(paseChannel, paseSecureSession);
}

Expand Down Expand Up @@ -584,10 +587,8 @@ export class MatterController {
}

const operationalChannel = await operationalInterface.openChannel(operationalServerAddress);
const operationalUnsecureMessageExchange = new MessageChannel(
operationalChannel,
this.sessionManager.getUnsecureSession(),
);
const unsecureSession = this.sessionManager.createUnsecureSession();
const operationalUnsecureMessageExchange = new MessageChannel(operationalChannel, unsecureSession);
const operationalSecureSession = await tryCatchAsync(
async () => {
const exchange = this.exchangeManager.initiateExchangeWithChannel(
Expand All @@ -609,6 +610,8 @@ export class MatterController {
throw new PairRetransmissionLimitReachedError(error.message);
}, // Convert error
);
await operationalUnsecureMessageExchange.close(); // We reconnect using CASE, so close unsecure connection
await unsecureSession.destroy();
const channel = new MessageChannel(operationalChannel, operationalSecureSession);
await this.channelManager.setChannel(this.fabric, peerNodeId, channel);
return channel;
Expand Down Expand Up @@ -662,40 +665,31 @@ export class MatterController {
);
}

getNextAvailableSessionId() {
async getNextAvailableSessionId() {
return this.sessionManager.getNextAvailableSessionId();
}

async createSecureSession(
sessionId: number,
fabric: Fabric | undefined,
peerNodeId: NodeId,
peerSessionId: number,
sharedSecret: ByteArray,
salt: ByteArray,
isInitiator: boolean,
isResumption: boolean,
idleRetransmissionTimeoutMs?: number,
activeRetransmissionTimeoutMs?: number,
) {
async createSecureSession(args: {
sessionId: number;
fabric: Fabric | undefined;
peerNodeId: NodeId;
peerSessionId: number;
sharedSecret: ByteArray;
salt: ByteArray;
isInitiator: boolean;
isResumption: boolean;
idleRetransmissionTimeoutMs?: number;
activeRetransmissionTimeoutMs?: number;
}) {
const session = await this.sessionManager.createSecureSession({
sessionId,
fabric,
peerNodeId,
peerSessionId,
sharedSecret,
salt,
isInitiator,
isResumption,
idleRetransmissionTimeoutMs,
activeRetransmissionTimeoutMs,
...args,
closeCallback: async () => {
logger.debug(`Remove ${session.isPase() ? "PASE" : "CASE"} session`, session.name);
if (!session.closingAfterExchangeFinished) {
// Delayed closing is executed when exchange is closed
await this.exchangeManager.closeSession(session);
}
this.sessionClosedCallback?.(peerNodeId);
this.sessionClosedCallback?.(args.peerNodeId);
},
});
return session;
Expand Down
44 changes: 16 additions & 28 deletions packages/matter.js/src/MatterDevice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,7 @@ export class MatterDevice {
let fabricsWithoutSessions = 0;
for (const fabric of fabrics) {
const session = this.sessionManager.getSessionForNode(fabric, fabric.rootNodeId);
if (
session === undefined ||
!session.isSecure() ||
(session as SecureSession<this>).numberOfActiveSubscriptions === 0
) {
if (session === undefined || !session.isSecure() || session.numberOfActiveSubscriptions === 0) {
fabricsWithoutSessions++;
logger.debug("Announcing", Logger.dict({ fabric: fabric.fabricId }));
}
Expand Down Expand Up @@ -226,33 +222,25 @@ export class MatterDevice {
await this.startAnnouncement();
}

getNextAvailableSessionId() {
async getNextAvailableSessionId() {
return this.sessionManager.getNextAvailableSessionId();
}

async createSecureSession(
sessionId: number,
fabric: Fabric | undefined,
peerNodeId: NodeId,
peerSessionId: number,
sharedSecret: ByteArray,
salt: ByteArray,
isInitiator: boolean,
isResumption: boolean,
idleRetransmissionTimeoutMs?: number,
activeRetransmissionTimeoutMs?: number,
) {
async createSecureSession(args: {
sessionId: number;
fabric: Fabric | undefined;
peerNodeId: NodeId;
peerSessionId: number;
sharedSecret: ByteArray;
salt: ByteArray;
isInitiator: boolean;
isResumption: boolean;
idleRetransmissionTimeoutMs?: number;
activeRetransmissionTimeoutMs?: number;
}) {
const { fabric } = args;
const session = await this.sessionManager.createSecureSession({
sessionId,
fabric,
peerNodeId,
peerSessionId,
sharedSecret,
salt,
isInitiator,
isResumption,
idleRetransmissionTimeoutMs,
activeRetransmissionTimeoutMs,
...args,
closeCallback: async () => {
logger.debug(`Remove ${session.isPase() ? "PASE" : "CASE"} session`, session.name);
if (session.isPase() && this.failSafeContext !== undefined) {
Expand Down
14 changes: 9 additions & 5 deletions packages/matter.js/src/codec/MessageCodec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,16 +241,20 @@ export class MessageCodec {
return writer.toByteArray();
}

static messageDiagnostics({
packetHeader: { messageId, sessionId },
payloadHeader: { exchangeId, messageType, protocolId, ackedMessageId, requiresAck },
payload,
}: Message) {
static messageDiagnostics(
{
packetHeader: { messageId, sessionId },
payloadHeader: { exchangeId, messageType, protocolId, ackedMessageId, requiresAck },
payload,
}: Message,
isDuplicate = false,
) {
return new DiagnosticDictionary({
id: `${sessionId}/${exchangeId}/${messageId}`,
type: `${protocolId}/${messageType}`,
acked: ackedMessageId,
reqAck: requiresAck,
duplicate: isDuplicate,
payload: payload,
});
}
Expand Down
Loading

0 comments on commit ec1d465

Please sign in to comment.