connectedhomeip icon indicating copy to clipboard operation
connectedhomeip copied to clipboard

Consider enforcing various spec constraints on messages in low-level secure channel code

Open bzbarsky-apple opened this issue 4 years ago • 3 comments

Problem

A few places recently where this has come up for me recently:

  1. Some messages should have the "C Flag" set. In the spec this is a very limited set of messages, and every message type has a unique corresponding "C Flag" value. So if we assume that all non-spec-defined messages are not control messages, we could derive the "C Flag" automatically from the message type. See also https://github.com/project-chip/connectedhomeip/pull/4771
  2. Some messages are sent encrypted and some are sent unencrypted. In spec terms, the latter are a very small set: they're they PASE/CASE session establishment messages. Ideally we would enforce via some sort of compile-time API boundary that other types of messages cannot be sent unencrypted.

Proposed Solution

Investigate what we can do to make it impossible, or at least very difficult, to misuse our message layer APIs to send non-spec-compliant messages.

bzbarsky-apple avatar Feb 10 '21 17:02 bzbarsky-apple

@pan-apple @turon @yufengwangca

bzbarsky-apple avatar Feb 10 '21 17:02 bzbarsky-apple

For 1, I think deriving the "C Flag" from protocol/message type restricts the flexibility, even we only have "MsgCounterSyncReq" and "MsgCounterSyncRsp" defined to set this flag, but the spec keep evolving .

I noticed we also have defined the Status Report message, but there is nowhere say how this Status Report is used https://github.com/CHIP-Specifications/connectedhomeip-spec/blob/master/src/common_protocol/Status_Report.adoc.

We need to answer, if we want the same message can be used as a Control message or general message within different scenarios.

For 2, I think we should restrict the apps from accessing the APIs in the transport layer at all to have better security.

yufengwangca avatar Feb 10 '21 20:02 yufengwangca

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

stale[bot] avatar Sep 15 '22 23:09 stale[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

stale[bot] avatar Mar 22 '23 22:03 stale[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

stale[bot] avatar Oct 15 '23 07:10 stale[bot]