From b477a871e948febef8c50a29d7dfd6f30579fec2 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 10 Jan 2023 08:53:04 +0100 Subject: [PATCH 01/12] Implemented the Clippy recommendations --- boxslab/src/lib.rs | 32 ++++++------ examples/onoff_light/src/main.rs | 2 +- matter/src/acl.rs | 1 + matter/src/cert/asn1_writer.rs | 17 ++++++- matter/src/cert/mod.rs | 29 +++++------ matter/src/cert/printer.rs | 25 +++++++--- matter/src/core.rs | 4 +- .../data_model/cluster_basic_information.rs | 2 +- matter/src/data_model/core.rs | 4 +- matter/src/data_model/device_types.rs | 2 +- matter/src/data_model/objects/attribute.rs | 1 + matter/src/data_model/objects/cluster.rs | 2 - matter/src/data_model/objects/encoder.rs | 2 +- matter/src/data_model/objects/endpoint.rs | 6 ++- matter/src/data_model/objects/node.rs | 8 +-- matter/src/data_model/objects/privilege.rs | 1 + matter/src/data_model/sdm/failsafe.rs | 1 + matter/src/data_model/sdm/noc.rs | 6 +-- .../data_model/system_model/access_control.rs | 29 ++++++----- .../src/data_model/system_model/descriptor.rs | 1 + matter/src/error.rs | 3 +- matter/src/fabric.rs | 2 +- matter/src/interaction_model/command.rs | 4 +- matter/src/interaction_model/core.rs | 2 +- matter/src/interaction_model/messages.rs | 26 +++++----- matter/src/interaction_model/write.rs | 2 +- matter/src/lib.rs | 3 +- matter/src/mdns.rs | 1 + matter/src/secure_channel/core.rs | 2 +- matter/src/secure_channel/pake.rs | 2 +- matter/src/tlv/parser.rs | 45 ++++++++--------- matter/src/tlv/traits.rs | 12 ++--- matter/src/tlv/writer.rs | 14 +++--- matter/src/transport/exchange.rs | 50 +++++++++---------- matter/src/transport/mgr.rs | 4 +- matter/src/transport/mod.rs | 1 - matter/src/transport/packet.rs | 4 +- matter/src/transport/queue.rs | 1 - matter/src/transport/session.rs | 3 +- matter/src/utils/parsebuf.rs | 20 +++----- matter/src/utils/writebuf.rs | 30 +++++------ matter/tests/common/echo_cluster.rs | 14 +++--- matter/tests/common/im_engine.rs | 13 ++--- matter/tests/data_model/timed_requests.rs | 18 +++---- matter/tests/interaction_model.rs | 4 +- matter_macro_derive/src/lib.rs | 32 ++++++------ 46 files changed, 240 insertions(+), 247 deletions(-) diff --git a/boxslab/src/lib.rs b/boxslab/src/lib.rs index 33bbe6e..f25cbd4 100644 --- a/boxslab/src/lib.rs +++ b/boxslab/src/lib.rs @@ -15,7 +15,6 @@ * limitations under the License. */ - use std::{ mem::MaybeUninit, ops::{Deref, DerefMut}, @@ -115,7 +114,7 @@ impl Slab { })) } - pub fn new(new_object: T::SlabType) -> Option> { + pub fn try_new(new_object: T::SlabType) -> Option> { let slab = T::get_slab(); let mut inner = slab.0.lock().unwrap(); if let Some(index) = inner.map.first_false_index() { @@ -180,14 +179,14 @@ mod tests { #[test] fn simple_alloc_free() { { - let a = Slab::::new(Test { val: Arc::new(10) }).unwrap(); + let a = Slab::::try_new(Test { val: Arc::new(10) }).unwrap(); assert_eq!(*a.val.deref(), 10); let inner = TestSlab::get_slab().0.lock().unwrap(); - assert_eq!(inner.map.is_empty(), false); + assert!(!inner.map.is_empty()); } // Validates that the 'Drop' got executed let inner = TestSlab::get_slab().0.lock().unwrap(); - assert_eq!(inner.map.is_empty(), true); + assert!(inner.map.is_empty()); println!("Box Size {}", std::mem::size_of::>()); println!("BoxSlab Size {}", std::mem::size_of::>()); } @@ -195,25 +194,22 @@ mod tests { #[test] fn alloc_full_block() { { - let a = Slab::::new(Test { val: Arc::new(10) }).unwrap(); - let b = Slab::::new(Test { val: Arc::new(11) }).unwrap(); - let c = Slab::::new(Test { val: Arc::new(12) }).unwrap(); + let a = Slab::::try_new(Test { val: Arc::new(10) }).unwrap(); + let b = Slab::::try_new(Test { val: Arc::new(11) }).unwrap(); + let c = Slab::::try_new(Test { val: Arc::new(12) }).unwrap(); // Test that at overflow, we return None - assert_eq!( - Slab::::new(Test { val: Arc::new(13) }).is_none(), - true - ); + assert!(Slab::::try_new(Test { val: Arc::new(13) }).is_none(),); assert_eq!(*b.val.deref(), 11); { let inner = TestSlab::get_slab().0.lock().unwrap(); // Test that the bitmap is marked as full - assert_eq!(inner.map.is_full(), true); + assert!(inner.map.is_full()); } // Purposefully drop, to test that new allocation is possible std::mem::drop(b); - let d = Slab::::new(Test { val: Arc::new(21) }).unwrap(); + let d = Slab::::try_new(Test { val: Arc::new(21) }).unwrap(); assert_eq!(*d.val.deref(), 21); // Ensure older allocations are still valid @@ -223,16 +219,16 @@ mod tests { // Validates that the 'Drop' got executed - test that the bitmap is empty let inner = TestSlab::get_slab().0.lock().unwrap(); - assert_eq!(inner.map.is_empty(), true); + assert!(inner.map.is_empty()); } #[test] fn test_drop_logic() { let root = Arc::new(10); { - let _a = Slab::::new(Test { val: root.clone() }).unwrap(); - let _b = Slab::::new(Test { val: root.clone() }).unwrap(); - let _c = Slab::::new(Test { val: root.clone() }).unwrap(); + let _a = Slab::::try_new(Test { val: root.clone() }).unwrap(); + let _b = Slab::::try_new(Test { val: root.clone() }).unwrap(); + let _c = Slab::::try_new(Test { val: root.clone() }).unwrap(); assert_eq!(Arc::strong_count(&root), 4); } // Test that Drop was correctly called on all the members of the pool diff --git a/examples/onoff_light/src/main.rs b/examples/onoff_light/src/main.rs index 15bfc4f..ad36f07 100644 --- a/examples/onoff_light/src/main.rs +++ b/examples/onoff_light/src/main.rs @@ -40,7 +40,7 @@ fn main() { }; let dev_att = Box::new(dev_att::HardCodedDevAtt::new()); - let mut matter = core::Matter::new(dev_info, dev_att, comm_data).unwrap(); + let mut matter = core::Matter::new(&dev_info, dev_att, &comm_data).unwrap(); let dm = matter.get_data_model(); { let mut node = dm.node.write().unwrap(); diff --git a/matter/src/acl.rs b/matter/src/acl.rs index 433f1cb..d46d46d 100644 --- a/matter/src/acl.rs +++ b/matter/src/acl.rs @@ -487,6 +487,7 @@ impl std::fmt::Display for AclMgr { } #[cfg(test)] +#[allow(clippy::bool_assert_comparison)] mod tests { use crate::{ data_model::objects::{Access, Privilege}, diff --git a/matter/src/cert/asn1_writer.rs b/matter/src/cert/asn1_writer.rs index db9fd08..cec7e7d 100644 --- a/matter/src/cert/asn1_writer.rs +++ b/matter/src/cert/asn1_writer.rs @@ -18,6 +18,7 @@ use super::{CertConsumer, MAX_DEPTH}; use crate::error::Error; use chrono::{TimeZone, Utc}; +use log::warn; #[derive(Debug)] pub struct ASN1Writer<'a> { @@ -255,10 +256,22 @@ impl<'a> CertConsumer for ASN1Writer<'a> { } fn utctime(&mut self, _tag: &str, epoch: u32) -> Result<(), Error> { - let mut matter_epoch = Utc.ymd(2000, 1, 1).and_hms(0, 0, 0).timestamp(); + let mut matter_epoch = Utc + .with_ymd_and_hms(2000, 1, 1, 0, 0, 0) + .unwrap() + .timestamp(); + matter_epoch += epoch as i64; - let dt = Utc.timestamp(matter_epoch, 0); + let dt = match Utc.timestamp_opt(matter_epoch, 0) { + chrono::LocalResult::None => return Err(Error::InvalidTime), + chrono::LocalResult::Single(s) => s, + chrono::LocalResult::Ambiguous(_, a) => { + warn!("Ambiguous time for epoch {epoch}; returning latest timestamp: {a}"); + a + } + }; + let time_str = format!("{}Z", dt.format("%y%m%d%H%M%S")); self.write_str(0x17, time_str.as_bytes()) } diff --git a/matter/src/cert/mod.rs b/matter/src/cert/mod.rs index add3b22..e1191d3 100644 --- a/matter/src/cert/mod.rs +++ b/matter/src/cert/mod.rs @@ -348,20 +348,17 @@ impl DistNames { w.start_seq(tag)?; for (id, value) in &self.dn { if let Ok(tag) = num::FromPrimitive::from_u8(*id).ok_or(Error::InvalidData) { - match tag { - DnTags::NocCat => { - w.start_set("")?; - w.start_seq("")?; - w.oid("Chip NOC CAT Id:", &OID_MATTER_NOC_CAT_ID)?; - w.utf8str("", format!("{:08X}", value).as_str())?; - w.end_seq()?; - w.end_set()?; - } - _ => { - let index: usize = (*id as usize) - (DnTags::NodeId as usize); - let this = &dn_encoding[index]; - encode_u64_dn(*value, this.0, this.1, w)?; - } + if let DnTags::NocCat = tag { + w.start_set("")?; + w.start_seq("")?; + w.oid("Chip NOC CAT Id:", &OID_MATTER_NOC_CAT_ID)?; + w.utf8str("", format!("{:08X}", value).as_str())?; + w.end_seq()?; + w.end_set()?; + } else { + let index: usize = (*id as usize) - (DnTags::NodeId as usize); + let this = &dn_encoding[index]; + encode_u64_dn(*value, this.0, this.1, w)?; } } else { error!("Non Matter DNs are not yet supported {}", id); @@ -452,7 +449,7 @@ impl Cert { pub fn as_asn1(&self, buf: &mut [u8]) -> Result { let mut w = ASN1Writer::new(buf); - let _ = self.encode(&mut w)?; + self.encode(&mut w)?; Ok(w.as_slice().len()) } @@ -657,7 +654,7 @@ mod tests { for input in test_input.iter() { println!("Testing next input..."); - let root = tlv::get_root_node(*input).unwrap(); + let root = tlv::get_root_node(input).unwrap(); let cert = Cert::from_tlv(&root).unwrap(); let mut buf = [0u8; 1024]; let buf_len = buf.len(); diff --git a/matter/src/cert/printer.rs b/matter/src/cert/printer.rs index 23b7020..4b6d4fe 100644 --- a/matter/src/cert/printer.rs +++ b/matter/src/cert/printer.rs @@ -18,6 +18,7 @@ use super::{CertConsumer, MAX_DEPTH}; use crate::error::Error; use chrono::{TimeZone, Utc}; +use log::warn; use std::fmt; pub struct CertPrinter<'a, 'b> { @@ -118,15 +119,23 @@ impl<'a, 'b> CertConsumer for CertPrinter<'a, 'b> { Ok(()) } fn utctime(&mut self, tag: &str, epoch: u32) -> Result<(), Error> { - let mut matter_epoch = Utc.ymd(2000, 1, 1).and_hms(0, 0, 0).timestamp(); + let mut matter_epoch = Utc + .with_ymd_and_hms(2000, 1, 1, 0, 0, 0) + .unwrap() + .timestamp(); + matter_epoch += epoch as i64; - let _ = writeln!( - self.f, - "{} {} {}", - SPACE[self.level], - tag, - Utc.timestamp(matter_epoch, 0) - ); + + let dt = match Utc.timestamp_opt(matter_epoch, 0) { + chrono::LocalResult::None => return Err(Error::InvalidTime), + chrono::LocalResult::Single(s) => s, + chrono::LocalResult::Ambiguous(_, a) => { + warn!("Ambiguous time for epoch {epoch}; returning latest timestamp: {a}"); + a + } + }; + + let _ = writeln!(self.f, "{} {} {}", SPACE[self.level], tag, dt); Ok(()) } } diff --git a/matter/src/core.rs b/matter/src/core.rs index 0706413..6aee3ce 100644 --- a/matter/src/core.rs +++ b/matter/src/core.rs @@ -57,9 +57,9 @@ impl Matter { /// requires a set of device attestation certificates and keys. It is the responsibility of /// this object to return the device attestation details when queried upon. pub fn new( - dev_det: BasicInfoConfig, + dev_det: &BasicInfoConfig, dev_att: Box, - dev_comm: CommissioningData, + dev_comm: &CommissioningData, ) -> Result, Error> { let mdns = Mdns::get()?; mdns.set_values(dev_det.vid, dev_det.pid, dev_comm.discriminator); diff --git a/matter/src/data_model/cluster_basic_information.rs b/matter/src/data_model/cluster_basic_information.rs index fbc33a4..c480f48 100644 --- a/matter/src/data_model/cluster_basic_information.rs +++ b/matter/src/data_model/cluster_basic_information.rs @@ -74,7 +74,7 @@ pub struct BasicInfoCluster { } impl BasicInfoCluster { - pub fn new(cfg: BasicInfoConfig) -> Result, Error> { + pub fn new(cfg: &BasicInfoConfig) -> Result, Error> { let mut cluster = Box::new(BasicInfoCluster { base: Cluster::new(ID)?, }); diff --git a/matter/src/data_model/core.rs b/matter/src/data_model/core.rs index 4b0db23..f269cdb 100644 --- a/matter/src/data_model/core.rs +++ b/matter/src/data_model/core.rs @@ -50,7 +50,7 @@ pub struct DataModel { impl DataModel { pub fn new( - dev_details: BasicInfoConfig, + dev_details: &BasicInfoConfig, dev_att: Box, fabric_mgr: Arc, acl_mgr: Arc, @@ -171,7 +171,7 @@ impl DataModel { // Set the cluster's data version attr_encoder.set_data_ver(cluster_data_ver); let mut access_req = AccessReq::new(accessor, path, Access::READ); - Cluster::read_attribute(c, &mut access_req, attr_encoder, &attr_details); + Cluster::read_attribute(c, &mut access_req, attr_encoder, attr_details); Ok(()) }); if let Err(e) = result { diff --git a/matter/src/data_model/device_types.rs b/matter/src/data_model/device_types.rs index 62b81f5..47fc022 100644 --- a/matter/src/data_model/device_types.rs +++ b/matter/src/data_model/device_types.rs @@ -34,7 +34,7 @@ type WriteNode<'a> = RwLockWriteGuard<'a, Box>; pub fn device_type_add_root_node( node: &mut WriteNode, - dev_info: BasicInfoConfig, + dev_info: &BasicInfoConfig, dev_att: Box, fabric_mgr: Arc, acl_mgr: Arc, diff --git a/matter/src/data_model/objects/attribute.rs b/matter/src/data_model/objects/attribute.rs index 4e8a6bd..5498875 100644 --- a/matter/src/data_model/objects/attribute.rs +++ b/matter/src/data_model/objects/attribute.rs @@ -203,6 +203,7 @@ impl std::fmt::Display for Attribute { } #[cfg(test)] +#[allow(clippy::bool_assert_comparison)] mod tests { use super::Access; use crate::data_model::objects::Privilege; diff --git a/matter/src/data_model/objects/cluster.rs b/matter/src/data_model/objects/cluster.rs index d822b04..a42aac2 100644 --- a/matter/src/data_model/objects/cluster.rs +++ b/matter/src/data_model/objects/cluster.rs @@ -291,7 +291,6 @@ impl Cluster { a.set_value(value) .map(|_| { self.cluster_changed(); - () }) .map_err(|_| IMStatusCode::UnsupportedWrite) } else { @@ -303,7 +302,6 @@ impl Cluster { let a = self.get_attribute_mut(attr_id)?; a.set_value(value).map(|_| { self.cluster_changed(); - () }) } diff --git a/matter/src/data_model/objects/encoder.rs b/matter/src/data_model/objects/encoder.rs index 8af9253..3e756d8 100644 --- a/matter/src/data_model/objects/encoder.rs +++ b/matter/src/data_model/objects/encoder.rs @@ -95,7 +95,7 @@ impl<'a> ToTLV for EncodeValue<'a> { (f)(tag_type, tw); Ok(()) } - EncodeValue::Tlv(_) => (panic!("This looks invalid")), + EncodeValue::Tlv(_) => panic!("This looks invalid"), EncodeValue::Value(v) => v.to_tlv(tw, tag_type), } } diff --git a/matter/src/data_model/objects/endpoint.rs b/matter/src/data_model/objects/endpoint.rs index a790fc6..220119b 100644 --- a/matter/src/data_model/objects/endpoint.rs +++ b/matter/src/data_model/objects/endpoint.rs @@ -25,6 +25,8 @@ pub struct Endpoint { clusters: Vec>, } +pub type BoxedClusters = [Box]; + impl Endpoint { pub fn new() -> Result, Error> { Ok(Box::new(Endpoint { @@ -63,7 +65,7 @@ impl Endpoint { pub fn get_wildcard_clusters( &self, cluster: Option, - ) -> Result<(&[Box], bool), IMStatusCode> { + ) -> Result<(&BoxedClusters, bool), IMStatusCode> { if let Some(c) = cluster { if let Some(i) = self.get_cluster_index(c) { Ok((&self.clusters[i..i + 1], false)) @@ -79,7 +81,7 @@ impl Endpoint { pub fn get_wildcard_clusters_mut( &mut self, cluster: Option, - ) -> Result<(&mut [Box], bool), IMStatusCode> { + ) -> Result<(&mut BoxedClusters, bool), IMStatusCode> { if let Some(c) = cluster { if let Some(i) = self.get_cluster_index(c) { Ok((&mut self.clusters[i..i + 1], false)) diff --git a/matter/src/data_model/objects/node.rs b/matter/src/data_model/objects/node.rs index e3b1598..8e391cf 100644 --- a/matter/src/data_model/objects/node.rs +++ b/matter/src/data_model/objects/node.rs @@ -29,6 +29,8 @@ pub trait ChangeConsumer { pub const ENDPTS_PER_ACC: usize = 3; +pub type BoxedEndpoints = [Option>]; + #[derive(Default)] pub struct Node { endpoints: [Option>; ENDPTS_PER_ACC], @@ -49,7 +51,7 @@ impl std::fmt::Display for Node { impl Node { pub fn new() -> Result, Error> { - let node = Box::new(Node::default()); + let node = Box::default(); Ok(node) } @@ -121,7 +123,7 @@ impl Node { pub fn get_wildcard_endpoints( &self, endpoint: Option, - ) -> Result<(&[Option>], usize, bool), IMStatusCode> { + ) -> Result<(&BoxedEndpoints, usize, bool), IMStatusCode> { if let Some(e) = endpoint { let e = e as usize; if self.endpoints.len() <= e || self.endpoints[e].is_none() { @@ -137,7 +139,7 @@ impl Node { pub fn get_wildcard_endpoints_mut( &mut self, endpoint: Option, - ) -> Result<(&mut [Option>], usize, bool), IMStatusCode> { + ) -> Result<(&mut BoxedEndpoints, usize, bool), IMStatusCode> { if let Some(e) = endpoint { let e = e as usize; if self.endpoints.len() <= e || self.endpoints[e].is_none() { diff --git a/matter/src/data_model/objects/privilege.rs b/matter/src/data_model/objects/privilege.rs index 10069d5..6b4e3a5 100644 --- a/matter/src/data_model/objects/privilege.rs +++ b/matter/src/data_model/objects/privilege.rs @@ -58,6 +58,7 @@ impl FromTLV<'_> for Privilege { } impl ToTLV for Privilege { + #[allow(clippy::bool_to_int_with_if)] fn to_tlv( &self, tw: &mut crate::tlv::TLVWriter, diff --git a/matter/src/data_model/sdm/failsafe.rs b/matter/src/data_model/sdm/failsafe.rs index 318b890..12a5d11 100644 --- a/matter/src/data_model/sdm/failsafe.rs +++ b/matter/src/data_model/sdm/failsafe.rs @@ -21,6 +21,7 @@ use std::sync::RwLock; #[derive(PartialEq)] #[allow(dead_code)] +#[allow(clippy::enum_variant_names)] enum NocState { NocNotRecvd, // This is the local fabric index diff --git a/matter/src/data_model/sdm/noc.rs b/matter/src/data_model/sdm/noc.rs index 68432dc..82bf61b 100644 --- a/matter/src/data_model/sdm/noc.rs +++ b/matter/src/data_model/sdm/noc.rs @@ -129,7 +129,7 @@ impl NocCluster { } fn add_acl(&self, fab_idx: u8, admin_subject: u64) -> Result<(), Error> { - let mut acl = AclEntry::new(fab_idx as u8, Privilege::ADMIN, AuthMode::Case); + let mut acl = AclEntry::new(fab_idx, Privilege::ADMIN, AuthMode::Case); acl.add_subject(admin_subject)?; self.acl_mgr.add(acl) } @@ -154,7 +154,7 @@ impl NocCluster { let noc_value = Cert::new(r.noc_value.0).map_err(|_| NocStatus::InvalidNOC)?; info!("Received NOC as: {}", noc_value); - let icac_value = if r.icac_value.0.len() != 0 { + let icac_value = if !r.icac_value.0.is_empty() { let cert = Cert::new(r.icac_value.0).map_err(|_| NocStatus::InvalidNOC)?; info!("Received ICAC as: {}", cert); Some(cert) @@ -176,7 +176,7 @@ impl NocCluster { .add(fabric) .map_err(|_| NocStatus::TableFull)?; - if self.add_acl(fab_idx as u8, r.case_admin_subject).is_err() { + if self.add_acl(fab_idx, r.case_admin_subject).is_err() { error!("Failed to add ACL, what to do?"); } diff --git a/matter/src/data_model/system_model/access_control.rs b/matter/src/data_model/system_model/access_control.rs index bf05bd7..2e5a992 100644 --- a/matter/src/data_model/system_model/access_control.rs +++ b/matter/src/data_model/system_model/access_control.rs @@ -63,7 +63,7 @@ impl AccessControlCluster { /// Care about fabric-scoped behaviour is taken fn write_acl_attr( &mut self, - op: ListOperation, + op: &ListOperation, data: &TLVElement, fab_idx: u8, ) -> Result<(), IMStatusCode> { @@ -77,12 +77,12 @@ impl AccessControlCluster { acl_entry.fab_idx = Some(fab_idx); if let ListOperation::EditItem(index) = op { - self.acl_mgr.edit(index as u8, fab_idx, acl_entry) + self.acl_mgr.edit(*index as u8, fab_idx, acl_entry) } else { self.acl_mgr.add(acl_entry) } } - ListOperation::DeleteItem(index) => self.acl_mgr.delete(index as u8, fab_idx), + ListOperation::DeleteItem(index) => self.acl_mgr.delete(*index as u8, fab_idx), ListOperation::DeleteList => self.acl_mgr.delete_for_fabric(fab_idx), }; match result { @@ -128,14 +128,13 @@ impl ClusterType for AccessControlCluster { attr: &AttrDetails, data: &TLVElement, ) -> Result<(), IMStatusCode> { - let result = match num::FromPrimitive::from_u16(attr.attr_id) { - Some(Attributes::Acl) => attr_list_write(attr, data, |op, data| { - self.write_acl_attr(op, data, attr.fab_idx) - }), - _ => { - error!("Attribute not yet supported: this shouldn't happen"); - Err(IMStatusCode::NotFound) - } + let result = if let Some(Attributes::Acl) = num::FromPrimitive::from_u16(attr.attr_id) { + attr_list_write(attr, data, |op, data| { + self.write_acl_attr(&op, data, attr.fab_idx) + }) + } else { + error!("Attribute not yet supported: this shouldn't happen"); + Err(IMStatusCode::NotFound) }; if result.is_ok() { self.base.cluster_changed(); @@ -223,7 +222,7 @@ mod tests { // Test, ACL has fabric index 2, but the accessing fabric is 1 // the fabric index in the TLV should be ignored and the ACL should be created with entry 1 - let result = acl.write_acl_attr(ListOperation::AddItem, &data, 1); + let result = acl.write_acl_attr(&ListOperation::AddItem, &data, 1); assert_eq!(result, Ok(())); let verifier = AclEntry::new(1, Privilege::VIEW, AuthMode::Case); @@ -259,7 +258,7 @@ mod tests { let data = get_root_node_struct(writebuf.as_borrow_slice()).unwrap(); // Test, Edit Fabric 2's index 1 - with accessing fabring as 2 - allow - let result = acl.write_acl_attr(ListOperation::EditItem(1), &data, 2); + let result = acl.write_acl_attr(&ListOperation::EditItem(1), &data, 2); // Fabric 2's index 1, is actually our index 2, update the verifier verifier[2] = new; assert_eq!(result, Ok(())); @@ -292,7 +291,7 @@ mod tests { let data = TLVElement::new(TagType::Anonymous, ElementType::True); // Test , Delete Fabric 1's index 0 - let result = acl.write_acl_attr(ListOperation::DeleteItem(0), &data, 1); + let result = acl.write_acl_attr(&ListOperation::DeleteItem(0), &data, 1); assert_eq!(result, Ok(())); let verifier = [input[0], input[2]]; @@ -323,7 +322,7 @@ mod tests { for i in input { acl_mgr.add(i).unwrap(); } - let acl = AccessControlCluster::new(acl_mgr.clone()).unwrap(); + let acl = AccessControlCluster::new(acl_mgr).unwrap(); // Test 1, all 3 entries are read in the response without fabric filtering { let mut tw = TLVWriter::new(&mut writebuf); diff --git a/matter/src/data_model/system_model/descriptor.rs b/matter/src/data_model/system_model/descriptor.rs index 0790f82..4609a73 100644 --- a/matter/src/data_model/system_model/descriptor.rs +++ b/matter/src/data_model/system_model/descriptor.rs @@ -27,6 +27,7 @@ use log::error; pub const ID: u32 = 0x001D; #[derive(FromPrimitive)] +#[allow(clippy::enum_variant_names)] enum Attributes { DeviceTypeList = 0, ServerList = 1, diff --git a/matter/src/error.rs b/matter/src/error.rs index 03d6b6d..aa326d0 100644 --- a/matter/src/error.rs +++ b/matter/src/error.rs @@ -57,6 +57,7 @@ pub enum Error { InvalidAuthKey, InvalidSignature, InvalidState, + InvalidTime, RwLock, TLVNotFound, TLVTypeMismatch, @@ -118,7 +119,7 @@ impl From for Error { } } -impl<'a> fmt::Display for Error { +impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{:?}", self) } diff --git a/matter/src/fabric.rs b/matter/src/fabric.rs index c39bb25..97e07bd 100644 --- a/matter/src/fabric.rs +++ b/matter/src/fabric.rs @@ -228,7 +228,7 @@ impl Fabric { let mut icac = Vec::new(); psm.get_kv_slice(fb_key!(index, ST_ICA), &mut icac)?; - let icac = if icac.len() != 0 { + let icac = if !icac.is_empty() { Some(Cert::new(icac.as_slice())?) } else { None diff --git a/matter/src/interaction_model/command.rs b/matter/src/interaction_model/command.rs index 4867ac9..4ee17bc 100644 --- a/matter/src/interaction_model/command.rs +++ b/matter/src/interaction_model/command.rs @@ -51,7 +51,7 @@ impl InteractionModel { rx_buf: &[u8], proto_tx: &mut Packet, ) -> Result { - if InteractionModel::req_timeout_handled(trans, proto_tx)? == true { + if InteractionModel::req_timeout_handled(trans, proto_tx)? { return Ok(ResponseRequired::Yes); } @@ -61,7 +61,7 @@ impl InteractionModel { let inv_req = InvReq::from_tlv(&root)?; let timed_tx = trans.get_timeout().map(|_| true); - let timed_request = inv_req.timed_request.filter(|a| *a == true); + let timed_request = inv_req.timed_request.filter(|a| *a); // Either both should be None, or both should be Some(true) if timed_tx != timed_request { InteractionModel::create_status_response(proto_tx, IMStatusCode::TimedRequestMisMatch)?; diff --git a/matter/src/interaction_model/core.rs b/matter/src/interaction_model/core.rs index 2540c02..326fbcc 100644 --- a/matter/src/interaction_model/core.rs +++ b/matter/src/interaction_model/core.rs @@ -179,7 +179,7 @@ impl proto_demux::HandleProto for InteractionModel { } fn get_proto_id(&self) -> usize { - PROTO_ID_INTERACTION_MODEL as usize + PROTO_ID_INTERACTION_MODEL } } diff --git a/matter/src/interaction_model/messages.rs b/matter/src/interaction_model/messages.rs index c5c1ca0..2faa9d3 100644 --- a/matter/src/interaction_model/messages.rs +++ b/matter/src/interaction_model/messages.rs @@ -348,22 +348,20 @@ pub mod ib { } else { f(ListOperation::EditItem(index), data) } - } else { - if data.confirm_array().is_ok() { - // If data is list, this is either Delete List or OverWrite List operation - // in either case, we have to first delete the whole list - f(ListOperation::DeleteList, data)?; - // Now the data must be a list, that should be added item by item + } else if data.confirm_array().is_ok() { + // If data is list, this is either Delete List or OverWrite List operation + // in either case, we have to first delete the whole list + f(ListOperation::DeleteList, data)?; + // Now the data must be a list, that should be added item by item - let container = data.enter().ok_or(Error::Invalid)?; - for d in container { - f(ListOperation::AddItem, &d)?; - } - Ok(()) - } else { - // If data is not a list, this must be an add operation - f(ListOperation::AddItem, data) + let container = data.enter().ok_or(Error::Invalid)?; + for d in container { + f(ListOperation::AddItem, &d)?; } + Ok(()) + } else { + // If data is not a list, this must be an add operation + f(ListOperation::AddItem, data) } } diff --git a/matter/src/interaction_model/write.rs b/matter/src/interaction_model/write.rs index cad15eb..48b7903 100644 --- a/matter/src/interaction_model/write.rs +++ b/matter/src/interaction_model/write.rs @@ -32,7 +32,7 @@ impl InteractionModel { rx_buf: &[u8], proto_tx: &mut Packet, ) -> Result { - if InteractionModel::req_timeout_handled(trans, proto_tx)? == true { + if InteractionModel::req_timeout_handled(trans, proto_tx)? { return Ok(ResponseRequired::Yes); } proto_tx.set_proto_opcode(OpCode::WriteResponse as u8); diff --git a/matter/src/lib.rs b/matter/src/lib.rs index 728c484..ad677b5 100644 --- a/matter/src/lib.rs +++ b/matter/src/lib.rs @@ -15,7 +15,6 @@ * limitations under the License. */ - //! Native Rust Implementation of Matter (Smart-Home) //! //! This crate implements the Matter specification that can be run on embedded devices @@ -56,7 +55,7 @@ //! //! /// Get the Matter Object //! /// The dev_att is an object that implements the DevAttDataFetcher trait. -//! let mut matter = Matter::new(dev_info, dev_att, comm_data).unwrap(); +//! let mut matter = Matter::new(&dev_info, dev_att, &comm_data).unwrap(); //! let dm = matter.get_data_model(); //! { //! let mut node = dm.node.write().unwrap(); diff --git a/matter/src/mdns.rs b/matter/src/mdns.rs index f5aeb7c..c1180a6 100644 --- a/matter/src/mdns.rs +++ b/matter/src/mdns.rs @@ -44,6 +44,7 @@ const SHORT_DISCRIMINATOR_SHIFT: u16 = 8; static mut G_MDNS: Option> = None; static INIT: Once = Once::new(); +#[derive(Clone, Copy)] pub enum ServiceMode { Commissioned, Commissionable, diff --git a/matter/src/secure_channel/core.rs b/matter/src/secure_channel/core.rs index 8a2774c..679b1f5 100644 --- a/matter/src/secure_channel/core.rs +++ b/matter/src/secure_channel/core.rs @@ -143,6 +143,6 @@ impl proto_demux::HandleProto for SecureChannel { } fn get_proto_id(&self) -> usize { - PROTO_ID_SECURE_CHANNEL as usize + PROTO_ID_SECURE_CHANNEL } } diff --git a/matter/src/secure_channel/pake.rs b/matter/src/secure_channel/pake.rs index 008f193..bb6503d 100644 --- a/matter/src/secure_channel/pake.rs +++ b/matter/src/secure_channel/pake.rs @@ -219,7 +219,7 @@ impl PAKE { let local_sessid = ctx.exch_ctx.sess.reserve_new_sess_id(); let spake2p_data: u32 = ((local_sessid as u32) << 16) | a.initiator_ssid as u32; let mut spake2p = Box::new(Spake2P::new()); - spake2p.set_app_data(spake2p_data as u32); + spake2p.set_app_data(spake2p_data); // Generate response let mut tw = TLVWriter::new(ctx.tx.get_writebuf()?); diff --git a/matter/src/tlv/parser.rs b/matter/src/tlv/parser.rs index bb07ae6..0bad933 100644 --- a/matter/src/tlv/parser.rs +++ b/matter/src/tlv/parser.rs @@ -88,7 +88,7 @@ static TAG_EXTRACTOR: [ExtractTag; 8] = [ // ImplPrf32 5 |t| TagType::ImplPrf32(LittleEndian::read_u32(&t.buf[t.current..])), // FullQual48 6 - |t| TagType::FullQual48(LittleEndian::read_u48(&t.buf[t.current..]) as u64), + |t| TagType::FullQual48(LittleEndian::read_u48(&t.buf[t.current..])), // FullQual64 7 |t| TagType::FullQual64(LittleEndian::read_u64(&t.buf[t.current..])), ]; @@ -330,32 +330,28 @@ impl<'a> PartialEq for TLVElement<'a> { let ours = ours.unwrap(); let theirs = theirs.unwrap(); - match ours.element_type { - ElementType::EndCnt => { - if nest_level == 0 { - break; - } else { - nest_level -= 1; - } + if let ElementType::EndCnt = ours.element_type { + if nest_level == 0 { + break; } - _ => { - if is_container(ours.element_type) { - nest_level += 1; - // Only compare the discriminants in case of array/list/structures, - // instead of actual element values. Those will be subsets within this same - // list that will get validated anyway - if std::mem::discriminant(&ours.element_type) - != std::mem::discriminant(&theirs.element_type) - { - return false; - } - } else if ours.element_type != theirs.element_type { + nest_level -= 1; + } else { + if is_container(ours.element_type) { + nest_level += 1; + // Only compare the discriminants in case of array/list/structures, + // instead of actual element values. Those will be subsets within this same + // list that will get validated anyway + if std::mem::discriminant(&ours.element_type) + != std::mem::discriminant(&theirs.element_type) + { return false; } + } else if ours.element_type != theirs.element_type { + return false; + } - if ours.tag_type != theirs.tag_type { - return false; - } + if ours.tag_type != theirs.tag_type { + return false; } } } @@ -668,9 +664,8 @@ impl<'a> TLVContainerIterator<'a> { } _ => return Some(last_elem), } - } else { - nest_level -= 1; } + nest_level -= 1; } _ => { if is_container(element.element_type) { diff --git a/matter/src/tlv/traits.rs b/matter/src/tlv/traits.rs index ab03a63..d9b7d10 100644 --- a/matter/src/tlv/traits.rs +++ b/matter/src/tlv/traits.rs @@ -88,7 +88,7 @@ macro_rules! totlv_for { }; } -impl<'a, T: ToTLV, const N: usize> ToTLV for [T; N] { +impl ToTLV for [T; N] { fn to_tlv(&self, tw: &mut TLVWriter, tag: TagType) -> Result<(), Error> { tw.start_array(tag)?; for i in self { @@ -201,7 +201,7 @@ impl<'a, T: FromTLV<'a>> FromTLV<'a> for Option { impl ToTLV for Option { fn to_tlv(&self, tw: &mut TLVWriter, tag: TagType) -> Result<(), Error> { match self { - Some(s) => (s.to_tlv(tw, tag)), + Some(s) => s.to_tlv(tw, tag), None => Ok(()), } } @@ -311,12 +311,10 @@ impl<'a, T: ToTLV> TLVArray<'a, T> { impl<'a, T: ToTLV + FromTLV<'a> + Copy> TLVArray<'a, T> { pub fn get_index(&self, index: usize) -> T { - let mut curr = 0; - for element in self.iter() { + for (curr, element) in self.iter().enumerate() { if curr == index { return element; } - curr += 1; } panic!("Out of bounds"); } @@ -350,7 +348,7 @@ where { fn eq(&self, other: &&[T]) -> bool { let mut iter1 = self.iter(); - let mut iter2 = other.into_iter(); + let mut iter2 = other.iter(); loop { match (iter1.next(), iter2.next()) { (None, None) => return true, @@ -392,7 +390,7 @@ impl<'a, T: Debug + ToTLV + FromTLV<'a> + Copy> Debug for TLVArray<'a, T> { for i in self.iter() { writeln!(f, "{:?}", i)?; } - writeln!(f, "") + writeln!(f) } } diff --git a/matter/src/tlv/writer.rs b/matter/src/tlv/writer.rs index e229bf0..9cefb0f 100644 --- a/matter/src/tlv/writer.rs +++ b/matter/src/tlv/writer.rs @@ -74,8 +74,8 @@ impl<'a, 'b> TLVWriter<'a, 'b> { TagType::CommonPrf32(v) => (3, v as u64), TagType::ImplPrf16(v) => (4, v as u64), TagType::ImplPrf32(v) => (5, v as u64), - TagType::FullQual48(v) => (6, v as u64), - TagType::FullQual64(v) => (7, v as u64), + TagType::FullQual48(v) => (6, v), + TagType::FullQual64(v) => (7, v), }; self.buf .le_u8(((tag_id) << TAG_SHIFT_BITS) | (val_type as u8))?; @@ -269,13 +269,11 @@ mod tests { tw.u8(TagType::Anonymous, 12).unwrap(); tw.u8(TagType::Context(1), 13).unwrap(); - match tw.u16(TagType::Anonymous, 12) { - Ok(_) => panic!("This should have returned error"), - _ => (), + if tw.u16(TagType::Anonymous, 12).is_ok() { + panic!("This should have returned error") } - match tw.u16(TagType::Context(2), 13) { - Ok(_) => panic!("This should have returned error"), - _ => (), + if tw.u16(TagType::Context(2), 13).is_ok() { + panic!("This should have returned error") } assert_eq!(buf, [4, 12, 36, 1, 13, 4]); } diff --git a/matter/src/transport/exchange.rs b/matter/src/transport/exchange.rs index 191c3e4..2e67636 100644 --- a/matter/src/transport/exchange.rs +++ b/matter/src/transport/exchange.rs @@ -123,10 +123,7 @@ impl Exchange { } pub fn is_data_none(&self) -> bool { - match self.data { - DataOption::None => true, - _ => false, - } + matches!(self.data, DataOption::None) } pub fn set_data_boxed(&mut self, data: Box) { @@ -147,12 +144,11 @@ impl Exchange { pub fn take_data_boxed(&mut self) -> Option> { let old = std::mem::replace(&mut self.data, DataOption::None); - match old { - DataOption::Boxed(d) => d.downcast::().ok(), - _ => { - self.data = old; - None - } + if let DataOption::Boxed(d) = old { + d.downcast::().ok() + } else { + self.data = old; + None } } @@ -293,15 +289,14 @@ impl ExchangeMgr { // Get the session let (mut proto_rx, index) = self.sess_mgr.recv()?; - let index = match index { - Some(s) => s, - None => { - // The sessions were full, evict one session, and re-perform post-recv - let evict_index = self.sess_mgr.get_lru(); - self.evict_session(evict_index)?; - info!("Reattempting session creation"); - self.sess_mgr.post_recv(&proto_rx)?.ok_or(Error::Invalid)? - } + let index = if let Some(s) = index { + s + } else { + // The sessions were full, evict one session, and re-perform post-recv + let evict_index = self.sess_mgr.get_lru(); + self.evict_session(evict_index)?; + info!("Reattempting session creation"); + self.sess_mgr.post_recv(&proto_rx)?.ok_or(Error::Invalid)? }; let mut session = self.sess_mgr.get_session_handle(index); @@ -352,7 +347,7 @@ impl ExchangeMgr { } } for (exch_id, _) in to_purge.iter() { - self.exchanges.remove(&*exch_id); + self.exchanges.remove(exch_id); } } @@ -370,7 +365,7 @@ impl ExchangeMgr { // As per the spec, we need to send a CLOSE here let mut session = self.sess_mgr.get_session_handle(index); - let mut tx = Slab::::new(Packet::new_tx()?).ok_or(Error::NoSpace)?; + let mut tx = Slab::::try_new(Packet::new_tx()?).ok_or(Error::NoSpace)?; secure_channel::common::create_sc_status_report( &mut tx, secure_channel::common::SCStatusCodes::CloseSession, @@ -408,13 +403,13 @@ impl ExchangeMgr { Ok(()) } - pub fn add_session(&mut self, clone_data: CloneData) -> Result { - let sess_idx = match self.sess_mgr.clone_session(&clone_data) { + pub fn add_session(&mut self, clone_data: &CloneData) -> Result { + let sess_idx = match self.sess_mgr.clone_session(clone_data) { Ok(idx) => idx, Err(Error::NoSpace) => { let evict_index = self.sess_mgr.get_lru(); self.evict_session(evict_index)?; - self.sess_mgr.clone_session(&clone_data)? + self.sess_mgr.clone_session(clone_data)? } Err(e) => { return Err(e); @@ -437,6 +432,7 @@ impl fmt::Display for ExchangeMgr { } #[cfg(test)] +#[allow(clippy::bool_assert_comparison)] mod tests { use crate::{ @@ -496,8 +492,8 @@ mod tests { let mut peer_sess_id = 100; for _ in 1..count { let clone_data = get_clone_data(peer_sess_id, local_sess_id); - match mgr.add_session(clone_data) { - Ok(s) => (assert_eq!(peer_sess_id, s.get_peer_sess_id())), + match mgr.add_session(&clone_data) { + Ok(s) => assert_eq!(peer_sess_id, s.get_peer_sess_id()), Err(Error::NoSpace) => break, _ => { panic!("Couldn't, create session"); @@ -558,7 +554,7 @@ mod tests { for i in 1..(MAX_SESSIONS + 1) { // Now purposefully overflow the sessions by adding another session let session = mgr - .add_session(get_clone_data(new_peer_sess_id, new_local_sess_id)) + .add_session(&get_clone_data(new_peer_sess_id, new_local_sess_id)) .unwrap(); assert_eq!(session.get_peer_sess_id(), new_peer_sess_id); diff --git a/matter/src/transport/mgr.rs b/matter/src/transport/mgr.rs index 4a241f2..76c506c 100644 --- a/matter/src/transport/mgr.rs +++ b/matter/src/transport/mgr.rs @@ -118,7 +118,7 @@ impl Mgr { // If a new session was created, add it let _ = self .exch_mgr - .add_session(clone_data) + .add_session(&clone_data) .map_err(|e| error!("Error adding new session {:?}", e)); } _ => { @@ -170,6 +170,6 @@ impl Mgr { } fn new_tx() -> Result, Error> { - Slab::::new(Packet::new_tx()?).ok_or(Error::PacketPoolExhaust) + Slab::::try_new(Packet::new_tx()?).ok_or(Error::PacketPoolExhaust) } } diff --git a/matter/src/transport/mod.rs b/matter/src/transport/mod.rs index 228421e..bc0ab6f 100644 --- a/matter/src/transport/mod.rs +++ b/matter/src/transport/mod.rs @@ -15,7 +15,6 @@ * limitations under the License. */ - pub mod exchange; pub mod mgr; pub mod mrp; diff --git a/matter/src/transport/packet.rs b/matter/src/transport/packet.rs index ecfdcc5..18af1b5 100644 --- a/matter/src/transport/packet.rs +++ b/matter/src/transport/packet.rs @@ -138,8 +138,8 @@ impl<'a> Packet<'a> { pub fn as_borrow_slice(&mut self) -> &mut [u8] { match &mut self.data { - Direction::Rx(pb, _) => (pb.as_borrow_slice()), - Direction::Tx(wb) => (wb.as_mut_slice()), + Direction::Rx(pb, _) => pb.as_borrow_slice(), + Direction::Tx(wb) => wb.as_mut_slice(), } } diff --git a/matter/src/transport/queue.rs b/matter/src/transport/queue.rs index 1507000..b0c0f37 100644 --- a/matter/src/transport/queue.rs +++ b/matter/src/transport/queue.rs @@ -15,7 +15,6 @@ * limitations under the License. */ - use std::sync::Once; use async_channel::{bounded, Receiver, Sender}; diff --git a/matter/src/transport/session.rs b/matter/src/transport/session.rs index 2b41b6e..db0c845 100644 --- a/matter/src/transport/session.rs +++ b/matter/src/transport/session.rs @@ -466,7 +466,8 @@ impl SessionMgr { } pub fn recv(&mut self) -> Result<(BoxSlab, Option), Error> { - let mut rx = Slab::::new(Packet::new_rx()?).ok_or(Error::PacketPoolExhaust)?; + let mut rx = + Slab::::try_new(Packet::new_rx()?).ok_or(Error::PacketPoolExhaust)?; let network = self.network.as_ref().ok_or(Error::NoNetworkInterface)?; diff --git a/matter/src/utils/parsebuf.rs b/matter/src/utils/parsebuf.rs index e0dcbf8..b5342c0 100644 --- a/matter/src/utils/parsebuf.rs +++ b/matter/src/utils/parsebuf.rs @@ -117,22 +117,19 @@ mod tests { assert_eq!(buf.le_u8().unwrap(), 0x01); - match buf.le_u16() { - Ok(_) => panic!("This should have returned error"), - Err(_) => (), + if buf.le_u16().is_ok() { + panic!("This should have returned error") } - match buf.le_u32() { - Ok(_) => panic!("This should have returned error"), - Err(_) => (), + if buf.le_u32().is_ok() { + panic!("This should have returned error") } // Now consume the leftover byte assert_eq!(buf.le_u8().unwrap(), 65); - match buf.le_u8() { - Ok(_) => panic!("This should have returned error"), - Err(_) => (), + if buf.le_u8().is_ok() { + panic!("This should have returned error") } assert_eq!(buf.as_slice(), []); } @@ -161,9 +158,8 @@ mod tests { assert_eq!(buf.le_u8().unwrap(), 0x01); assert_eq!(buf.le_u16().unwrap(), 65); assert_eq!(buf.le_u32().unwrap(), 0xcafebabe); - match buf.tail(5) { - Ok(_) => panic!("This should have returned error"), - Err(_) => (), + if buf.tail(5).is_ok() { + panic!("This should have returned error") } assert_eq!(buf.tail(2).unwrap(), [0xc, 0xd]); } diff --git a/matter/src/utils/writebuf.rs b/matter/src/utils/writebuf.rs index 1b075cc..b82bb02 100644 --- a/matter/src/utils/writebuf.rs +++ b/matter/src/utils/writebuf.rs @@ -199,24 +199,20 @@ mod tests { buf.le_u64(0xcafebabecafebabe).unwrap(); // Now the buffer is fully filled up, so no further puts will happen - match buf.le_u8(1) { - Ok(_) => panic!("Should return error"), - _ => (), + if buf.le_u8(1).is_ok() { + panic!("Should return error") } - match buf.le_u16(65) { - Ok(_) => panic!("Should return error"), - _ => (), + if buf.le_u16(65).is_ok() { + panic!("Should return error") } - match buf.le_u32(0xcafebabe) { - Ok(_) => panic!("Should return error"), - _ => (), + if buf.le_u32(0xcafebabe).is_ok() { + panic!("Should return error") } - match buf.le_u64(0xcafebabecafebabe) { - Ok(_) => panic!("Should return error"), - _ => (), + if buf.le_u64(0xcafebabecafebabe).is_ok() { + panic!("Should return error") } } @@ -267,9 +263,8 @@ mod tests { buf.le_u16(65).unwrap(); let new_slice: [u8; 5] = [0xaa, 0xbb, 0xcc, 0xdd, 0xee]; - match buf.copy_from_slice(&new_slice) { - Ok(_) => panic!("This should have returned error"), - Err(_) => (), + if buf.copy_from_slice(&new_slice).is_ok() { + panic!("This should have returned error") } } @@ -296,9 +291,8 @@ mod tests { buf.le_u16(65).unwrap(); let new_slice: [u8; 6] = [0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff]; - match buf.prepend(&new_slice) { - Ok(_) => panic!("Prepend should return error"), - Err(_) => (), + if buf.prepend(&new_slice).is_ok() { + panic!("Prepend should return error") } } diff --git a/matter/tests/common/echo_cluster.rs b/matter/tests/common/echo_cluster.rs index 381bc6f..a65872f 100644 --- a/matter/tests/common/echo_cluster.rs +++ b/matter/tests/common/echo_cluster.rs @@ -121,7 +121,7 @@ impl ClusterType for EchoCluster { ) -> Result<(), IMStatusCode> { match num::FromPrimitive::from_u16(attr.attr_id) { Some(Attributes::AttWriteList) => { - attr_list_write(attr, data, |op, data| self.write_attr_list(op, data)) + attr_list_write(attr, data, |op, data| self.write_attr_list(&op, data)) } _ => self.base.write_attribute_from_tlv(attr.attr_id, data), } @@ -132,7 +132,7 @@ impl ClusterType for EchoCluster { .cmd .path .leaf - .map(|c| num::FromPrimitive::from_u32(c)) + .map(num::FromPrimitive::from_u32) .ok_or(IMStatusCode::UnsupportedCommand)? .ok_or(IMStatusCode::UnsupportedCommand)?; match cmd { @@ -206,7 +206,7 @@ impl EchoCluster { fn write_attr_list( &mut self, - op: ListOperation, + op: &ListOperation, data: &TLVElement, ) -> Result<(), IMStatusCode> { let tc_handle = TestChecker::get().unwrap(); @@ -224,16 +224,16 @@ impl EchoCluster { } ListOperation::EditItem(index) => { let data = data.u16().map_err(|_| IMStatusCode::Failure)?; - if tc.write_list[index as usize].is_some() { - tc.write_list[index as usize] = Some(data); + if tc.write_list[*index as usize].is_some() { + tc.write_list[*index as usize] = Some(data); Ok(()) } else { Err(IMStatusCode::InvalidAction) } } ListOperation::DeleteItem(index) => { - if tc.write_list[index as usize].is_some() { - tc.write_list[index as usize] = None; + if tc.write_list[*index as usize].is_some() { + tc.write_list[*index as usize] = None; Ok(()) } else { Err(IMStatusCode::InvalidAction) diff --git a/matter/tests/common/im_engine.rs b/matter/tests/common/im_engine.rs index ddd8b30..81c2c74 100644 --- a/matter/tests/common/im_engine.rs +++ b/matter/tests/common/im_engine.rs @@ -102,7 +102,7 @@ impl ImEngine { // Only allow the standard peer node id of the IM Engine default_acl.add_subject(IM_ENGINE_PEER_ID).unwrap(); acl_mgr.add(default_acl).unwrap(); - let dm = DataModel::new(dev_det, dev_att, fabric_mgr.clone(), acl_mgr.clone()).unwrap(); + let dm = DataModel::new(&dev_det, dev_att, fabric_mgr, acl_mgr.clone()).unwrap(); { let mut d = dm.node.write().unwrap(); @@ -127,7 +127,7 @@ impl ImEngine { pub fn process<'a>(&mut self, input: &ImInput, data_out: &'a mut [u8]) -> (u8, &'a mut [u8]) { let mut new_exch = Exchange::new(1, 0, exchange::Role::Responder); // Choose whether to use a new exchange, or use the one from the ImEngine configuration - let mut exch = self.exch.as_mut().unwrap_or_else(|| &mut new_exch); + let exch = self.exch.as_mut().unwrap_or(&mut new_exch); let mut sess_mgr: SessionMgr = Default::default(); @@ -144,12 +144,9 @@ impl ImEngine { ); let sess_idx = sess_mgr.clone_session(&clone_data).unwrap(); let sess = sess_mgr.get_session_handle(sess_idx); - let exch_ctx = ExchangeCtx { - exch: &mut exch, - sess, - }; - let mut rx = Slab::::new(Packet::new_rx().unwrap()).unwrap(); - let tx = Slab::::new(Packet::new_tx().unwrap()).unwrap(); + let exch_ctx = ExchangeCtx { exch, sess }; + let mut rx = Slab::::try_new(Packet::new_rx().unwrap()).unwrap(); + let tx = Slab::::try_new(Packet::new_tx().unwrap()).unwrap(); // Create fake rx packet rx.set_proto_id(0x01); rx.set_proto_opcode(input.action as u8); diff --git a/matter/tests/data_model/timed_requests.rs b/matter/tests/data_model/timed_requests.rs index 75af2e4..a502f55 100644 --- a/matter/tests/data_model/timed_requests.rs +++ b/matter/tests/data_model/timed_requests.rs @@ -83,7 +83,7 @@ enum WriteResponse<'a> { // Helper for handling Write Attribute sequences fn handle_timed_write_reqs( input: &[AttrData], - expected: WriteResponse, + expected: &WriteResponse, timeout: u16, delay: u16, ) -> DataModel { @@ -159,10 +159,10 @@ fn test_timed_write_fail_and_success() { ]; // Test with incorrect handling - handle_timed_write_reqs(input, WriteResponse::TransactionError, 400, 500); + handle_timed_write_reqs(input, &WriteResponse::TransactionError, 400, 500); // Test with correct handling - let dm = handle_timed_write_reqs(input, WriteResponse::TransactionSuccess(expected), 400, 0); + let dm = handle_timed_write_reqs(input, &WriteResponse::TransactionSuccess(expected), 400, 0); assert_eq!( AttrValue::Uint16(val0), dm.read_attribute_raw( @@ -190,7 +190,7 @@ enum TimedInvResponse<'a> { // Helper for handling Invoke Command sequences fn handle_timed_commands( input: &[CmdData], - expected: TimedInvResponse, + expected: &TimedInvResponse, timeout: u16, delay: u16, set_timed_request: bool, @@ -222,7 +222,7 @@ fn handle_timed_commands( Some(OpCode::StatusResponse) ); let status_resp = StatusResp::from_tlv(&root).unwrap(); - assert_eq!(status_resp.status, e); + assert_eq!(status_resp.status, *e); } } dm @@ -237,7 +237,7 @@ fn test_timed_cmd_success() { let expected = &[echo_resp!(0, 10), echo_resp!(1, 30)]; handle_timed_commands( input, - TimedInvResponse::TransactionSuccess(expected), + &TimedInvResponse::TransactionSuccess(expected), 400, 0, true, @@ -252,7 +252,7 @@ fn test_timed_cmd_timeout() { let input = &[echo_req!(0, 5), echo_req!(1, 10)]; handle_timed_commands( input, - TimedInvResponse::TransactionError(IMStatusCode::Timeout), + &TimedInvResponse::TransactionError(IMStatusCode::Timeout), 400, 500, true, @@ -267,7 +267,7 @@ fn test_timed_cmd_timedout_mismatch() { let input = &[echo_req!(0, 5), echo_req!(1, 10)]; handle_timed_commands( input, - TimedInvResponse::TransactionError(IMStatusCode::TimedRequestMisMatch), + &TimedInvResponse::TransactionError(IMStatusCode::TimedRequestMisMatch), 400, 0, false, @@ -276,7 +276,7 @@ fn test_timed_cmd_timedout_mismatch() { let input = &[echo_req!(0, 5), echo_req!(1, 10)]; handle_timed_commands( input, - TimedInvResponse::TransactionError(IMStatusCode::TimedRequestMisMatch), + &TimedInvResponse::TransactionError(IMStatusCode::TimedRequestMisMatch), 0, 0, true, diff --git a/matter/tests/interaction_model.rs b/matter/tests/interaction_model.rs index 06d31a1..3f831c4 100644 --- a/matter/tests/interaction_model.rs +++ b/matter/tests/interaction_model.rs @@ -136,8 +136,8 @@ fn handle_data(action: OpCode, data_in: &[u8], data_out: &mut [u8]) -> (DataMode exch: &mut exch, sess, }; - let mut rx = Slab::::new(Packet::new_rx().unwrap()).unwrap(); - let tx = Slab::::new(Packet::new_tx().unwrap()).unwrap(); + let mut rx = Slab::::try_new(Packet::new_rx().unwrap()).unwrap(); + let tx = Slab::::try_new(Packet::new_tx().unwrap()).unwrap(); // Create fake rx packet rx.set_proto_id(0x01); rx.set_proto_opcode(action as u8); diff --git a/matter_macro_derive/src/lib.rs b/matter_macro_derive/src/lib.rs index 91f2734..0fc358f 100644 --- a/matter_macro_derive/src/lib.rs +++ b/matter_macro_derive/src/lib.rs @@ -47,7 +47,7 @@ impl Default for TlvArgs { fn parse_tlvargs(ast: &DeriveInput) -> TlvArgs { let mut tlvargs: TlvArgs = Default::default(); - if ast.attrs.len() > 0 { + if !ast.attrs.is_empty() { if let List(MetaList { path, paren_token: _, @@ -87,7 +87,7 @@ fn parse_tlvargs(ast: &DeriveInput) -> TlvArgs { } fn parse_tag_val(field: &syn::Field) -> Option { - if field.attrs.len() > 0 { + if !field.attrs.is_empty() { if let List(MetaList { path, paren_token: _, @@ -110,8 +110,8 @@ fn parse_tag_val(field: &syn::Field) -> Option { fn gen_totlv_for_struct( fields: &syn::FieldsNamed, struct_name: &proc_macro2::Ident, - tlvargs: TlvArgs, - generics: syn::Generics, + tlvargs: &TlvArgs, + generics: &syn::Generics, ) -> TokenStream { let mut tag_start = tlvargs.start; let datatype = format_ident!("start_{}", tlvargs.datatype); @@ -127,7 +127,7 @@ fn gen_totlv_for_struct( // keys.push(quote! { #literal_key_str }); idents.push(&field.ident); // types.push(type_name.to_token_stream()); - if let Some(a) = parse_tag_val(&field) { + if let Some(a) = parse_tag_val(field) { tags.push(a); } else { tags.push(tag_start); @@ -152,10 +152,10 @@ fn gen_totlv_for_struct( /// Generate a ToTlv implementation for an enum fn gen_totlv_for_enum( - data_enum: syn::DataEnum, + data_enum: &syn::DataEnum, enum_name: &proc_macro2::Ident, - tlvargs: TlvArgs, - generics: syn::Generics, + tlvargs: &TlvArgs, + generics: &syn::Generics, ) -> TokenStream { let mut tag_start = tlvargs.start; @@ -232,9 +232,9 @@ pub fn derive_totlv(item: TokenStream) -> TokenStream { .. }) = ast.data { - gen_totlv_for_struct(fields, name, tlvargs, generics) + gen_totlv_for_struct(fields, name, &tlvargs, &generics) } else if let syn::Data::Enum(data_enum) = ast.data { - gen_totlv_for_enum(data_enum, name, tlvargs, generics) + gen_totlv_for_enum(&data_enum, name, &tlvargs, &generics) } else { panic!( "Derive ToTLV - Only supported Struct for now {:?}", @@ -248,7 +248,7 @@ fn gen_fromtlv_for_struct( fields: &syn::FieldsNamed, struct_name: &proc_macro2::Ident, tlvargs: TlvArgs, - generics: syn::Generics, + generics: &syn::Generics, ) -> TokenStream { let mut tag_start = tlvargs.start; let lifetime = tlvargs.lifetime; @@ -260,7 +260,7 @@ fn gen_fromtlv_for_struct( for field in fields.named.iter() { let type_name = &field.ty; - if let Some(a) = parse_tag_val(&field) { + if let Some(a) = parse_tag_val(field) { // TODO: The current limitation with this is that a hard-coded integer // value has to be mentioned in the tagval attribute. This is because // our tags vector is for integers, and pushing an 'identifier' on it @@ -330,10 +330,10 @@ fn gen_fromtlv_for_struct( /// Generate a FromTlv implementation for an enum fn gen_fromtlv_for_enum( - data_enum: syn::DataEnum, + data_enum: &syn::DataEnum, enum_name: &proc_macro2::Ident, tlvargs: TlvArgs, - generics: syn::Generics, + generics: &syn::Generics, ) -> TokenStream { let mut tag_start = tlvargs.start; let lifetime = tlvargs.lifetime; @@ -422,9 +422,9 @@ pub fn derive_fromtlv(item: TokenStream) -> TokenStream { .. }) = ast.data { - gen_fromtlv_for_struct(fields, name, tlvargs, generics) + gen_fromtlv_for_struct(fields, name, tlvargs, &generics) } else if let syn::Data::Enum(data_enum) = ast.data { - gen_fromtlv_for_enum(data_enum, name, tlvargs, generics) + gen_fromtlv_for_enum(&data_enum, name, tlvargs, &generics) } else { panic!( "Derive FromTLV - Only supported Struct for now {:?}", From b74d626efcef7aad2a67051bb6b1bb673b58836c Mon Sep 17 00:00:00 2001 From: Kedar Sovani Date: Sat, 7 Jan 2023 21:47:33 +0530 Subject: [PATCH 02/12] AdminCommissioning: Baseline support Just add all the 3 attributes, and the command open-commissioning-window that simply sets a variable for now --- matter/src/data_model/device_types.rs | 2 + matter/src/data_model/objects/endpoint.rs | 2 +- .../src/data_model/sdm/admin_commissioning.rs | 163 ++++++++++++++++++ matter/src/data_model/sdm/mod.rs | 1 + 4 files changed, 167 insertions(+), 1 deletion(-) create mode 100644 matter/src/data_model/sdm/admin_commissioning.rs diff --git a/matter/src/data_model/device_types.rs b/matter/src/data_model/device_types.rs index 47fc022..e1c74ec 100644 --- a/matter/src/data_model/device_types.rs +++ b/matter/src/data_model/device_types.rs @@ -19,6 +19,7 @@ use super::cluster_basic_information::BasicInfoCluster; use super::cluster_basic_information::BasicInfoConfig; use super::cluster_on_off::OnOffCluster; use super::objects::*; +use super::sdm::admin_commissioning::AdminCommCluster; use super::sdm::dev_att::DevAttDataFetcher; use super::sdm::general_commissioning::GenCommCluster; use super::sdm::noc::NocCluster; @@ -51,6 +52,7 @@ pub fn device_type_add_root_node( let failsafe = general_commissioning.failsafe(); node.add_cluster(0, general_commissioning)?; node.add_cluster(0, NwCommCluster::new()?)?; + node.add_cluster(0, AdminCommCluster::new()?)?; node.add_cluster( 0, NocCluster::new(dev_att, fabric_mgr, acl_mgr.clone(), failsafe)?, diff --git a/matter/src/data_model/objects/endpoint.rs b/matter/src/data_model/objects/endpoint.rs index 220119b..a87f887 100644 --- a/matter/src/data_model/objects/endpoint.rs +++ b/matter/src/data_model/objects/endpoint.rs @@ -19,7 +19,7 @@ use crate::{data_model::objects::ClusterType, error::*, interaction_model::core: use std::fmt; -pub const CLUSTERS_PER_ENDPT: usize = 7; +pub const CLUSTERS_PER_ENDPT: usize = 9; pub struct Endpoint { clusters: Vec>, diff --git a/matter/src/data_model/sdm/admin_commissioning.rs b/matter/src/data_model/sdm/admin_commissioning.rs new file mode 100644 index 0000000..6e7d7b7 --- /dev/null +++ b/matter/src/data_model/sdm/admin_commissioning.rs @@ -0,0 +1,163 @@ +/* + * + * Copyright (c) 2023 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +use crate::cmd_enter; +use crate::data_model::objects::*; +use crate::interaction_model::core::IMStatusCode; +use crate::tlv::{FromTLV, Nullable, OctetStr, TLVElement}; +use crate::{error::*, interaction_model::command::CommandReq}; +use log::{error, info}; +use num_derive::FromPrimitive; + +pub const ID: u32 = 0x003C; + +#[derive(FromPrimitive, Debug, Copy, Clone, PartialEq)] +pub enum WindowStatus { + WindowNotOpen = 0, + EnhancedWindowOpen = 1, + BasicWindowOpen = 2, +} + +#[derive(FromPrimitive)] +pub enum Attributes { + WindowStatus = 0, + AdminFabricIndex = 1, + AdminVendorId = 2, +} + +#[derive(FromPrimitive)] +pub enum Commands { + OpenCommWindow = 0x00, + OpenBasicCommWindow = 0x01, + RevokeComm = 0x02, +} + +fn attr_window_status_new() -> Result { + Attribute::new( + Attributes::WindowStatus as u16, + AttrValue::Custom, + Access::RV, + Quality::NONE, + ) +} + +fn attr_admin_fabid_new() -> Result { + Attribute::new( + Attributes::AdminFabricIndex as u16, + AttrValue::Custom, + Access::RV, + Quality::NULLABLE, + ) +} + +fn attr_admin_vid_new() -> Result { + Attribute::new( + Attributes::AdminVendorId as u16, + AttrValue::Custom, + Access::RV, + Quality::NULLABLE, + ) +} + +pub struct AdminCommCluster { + window_status: WindowStatus, + base: Cluster, +} + +impl ClusterType for AdminCommCluster { + fn base(&self) -> &Cluster { + &self.base + } + fn base_mut(&mut self) -> &mut Cluster { + &mut self.base + } + + fn read_custom_attribute(&self, encoder: &mut dyn Encoder, attr: &AttrDetails) { + match num::FromPrimitive::from_u16(attr.attr_id) { + Some(Attributes::WindowStatus) => { + let status = self.window_status as u8; + encoder.encode(EncodeValue::Value(&status)) + } + Some(Attributes::AdminVendorId) => { + let vid = if self.window_status == WindowStatus::WindowNotOpen { + Nullable::Null + } else { + Nullable::NotNull(1_u8) + }; + encoder.encode(EncodeValue::Value(&vid)) + } + Some(Attributes::AdminFabricIndex) => { + let vid = if self.window_status == WindowStatus::WindowNotOpen { + Nullable::Null + } else { + Nullable::NotNull(1_u8) + }; + encoder.encode(EncodeValue::Value(&vid)) + } + _ => { + error!("Unsupported Attribute: this shouldn't happen"); + } + } + } + fn handle_command(&mut self, cmd_req: &mut CommandReq) -> Result<(), IMStatusCode> { + let cmd = cmd_req + .cmd + .path + .leaf + .map(num::FromPrimitive::from_u32) + .ok_or(IMStatusCode::UnsupportedCommand)? + .ok_or(IMStatusCode::UnsupportedCommand)?; + match cmd { + Commands::OpenCommWindow => self.handle_command_opencomm_win(cmd_req), + _ => Err(IMStatusCode::UnsupportedCommand), + } + } +} + +impl AdminCommCluster { + pub fn new() -> Result, Error> { + let mut c = Box::new(AdminCommCluster { + window_status: WindowStatus::WindowNotOpen, + base: Cluster::new(ID)?, + }); + c.base.add_attribute(attr_window_status_new()?)?; + c.base.add_attribute(attr_admin_fabid_new()?)?; + c.base.add_attribute(attr_admin_vid_new()?)?; + Ok(c) + } + + fn handle_command_opencomm_win( + &mut self, + cmd_req: &mut CommandReq, + ) -> Result<(), IMStatusCode> { + cmd_enter!("Open Commissioning Window"); + let _req = + OpenCommWindowReq::from_tlv(&cmd_req.data).map_err(|_| IMStatusCode::InvalidCommand)?; + self.window_status = WindowStatus::EnhancedWindowOpen; + Err(IMStatusCode::Sucess) + } +} + +#[derive(FromTLV)] +#[tlvargs(lifetime = "'a")] +pub struct OpenCommWindowReq<'a> { + _timeout: u16, + _verifier: OctetStr<'a>, + _discriminator: u16, + _iterations: u32, + _salt: OctetStr<'a>, +} diff --git a/matter/src/data_model/sdm/mod.rs b/matter/src/data_model/sdm/mod.rs index cec166d..1ce25ad 100644 --- a/matter/src/data_model/sdm/mod.rs +++ b/matter/src/data_model/sdm/mod.rs @@ -15,6 +15,7 @@ * limitations under the License. */ +pub mod admin_commissioning; pub mod dev_att; pub mod failsafe; pub mod general_commissioning; From 98fccca39239f4d37e31ba17a13d2049019d5c84 Mon Sep 17 00:00:00 2001 From: Kedar Sovani Date: Sun, 8 Jan 2023 11:13:12 +0530 Subject: [PATCH 03/12] mdns: Discriminator is a property of the 'commissionable' state --- matter/src/core.rs | 8 ++++++-- matter/src/mdns.rs | 19 ++++++++----------- matter/src/secure_channel/core.rs | 10 ++++++++-- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/matter/src/core.rs b/matter/src/core.rs index 6aee3ce..088b028 100644 --- a/matter/src/core.rs +++ b/matter/src/core.rs @@ -62,7 +62,7 @@ impl Matter { dev_comm: &CommissioningData, ) -> Result, Error> { let mdns = Mdns::get()?; - mdns.set_values(dev_det.vid, dev_det.pid, dev_comm.discriminator); + mdns.set_values(dev_det.vid, dev_det.pid); let fabric_mgr = Arc::new(FabricMgr::new()?); let acl_mgr = Arc::new(AclMgr::new()?); @@ -78,7 +78,11 @@ impl Matter { matter.transport_mgr.register_protocol(interaction_model)?; let mut secure_channel = Box::new(SecureChannel::new(matter.fabric_mgr.clone())); if open_comm_window { - secure_channel.open_comm_window(&dev_comm.salt, dev_comm.passwd)?; + secure_channel.open_comm_window( + &dev_comm.salt, + dev_comm.passwd, + dev_comm.discriminator, + )?; } matter.transport_mgr.register_protocol(secure_channel)?; diff --git a/matter/src/mdns.rs b/matter/src/mdns.rs index c1180a6..6a971e4 100644 --- a/matter/src/mdns.rs +++ b/matter/src/mdns.rs @@ -30,8 +30,6 @@ pub struct MdnsInner { vid: u16, /// Product ID pid: u16, - /// Discriminator - discriminator: u16, } pub struct Mdns { @@ -46,8 +44,10 @@ static INIT: Once = Once::new(); #[derive(Clone, Copy)] pub enum ServiceMode { + /// The commissioned state Commissioned, - Commissionable, + /// The commissionable state with the discriminator that should be used + Commissionable(u16), } impl Mdns { @@ -72,11 +72,10 @@ impl Mdns { /// Set mDNS service specific values /// Values like vid, pid, discriminator etc // TODO: More things like device-type etc can be added here - pub fn set_values(&self, vid: u16, pid: u16, discriminator: u16) { + pub fn set_values(&self, vid: u16, pid: u16) { let mut inner = self.inner.lock().unwrap(); inner.vid = vid; inner.pid = pid; - inner.discriminator = discriminator; } /// Publish a mDNS service @@ -87,13 +86,11 @@ impl Mdns { ServiceMode::Commissioned => { sys_publish_service(name, "_matter._tcp", MATTER_PORT, &[]) } - ServiceMode::Commissionable => { - let inner = self.inner.lock().unwrap(); - let short = - (inner.discriminator & SHORT_DISCRIMINATOR_MASK) >> SHORT_DISCRIMINATOR_SHIFT; - let serv_type = format!("_matterc._udp,_S{},_L{}", short, inner.discriminator); + ServiceMode::Commissionable(discriminator) => { + let short = (discriminator & SHORT_DISCRIMINATOR_MASK) >> SHORT_DISCRIMINATOR_SHIFT; + let serv_type = format!("_matterc._udp,_S{},_L{}", short, discriminator); - let str_discriminator = format!("{}", inner.discriminator); + let str_discriminator = format!("{}", discriminator); let txt_kvs = [["D", &str_discriminator], ["CM", "1"]]; sys_publish_service(name, &serv_type, MATTER_PORT, &txt_kvs) } diff --git a/matter/src/secure_channel/core.rs b/matter/src/secure_channel/core.rs index 679b1f5..f855802 100644 --- a/matter/src/secure_channel/core.rs +++ b/matter/src/secure_channel/core.rs @@ -48,10 +48,16 @@ impl SecureChannel { } } - pub fn open_comm_window(&mut self, salt: &[u8; 16], passwd: u32) -> Result<(), Error> { + pub fn open_comm_window( + &mut self, + salt: &[u8; 16], + passwd: u32, + discriminator: u16, + ) -> Result<(), Error> { let name: u64 = rand::thread_rng().gen_range(0..0xFFFFFFFFFFFFFFFF); let name = format!("{:016X}", name); - let mdns = Mdns::get()?.publish_service(&name, mdns::ServiceMode::Commissionable)?; + let mdns = Mdns::get()? + .publish_service(&name, mdns::ServiceMode::Commissionable(discriminator))?; self.pake = Some((PAKE::new(salt, passwd), mdns)); Ok(()) } From 311cc412e2ab83f81e86acb360fad8703e12a15c Mon Sep 17 00:00:00 2001 From: Kedar Sovani Date: Sun, 8 Jan 2023 12:27:27 +0530 Subject: [PATCH 04/12] pase: Create provision to accept either password or verifier for configuring PASE --- examples/onoff_light/src/main.rs | 8 +- matter/src/core.rs | 16 +--- matter/src/lib.rs | 9 +-- matter/src/secure_channel/core.rs | 7 +- matter/src/secure_channel/crypto.rs | 4 +- matter/src/secure_channel/crypto_mbedtls.rs | 7 +- matter/src/secure_channel/crypto_openssl.rs | 7 +- matter/src/secure_channel/pake.rs | 21 ++---- matter/src/secure_channel/spake2p.rs | 82 ++++++++++++++++++--- 9 files changed, 109 insertions(+), 52 deletions(-) diff --git a/examples/onoff_light/src/main.rs b/examples/onoff_light/src/main.rs index ad36f07..39de6e4 100644 --- a/examples/onoff_light/src/main.rs +++ b/examples/onoff_light/src/main.rs @@ -19,17 +19,15 @@ mod dev_att; use matter::core::{self, CommissioningData}; use matter::data_model::cluster_basic_information::BasicInfoConfig; use matter::data_model::device_types::device_type_add_on_off_light; -use rand::prelude::*; +use matter::secure_channel::spake2p::VerifierData; fn main() { env_logger::init(); - let mut comm_data = CommissioningData { + let comm_data = CommissioningData { // TODO: Hard-coded for now - passwd: 123456, + verifier: VerifierData::new_with_pw(123456), discriminator: 250, - ..Default::default() }; - rand::thread_rng().fill_bytes(&mut comm_data.salt); // vid/pid should match those in the DAC let dev_info = BasicInfoConfig { diff --git a/matter/src/core.rs b/matter/src/core.rs index 088b028..0421977 100644 --- a/matter/src/core.rs +++ b/matter/src/core.rs @@ -25,19 +25,15 @@ use crate::{ fabric::FabricMgr, interaction_model::InteractionModel, mdns::Mdns, - secure_channel::core::SecureChannel, + secure_channel::{core::SecureChannel, spake2p::VerifierData}, transport, }; use std::sync::Arc; -#[derive(Default)] /// Device Commissioning Data pub struct CommissioningData { - /// The commissioning salt - pub salt: [u8; 16], - /// The password for commissioning the device - // TODO: We should replace this with verifier instead of password - pub passwd: u32, + /// The data like password or verifier that is required to authenticate + pub verifier: VerifierData, /// The 12-bit discriminator used to differentiate between multiple devices pub discriminator: u16, } @@ -78,11 +74,7 @@ impl Matter { matter.transport_mgr.register_protocol(interaction_model)?; let mut secure_channel = Box::new(SecureChannel::new(matter.fabric_mgr.clone())); if open_comm_window { - secure_channel.open_comm_window( - &dev_comm.salt, - dev_comm.passwd, - dev_comm.discriminator, - )?; + secure_channel.open_comm_window(dev_comm.verifier, dev_comm.discriminator)?; } matter.transport_mgr.register_protocol(secure_channel)?; diff --git a/matter/src/lib.rs b/matter/src/lib.rs index ad677b5..08f2c19 100644 --- a/matter/src/lib.rs +++ b/matter/src/lib.rs @@ -27,7 +27,7 @@ //! use matter::{Matter, CommissioningData}; //! use matter::data_model::device_types::device_type_add_on_off_light; //! use matter::data_model::cluster_basic_information::BasicInfoConfig; -//! use rand::prelude::*; +//! use matter::secure_channel::spake2p::VerifierData; //! //! # use matter::data_model::sdm::dev_att::{DataType, DevAttDataFetcher}; //! # use matter::error::Error; @@ -38,12 +38,11 @@ //! # let dev_att = Box::new(DevAtt{}); //! //! /// The commissioning data for this device -//! let mut comm_data = CommissioningData { -//! passwd: 123456, +//! let comm_data = CommissioningData { +//! verifier: VerifierData::new_with_pw(123456), //! discriminator: 250, -//! ..Default::default() +//! //! }; -//! rand::thread_rng().fill_bytes(&mut comm_data.salt); //! //! /// The basic information about this device //! let dev_info = BasicInfoConfig { diff --git a/matter/src/secure_channel/core.rs b/matter/src/secure_channel/core.rs index f855802..d4355f3 100644 --- a/matter/src/secure_channel/core.rs +++ b/matter/src/secure_channel/core.rs @@ -30,7 +30,7 @@ use log::{error, info}; use num; use rand::prelude::*; -use super::case::Case; +use super::{case::Case, spake2p::VerifierData}; /* Handle messages related to the Secure Channel */ @@ -50,15 +50,14 @@ impl SecureChannel { pub fn open_comm_window( &mut self, - salt: &[u8; 16], - passwd: u32, + verifier: VerifierData, discriminator: u16, ) -> Result<(), Error> { let name: u64 = rand::thread_rng().gen_range(0..0xFFFFFFFFFFFFFFFF); let name = format!("{:016X}", name); let mdns = Mdns::get()? .publish_service(&name, mdns::ServiceMode::Commissionable(discriminator))?; - self.pake = Some((PAKE::new(salt, passwd), mdns)); + self.pake = Some((PAKE::new(verifier), mdns)); Ok(()) } diff --git a/matter/src/secure_channel/crypto.rs b/matter/src/secure_channel/crypto.rs index 68dac8d..f9481ba 100644 --- a/matter/src/secure_channel/crypto.rs +++ b/matter/src/secure_channel/crypto.rs @@ -38,7 +38,9 @@ pub trait CryptoSpake2 { fn set_w1(&mut self, w1: &[u8]) -> Result<(), Error>; #[allow(non_snake_case)] - fn set_L(&mut self, w1s: &[u8]) -> Result<(), Error>; + fn set_L(&mut self, l: &[u8]) -> Result<(), Error>; + #[allow(non_snake_case)] + fn set_L_from_w1s(&mut self, w1s: &[u8]) -> Result<(), Error>; #[allow(non_snake_case)] fn get_pB(&mut self, pB: &mut [u8]) -> Result<(), Error>; #[allow(non_snake_case)] diff --git a/matter/src/secure_channel/crypto_mbedtls.rs b/matter/src/secure_channel/crypto_mbedtls.rs index 57eeff2..7231f63 100644 --- a/matter/src/secure_channel/crypto_mbedtls.rs +++ b/matter/src/secure_channel/crypto_mbedtls.rs @@ -114,9 +114,14 @@ impl CryptoSpake2 for CryptoMbedTLS { Ok(()) } + fn set_L(&mut self, l: &[u8]) -> Result<(), Error> { + self.L = EcPoint::from_binary(&mut self.group, l)?; + Ok(()) + } + #[allow(non_snake_case)] #[allow(dead_code)] - fn set_L(&mut self, w1s: &[u8]) -> Result<(), Error> { + fn set_L_from_w1s(&mut self, w1s: &[u8]) -> Result<(), Error> { // From the Matter spec, // L = w1 * P // where P is the generator of the underlying elliptic curve diff --git a/matter/src/secure_channel/crypto_openssl.rs b/matter/src/secure_channel/crypto_openssl.rs index 0f80f4c..84d6793 100644 --- a/matter/src/secure_channel/crypto_openssl.rs +++ b/matter/src/secure_channel/crypto_openssl.rs @@ -117,9 +117,14 @@ impl CryptoSpake2 for CryptoOpenSSL { Ok(()) } + fn set_L(&mut self, l: &[u8]) -> Result<(), Error> { + self.L = EcPoint::from_bytes(&self.group, l, &mut self.bn_ctx)?; + Ok(()) + } + #[allow(non_snake_case)] #[allow(dead_code)] - fn set_L(&mut self, w1s: &[u8]) -> Result<(), Error> { + fn set_L_from_w1s(&mut self, w1s: &[u8]) -> Result<(), Error> { // From the Matter spec, // L = w1 * P // where P is the generator of the underlying elliptic curve diff --git a/matter/src/secure_channel/pake.rs b/matter/src/secure_channel/pake.rs index bb6503d..3655c50 100644 --- a/matter/src/secure_channel/pake.rs +++ b/matter/src/secure_channel/pake.rs @@ -19,12 +19,11 @@ use std::time::{Duration, SystemTime}; use super::{ common::{create_sc_status_report, SCStatusCodes}, - spake2p::Spake2P, + spake2p::{Spake2P, VerifierData}, }; use crate::{ crypto, error::Error, - sys::SPAKE2_ITERATION_COUNT, tlv::{self, get_root_node_struct, FromTLV, OctetStr, TLVElement, TLVWriter, TagType, ToTLV}, transport::{ exchange::ExchangeCtx, @@ -111,20 +110,17 @@ impl Default for PakeState { } } -#[derive(Default)] pub struct PAKE { - salt: [u8; 16], - passwd: u32, + verifier: VerifierData, state: PakeState, } impl PAKE { - pub fn new(salt: &[u8; 16], passwd: u32) -> Self { + pub fn new(verifier: VerifierData) -> Self { // TODO: Can any PBKDF2 calculation be pre-computed here PAKE { - passwd, - salt: *salt, - ..Default::default() + verifier, + state: Default::default(), } } @@ -176,8 +172,7 @@ impl PAKE { let pA = extract_pasepake_1_or_3_params(ctx.rx.as_borrow_slice())?; let mut pB: [u8; 65] = [0; 65]; let mut cB: [u8; 32] = [0; 32]; - sd.spake2p - .start_verifier(self.passwd, SPAKE2_ITERATION_COUNT, &self.salt)?; + sd.spake2p.start_verifier(&self.verifier)?; sd.spake2p.handle_pA(pA, &mut pB, &mut cB)?; let mut tw = TLVWriter::new(ctx.tx.get_writebuf()?); @@ -231,8 +226,8 @@ impl PAKE { }; if !a.has_params { let params_resp = PBKDFParamRespParams { - count: SPAKE2_ITERATION_COUNT, - salt: OctetStr(&self.salt), + count: self.verifier.count, + salt: OctetStr(&self.verifier.salt), }; resp.params = Some(params_resp); } diff --git a/matter/src/secure_channel/spake2p.rs b/matter/src/secure_channel/spake2p.rs index 3870bcd..5e82f9f 100644 --- a/matter/src/secure_channel/spake2p.rs +++ b/matter/src/secure_channel/spake2p.rs @@ -15,8 +15,13 @@ * limitations under the License. */ -use crate::crypto::{self, HmacSha256}; +use crate::{ + crypto::{self, HmacSha256}, + sys, +}; use byteorder::{ByteOrder, LittleEndian}; +use log::error; +use rand::prelude::*; use subtle::ConstantTimeEq; use crate::{ @@ -74,6 +79,10 @@ const SPAKE2P_KEY_CONFIRM_INFO: [u8; 16] = *b"ConfirmationKeys"; const SPAKE2P_CONTEXT_PREFIX: [u8; 26] = *b"CHIP PAKE V1 Commissioning"; const CRYPTO_GROUP_SIZE_BYTES: usize = 32; const CRYPTO_W_SIZE_BYTES: usize = CRYPTO_GROUP_SIZE_BYTES + 8; +const CRYPTO_PUBLIC_KEY_SIZE_BYTES: usize = (2 * CRYPTO_GROUP_SIZE_BYTES) + 1; + +const MAX_SALT_SIZE_BYTES: usize = 32; +const VERIFIER_SIZE_BYTES: usize = CRYPTO_W_SIZE_BYTES + CRYPTO_PUBLIC_KEY_SIZE_BYTES; #[cfg(feature = "crypto_openssl")] fn crypto_spake2_new() -> Result, Error> { @@ -96,6 +105,45 @@ impl Default for Spake2P { } } +pub struct VerifierData { + pub data: VerifierOption, + // For the VerifierOption::Verifier, the following fields only serve + // information purposes + pub salt: [u8; MAX_SALT_SIZE_BYTES], + pub count: u32, +} + +pub enum VerifierOption { + /// With Password + Password(u32), + /// With Verifier + Verifier([u8; VERIFIER_SIZE_BYTES]), +} + +impl VerifierData { + pub fn new_with_pw(pw: u32) -> Self { + let mut s = Self { + salt: [0; MAX_SALT_SIZE_BYTES], + count: sys::SPAKE2_ITERATION_COUNT, + data: VerifierOption::Password(pw), + }; + rand::thread_rng().fill_bytes(&mut s.salt); + s + } + + pub fn new( + verifier: [u8; VERIFIER_SIZE_BYTES], + count: u32, + salt: [u8; MAX_SALT_SIZE_BYTES], + ) -> Self { + Self { + data: VerifierOption::Verifier(verifier), + count, + salt, + } + } +} + impl Spake2P { pub fn new() -> Self { Spake2P { @@ -132,17 +180,31 @@ impl Spake2P { let _ = pbkdf2_hmac(&pw_str, iter as usize, salt, w0w1s); } - pub fn start_verifier(&mut self, pw: u32, iter: u32, salt: &[u8]) -> Result<(), Error> { - let mut w0w1s: [u8; (2 * CRYPTO_W_SIZE_BYTES)] = [0; (2 * CRYPTO_W_SIZE_BYTES)]; - Spake2P::get_w0w1s(pw, iter, salt, &mut w0w1s); - self.crypto_spake2 = Some(crypto_spake2_new()?); + pub fn start_verifier(&mut self, verifier: &VerifierData) -> Result<(), Error> { + match verifier.data { + VerifierOption::Password(pw) => { + // Derive w0 and L from the password + let mut w0w1s: [u8; (2 * CRYPTO_W_SIZE_BYTES)] = [0; (2 * CRYPTO_W_SIZE_BYTES)]; + Spake2P::get_w0w1s(pw, verifier.count, &verifier.salt, &mut w0w1s); + self.crypto_spake2 = Some(crypto_spake2_new()?); - let w0s_len = w0w1s.len() / 2; - if let Some(crypto_spake2) = &mut self.crypto_spake2 { - crypto_spake2.set_w0_from_w0s(&w0w1s[0..w0s_len])?; - crypto_spake2.set_L(&w0w1s[w0s_len..])?; + let w0s_len = w0w1s.len() / 2; + if let Some(crypto_spake2) = &mut self.crypto_spake2 { + crypto_spake2.set_w0_from_w0s(&w0w1s[0..w0s_len])?; + crypto_spake2.set_L_from_w1s(&w0w1s[w0s_len..])?; + } + } + VerifierOption::Verifier(v) => { + // Extract w0 and L from the verifier + if v.len() != CRYPTO_GROUP_SIZE_BYTES + CRYPTO_PUBLIC_KEY_SIZE_BYTES { + error!("Verifier of invalid length"); + } + if let Some(crypto_spake2) = &mut self.crypto_spake2 { + crypto_spake2.set_w0(&v[0..CRYPTO_GROUP_SIZE_BYTES])?; + crypto_spake2.set_L(&v[CRYPTO_GROUP_SIZE_BYTES..])?; + } + } } - self.mode = Spake2Mode::Verifier(Spake2VerifierState::Init); Ok(()) } From 5c7e13c649ad01f5f9bdf9853f6b40dc09acee1a Mon Sep 17 00:00:00 2001 From: Kedar Sovani Date: Tue, 10 Jan 2023 10:48:20 +0530 Subject: [PATCH 05/12] pase: Split out a separate Pase Mgr - This is required so that the AdminCommissioning Cluster has a reference to the Pase Mgr - The Cluster can then open commissioning window accordingly - Right now, for the error paths in the PASE/CASE sessions, I have set ResponseRequired to No, but I am not quite sure if this is the expected behaviour. Need to check --- matter/src/core.rs | 8 ++- matter/src/secure_channel/case.rs | 24 +++---- matter/src/secure_channel/core.rs | 100 ++++-------------------------- matter/src/secure_channel/pake.rs | 83 ++++++++++++++++++++++++- 4 files changed, 112 insertions(+), 103 deletions(-) diff --git a/matter/src/core.rs b/matter/src/core.rs index 0421977..ce31278 100644 --- a/matter/src/core.rs +++ b/matter/src/core.rs @@ -25,7 +25,7 @@ use crate::{ fabric::FabricMgr, interaction_model::InteractionModel, mdns::Mdns, - secure_channel::{core::SecureChannel, spake2p::VerifierData}, + secure_channel::{core::SecureChannel, pake::PaseMgr, spake2p::VerifierData}, transport, }; use std::sync::Arc; @@ -72,11 +72,13 @@ impl Matter { let interaction_model = Box::new(InteractionModel::new(Box::new(matter.data_model.clone()))); matter.transport_mgr.register_protocol(interaction_model)?; - let mut secure_channel = Box::new(SecureChannel::new(matter.fabric_mgr.clone())); + + let mut pase = PaseMgr::new(); if open_comm_window { - secure_channel.open_comm_window(dev_comm.verifier, dev_comm.discriminator)?; + pase.enable_pase_session(dev_comm.verifier, dev_comm.discriminator)?; } + let secure_channel = Box::new(SecureChannel::new(pase.clone(), matter.fabric_mgr.clone())); matter.transport_mgr.register_protocol(secure_channel)?; Ok(matter) } diff --git a/matter/src/secure_channel/case.rs b/matter/src/secure_channel/case.rs index 75d1fc9..33c8e47 100644 --- a/matter/src/secure_channel/case.rs +++ b/matter/src/secure_channel/case.rs @@ -26,12 +26,12 @@ use crate::{ crypto::{self, CryptoKeyPair, KeyPair, Sha256}, error::Error, fabric::{Fabric, FabricMgr, FabricMgrInner}, - secure_channel::common, secure_channel::common::SCStatusCodes, + secure_channel::common::{self, OpCode}, tlv::{get_root_node_struct, FromTLV, OctetStr, TLVElement, TLVWriter, TagType}, transport::{ network::Address, - proto_demux::ProtoCtx, + proto_demux::{ProtoCtx, ResponseRequired}, queue::{Msg, WorkQ}, session::{CloneData, SessionMode}, }, @@ -78,7 +78,7 @@ impl Case { Self { fabric_mgr } } - pub fn handle_casesigma3(&mut self, ctx: &mut ProtoCtx) -> Result<(), Error> { + pub fn casesigma3_handler(&mut self, ctx: &mut ProtoCtx) -> Result { let mut case_session = ctx .exch_ctx .exch @@ -97,7 +97,7 @@ impl Case { None, )?; ctx.exch_ctx.exch.close(); - return Ok(()); + return Ok(ResponseRequired::Yes); } // Safe to unwrap here let fabric = fabric.as_ref().as_ref().unwrap(); @@ -132,7 +132,7 @@ impl Case { None, )?; ctx.exch_ctx.exch.close(); - return Ok(()); + return Ok(ResponseRequired::Yes); } if Case::validate_sigma3_sign( @@ -151,7 +151,7 @@ impl Case { None, )?; ctx.exch_ctx.exch.close(); - return Ok(()); + return Ok(ResponseRequired::Yes); } // Only now do we add this message to the TT Hash @@ -174,10 +174,12 @@ impl Case { ctx.exch_ctx.exch.clear_data_boxed(); ctx.exch_ctx.exch.close(); - Ok(()) + Ok(ResponseRequired::Yes) } - pub fn handle_casesigma1(&mut self, ctx: &mut ProtoCtx) -> Result<(), Error> { + pub fn casesigma1_handler(&mut self, ctx: &mut ProtoCtx) -> Result { + ctx.tx.set_proto_opcode(OpCode::CASESigma2 as u8); + let rx_buf = ctx.rx.as_borrow_slice(); let root = get_root_node_struct(rx_buf)?; let r = Sigma1Req::from_tlv(&root)?; @@ -193,7 +195,7 @@ impl Case { None, )?; ctx.exch_ctx.exch.close(); - return Ok(()); + return Ok(ResponseRequired::Yes); } let local_sessid = ctx.exch_ctx.sess.reserve_new_sess_id(); @@ -239,7 +241,7 @@ impl Case { None, )?; ctx.exch_ctx.exch.close(); - return Ok(()); + return Ok(ResponseRequired::Yes); } let sign_len = Case::get_sigma2_sign( @@ -270,7 +272,7 @@ impl Case { tw.end_container()?; case_session.tt_hash.update(ctx.tx.as_borrow_slice())?; ctx.exch_ctx.exch.set_data_boxed(case_session); - Ok(()) + Ok(ResponseRequired::Yes) } fn get_session_clone_data( diff --git a/matter/src/secure_channel/core.rs b/matter/src/secure_channel/core.rs index d4355f3..9f7d16b 100644 --- a/matter/src/secure_channel/core.rs +++ b/matter/src/secure_channel/core.rs @@ -20,105 +20,30 @@ use std::sync::Arc; use crate::{ error::*, fabric::FabricMgr, - mdns::{self, Mdns}, - secure_channel::{common::*, pake::PAKE}, - sys::SysMdnsService, + secure_channel::common::*, tlv, transport::proto_demux::{self, ProtoCtx, ResponseRequired}, }; use log::{error, info}; use num; -use rand::prelude::*; -use super::{case::Case, spake2p::VerifierData}; +use super::{case::Case, pake::PaseMgr}; /* Handle messages related to the Secure Channel */ pub struct SecureChannel { case: Case, - pake: Option<(PAKE, SysMdnsService)>, + pase: PaseMgr, } impl SecureChannel { - pub fn new(fabric_mgr: Arc) -> SecureChannel { + pub fn new(pase: PaseMgr, fabric_mgr: Arc) -> SecureChannel { SecureChannel { - pake: None, + pase, case: Case::new(fabric_mgr), } } - - pub fn open_comm_window( - &mut self, - verifier: VerifierData, - discriminator: u16, - ) -> Result<(), Error> { - let name: u64 = rand::thread_rng().gen_range(0..0xFFFFFFFFFFFFFFFF); - let name = format!("{:016X}", name); - let mdns = Mdns::get()? - .publish_service(&name, mdns::ServiceMode::Commissionable(discriminator))?; - self.pake = Some((PAKE::new(verifier), mdns)); - Ok(()) - } - - pub fn close_comm_window(&mut self) { - self.pake = None; - } - - fn mrpstandaloneack_handler(&mut self, _ctx: &mut ProtoCtx) -> Result { - info!("In MRP StandAlone ACK Handler"); - Ok(ResponseRequired::No) - } - - fn pbkdfparamreq_handler(&mut self, ctx: &mut ProtoCtx) -> Result { - info!("In PBKDF Param Request Handler"); - ctx.tx.set_proto_opcode(OpCode::PBKDFParamResponse as u8); - if let Some((pake, _)) = &mut self.pake { - pake.handle_pbkdfparamrequest(ctx)?; - } else { - error!("PASE Not enabled"); - create_sc_status_report(&mut ctx.tx, SCStatusCodes::InvalidParameter, None)?; - } - Ok(ResponseRequired::Yes) - } - - fn pasepake1_handler(&mut self, ctx: &mut ProtoCtx) -> Result { - info!("In PASE Pake1 Handler"); - ctx.tx.set_proto_opcode(OpCode::PASEPake2 as u8); - if let Some((pake, _)) = &mut self.pake { - pake.handle_pasepake1(ctx)?; - } else { - error!("PASE Not enabled"); - create_sc_status_report(&mut ctx.tx, SCStatusCodes::InvalidParameter, None)?; - } - Ok(ResponseRequired::Yes) - } - - fn pasepake3_handler(&mut self, ctx: &mut ProtoCtx) -> Result { - info!("In PASE Pake3 Handler"); - if let Some((pake, _)) = &mut self.pake { - pake.handle_pasepake3(ctx)?; - // TODO: Currently we assume that PAKE is not successful and reset the PAKE object - self.pake = None; - } else { - error!("PASE Not enabled"); - create_sc_status_report(&mut ctx.tx, SCStatusCodes::InvalidParameter, None)?; - } - Ok(ResponseRequired::Yes) - } - - fn casesigma1_handler(&mut self, ctx: &mut ProtoCtx) -> Result { - info!("In CASE Sigma1 Handler"); - ctx.tx.set_proto_opcode(OpCode::CASESigma2 as u8); - self.case.handle_casesigma1(ctx)?; - Ok(ResponseRequired::Yes) - } - - fn casesigma3_handler(&mut self, ctx: &mut ProtoCtx) -> Result { - info!("In CASE Sigma3 Handler"); - self.case.handle_casesigma3(ctx)?; - Ok(ResponseRequired::Yes) - } } impl proto_demux::HandleProto for SecureChannel { @@ -126,15 +51,16 @@ impl proto_demux::HandleProto for SecureChannel { let proto_opcode: OpCode = num::FromPrimitive::from_u8(ctx.rx.get_proto_opcode()).ok_or(Error::Invalid)?; ctx.tx.set_proto_id(PROTO_ID_SECURE_CHANNEL as u16); - info!("Received Data"); + info!("Received Opcode: {:?}", proto_opcode); + info!("Received Data:"); tlv::print_tlv_list(ctx.rx.as_borrow_slice()); let result = match proto_opcode { - OpCode::MRPStandAloneAck => self.mrpstandaloneack_handler(ctx), - OpCode::PBKDFParamRequest => self.pbkdfparamreq_handler(ctx), - OpCode::PASEPake1 => self.pasepake1_handler(ctx), - OpCode::PASEPake3 => self.pasepake3_handler(ctx), - OpCode::CASESigma1 => self.casesigma1_handler(ctx), - OpCode::CASESigma3 => self.casesigma3_handler(ctx), + OpCode::MRPStandAloneAck => Ok(ResponseRequired::No), + OpCode::PBKDFParamRequest => self.pase.pbkdfparamreq_handler(ctx), + OpCode::PASEPake1 => self.pase.pasepake1_handler(ctx), + OpCode::PASEPake3 => self.pase.pasepake3_handler(ctx), + OpCode::CASESigma1 => self.case.casesigma1_handler(ctx), + OpCode::CASESigma3 => self.case.casesigma3_handler(ctx), _ => { error!("OpCode Not Handled: {:?}", proto_opcode); Err(Error::InvalidOpcode) diff --git a/matter/src/secure_channel/pake.rs b/matter/src/secure_channel/pake.rs index 3655c50..99c0063 100644 --- a/matter/src/secure_channel/pake.rs +++ b/matter/src/secure_channel/pake.rs @@ -15,7 +15,10 @@ * limitations under the License. */ -use std::time::{Duration, SystemTime}; +use std::{ + sync::{Arc, Mutex}, + time::{Duration, SystemTime}, +}; use super::{ common::{create_sc_status_report, SCStatusCodes}, @@ -24,11 +27,14 @@ use super::{ use crate::{ crypto, error::Error, + mdns::{self, Mdns}, + secure_channel::common::OpCode, + sys::SysMdnsService, tlv::{self, get_root_node_struct, FromTLV, OctetStr, TLVElement, TLVWriter, TagType, ToTLV}, transport::{ exchange::ExchangeCtx, network::Address, - proto_demux::ProtoCtx, + proto_demux::{ProtoCtx, ResponseRequired}, queue::{Msg, WorkQ}, session::{CloneData, SessionMode}, }, @@ -36,6 +42,79 @@ use crate::{ use log::{error, info}; use rand::prelude::*; +enum PaseSessionState { + Enabled(PAKE, SysMdnsService), + Disabled, +} + +pub struct PaseMgrInternal { + state: PaseSessionState, +} + +#[derive(Clone)] +// Could this lock be avoided? +pub struct PaseMgr(Arc>); + +impl PaseMgr { + pub fn new() -> Self { + Self(Arc::new(Mutex::new(PaseMgrInternal { + state: PaseSessionState::Disabled, + }))) + } + + pub fn enable_pase_session( + &mut self, + verifier: VerifierData, + discriminator: u16, + ) -> Result<(), Error> { + let mut s = self.0.lock().unwrap(); + let name: u64 = rand::thread_rng().gen_range(0..0xFFFFFFFFFFFFFFFF); + let name = format!("{:016X}", name); + let mdns = Mdns::get()? + .publish_service(&name, mdns::ServiceMode::Commissionable(discriminator))?; + s.state = PaseSessionState::Enabled(PAKE::new(verifier), mdns); + Ok(()) + } + + pub fn disable_pase_session(&mut self) { + let mut s = self.0.lock().unwrap(); + s.state = PaseSessionState::Disabled; + } + + /// If the PASE Session is enabled, execute the closure, + /// if not enabled, generate SC Status Report + fn if_enabled(&mut self, ctx: &mut ProtoCtx, f: F) -> Result<(), Error> + where + F: FnOnce(&mut PAKE, &mut ProtoCtx) -> Result<(), Error>, + { + let mut s = self.0.lock().unwrap(); + if let PaseSessionState::Enabled(pake, _) = &mut s.state { + f(pake, ctx) + } else { + error!("PASE Not enabled"); + create_sc_status_report(&mut ctx.tx, SCStatusCodes::InvalidParameter, None) + } + } + + pub fn pbkdfparamreq_handler(&mut self, ctx: &mut ProtoCtx) -> Result { + ctx.tx.set_proto_opcode(OpCode::PBKDFParamResponse as u8); + self.if_enabled(ctx, |pake, ctx| pake.handle_pbkdfparamrequest(ctx))?; + Ok(ResponseRequired::Yes) + } + + pub fn pasepake1_handler(&mut self, ctx: &mut ProtoCtx) -> Result { + ctx.tx.set_proto_opcode(OpCode::PASEPake2 as u8); + self.if_enabled(ctx, |pake, ctx| pake.handle_pasepake1(ctx))?; + Ok(ResponseRequired::Yes) + } + + pub fn pasepake3_handler(&mut self, ctx: &mut ProtoCtx) -> Result { + self.if_enabled(ctx, |pake, ctx| pake.handle_pasepake3(ctx))?; + self.disable_pase_session(); + Ok(ResponseRequired::Yes) + } +} + // This file basically deals with the handlers for the PASE secure channel protocol // TLV extraction and encoding is done in this file. // We create a Spake2p object and set it up in the exchange-data. This object then From 8d4f91b3eb5cfe8e8296bb545b1ba6d0e118f128 Mon Sep 17 00:00:00 2001 From: Kedar Sovani Date: Tue, 10 Jan 2023 18:37:28 +0530 Subject: [PATCH 06/12] mdns: Fix incorrect mask for short discriminator --- matter/src/mdns.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matter/src/mdns.rs b/matter/src/mdns.rs index 6a971e4..4995bb2 100644 --- a/matter/src/mdns.rs +++ b/matter/src/mdns.rs @@ -36,7 +36,7 @@ pub struct Mdns { inner: Mutex, } -const SHORT_DISCRIMINATOR_MASK: u16 = 0x700; +const SHORT_DISCRIMINATOR_MASK: u16 = 0xf00; const SHORT_DISCRIMINATOR_SHIFT: u16 = 8; static mut G_MDNS: Option> = None; From 471c264a097d9cffda1895b82e68bc3df608dcc7 Mon Sep 17 00:00:00 2001 From: Kedar Sovani Date: Tue, 10 Jan 2023 18:39:14 +0530 Subject: [PATCH 07/12] spake2p: Fix issues - Verifier length was incorrect - spake2 was not being initialised in the 'Verifier' Option - change method for new() --- matter/src/secure_channel/spake2p.rs | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/matter/src/secure_channel/spake2p.rs b/matter/src/secure_channel/spake2p.rs index 5e82f9f..ad0cac5 100644 --- a/matter/src/secure_channel/spake2p.rs +++ b/matter/src/secure_channel/spake2p.rs @@ -82,7 +82,7 @@ const CRYPTO_W_SIZE_BYTES: usize = CRYPTO_GROUP_SIZE_BYTES + 8; const CRYPTO_PUBLIC_KEY_SIZE_BYTES: usize = (2 * CRYPTO_GROUP_SIZE_BYTES) + 1; const MAX_SALT_SIZE_BYTES: usize = 32; -const VERIFIER_SIZE_BYTES: usize = CRYPTO_W_SIZE_BYTES + CRYPTO_PUBLIC_KEY_SIZE_BYTES; +const VERIFIER_SIZE_BYTES: usize = CRYPTO_GROUP_SIZE_BYTES + CRYPTO_PUBLIC_KEY_SIZE_BYTES; #[cfg(feature = "crypto_openssl")] fn crypto_spake2_new() -> Result, Error> { @@ -131,15 +131,20 @@ impl VerifierData { s } - pub fn new( - verifier: [u8; VERIFIER_SIZE_BYTES], - count: u32, - salt: [u8; MAX_SALT_SIZE_BYTES], - ) -> Self { + pub fn new(verifier: &[u8], count: u32, salt: &[u8]) -> Self { + let mut v = [0_u8; VERIFIER_SIZE_BYTES]; + let mut s = [0_u8; MAX_SALT_SIZE_BYTES]; + + let slice = &mut v[..verifier.len()]; + slice.copy_from_slice(verifier); + + let slice = &mut s[..salt.len()]; + slice.copy_from_slice(salt); + Self { - data: VerifierOption::Verifier(verifier), + data: VerifierOption::Verifier(v), count, - salt, + salt: s, } } } @@ -181,12 +186,12 @@ impl Spake2P { } pub fn start_verifier(&mut self, verifier: &VerifierData) -> Result<(), Error> { + self.crypto_spake2 = Some(crypto_spake2_new()?); match verifier.data { VerifierOption::Password(pw) => { // Derive w0 and L from the password let mut w0w1s: [u8; (2 * CRYPTO_W_SIZE_BYTES)] = [0; (2 * CRYPTO_W_SIZE_BYTES)]; Spake2P::get_w0w1s(pw, verifier.count, &verifier.salt, &mut w0w1s); - self.crypto_spake2 = Some(crypto_spake2_new()?); let w0s_len = w0w1s.len() / 2; if let Some(crypto_spake2) = &mut self.crypto_spake2 { @@ -226,6 +231,7 @@ impl Spake2P { Spake2P::get_Ke_and_cAcB(&TT, pA, pB, &mut self.Ke, &mut self.cA, cB)?; } } + // We are finished with using the crypto_spake2 now self.crypto_spake2 = None; self.mode = Spake2Mode::Verifier(Spake2VerifierState::PendingConfirmation); From 40b6b0c5052af7428e06ddfbe81e3703ecb81331 Mon Sep 17 00:00:00 2001 From: Kedar Sovani Date: Tue, 10 Jan 2023 18:40:52 +0530 Subject: [PATCH 08/12] admin-commissioning: Link PaseMgr with Admin Commissioning Adding the second commissioner now works successfully through iOS! Yay! --- matter/src/core.rs | 7 ++-- matter/src/data_model/core.rs | 11 +++++- matter/src/data_model/device_types.rs | 4 +- .../src/data_model/sdm/admin_commissioning.rs | 37 +++++++++---------- matter/src/secure_channel/pake.rs | 14 +++---- matter/tests/common/im_engine.rs | 11 +++++- 6 files changed, 51 insertions(+), 33 deletions(-) diff --git a/matter/src/core.rs b/matter/src/core.rs index ce31278..7d779a4 100644 --- a/matter/src/core.rs +++ b/matter/src/core.rs @@ -62,8 +62,10 @@ impl Matter { let fabric_mgr = Arc::new(FabricMgr::new()?); let acl_mgr = Arc::new(AclMgr::new()?); + let mut pase = PaseMgr::new(); let open_comm_window = fabric_mgr.is_empty(); - let data_model = DataModel::new(dev_det, dev_att, fabric_mgr.clone(), acl_mgr)?; + let data_model = + DataModel::new(dev_det, dev_att, fabric_mgr.clone(), acl_mgr, pase.clone())?; let mut matter = Box::new(Matter { transport_mgr: transport::mgr::Mgr::new()?, data_model, @@ -73,12 +75,11 @@ impl Matter { Box::new(InteractionModel::new(Box::new(matter.data_model.clone()))); matter.transport_mgr.register_protocol(interaction_model)?; - let mut pase = PaseMgr::new(); if open_comm_window { pase.enable_pase_session(dev_comm.verifier, dev_comm.discriminator)?; } - let secure_channel = Box::new(SecureChannel::new(pase.clone(), matter.fabric_mgr.clone())); + let secure_channel = Box::new(SecureChannel::new(pase, matter.fabric_mgr.clone())); matter.transport_mgr.register_protocol(secure_channel)?; Ok(matter) } diff --git a/matter/src/data_model/core.rs b/matter/src/data_model/core.rs index f269cdb..11bf060 100644 --- a/matter/src/data_model/core.rs +++ b/matter/src/data_model/core.rs @@ -36,6 +36,7 @@ use crate::{ }, InteractionConsumer, Transaction, }, + secure_channel::pake::PaseMgr, tlv::{TLVArray, TLVWriter, TagType, ToTLV}, transport::session::{Session, SessionMode}, }; @@ -54,6 +55,7 @@ impl DataModel { dev_att: Box, fabric_mgr: Arc, acl_mgr: Arc, + pase_mgr: PaseMgr, ) -> Result { let dm = DataModel { node: Arc::new(RwLock::new(Node::new()?)), @@ -62,7 +64,14 @@ impl DataModel { { let mut node = dm.node.write()?; node.set_changes_cb(Box::new(dm.clone())); - device_type_add_root_node(&mut node, dev_details, dev_att, fabric_mgr, acl_mgr)?; + device_type_add_root_node( + &mut node, + dev_details, + dev_att, + fabric_mgr, + acl_mgr, + pase_mgr, + )?; } Ok(dm) } diff --git a/matter/src/data_model/device_types.rs b/matter/src/data_model/device_types.rs index e1c74ec..a0c9c28 100644 --- a/matter/src/data_model/device_types.rs +++ b/matter/src/data_model/device_types.rs @@ -28,6 +28,7 @@ use super::system_model::access_control::AccessControlCluster; use crate::acl::AclMgr; use crate::error::*; use crate::fabric::FabricMgr; +use crate::secure_channel::pake::PaseMgr; use std::sync::Arc; use std::sync::RwLockWriteGuard; @@ -39,6 +40,7 @@ pub fn device_type_add_root_node( dev_att: Box, fabric_mgr: Arc, acl_mgr: Arc, + pase_mgr: PaseMgr, ) -> Result { // Add the root endpoint let endpoint = node.add_endpoint()?; @@ -52,7 +54,7 @@ pub fn device_type_add_root_node( let failsafe = general_commissioning.failsafe(); node.add_cluster(0, general_commissioning)?; node.add_cluster(0, NwCommCluster::new()?)?; - node.add_cluster(0, AdminCommCluster::new()?)?; + node.add_cluster(0, AdminCommCluster::new(pase_mgr)?)?; node.add_cluster( 0, NocCluster::new(dev_att, fabric_mgr, acl_mgr.clone(), failsafe)?, diff --git a/matter/src/data_model/sdm/admin_commissioning.rs b/matter/src/data_model/sdm/admin_commissioning.rs index 6e7d7b7..af8149b 100644 --- a/matter/src/data_model/sdm/admin_commissioning.rs +++ b/matter/src/data_model/sdm/admin_commissioning.rs @@ -18,6 +18,8 @@ use crate::cmd_enter; use crate::data_model::objects::*; use crate::interaction_model::core::IMStatusCode; +use crate::secure_channel::pake::PaseMgr; +use crate::secure_channel::spake2p::VerifierData; use crate::tlv::{FromTLV, Nullable, OctetStr, TLVElement}; use crate::{error::*, interaction_model::command::CommandReq}; use log::{error, info}; @@ -74,7 +76,7 @@ fn attr_admin_vid_new() -> Result { } pub struct AdminCommCluster { - window_status: WindowStatus, + pase_mgr: PaseMgr, base: Cluster, } @@ -89,23 +91,16 @@ impl ClusterType for AdminCommCluster { fn read_custom_attribute(&self, encoder: &mut dyn Encoder, attr: &AttrDetails) { match num::FromPrimitive::from_u16(attr.attr_id) { Some(Attributes::WindowStatus) => { - let status = self.window_status as u8; + let status = 1_u8; encoder.encode(EncodeValue::Value(&status)) } Some(Attributes::AdminVendorId) => { - let vid = if self.window_status == WindowStatus::WindowNotOpen { - Nullable::Null - } else { - Nullable::NotNull(1_u8) - }; + let vid = Nullable::NotNull(1_u8); + encoder.encode(EncodeValue::Value(&vid)) } Some(Attributes::AdminFabricIndex) => { - let vid = if self.window_status == WindowStatus::WindowNotOpen { - Nullable::Null - } else { - Nullable::NotNull(1_u8) - }; + let vid = Nullable::NotNull(1_u8); encoder.encode(EncodeValue::Value(&vid)) } _ => { @@ -129,9 +124,9 @@ impl ClusterType for AdminCommCluster { } impl AdminCommCluster { - pub fn new() -> Result, Error> { + pub fn new(pase_mgr: PaseMgr) -> Result, Error> { let mut c = Box::new(AdminCommCluster { - window_status: WindowStatus::WindowNotOpen, + pase_mgr, base: Cluster::new(ID)?, }); c.base.add_attribute(attr_window_status_new()?)?; @@ -145,9 +140,11 @@ impl AdminCommCluster { cmd_req: &mut CommandReq, ) -> Result<(), IMStatusCode> { cmd_enter!("Open Commissioning Window"); - let _req = + let req = OpenCommWindowReq::from_tlv(&cmd_req.data).map_err(|_| IMStatusCode::InvalidCommand)?; - self.window_status = WindowStatus::EnhancedWindowOpen; + let verifier = VerifierData::new(req.verifier.0, req.iterations, req.salt.0); + self.pase_mgr + .enable_pase_session(verifier, req.discriminator)?; Err(IMStatusCode::Sucess) } } @@ -156,8 +153,8 @@ impl AdminCommCluster { #[tlvargs(lifetime = "'a")] pub struct OpenCommWindowReq<'a> { _timeout: u16, - _verifier: OctetStr<'a>, - _discriminator: u16, - _iterations: u32, - _salt: OctetStr<'a>, + verifier: OctetStr<'a>, + discriminator: u16, + iterations: u32, + salt: OctetStr<'a>, } diff --git a/matter/src/secure_channel/pake.rs b/matter/src/secure_channel/pake.rs index 99c0063..b096cf6 100644 --- a/matter/src/secure_channel/pake.rs +++ b/matter/src/secure_channel/pake.rs @@ -42,13 +42,13 @@ use crate::{ use log::{error, info}; use rand::prelude::*; -enum PaseSessionState { +enum PaseMgrState { Enabled(PAKE, SysMdnsService), Disabled, } pub struct PaseMgrInternal { - state: PaseSessionState, + state: PaseMgrState, } #[derive(Clone)] @@ -58,7 +58,7 @@ pub struct PaseMgr(Arc>); impl PaseMgr { pub fn new() -> Self { Self(Arc::new(Mutex::new(PaseMgrInternal { - state: PaseSessionState::Disabled, + state: PaseMgrState::Disabled, }))) } @@ -72,13 +72,13 @@ impl PaseMgr { let name = format!("{:016X}", name); let mdns = Mdns::get()? .publish_service(&name, mdns::ServiceMode::Commissionable(discriminator))?; - s.state = PaseSessionState::Enabled(PAKE::new(verifier), mdns); + s.state = PaseMgrState::Enabled(PAKE::new(verifier), mdns); Ok(()) } pub fn disable_pase_session(&mut self) { let mut s = self.0.lock().unwrap(); - s.state = PaseSessionState::Disabled; + s.state = PaseMgrState::Disabled; } /// If the PASE Session is enabled, execute the closure, @@ -88,7 +88,7 @@ impl PaseMgr { F: FnOnce(&mut PAKE, &mut ProtoCtx) -> Result<(), Error>, { let mut s = self.0.lock().unwrap(); - if let PaseSessionState::Enabled(pake, _) = &mut s.state { + if let PaseMgrState::Enabled(pake, _) = &mut s.state { f(pake, ctx) } else { error!("PASE Not enabled"); @@ -190,7 +190,7 @@ impl Default for PakeState { } pub struct PAKE { - verifier: VerifierData, + pub verifier: VerifierData, state: PakeState, } diff --git a/matter/tests/common/im_engine.rs b/matter/tests/common/im_engine.rs index 81c2c74..feddb96 100644 --- a/matter/tests/common/im_engine.rs +++ b/matter/tests/common/im_engine.rs @@ -29,6 +29,7 @@ use matter::{ error::Error, fabric::FabricMgr, interaction_model::{core::OpCode, InteractionModel}, + secure_channel::pake::PaseMgr, tlv::{TLVWriter, TagType, ToTLV}, transport::packet::Packet, transport::proto_demux::HandleProto, @@ -97,12 +98,20 @@ impl ImEngine { let dev_att = Box::new(DummyDevAtt {}); let fabric_mgr = Arc::new(FabricMgr::new().unwrap()); let acl_mgr = Arc::new(AclMgr::new_with(false).unwrap()); + let pase_mgr = PaseMgr::new(); acl_mgr.erase_all(); let mut default_acl = AclEntry::new(1, Privilege::ADMIN, AuthMode::Case); // Only allow the standard peer node id of the IM Engine default_acl.add_subject(IM_ENGINE_PEER_ID).unwrap(); acl_mgr.add(default_acl).unwrap(); - let dm = DataModel::new(&dev_det, dev_att, fabric_mgr, acl_mgr.clone()).unwrap(); + let dm = DataModel::new( + &dev_det, + dev_att, + fabric_mgr.clone(), + acl_mgr.clone(), + pase_mgr, + ) + .unwrap(); { let mut d = dm.node.write().unwrap(); From e6238ee2e6e0a418519d889f24de6f747f395100 Mon Sep 17 00:00:00 2001 From: Kedar Sovani Date: Wed, 11 Jan 2023 13:34:59 +0530 Subject: [PATCH 09/12] noc: Support UpdateFabricLabel command --- matter/src/data_model/sdm/noc.rs | 69 ++++++++++++++++++++++++-------- matter/src/fabric.rs | 52 +++++++++++++++++++++--- matter/src/tlv/traits.rs | 10 +++++ 3 files changed, 108 insertions(+), 23 deletions(-) diff --git a/matter/src/data_model/sdm/noc.rs b/matter/src/data_model/sdm/noc.rs index 82bf61b..c15f607 100644 --- a/matter/src/data_model/sdm/noc.rs +++ b/matter/src/data_model/sdm/noc.rs @@ -27,7 +27,7 @@ use crate::fabric::{Fabric, FabricMgr}; use crate::interaction_model::command::CommandReq; use crate::interaction_model::core::IMStatusCode; use crate::interaction_model::messages::ib; -use crate::tlv::{FromTLV, OctetStr, TLVElement, TLVWriter, TagType, ToTLV}; +use crate::tlv::{FromTLV, OctetStr, TLVElement, TLVWriter, TagType, ToTLV, UtfStr}; use crate::transport::session::SessionMode; use crate::utils::writebuf::WriteBuf; use crate::{cmd_enter, error::*}; @@ -75,6 +75,8 @@ pub enum Commands { CSRResp = 0x05, AddNOC = 0x06, NOCResp = 0x08, + UpdateFabricLabel = 0x09, + RemoveFabric = 0x0a, AddTrustedRootCert = 0x0b, } @@ -183,19 +185,55 @@ impl NocCluster { if self.failsafe.record_add_noc(fab_idx).is_err() { error!("Failed to record NoC in the FailSafe, what to do?"); } + NocCluster::create_nocresponse(cmd_req.resp, NocStatus::Ok, fab_idx, "".to_owned()); + cmd_req.trans.complete(); + Ok(()) + } + fn create_nocresponse( + tw: &mut TLVWriter, + status_code: NocStatus, + fab_idx: u8, + debug_txt: String, + ) { let cmd_data = NocResp { - status_code: NocStatus::Ok as u8, + status_code: status_code as u8, fab_idx, - debug_txt: "".to_owned(), + debug_txt, }; - let resp = ib::InvResp::cmd_new( + let invoke_resp = ib::InvResp::cmd_new( 0, ID, Commands::NOCResp as u16, EncodeValue::Value(&cmd_data), ); - let _ = resp.to_tlv(cmd_req.resp, TagType::Anonymous); + let _ = invoke_resp.to_tlv(tw, TagType::Anonymous); + } + + fn handle_command_updatefablabel( + &mut self, + cmd_req: &mut CommandReq, + ) -> Result<(), IMStatusCode> { + cmd_enter!("Update Fabric Label"); + let req = UpdateFabricLabelReq::from_tlv(&cmd_req.data) + .map_err(|_| IMStatusCode::InvalidDataType)?; + let label = req + .label + .to_string() + .map_err(|_| IMStatusCode::InvalidDataType)?; + + let (result, fab_idx) = + if let SessionMode::Case(fab_idx) = cmd_req.trans.session.get_session_mode() { + if self.fabric_mgr.set_label(fab_idx, label).is_err() { + (NocStatus::LabelConflict, fab_idx) + } else { + (NocStatus::Ok, fab_idx) + } + } else { + // Update Fabric Label not allowed + (NocStatus::InvalidFabricIndex, 0) + }; + NocCluster::create_nocresponse(cmd_req.resp, result, fab_idx, "".to_string()); cmd_req.trans.complete(); Ok(()) } @@ -203,18 +241,8 @@ impl NocCluster { fn handle_command_addnoc(&mut self, cmd_req: &mut CommandReq) -> Result<(), IMStatusCode> { cmd_enter!("AddNOC"); if let Err(e) = self._handle_command_addnoc(cmd_req) { - let cmd_data = NocResp { - status_code: e as u8, - fab_idx: 0, - debug_txt: "".to_owned(), - }; - let invoke_resp = ib::InvResp::cmd_new( - 0, - ID, - Commands::NOCResp as u16, - EncodeValue::Value(&cmd_data), - ); - let _ = invoke_resp.to_tlv(cmd_req.resp, TagType::Anonymous); + //TODO: Fab-idx 0? + NocCluster::create_nocresponse(cmd_req.resp, e, 0, "".to_owned()); cmd_req.trans.complete(); } Ok(()) @@ -401,6 +429,7 @@ impl ClusterType for NocCluster { Commands::AddTrustedRootCert => self.handle_command_addtrustedrootcert(cmd_req), Commands::AttReq => self.handle_command_attrequest(cmd_req), Commands::CertChainReq => self.handle_command_certchainrequest(cmd_req), + Commands::UpdateFabricLabel => self.handle_command_updatefablabel(cmd_req), _ => Err(IMStatusCode::UnsupportedCommand), } } @@ -515,6 +544,12 @@ struct CommonReq<'a> { str: OctetStr<'a>, } +#[derive(FromTLV)] +#[tlvargs(lifetime = "'a")] +struct UpdateFabricLabelReq<'a> { + label: UtfStr<'a>, +} + #[derive(FromTLV)] struct CertChainReq { cert_type: u8, diff --git a/matter/src/fabric.rs b/matter/src/fabric.rs index 97e07bd..1e91617 100644 --- a/matter/src/fabric.rs +++ b/matter/src/fabric.rs @@ -18,7 +18,7 @@ use std::sync::{Arc, Mutex, MutexGuard, RwLock}; use byteorder::{BigEndian, ByteOrder, LittleEndian}; -use log::info; +use log::{error, info}; use owning_ref::RwLockReadGuardRef; use crate::{ @@ -45,6 +45,7 @@ const ST_RCA: &str = "rca"; const ST_ICA: &str = "ica"; const ST_NOC: &str = "noc"; const ST_IPK: &str = "ipk"; +const ST_LBL: &str = "label"; const ST_PBKEY: &str = "pubkey"; const ST_PRKEY: &str = "privkey"; @@ -58,6 +59,7 @@ pub struct Fabric { pub icac: Option, pub noc: Cert, pub ipk: KeySet, + label: String, compressed_id: [u8; COMPRESSED_FABRIC_ID_LEN], mdns_service: Option, } @@ -97,6 +99,7 @@ impl Fabric { noc, ipk: KeySet::default(), compressed_id: [0; COMPRESSED_FABRIC_ID_LEN], + label: "".into(), mdns_service: None, }; Fabric::get_compressed_id(f.root_ca.get_pubkey(), fabric_id, &mut f.compressed_id)?; @@ -129,6 +132,7 @@ impl Fabric { icac: Some(Cert::default()), noc: Cert::default(), ipk: KeySet::default(), + label: "".into(), compressed_id: [0; COMPRESSED_FABRIC_ID_LEN], mdns_service: None, }) @@ -186,7 +190,7 @@ impl Fabric { vendor_id: self.vendor_id, fabric_id: self.fabric_id, node_id: self.node_id, - label: UtfStr::new(b""), + label: UtfStr(self.label.as_bytes()), fab_idx: Some(fab_idx), } } @@ -206,6 +210,7 @@ impl Fabric { let len = self.noc.as_tlv(&mut key)?; psm.set_kv_slice(fb_key!(index, ST_NOC), &key[..len])?; psm.set_kv_slice(fb_key!(index, ST_IPK), self.ipk.epoch_key())?; + psm.set_kv_slice(fb_key!(index, ST_LBL), self.label.as_bytes())?; let mut key = [0_u8; crypto::EC_POINT_LEN_BYTES]; let len = self.key_pair.get_public_key(&mut key)?; @@ -217,7 +222,7 @@ impl Fabric { let key = &key[..len]; psm.set_kv_slice(fb_key!(index, ST_PRKEY), key)?; - psm.set_kv_u64(ST_VID, self.vendor_id.into())?; + psm.set_kv_u64(fb_key!(index, ST_VID), self.vendor_id.into())?; Ok(()) } @@ -241,6 +246,13 @@ impl Fabric { let mut ipk = Vec::new(); psm.get_kv_slice(fb_key!(index, ST_IPK), &mut ipk)?; + let mut label = Vec::new(); + psm.get_kv_slice(fb_key!(index, ST_LBL), &mut label)?; + let label = String::from_utf8(label).map_err(|_| { + error!("Couldn't read label"); + Error::Invalid + })?; + let mut pub_key = Vec::new(); psm.get_kv_slice(fb_key!(index, ST_PBKEY), &mut pub_key)?; let mut priv_key = Vec::new(); @@ -248,16 +260,20 @@ impl Fabric { let keypair = KeyPair::new_from_components(pub_key.as_slice(), priv_key.as_slice())?; let mut vendor_id = 0; - psm.get_kv_u64(ST_VID, &mut vendor_id)?; + psm.get_kv_u64(fb_key!(index, ST_VID), &mut vendor_id)?; - Fabric::new( + let f = Fabric::new( keypair, root_ca, icac, noc, ipk.as_slice(), vendor_id as u16, - ) + ); + f.map(|mut f| { + f.label = label; + f + }) } } @@ -361,4 +377,28 @@ impl FabricMgr { } Ok(()) } + + pub fn set_label(&self, index: u8, label: String) -> Result<(), Error> { + let index = index as usize; + let mut mgr = self.inner.write()?; + if label != "" { + for i in 1..MAX_SUPPORTED_FABRICS { + if let Some(fabric) = &mgr.fabrics[i] { + if fabric.label == label { + return Err(Error::Invalid); + } + } + } + } + if let Some(fabric) = &mut mgr.fabrics[index] { + let old = fabric.label.clone(); + fabric.label = label; + let psm = self.psm.lock().unwrap(); + if fabric.store(index, &psm).is_err() { + fabric.label = old; + return Err(Error::StdIoError); + } + } + Ok(()) + } } diff --git a/matter/src/tlv/traits.rs b/matter/src/tlv/traits.rs index d9b7d10..60bf73a 100644 --- a/matter/src/tlv/traits.rs +++ b/matter/src/tlv/traits.rs @@ -119,6 +119,10 @@ impl<'a> UtfStr<'a> { pub fn new(str: &'a [u8]) -> Self { Self(str) } + + pub fn to_string(self) -> Result { + String::from_utf8(self.0.to_vec()).map_err(|_| Error::Invalid) + } } impl<'a> ToTLV for UtfStr<'a> { @@ -127,6 +131,12 @@ impl<'a> ToTLV for UtfStr<'a> { } } +impl<'a> FromTLV<'a> for UtfStr<'a> { + fn from_tlv(t: &TLVElement<'a>) -> Result, Error> { + t.slice().map(UtfStr) + } +} + /// Implements OctetString from the spec #[derive(Debug, Copy, Clone, PartialEq)] pub struct OctetStr<'a>(pub &'a [u8]); From 8714758ae44e6cba586f9912f76e37e60cd06849 Mon Sep 17 00:00:00 2001 From: Kedar Sovani Date: Thu, 12 Jan 2023 16:56:02 +0530 Subject: [PATCH 10/12] data_model: A number of changes required for iOS to work --- examples/onoff_light/src/main.rs | 1 + .../data_model/cluster_basic_information.rs | 27 +++++++++++ matter/src/data_model/core.rs | 2 +- matter/src/data_model/device_types.rs | 14 +++++- matter/src/data_model/objects/attribute.rs | 5 +- matter/src/data_model/objects/cluster.rs | 29 +++++------ matter/src/data_model/objects/encoder.rs | 6 +++ matter/src/data_model/objects/endpoint.rs | 10 +++- matter/src/data_model/objects/node.rs | 6 ++- .../src/data_model/system_model/descriptor.rs | 48 ++++++++++++++++++- matter/tests/common/im_engine.rs | 1 + matter/tests/data_model/acl_and_dataver.rs | 4 +- 12 files changed, 125 insertions(+), 28 deletions(-) diff --git a/examples/onoff_light/src/main.rs b/examples/onoff_light/src/main.rs index 39de6e4..9350451 100644 --- a/examples/onoff_light/src/main.rs +++ b/examples/onoff_light/src/main.rs @@ -35,6 +35,7 @@ fn main() { pid: 0x8002, hw_ver: 2, sw_ver: 1, + serial_no: "aabbccdd".to_string(), }; let dev_att = Box::new(dev_att::HardCodedDevAtt::new()); diff --git a/matter/src/data_model/cluster_basic_information.rs b/matter/src/data_model/cluster_basic_information.rs index c480f48..d09e4dc 100644 --- a/matter/src/data_model/cluster_basic_information.rs +++ b/matter/src/data_model/cluster_basic_information.rs @@ -17,13 +17,18 @@ use super::objects::*; use crate::error::*; +use num_derive::FromPrimitive; pub const ID: u32 = 0x0028; + +#[derive(FromPrimitive)] enum Attributes { + DMRevision = 0, VendorId = 2, ProductId = 4, HwVer = 7, SwVer = 9, + SerialNo = 0x0f, } pub struct BasicInfoConfig { @@ -31,6 +36,16 @@ pub struct BasicInfoConfig { pub pid: u16, pub hw_ver: u16, pub sw_ver: u32, + pub serial_no: String, +} + +fn attr_dm_rev_new() -> Result { + Attribute::new( + Attributes::DMRevision as u16, + AttrValue::Uint8(1), + Access::RV, + Quality::FIXED, + ) } fn attr_vid_new(vid: u16) -> Result { @@ -69,6 +84,14 @@ fn attr_sw_ver_new(sw_ver: u32) -> Result { ) } +fn attr_serial_no_new(label: String) -> Result { + Attribute::new( + Attributes::SerialNo as u16, + AttrValue::Utf8(label), + Access::RV, + Quality::FIXED, + ) +} pub struct BasicInfoCluster { base: Cluster, } @@ -78,10 +101,14 @@ impl BasicInfoCluster { let mut cluster = Box::new(BasicInfoCluster { base: Cluster::new(ID)?, }); + cluster.base.add_attribute(attr_dm_rev_new()?)?; cluster.base.add_attribute(attr_vid_new(cfg.vid)?)?; cluster.base.add_attribute(attr_pid_new(cfg.pid)?)?; cluster.base.add_attribute(attr_hw_ver_new(cfg.hw_ver)?)?; cluster.base.add_attribute(attr_sw_ver_new(cfg.sw_ver)?)?; + cluster + .base + .add_attribute(attr_serial_no_new(cfg.serial_no)?)?; Ok(cluster) } } diff --git a/matter/src/data_model/core.rs b/matter/src/data_model/core.rs index 11bf060..e6fcf7c 100644 --- a/matter/src/data_model/core.rs +++ b/matter/src/data_model/core.rs @@ -84,7 +84,7 @@ impl DataModel { ) -> Result { let node = self.node.read().unwrap(); let cluster = node.get_cluster(endpoint, cluster)?; - cluster.base().read_attribute_raw(attr).map(|a| *a) + cluster.base().read_attribute_raw(attr).map(|a| a.clone()) } // Encode a write attribute from a path that may or may not be wildcard diff --git a/matter/src/data_model/device_types.rs b/matter/src/data_model/device_types.rs index a0c9c28..b621ce1 100644 --- a/matter/src/data_model/device_types.rs +++ b/matter/src/data_model/device_types.rs @@ -32,6 +32,11 @@ use crate::secure_channel::pake::PaseMgr; use std::sync::Arc; use std::sync::RwLockWriteGuard; +pub const DEV_TYPE_ROOT_NODE: DeviceType = DeviceType { + dtype: 0x0016, + drev: 1, +}; + type WriteNode<'a> = RwLockWriteGuard<'a, Box>; pub fn device_type_add_root_node( @@ -43,7 +48,7 @@ pub fn device_type_add_root_node( pase_mgr: PaseMgr, ) -> Result { // Add the root endpoint - let endpoint = node.add_endpoint()?; + let endpoint = node.add_endpoint(DEV_TYPE_ROOT_NODE)?; if endpoint != 0 { // Somehow endpoint 0 was already added, this shouldn't be the case return Err(Error::Invalid); @@ -63,8 +68,13 @@ pub fn device_type_add_root_node( Ok(endpoint) } +const DEV_TYPE_ON_OFF_LIGHT: DeviceType = DeviceType { + dtype: 0x0100, + drev: 2, +}; + pub fn device_type_add_on_off_light(node: &mut WriteNode) -> Result { - let endpoint = node.add_endpoint()?; + let endpoint = node.add_endpoint(DEV_TYPE_ON_OFF_LIGHT)?; node.add_cluster(endpoint, OnOffCluster::new()?)?; Ok(endpoint) } diff --git a/matter/src/data_model/objects/attribute.rs b/matter/src/data_model/objects/attribute.rs index 5498875..0ffbc73 100644 --- a/matter/src/data_model/objects/attribute.rs +++ b/matter/src/data_model/objects/attribute.rs @@ -88,7 +88,7 @@ bitflags! { * - instead of arrays, can use linked-lists to conserve space and avoid the internal fragmentation */ -#[derive(PartialEq, Copy, Clone)] +#[derive(PartialEq, Clone)] pub enum AttrValue { Int64(i64), Uint8(u8), @@ -96,6 +96,7 @@ pub enum AttrValue { Uint32(u32), Uint64(u64), Bool(bool), + Utf8(String), Custom, } @@ -108,6 +109,7 @@ impl Debug for AttrValue { AttrValue::Uint32(v) => write!(f, "{:?}", *v), AttrValue::Uint64(v) => write!(f, "{:?}", *v), AttrValue::Bool(v) => write!(f, "{:?}", *v), + AttrValue::Utf8(v) => write!(f, "{:?}", *v), AttrValue::Custom => write!(f, "custom-attribute"), }?; Ok(()) @@ -123,6 +125,7 @@ impl ToTLV for AttrValue { AttrValue::Uint16(v) => tw.u16(tag_type, *v), AttrValue::Uint32(v) => tw.u32(tag_type, *v), AttrValue::Uint64(v) => tw.u64(tag_type, *v), + AttrValue::Utf8(v) => tw.utf8(tag_type, v.as_bytes()), _ => { error!("Attribute type not yet supported"); Err(Error::AttributeNotFound) diff --git a/matter/src/data_model/objects/cluster.rs b/matter/src/data_model/objects/cluster.rs index a42aac2..d279363 100644 --- a/matter/src/data_model/objects/cluster.rs +++ b/matter/src/data_model/objects/cluster.rs @@ -87,7 +87,6 @@ pub trait ClusterType { pub struct Cluster { pub(super) id: u32, attributes: Vec, - feature_map: Option, data_ver: u32, } @@ -96,7 +95,6 @@ impl Cluster { let mut c = Cluster { id, attributes: Vec::with_capacity(ATTRS_PER_CLUSTER), - feature_map: None, data_ver: rand::thread_rng().gen_range(0..0xFFFFFFFF), }; c.add_default_attributes()?; @@ -112,22 +110,20 @@ impl Cluster { } pub fn set_feature_map(&mut self, map: u32) -> Result<(), Error> { - if self.feature_map.is_none() { - self.add_attribute(Attribute::new( - GlobalElements::FeatureMap as u16, - AttrValue::Uint32(map), - Access::RV, - Quality::NONE, - )?)?; - } else { - self.write_attribute_raw(GlobalElements::FeatureMap as u16, AttrValue::Uint32(map)) - .map_err(|_| Error::Invalid)?; - } - self.feature_map = Some(map); + self.write_attribute_raw(GlobalElements::FeatureMap as u16, AttrValue::Uint32(map)) + .map_err(|_| Error::Invalid)?; Ok(()) } fn add_default_attributes(&mut self) -> Result<(), Error> { + // Default feature map is 0 + self.add_attribute(Attribute::new( + GlobalElements::FeatureMap as u16, + AttrValue::Uint32(0), + Access::RV, + Quality::NONE, + )?)?; + self.add_attribute(Attribute::new( GlobalElements::AttributeList as u16, AttrValue::Custom, @@ -233,8 +229,7 @@ impl Cluster { return; } GlobalElements::FeatureMap => { - let val = if let Some(m) = self.feature_map { m } else { 0 }; - encoder.encode(EncodeValue::Value(&val)); + encoder.encode(EncodeValue::Value(&attr.value)); return; } _ => { @@ -284,7 +279,7 @@ impl Cluster { ) -> Result<(), IMStatusCode> { let a = self.get_attribute_mut(attr_id)?; if a.value != AttrValue::Custom { - let mut value = a.value; + let mut value = a.value.clone(); value .update_from_tlv(data) .map_err(|_| IMStatusCode::Failure)?; diff --git a/matter/src/data_model/objects/encoder.rs b/matter/src/data_model/objects/encoder.rs index 3e756d8..24a81aa 100644 --- a/matter/src/data_model/objects/encoder.rs +++ b/matter/src/data_model/objects/encoder.rs @@ -115,3 +115,9 @@ pub trait Encoder { /// Encode a status report fn encode_status(&mut self, status: IMStatusCode, cluster_status: u16); } + +#[derive(ToTLV, Copy, Clone)] +pub struct DeviceType { + pub dtype: u16, + pub drev: u16, +} diff --git a/matter/src/data_model/objects/endpoint.rs b/matter/src/data_model/objects/endpoint.rs index a87f887..7ea22dd 100644 --- a/matter/src/data_model/objects/endpoint.rs +++ b/matter/src/data_model/objects/endpoint.rs @@ -19,17 +19,21 @@ use crate::{data_model::objects::ClusterType, error::*, interaction_model::core: use std::fmt; +use super::DeviceType; + pub const CLUSTERS_PER_ENDPT: usize = 9; pub struct Endpoint { + dev_type: DeviceType, clusters: Vec>, } pub type BoxedClusters = [Box]; impl Endpoint { - pub fn new() -> Result, Error> { + pub fn new(dev_type: DeviceType) -> Result, Error> { Ok(Box::new(Endpoint { + dev_type, clusters: Vec::with_capacity(CLUSTERS_PER_ENDPT), })) } @@ -43,6 +47,10 @@ impl Endpoint { } } + pub fn get_dev_type(&self) -> &DeviceType { + &self.dev_type + } + fn get_cluster_index(&self, cluster_id: u32) -> Option { self.clusters.iter().position(|c| c.base().id == cluster_id) } diff --git a/matter/src/data_model/objects/node.rs b/matter/src/data_model/objects/node.rs index 8e391cf..1f2b7cf 100644 --- a/matter/src/data_model/objects/node.rs +++ b/matter/src/data_model/objects/node.rs @@ -23,6 +23,8 @@ use crate::{ }; use std::fmt; +use super::DeviceType; + pub trait ChangeConsumer { fn endpoint_added(&self, id: u16, endpoint: &mut Endpoint) -> Result<(), Error>; } @@ -59,13 +61,13 @@ impl Node { self.changes_cb = Some(consumer); } - pub fn add_endpoint(&mut self) -> Result { + pub fn add_endpoint(&mut self, dev_type: DeviceType) -> Result { let index = self .endpoints .iter() .position(|x| x.is_none()) .ok_or(Error::NoSpace)?; - let mut endpoint = Endpoint::new()?; + let mut endpoint = Endpoint::new(dev_type)?; if let Some(cb) = &self.changes_cb { cb.endpoint_added(index as u16, &mut endpoint)?; } diff --git a/matter/src/data_model/system_model/descriptor.rs b/matter/src/data_model/system_model/descriptor.rs index 4609a73..29fb15b 100644 --- a/matter/src/data_model/system_model/descriptor.rs +++ b/matter/src/data_model/system_model/descriptor.rs @@ -21,7 +21,7 @@ use crate::data_model::core::DataModel; use crate::data_model::objects::*; use crate::error::*; use crate::interaction_model::messages::GenericPath; -use crate::tlv::{TLVWriter, TagType}; +use crate::tlv::{TLVWriter, TagType, ToTLV}; use log::error; pub const ID: u32 = 0x001D; @@ -48,11 +48,28 @@ impl DescriptorCluster { data_model, base: Cluster::new(ID)?, }); + c.base.add_attribute(attr_devtypelist_new()?)?; c.base.add_attribute(attr_serverlist_new()?)?; c.base.add_attribute(attr_partslist_new()?)?; Ok(c) } + fn encode_devtype_list(&self, tag: TagType, tw: &mut TLVWriter) { + let path = GenericPath { + endpoint: Some(self.endpoint_id), + cluster: None, + leaf: None, + }; + let _ = tw.start_array(tag); + let dm = self.data_model.node.read().unwrap(); + let _ = dm.for_each_endpoint(&path, |_, e| { + let dev_type = e.get_dev_type(); + let _ = dev_type.to_tlv(tw, TagType::Anonymous); + Ok(()) + }); + let _ = tw.end_container(); + } + fn encode_server_list(&self, tag: TagType, tw: &mut TLVWriter) { let path = GenericPath { endpoint: Some(self.endpoint_id), @@ -69,8 +86,24 @@ impl DescriptorCluster { } fn encode_parts_list(&self, tag: TagType, tw: &mut TLVWriter) { - // TODO: Support Partslist + let path = GenericPath { + endpoint: None, + cluster: None, + leaf: None, + }; let _ = tw.start_array(tag); + if self.endpoint_id == 0 { + // TODO: If endpoint is another than 0, need to figure out what to do + let dm = self.data_model.node.read().unwrap(); + let _ = dm.for_each_endpoint(&path, |current_path, _| { + if let Some(endpoint_id) = current_path.endpoint { + if endpoint_id != 0 { + let _ = tw.u16(TagType::Anonymous, endpoint_id); + } + } + Ok(()) + }); + } let _ = tw.end_container(); } } @@ -85,6 +118,9 @@ impl ClusterType for DescriptorCluster { fn read_custom_attribute(&self, encoder: &mut dyn Encoder, attr: &AttrDetails) { match num::FromPrimitive::from_u16(attr.attr_id) { + Some(Attributes::DeviceTypeList) => encoder.encode(EncodeValue::Closure(&|tag, tw| { + self.encode_devtype_list(tag, tw) + })), Some(Attributes::ServerList) => encoder.encode(EncodeValue::Closure(&|tag, tw| { self.encode_server_list(tag, tw) })), @@ -98,6 +134,14 @@ impl ClusterType for DescriptorCluster { } } +fn attr_devtypelist_new() -> Result { + Attribute::new( + Attributes::DeviceTypeList as u16, + AttrValue::Custom, + Access::RV, + Quality::NONE, + ) +} fn attr_serverlist_new() -> Result { Attribute::new( Attributes::ServerList as u16, diff --git a/matter/tests/common/im_engine.rs b/matter/tests/common/im_engine.rs index feddb96..45f8fe5 100644 --- a/matter/tests/common/im_engine.rs +++ b/matter/tests/common/im_engine.rs @@ -94,6 +94,7 @@ impl ImEngine { pid: 11, hw_ver: 12, sw_ver: 13, + serial_no: "aabbccdd".to_string(), }; let dev_att = Box::new(DummyDevAtt {}); let fabric_mgr = Arc::new(FabricMgr::new().unwrap()); diff --git a/matter/tests/data_model/acl_and_dataver.rs b/matter/tests/data_model/acl_and_dataver.rs index 11e3576..3dd2f60 100644 --- a/matter/tests/data_model/acl_and_dataver.rs +++ b/matter/tests/data_model/acl_and_dataver.rs @@ -204,10 +204,10 @@ fn read_cluster_id_write_attr(im: &ImEngine, endpoint: u16) -> AttrValue { let node = im.dm.node.read().unwrap(); let echo = node.get_cluster(endpoint, echo_cluster::ID).unwrap(); - *echo - .base() + echo.base() .read_attribute_raw(echo_cluster::Attributes::AttWrite as u16) .unwrap() + .clone() } fn read_cluster_id_data_ver(im: &ImEngine, endpoint: u16) -> u32 { From 9858fdefdce350c622a0aad8020510a6cd9f5d61 Mon Sep 17 00:00:00 2001 From: Kedar Sovani Date: Thu, 12 Jan 2023 18:31:13 +0530 Subject: [PATCH 11/12] tests: Fix test cases for featuremap --- matter/src/lib.rs | 1 + matter/tests/data_model/attributes.rs | 17 +++++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/matter/src/lib.rs b/matter/src/lib.rs index 08f2c19..a0a9603 100644 --- a/matter/src/lib.rs +++ b/matter/src/lib.rs @@ -50,6 +50,7 @@ //! pid: 0xFFF1, //! hw_ver: 2, //! sw_ver: 1, +//! serial_no: "aabbcc".to_string(), //! }; //! //! /// Get the Matter Object diff --git a/matter/tests/data_model/attributes.rs b/matter/tests/data_model/attributes.rs index 5e230c7..b53614f 100644 --- a/matter/tests/data_model/attributes.rs +++ b/matter/tests/data_model/attributes.rs @@ -238,6 +238,7 @@ fn test_read_wc_endpoint_wc_attribute() { let attr_list_tlvs = get_tlvs( &mut buf, &[ + GlobalElements::FeatureMap as u16, GlobalElements::AttributeList as u16, echo_cluster::Attributes::Att1 as u16, echo_cluster::Attributes::Att2 as u16, @@ -247,6 +248,14 @@ fn test_read_wc_endpoint_wc_attribute() { ); let expected = &[ + attr_data!( + GenericPath::new( + Some(0), + Some(echo_cluster::ID), + Some(GlobalElements::FeatureMap as u32), + ), + ElementType::U8(0) + ), attr_data!( GenericPath::new( Some(0), @@ -279,6 +288,14 @@ fn test_read_wc_endpoint_wc_attribute() { ), ElementType::U32(echo_cluster::ATTR_CUSTOM_VALUE) ), + attr_data!( + GenericPath::new( + Some(1), + Some(echo_cluster::ID), + Some(GlobalElements::FeatureMap as u32), + ), + ElementType::U8(0) + ), attr_data!( GenericPath::new( Some(1), From 34e137a346752b21b4f03bfead7926a5abd904c9 Mon Sep 17 00:00:00 2001 From: Kedar Sovani Date: Thu, 12 Jan 2023 18:32:53 +0530 Subject: [PATCH 12/12] Rebase changes on latest commit --- examples/onoff_light/src/main.rs | 2 +- matter/src/core.rs | 4 ++-- matter/src/data_model/cluster_basic_information.rs | 2 +- matter/src/data_model/core.rs | 2 +- matter/src/data_model/device_types.rs | 2 +- matter/src/lib.rs | 2 +- matter/tests/common/im_engine.rs | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/examples/onoff_light/src/main.rs b/examples/onoff_light/src/main.rs index 9350451..bbf1d8b 100644 --- a/examples/onoff_light/src/main.rs +++ b/examples/onoff_light/src/main.rs @@ -39,7 +39,7 @@ fn main() { }; let dev_att = Box::new(dev_att::HardCodedDevAtt::new()); - let mut matter = core::Matter::new(&dev_info, dev_att, &comm_data).unwrap(); + let mut matter = core::Matter::new(dev_info, dev_att, comm_data).unwrap(); let dm = matter.get_data_model(); { let mut node = dm.node.write().unwrap(); diff --git a/matter/src/core.rs b/matter/src/core.rs index 7d779a4..39a903f 100644 --- a/matter/src/core.rs +++ b/matter/src/core.rs @@ -53,9 +53,9 @@ impl Matter { /// requires a set of device attestation certificates and keys. It is the responsibility of /// this object to return the device attestation details when queried upon. pub fn new( - dev_det: &BasicInfoConfig, + dev_det: BasicInfoConfig, dev_att: Box, - dev_comm: &CommissioningData, + dev_comm: CommissioningData, ) -> Result, Error> { let mdns = Mdns::get()?; mdns.set_values(dev_det.vid, dev_det.pid); diff --git a/matter/src/data_model/cluster_basic_information.rs b/matter/src/data_model/cluster_basic_information.rs index d09e4dc..f78c93c 100644 --- a/matter/src/data_model/cluster_basic_information.rs +++ b/matter/src/data_model/cluster_basic_information.rs @@ -97,7 +97,7 @@ pub struct BasicInfoCluster { } impl BasicInfoCluster { - pub fn new(cfg: &BasicInfoConfig) -> Result, Error> { + pub fn new(cfg: BasicInfoConfig) -> Result, Error> { let mut cluster = Box::new(BasicInfoCluster { base: Cluster::new(ID)?, }); diff --git a/matter/src/data_model/core.rs b/matter/src/data_model/core.rs index e6fcf7c..8cddaf4 100644 --- a/matter/src/data_model/core.rs +++ b/matter/src/data_model/core.rs @@ -51,7 +51,7 @@ pub struct DataModel { impl DataModel { pub fn new( - dev_details: &BasicInfoConfig, + dev_details: BasicInfoConfig, dev_att: Box, fabric_mgr: Arc, acl_mgr: Arc, diff --git a/matter/src/data_model/device_types.rs b/matter/src/data_model/device_types.rs index b621ce1..12874f4 100644 --- a/matter/src/data_model/device_types.rs +++ b/matter/src/data_model/device_types.rs @@ -41,7 +41,7 @@ type WriteNode<'a> = RwLockWriteGuard<'a, Box>; pub fn device_type_add_root_node( node: &mut WriteNode, - dev_info: &BasicInfoConfig, + dev_info: BasicInfoConfig, dev_att: Box, fabric_mgr: Arc, acl_mgr: Arc, diff --git a/matter/src/lib.rs b/matter/src/lib.rs index a0a9603..cb4b1e2 100644 --- a/matter/src/lib.rs +++ b/matter/src/lib.rs @@ -55,7 +55,7 @@ //! //! /// Get the Matter Object //! /// The dev_att is an object that implements the DevAttDataFetcher trait. -//! let mut matter = Matter::new(&dev_info, dev_att, &comm_data).unwrap(); +//! let mut matter = Matter::new(dev_info, dev_att, comm_data).unwrap(); //! let dm = matter.get_data_model(); //! { //! let mut node = dm.node.write().unwrap(); diff --git a/matter/tests/common/im_engine.rs b/matter/tests/common/im_engine.rs index 45f8fe5..b0a1d64 100644 --- a/matter/tests/common/im_engine.rs +++ b/matter/tests/common/im_engine.rs @@ -106,7 +106,7 @@ impl ImEngine { default_acl.add_subject(IM_ENGINE_PEER_ID).unwrap(); acl_mgr.add(default_acl).unwrap(); let dm = DataModel::new( - &dev_det, + dev_det, dev_att, fabric_mgr.clone(), acl_mgr.clone(),