Skip to content

Commit

Permalink
CASE re-arm for a FailSafe armed over a PASE session initially (proje…
Browse files Browse the repository at this point in the history 
…ct-chip#170)

* PASE session upgrade on AddNOC; make fab_idx NonZeroU8

* Changes after code review feedback
  • Loading branch information
@ivmarkov
ivmarkov authored Jun 11, 2024
1 parent 271053d commit 1c2ee9c
Show file tree
Hide file tree
Showing 14 changed files with 402 additions and 226 deletions.
197 changes: 131 additions & 66 deletions rs-matter/src/acl.rs
View file

Large diffs are not rendered by default.

17 changes: 13 additions & 4 deletions rs-matter/src/data_model/core.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

use core::cell::{Cell, RefCell};
use core::iter::Peekable;
use core::num::NonZeroU8;
use core::pin::pin;
use core::time::Duration;

Expand Down Expand Up @@ -53,7 +54,7 @@ const MAX_WRITE_ATTRS_IN_ONE_TRANS: usize = 7;
pub type IMBuffer = heapless::Vec<u8, MAX_EXCHANGE_RX_BUF_SIZE>;

struct SubscriptionBuffer<B> {
fabric_idx: u8,
fabric_idx: NonZeroU8,
peer_node_id: u64,
subscription_id: u32,
buffer: B,
Expand Down Expand Up @@ -369,7 +370,8 @@ where
debug!("IM: Subscribe request: {:?}", req);

let (fabric_idx, peer_node_id) = exchange.with_session(|sess| {
let fabric_idx = sess.get_local_fabric_idx().ok_or(ErrorCode::Invalid)?;
let fabric_idx =
NonZeroU8::new(sess.get_local_fabric_idx()).ok_or(ErrorCode::Invalid)?;
let peer_node_id = sess.get_peer_node_id().ok_or(ErrorCode::Invalid)?;

Ok((fabric_idx, peer_node_id))
Expand Down Expand Up @@ -407,7 +409,7 @@ where
});

let primed = self
.report_data(id, fabric_idx, peer_node_id, &rx, &mut tx, exchange)
.report_data(id, fabric_idx.get(), peer_node_id, &rx, &mut tx, exchange)
.await?;

if primed {
Expand Down Expand Up @@ -523,7 +525,14 @@ where

if let Some(mut tx) = self.buffers.get().await {
let primed = self
.report_data(id, fabric_idx, peer_node_id, &rx, &mut tx, &mut exchange)
.report_data(
id,
fabric_idx.get(),
peer_node_id,
&rx,
&mut tx,
&mut exchange,
)
.await?;

exchange.acknowledge().await?;
Expand Down
46 changes: 30 additions & 16 deletions rs-matter/src/data_model/sdm/failsafe.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
* limitations under the License.
*/

use core::num::NonZeroU8;

use crate::{
error::{Error, ErrorCode},
transport::session::SessionMode,
Expand All @@ -27,13 +29,12 @@ use log::error;
enum NocState {
NocNotRecvd,
// This is the local fabric index
AddNocRecvd(u8),
UpdateNocRecvd(u8),
AddNocRecvd(NonZeroU8),
UpdateNocRecvd(NonZeroU8),
}

#[derive(PartialEq)]
pub struct ArmedCtx {
session_mode: SessionMode,
timeout: u16,
noc_state: NocState,
}
Expand All @@ -58,16 +59,26 @@ impl FailSafe {
match &mut self.state {
State::Idle => {
self.state = State::Armed(ArmedCtx {
session_mode,
timeout,
noc_state: NocState::NocNotRecvd,
})
}
State::Armed(c) => {
if c.session_mode != session_mode {
error!("Received Fail-Safe Arm with different session modes; current {:?}, incoming {:?}", c.session_mode, session_mode);
Err(ErrorCode::Invalid)?;
match c.noc_state {
NocState::NocNotRecvd => (),
NocState::AddNocRecvd(fab_idx) | NocState::UpdateNocRecvd(fab_idx) => {
if let Some(sess_fab_idx) = NonZeroU8::new(session_mode.fab_idx()) {
if sess_fab_idx != fab_idx {
error!("Received Fail-Safe Re-arm with a different fabric index from a previous Add/Update NOC");
Err(ErrorCode::Invalid)?;
}
} else {
error!("Received Fail-Safe Re-arm from a session that does not have a fabric index");
Err(ErrorCode::Invalid)?;
}
}
}

// re-arm
c.timeout = timeout;
}
Expand All @@ -83,17 +94,20 @@ impl FailSafe {
}
State::Armed(c) => {
match c.noc_state {
NocState::NocNotRecvd => Err(ErrorCode::Invalid)?,
NocState::AddNocRecvd(idx) | NocState::UpdateNocRecvd(idx) => {
if let SessionMode::Case(c) = session_mode {
if c.fab_idx != idx {
error!(
"Received disarm in separate session from previous Add/Update NOC"
);
NocState::NocNotRecvd => {
error!("Received Fail-Safe Disarm, yet the failsafe has not received Add/Update NOC first");
Err(ErrorCode::Invalid)?;
}
NocState::AddNocRecvd(fab_idx) | NocState::UpdateNocRecvd(fab_idx) => {
if let Some(sess_fab_idx) = NonZeroU8::new(session_mode.fab_idx()) {
if sess_fab_idx != fab_idx {
error!("Received disarm with different fabric index from a previous Add/Update NOC");
Err(ErrorCode::Invalid)?;
}
} else {
error!("Received disarm in a non-CASE session");
error!(
"Received disarm from a session that does not have a fabric index"
);
Err(ErrorCode::Invalid)?;
}
}
Expand All @@ -108,7 +122,7 @@ impl FailSafe {
self.state != State::Idle
}

pub fn record_add_noc(&mut self, fabric_index: u8) -> Result<(), Error> {
pub fn record_add_noc(&mut self, fabric_index: NonZeroU8) -> Result<(), Error> {
match &mut self.state {
State::Idle => Err(ErrorCode::Invalid.into()),
State::Armed(c) => {
Expand Down
6 changes: 3 additions & 3 deletions rs-matter/src/data_model/sdm/general_commissioning.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use crate::data_model::objects::*;
use crate::data_model::sdm::failsafe::FailSafe;
use crate::tlv::{FromTLV, TLVElement, ToTLV, UtfStr};
use crate::transport::exchange::Exchange;
use crate::transport::session::SessionMode;
use crate::utils::rand::Rand;
use crate::{attribute_enum, cmd_enter};
use crate::{command_enum, error::*};
Expand Down Expand Up @@ -267,9 +268,8 @@ impl<'a> GenCommCluster<'a> {
let mut status: u8 = CommissioningErrorEnum::OK as u8;

// Has to be a Case Session
if exchange
.with_session(|sess| Ok(sess.get_local_fabric_idx()))?
.is_none()
if !exchange
.with_session(|sess| Ok(matches!(sess.get_session_mode(), SessionMode::Case { .. })))?
{
status = CommissioningErrorEnum::InvalidAuthentication as u8;
}
Expand Down
73 changes: 45 additions & 28 deletions rs-matter/src/data_model/sdm/noc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/

use core::cell::RefCell;
use core::num::NonZeroU8;

use crate::acl::{AclEntry, AclMgr, AuthMode};
use crate::cert::{Cert, MAX_CERT_TLV_LEN};
Expand Down Expand Up @@ -211,7 +212,7 @@ struct CertChainReq {

#[derive(FromTLV)]
struct RemoveFabricReq {
fab_idx: u8,
fab_idx: NonZeroU8,
}

#[derive(Clone)]
Expand Down Expand Up @@ -265,7 +266,7 @@ impl<'a> NocCluster<'a> {
Attributes::Fabrics(_) => {
writer.start_array(AttrDataWriter::TAG)?;
self.fabric_mgr.borrow().for_each(|entry, fab_idx| {
if !attr.fab_filter || attr.fab_idx == fab_idx {
if !attr.fab_filter || attr.fab_idx == fab_idx.get() {
let root_ca_cert = entry.get_root_ca()?;

entry
Expand Down Expand Up @@ -321,17 +322,11 @@ impl<'a> NocCluster<'a> {
Ok(())
}

fn add_acl(&self, fab_idx: u8, admin_subject: u64) -> Result<(), Error> {
let mut acl = AclEntry::new(fab_idx, Privilege::ADMIN, AuthMode::Case);
acl.add_subject(admin_subject)?;
self.acl_mgr.borrow_mut().add(acl)
}

fn _handle_command_addnoc(
&self,
exchange: &Exchange,
data: &TLVElement,
) -> Result<u8, NocError> {
) -> Result<NonZeroU8, NocError> {
let noc_data = exchange
.with_session(|sess| Ok(sess.take_noc_data()))?
.ok_or(NocStatus::MissingCsr)?;
Expand All @@ -346,16 +341,6 @@ impl<'a> NocCluster<'a> {
Err(NocStatus::InsufficientPrivlege)?;
}

// TODO
// // This command's processing may take longer, send a stand alone ACK to the peer to avoid any retranmissions
// let ack_send = secure_channel::common::send_mrp_standalone_ack(
// trans.exch,
// trans.session,
// );
// if ack_send.is_err() {
// error!("Error sending Standalone ACK, falling back to piggybacked ACK");
// }

let r = AddNocReq::from_tlv(data).map_err(|_| NocStatus::InvalidNOC)?;

let noc_cert = Cert::new(r.noc_value.0).map_err(|_| NocStatus::InvalidNOC)?;
Expand Down Expand Up @@ -394,10 +379,37 @@ impl<'a> NocCluster<'a> {
.add(fabric, self.mdns)
.map_err(|_| NocStatus::TableFull)?;

self.add_acl(fab_idx, r.case_admin_subject)?;
let _fab_guard = scopeguard::guard(fab_idx, |fab_idx| {
// Remove the fabric if we fail further down this function
self.fabric_mgr
.borrow_mut()
.remove(fab_idx, self.mdns)
.unwrap();
});

let mut acl = AclEntry::new(fab_idx, Privilege::ADMIN, AuthMode::Case);
acl.add_subject(r.case_admin_subject)?;
let acl_entry_index = self.acl_mgr.borrow_mut().add(acl)?;

let _acl_guard = scopeguard::guard(fab_idx, |fab_idx| {
// Remove the ACL entry if we fail further down this function
self.acl_mgr
.borrow_mut()
.delete(acl_entry_index, fab_idx)
.unwrap();
});

self.failsafe.borrow_mut().record_add_noc(fab_idx)?;

// Finally, upgrade our session with the new fabric index
exchange.with_session(|sess| {
if matches!(sess.get_session_mode(), SessionMode::Pase { .. }) {
sess.upgrade_fabric_idx(fab_idx)?;
}

Ok(())
})?;

Ok(fab_idx)
}

Expand Down Expand Up @@ -426,21 +438,21 @@ impl<'a> NocCluster<'a> {
) -> Result<(), Error> {
cmd_enter!("Update Fabric Label");
let req = UpdateFabricLabelReq::from_tlv(data).map_err(Error::map_invalid_data_type)?;
let (result, fab_idx) = if let SessionMode::Case(c) =
let (result, fab_idx) = if let SessionMode::Case { fab_idx, .. } =
exchange.with_session(|sess| Ok(sess.get_session_mode().clone()))?
{
if self
.fabric_mgr
.borrow_mut()
.set_label(
c.fab_idx,
fab_idx,
req.label.as_str().map_err(Error::map_invalid_data_type)?,
)
.is_err()
{
(NocStatus::LabelConflict, c.fab_idx)
(NocStatus::LabelConflict, fab_idx.get())
} else {
(NocStatus::Ok, c.fab_idx)
(NocStatus::Ok, fab_idx.get())
}
} else {
// Update Fabric Label not allowed
Expand Down Expand Up @@ -478,7 +490,12 @@ impl<'a> NocCluster<'a> {

Ok(())
} else {
Self::create_nocresponse(encoder, NocStatus::InvalidFabricIndex, req.fab_idx, "")
Self::create_nocresponse(
encoder,
NocStatus::InvalidFabricIndex,
req.fab_idx.get(),
"",
)
}
}

Expand All @@ -491,7 +508,7 @@ impl<'a> NocCluster<'a> {
cmd_enter!("AddNOC");

let (status, fab_idx) = match self._handle_command_addnoc(exchange, data) {
Ok(fab_idx) => (NocStatus::Ok, fab_idx),
Ok(fab_idx) => (NocStatus::Ok, fab_idx.get()),
Err(NocError::Status(status)) => (status, 0),
Err(NocError::Error(error)) => Err(error)?,
};
Expand Down Expand Up @@ -644,11 +661,11 @@ impl<'a> NocCluster<'a> {

// This may happen on CASE or PASE. For PASE, the existence of NOC Data is necessary
match exchange.with_session(|sess| Ok(sess.get_session_mode().clone()))? {
SessionMode::Case(_) => {
SessionMode::Case { .. } => {
// TODO - Updating the Trusted RCA of an existing Fabric
Self::add_rca_to_session_noc_data(exchange, data)?;
}
SessionMode::Pase => {
SessionMode::Pase { .. } => {
Self::add_rca_to_session_noc_data(exchange, data)?;
}
_ => (),
Expand Down
19 changes: 13 additions & 6 deletions rs-matter/src/data_model/subscriptions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/

use core::cell::RefCell;
use core::num::NonZeroU8;

use embassy_sync::blocking_mutex::raw::NoopRawMutex;
use embassy_time::Instant;
Expand All @@ -25,7 +26,7 @@ use portable_atomic::{AtomicU32, Ordering};
use crate::utils::notification::Notification;

struct Subscription {
fabric_idx: u8,
fabric_idx: NonZeroU8,
peer_node_id: u64,
session_id: Option<u32>,
id: u32,
Expand Down Expand Up @@ -96,7 +97,7 @@ impl<const N: usize> Subscriptions<N> {

pub(crate) fn add(
&self,
fabric_idx: u8,
fabric_idx: NonZeroU8,
peer_node_id: u64,
session_id: u32,
min_int_secs: u16,
Expand Down Expand Up @@ -139,7 +140,7 @@ impl<const N: usize> Subscriptions<N> {

pub(crate) fn remove(
&self,
fabric_idx: Option<u8>,
fabric_idx: Option<NonZeroU8>,
peer_node_id: Option<u64>,
id: Option<u32>,
) {
Expand All @@ -153,7 +154,10 @@ impl<const N: usize> Subscriptions<N> {
}
}

pub(crate) fn find_removed_session<F>(&self, session_removed: F) -> Option<(u8, u64, u32, u32)>
pub(crate) fn find_removed_session<F>(
&self,
session_removed: F,
) -> Option<(NonZeroU8, u64, u32, u32)>
where
F: Fn(u32) -> bool,
{
Expand All @@ -170,7 +174,7 @@ impl<const N: usize> Subscriptions<N> {
})
}

pub(crate) fn find_expired(&self, now: Instant) -> Option<(u8, u64, Option<u32>, u32)> {
pub(crate) fn find_expired(&self, now: Instant) -> Option<(NonZeroU8, u64, Option<u32>, u32)> {
self.subscriptions.borrow().iter().find_map(|sub| {
sub.is_expired(now).then_some((
sub.fabric_idx,
Expand All @@ -183,7 +187,10 @@ impl<const N: usize> Subscriptions<N> {

/// Note that this method has a side effect:
/// it updates the `reported_at` field of the subscription that is returned.
pub(crate) fn find_report_due(&self, now: Instant) -> Option<(u8, u64, Option<u32>, u32)> {
pub(crate) fn find_report_due(
&self,
now: Instant,
) -> Option<(NonZeroU8, u64, Option<u32>, u32)> {
self.subscriptions
.borrow_mut()
.iter_mut()
Expand Down
Loading

0 comments on commit 1c2ee9c

Please sign in to comment.