From 72e703353d18305aa2c6cf114bac208674a37e2a Mon Sep 17 00:00:00 2001 From: Kedar Sovani Date: Sat, 31 Dec 2022 11:02:16 +0530 Subject: [PATCH 1/6] Debug: Include TLV dumps for Secure Channel exchanges --- matter/src/secure_channel/core.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/matter/src/secure_channel/core.rs b/matter/src/secure_channel/core.rs index eb36531..8a2774c 100644 --- a/matter/src/secure_channel/core.rs +++ b/matter/src/secure_channel/core.rs @@ -23,6 +23,7 @@ use crate::{ mdns::{self, Mdns}, secure_channel::{common::*, pake::PAKE}, sys::SysMdnsService, + tlv, transport::proto_demux::{self, ProtoCtx, ResponseRequired}, }; use log::{error, info}; @@ -120,7 +121,9 @@ 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); - match 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), @@ -131,7 +134,12 @@ impl proto_demux::HandleProto for SecureChannel { error!("OpCode Not Handled: {:?}", proto_opcode); Err(Error::InvalidOpcode) } + }; + if result == Ok(ResponseRequired::Yes) { + info!("Sending response"); + tlv::print_tlv_list(ctx.tx.as_borrow_slice()); } + result } fn get_proto_id(&self) -> usize { From 5843ebf2c08c4564cd9f9eedfe12a32056b0b9f6 Mon Sep 17 00:00:00 2001 From: Kedar Sovani Date: Sat, 31 Dec 2022 11:02:45 +0530 Subject: [PATCH 2/6] Network Commissioning: Fix incorrect reporting of transport type --- matter/src/data_model/sdm/nw_commissioning.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/matter/src/data_model/sdm/nw_commissioning.rs b/matter/src/data_model/sdm/nw_commissioning.rs index 3b08ec1..753347d 100644 --- a/matter/src/data_model/sdm/nw_commissioning.rs +++ b/matter/src/data_model/sdm/nw_commissioning.rs @@ -36,9 +36,9 @@ impl ClusterType for NwCommCluster { } enum FeatureMap { - _Wifi = 0, - _Thread = 1, - Ethernet = 2, + _Wifi = 0x01, + _Thread = 0x02, + Ethernet = 0x04, } impl NwCommCluster { From bd25ca8c85ab84272ba8efda09baf9383b63e09f Mon Sep 17 00:00:00 2001 From: Kedar Sovani Date: Sat, 31 Dec 2022 11:03:27 +0530 Subject: [PATCH 3/6] DM: Include placeholder support for PartsList --- .../src/data_model/system_model/descriptor.rs | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/matter/src/data_model/system_model/descriptor.rs b/matter/src/data_model/system_model/descriptor.rs index 722a822..0790f82 100644 --- a/matter/src/data_model/system_model/descriptor.rs +++ b/matter/src/data_model/system_model/descriptor.rs @@ -48,6 +48,7 @@ impl DescriptorCluster { base: Cluster::new(ID)?, }); c.base.add_attribute(attr_serverlist_new()?)?; + c.base.add_attribute(attr_partslist_new()?)?; Ok(c) } @@ -65,6 +66,12 @@ impl DescriptorCluster { }); let _ = tw.end_container(); } + + fn encode_parts_list(&self, tag: TagType, tw: &mut TLVWriter) { + // TODO: Support Partslist + let _ = tw.start_array(tag); + let _ = tw.end_container(); + } } impl ClusterType for DescriptorCluster { @@ -80,7 +87,9 @@ impl ClusterType for DescriptorCluster { Some(Attributes::ServerList) => encoder.encode(EncodeValue::Closure(&|tag, tw| { self.encode_server_list(tag, tw) })), - + Some(Attributes::PartsList) => encoder.encode(EncodeValue::Closure(&|tag, tw| { + self.encode_parts_list(tag, tw) + })), _ => { error!("Attribute not supported: this shouldn't happen"); } @@ -96,3 +105,12 @@ fn attr_serverlist_new() -> Result { Quality::NONE, ) } + +fn attr_partslist_new() -> Result { + Attribute::new( + Attributes::PartsList as u16, + AttrValue::Custom, + Access::RV, + Quality::NONE, + ) +} From 569f1bb19b29a0f855df17a8a49f60feca5f334b Mon Sep 17 00:00:00 2001 From: Kedar Sovani Date: Sat, 31 Dec 2022 11:04:33 +0530 Subject: [PATCH 4/6] OpCreds: ICAC is optional and may not be added as part of commissioning --- matter/src/data_model/sdm/noc.rs | 9 +++++++-- matter/src/fabric.rs | 20 +++++++++++++++----- matter/src/secure_channel/case.rs | 9 +++++++-- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/matter/src/data_model/sdm/noc.rs b/matter/src/data_model/sdm/noc.rs index 3812ed6..d03467e 100644 --- a/matter/src/data_model/sdm/noc.rs +++ b/matter/src/data_model/sdm/noc.rs @@ -141,8 +141,13 @@ impl NocCluster { let noc_value = Cert::new(r.noc_value.0).map_err(|_| NocStatus::InvalidNOC)?; info!("Received NOC as: {}", noc_value); - let icac_value = Cert::new(r.icac_value.0).map_err(|_| NocStatus::InvalidNOC)?; - info!("Received ICAC as: {}", icac_value); + let icac_value = if r.icac_value.0.len() != 0 { + let cert = Cert::new(r.icac_value.0).map_err(|_| NocStatus::InvalidNOC)?; + info!("Received ICAC as: {}", cert); + Some(cert) + } else { + None + }; let fabric = Fabric::new( noc_data.key_pair, diff --git a/matter/src/fabric.rs b/matter/src/fabric.rs index 8f85504..f868664 100644 --- a/matter/src/fabric.rs +++ b/matter/src/fabric.rs @@ -52,7 +52,7 @@ pub struct Fabric { fabric_id: u64, key_pair: Box, pub root_ca: Cert, - pub icac: Cert, + pub icac: Option, pub noc: Cert, pub ipk: KeySet, compressed_id: [u8; COMPRESSED_FABRIC_ID_LEN], @@ -63,7 +63,7 @@ impl Fabric { pub fn new( key_pair: KeyPair, root_ca: Cert, - icac: Cert, + icac: Option, noc: Cert, ipk: &[u8], ) -> Result { @@ -107,7 +107,7 @@ impl Fabric { fabric_id: 0, key_pair: Box::new(KeyPairDummy::new()?), root_ca: Cert::default(), - icac: Cert::default(), + icac: Some(Cert::default()), noc: Cert::default(), ipk: KeySet::default(), compressed_id: [0; COMPRESSED_FABRIC_ID_LEN], @@ -165,8 +165,14 @@ impl Fabric { let mut key = [0u8; MAX_CERT_TLV_LEN]; let len = self.root_ca.as_tlv(&mut key)?; psm.set_kv_slice(fb_key!(index, ST_RCA), &key[..len])?; - let len = self.icac.as_tlv(&mut key)?; + + let len = if let Some(icac) = &self.icac { + icac.as_tlv(&mut key)? + } else { + 0 + }; psm.set_kv_slice(fb_key!(index, ST_ICA), &key[..len])?; + 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())?; @@ -191,7 +197,11 @@ impl Fabric { let mut icac = Vec::new(); psm.get_kv_slice(fb_key!(index, ST_ICA), &mut icac)?; - let icac = Cert::new(icac.as_slice())?; + let icac = if icac.len() != 0 { + Some(Cert::new(icac.as_slice())?) + } else { + None + }; let mut noc = Vec::new(); psm.get_kv_slice(fb_key!(index, ST_NOC), &mut noc)?; diff --git a/matter/src/secure_channel/case.rs b/matter/src/secure_channel/case.rs index c034532..d75bf96 100644 --- a/matter/src/secure_channel/case.rs +++ b/matter/src/secure_channel/case.rs @@ -475,7 +475,10 @@ impl Case { let mut tw = TLVWriter::new(&mut write_buf); tw.start_struct(TagType::Anonymous)?; tw.str16_as(TagType::Context(1), |buf| fabric.noc.as_tlv(buf))?; - tw.str16_as(TagType::Context(2), |buf| fabric.icac.as_tlv(buf))?; + if let Some(icac_cert) = &fabric.icac { + tw.str16_as(TagType::Context(2), |buf| icac_cert.as_tlv(buf))? + }; + tw.str8(TagType::Context(3), signature)?; tw.str8(TagType::Context(4), &resumption_id)?; tw.end_container()?; @@ -515,7 +518,9 @@ impl Case { let mut tw = TLVWriter::new(&mut write_buf); tw.start_struct(TagType::Anonymous)?; tw.str16_as(TagType::Context(1), |buf| fabric.noc.as_tlv(buf))?; - tw.str16_as(TagType::Context(2), |buf| fabric.icac.as_tlv(buf))?; + if let Some(icac_cert) = &fabric.icac { + tw.str16_as(TagType::Context(2), |buf| icac_cert.as_tlv(buf))?; + } tw.str8(TagType::Context(3), our_pub_key)?; tw.str8(TagType::Context(4), peer_pub_key)?; tw.end_container()?; From 752a2f3880e078e47e50d371f389bba1c920f7ee Mon Sep 17 00:00:00 2001 From: Kedar Sovani Date: Sun, 1 Jan 2023 11:54:58 +0530 Subject: [PATCH 5/6] Make CASE work with iOS - The initiator ICAC is also optional in this scenario - Secondly, it was observed that the NOC was larger than 256, so we've got to use str16 instead --- matter/src/secure_channel/case.rs | 39 ++++++++++++++++++------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/matter/src/secure_channel/case.rs b/matter/src/secure_channel/case.rs index d75bf96..309b414 100644 --- a/matter/src/secure_channel/case.rs +++ b/matter/src/secure_channel/case.rs @@ -120,7 +120,10 @@ impl Case { let d = Sigma3Decrypt::from_tlv(&root)?; let initiator_noc = Cert::new(d.initiator_noc.0)?; - let initiator_icac = Cert::new(d.initiator_icac.0)?; + let mut initiator_icac = None; + if let Some(icac) = d.initiator_icac { + initiator_icac = Some(Cert::new(icac.0)?); + } if let Err(e) = Case::validate_certs(fabric, &initiator_noc, &initiator_icac) { error!("Certificate Chain doesn't match: {}", e); common::create_sc_status_report( @@ -134,7 +137,7 @@ impl Case { if Case::validate_sigma3_sign( d.initiator_noc.0, - d.initiator_icac.0, + d.initiator_icac.map(|a| a.0), &initiator_noc, d.signature.0, &case_session, @@ -304,7 +307,7 @@ impl Case { fn validate_sigma3_sign( initiator_noc: &[u8], - initiator_icac: &[u8], + initiator_icac: Option<&[u8]>, initiator_noc_cert: &Cert, sign: &[u8], case_session: &CaseSession, @@ -314,8 +317,10 @@ impl Case { let mut write_buf = WriteBuf::new(&mut buf, MAX_TBS_SIZE); let mut tw = TLVWriter::new(&mut write_buf); tw.start_struct(TagType::Anonymous)?; - tw.str8(TagType::Context(1), initiator_noc)?; - tw.str8(TagType::Context(2), initiator_icac)?; + tw.str16(TagType::Context(1), initiator_noc)?; + if let Some(icac) = initiator_icac { + tw.str16(TagType::Context(2), icac)?; + } tw.str8(TagType::Context(3), &case_session.peer_pub_key)?; tw.str8(TagType::Context(4), &case_session.our_pub_key)?; tw.end_container()?; @@ -325,22 +330,24 @@ impl Case { Ok(()) } - fn validate_certs(fabric: &Fabric, noc: &Cert, icac: &Cert) -> Result<(), Error> { - if let Ok(fid) = icac.get_fabric_id() { - if fid != fabric.get_fabric_id() { - return Err(Error::Invalid); - } - } + fn validate_certs(fabric: &Fabric, noc: &Cert, icac: &Option) -> Result<(), Error> { + let mut verifier = noc.verify_chain_start(); if fabric.get_fabric_id() != noc.get_fabric_id()? { return Err(Error::Invalid); } - noc.verify_chain_start() - .add_cert(icac)? - .add_cert(&fabric.root_ca)? - .finalise()?; + if let Some(icac) = icac { + // If ICAC is present handle it + if let Ok(fid) = icac.get_fabric_id() { + if fid != fabric.get_fabric_id() { + return Err(Error::Invalid); + } + } + verifier = verifier.add_cert(icac)?; + } + verifier.add_cert(&fabric.root_ca)?.finalise()?; Ok(()) } @@ -542,6 +549,6 @@ struct Sigma1Req<'a> { #[tlvargs(start = 1, lifetime = "'a")] struct Sigma3Decrypt<'a> { initiator_noc: OctetStr<'a>, - initiator_icac: OctetStr<'a>, + initiator_icac: Option>, signature: OctetStr<'a>, } From f096f5005442804f0d7fe8f4e30e85a67a7ee7b2 Mon Sep 17 00:00:00 2001 From: Kedar Sovani Date: Sun, 1 Jan 2023 11:55:52 +0530 Subject: [PATCH 6/6] Misc: Fix leftover debug print put_str16 -> str16 --- matter/src/tlv/writer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matter/src/tlv/writer.rs b/matter/src/tlv/writer.rs index 5600237..e229bf0 100644 --- a/matter/src/tlv/writer.rs +++ b/matter/src/tlv/writer.rs @@ -130,7 +130,7 @@ impl<'a, 'b> TLVWriter<'a, 'b> { pub fn str8(&mut self, tag_type: TagType, data: &[u8]) -> Result<(), Error> { if data.len() > 256 { - error!("use put_str16() instead"); + error!("use str16() instead"); return Err(Error::Invalid); } self.put_control_tag(tag_type, WriteElementType::Str8l)?;