Skip to content

Commit 224a626

Browse files
Alami-AmineAlami-Amine
committed
integrating comments
1 parent 13a8f48 commit 224a626

File tree

3 files changed

+58
-43
lines changed

3 files changed

+58
-43
lines changed

‎src/protocols/secure_channel/CASESession.cpp

+32-35
Original file line numberDiff line numberDiff line change
@@ -770,7 +770,7 @@ CHIP_ERROR CASESession::SendSigma1()
770770
System::PacketBufferHandle msg_R1;
771771
uint8_t destinationIdentifier[kSHA256_Hash_Length] = { 0 };
772772

773-
Sigma1Param encodeSigma1Params;
773+
EncodeSigma1Param encodeSigma1Params;
774774

775775
// Lookup fabric info.
776776
const auto * fabricInfo = mFabricsTable->FindFabricWithIndex(mFabricIndex);
@@ -784,9 +784,11 @@ CHIP_ERROR CASESession::SendSigma1()
784784
mEphemeralKey = mFabricsTable->AllocateEphemeralKeypairForCASE();
785785
VerifyOrReturnError(mEphemeralKey != nullptr, CHIP_ERROR_NO_MEMORY);
786786
ReturnErrorOnFailure(mEphemeralKey->Initialize(ECPKeyTarget::ECDH));
787+
encodeSigma1Params.pEphPubKey = &mEphemeralKey->Pubkey();
787788

788789
// Fill in the random value
789790
ReturnErrorOnFailure(DRBG_get_bytes(mInitiatorRandom, sizeof(mInitiatorRandom)));
791+
encodeSigma1Params.initiatorRandom = ByteSpan(mInitiatorRandom);
790792

791793
// Generate a Destination Identifier based on the node we are attempting to reach
792794
{
@@ -800,15 +802,15 @@ CHIP_ERROR CASESession::SendSigma1()
800802
Credentials::P256PublicKeySpan rootPubKeySpan{ rootPubKey.ConstBytes() };
801803

802804
MutableByteSpan destinationIdSpan(destinationIdentifier);
803-
ReturnErrorOnFailure(GenerateCaseDestinationId(ByteSpan(mIPK), ByteSpan(mInitiatorRandom), rootPubKeySpan, fabricId,
805+
ReturnErrorOnFailure(GenerateCaseDestinationId(ByteSpan(mIPK), encodeSigma1Params.initiatorRandom, rootPubKeySpan, fabricId,
804806
mPeerNodeId, destinationIdSpan));
805807
encodeSigma1Params.destinationId = destinationIdSpan;
806808
}
807809

808810
VerifyOrReturnError(mLocalMRPConfig.HasValue(), CHIP_ERROR_INCORRECT_STATE);
811+
encodeSigma1Params.initiatorMrpConfig = &mLocalMRPConfig.Value();
809812

810813
// Try to find persistent session, and resume it.
811-
bool resuming = false;
812814
if (mSessionResumptionStorage != nullptr)
813815
{
814816
CHIP_ERROR err = mSessionResumptionStorage->FindByScopedNodeId(fabricInfo->GetScopedNodeIdForNode(mPeerNodeId),
@@ -817,14 +819,13 @@ CHIP_ERROR CASESession::SendSigma1()
817819
{
818820
// Found valid resumption state, try to resume the session.
819821

822+
encodeSigma1Params.resumptionId = mResumeResumptionId;
820823
MutableByteSpan resumeMICSpan(encodeSigma1Params.initiatorResume1MIC);
821-
ReturnErrorOnFailure(GenerateSigmaResumeMIC(ByteSpan(mInitiatorRandom), ByteSpan(mResumeResumptionId),
824+
ReturnErrorOnFailure(GenerateSigmaResumeMIC(encodeSigma1Params.initiatorRandom, encodeSigma1Params.resumptionId,
822825
ByteSpan(kKDFS1RKeyInfo), ByteSpan(kResume1MIC_Nonce), resumeMICSpan));
823826

824827
encodeSigma1Params.initiatorResumeMICSpan = resumeMICSpan;
825828
encodeSigma1Params.sessionResumptionRequested = true;
826-
827-
resuming = true;
828829
}
829830
}
830831

@@ -837,7 +838,7 @@ CHIP_ERROR CASESession::SendSigma1()
837838
ReturnErrorOnFailure(mExchangeCtxt.Value()->SendMessage(Protocols::SecureChannel::MsgType::CASE_Sigma1, std::move(msg_R1),
838839
SendFlags(SendMessageFlags::kExpectResponse)));
839840

840-
if (resuming)
841+
if (encodeSigma1Params.sessionResumptionRequested)
841842
{
842843
mState = State::kSentSigma1Resume;
843844

@@ -861,17 +862,19 @@ CHIP_ERROR CASESession::SendSigma1()
861862
return CHIP_NO_ERROR;
862863
}
863864

864-
CHIP_ERROR CASESession::EncodeSigma1(System::PacketBufferHandle & msg, Sigma1Param & inputParams)
865+
CHIP_ERROR CASESession::EncodeSigma1(System::PacketBufferHandle & msg, EncodeSigma1Param & input)
865866
{
866867

867868
MATTER_TRACE_SCOPE("EncodeSigma1", "CASESession");
868869

869-
size_t data_len = TLV::EstimateStructOverhead(kSigmaParamRandomNumberSize, // initiatorRandom
870-
sizeof(uint16_t), // initiatorSessionId,
871-
kSHA256_Hash_Length, // destinationId
872-
kP256_PublicKey_Length, // InitiatorEphPubKey,
873-
SessionParameters::kEstimatedTLVSize, // initiatorSessionParams
874-
SessionResumptionStorage::kResumptionIdSize, CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES);
870+
size_t data_len = TLV::EstimateStructOverhead(kSigmaParamRandomNumberSize, // initiatorRandom
871+
sizeof(uint16_t), // initiatorSessionId,
872+
kSHA256_Hash_Length, // destinationId
873+
kP256_PublicKey_Length, // InitiatorEphPubKey,
874+
SessionParameters::kEstimatedTLVSize, // initiatorSessionParams
875+
SessionResumptionStorage::kResumptionIdSize, // resumptionId
876+
CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES // initiatorResumeMIC
877+
);
875878

876879
msg = System::PacketBufferHandle::New(data_len);
877880
VerifyOrReturnError(!msg.IsNull(), CHIP_ERROR_NO_MEMORY);
@@ -881,24 +884,21 @@ CHIP_ERROR CASESession::EncodeSigma1(System::PacketBufferHandle & msg, Sigma1Par
881884

882885
tlvWriter.Init(std::move(msg));
883886
ReturnErrorOnFailure(tlvWriter.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, outerContainerType));
884-
// TODO Pass this in the struct?
885-
ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(kInitiatorRandomTag), ByteSpan(mInitiatorRandom)));
886-
ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(kInitiatorSessionIdTag), inputParams.initiatorSessionId));
887-
888-
ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(kDestinationIdTag), inputParams.destinationId));
887+
ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(kInitiatorRandomTag), input.initiatorRandom));
888+
ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(kInitiatorSessionIdTag), input.initiatorSessionId));
889+
ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(kDestinationIdTag), input.destinationId));
889890

890-
// TODO Pass this in the struct?
891-
ReturnErrorOnFailure(tlvWriter.PutBytes(TLV::ContextTag(kInitiatorPubKeyTag), mEphemeralKey->Pubkey(),
892-
static_cast<uint32_t>(mEphemeralKey->Pubkey().Length())));
891+
VerifyOrReturnError(input.pEphPubKey != nullptr, CHIP_ERROR_INCORRECT_STATE);
892+
ReturnErrorOnFailure(tlvWriter.PutBytes(TLV::ContextTag(kInitiatorPubKeyTag), *input.pEphPubKey,
893+
static_cast<uint32_t>(input.pEphPubKey->Length())));
893894

894-
// TODO is it redudunt?
895-
VerifyOrReturnError(mLocalMRPConfig.HasValue(), CHIP_ERROR_INCORRECT_STATE);
896-
ReturnErrorOnFailure(EncodeSessionParameters(TLV::ContextTag(kInitiatorMRPParamsTag), mLocalMRPConfig.Value(), tlvWriter));
895+
VerifyOrReturnError(input.initiatorMrpConfig != nullptr, CHIP_ERROR_INCORRECT_STATE);
896+
ReturnErrorOnFailure(EncodeSessionParameters(TLV::ContextTag(kInitiatorMRPParamsTag), *input.initiatorMrpConfig, tlvWriter));
897897

898-
if (inputParams.sessionResumptionRequested)
898+
if (input.sessionResumptionRequested)
899899
{
900-
ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(kResumptionIDTag), mResumeResumptionId));
901-
ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(kResume1MICTag), inputParams.initiatorResumeMICSpan));
900+
ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(kResumptionIDTag), input.resumptionId));
901+
ReturnErrorOnFailure(tlvWriter.Put(TLV::ContextTag(kResume1MICTag), input.initiatorResumeMICSpan));
902902
}
903903

904904
ReturnErrorOnFailure(tlvWriter.EndContainer(outerContainerType));
@@ -1003,7 +1003,7 @@ CHIP_ERROR CASESession::HandleSigma1(System::PacketBufferHandle && msg)
10031003
CHIP_ERROR err = CHIP_NO_ERROR;
10041004
System::PacketBufferTLVReader tlvReader;
10051005

1006-
Sigma1Param parsedSigma1;
1006+
ParseSigma1Param parsedSigma1;
10071007

10081008
SuccessOrExit(err = mCommissioningHash.AddData(ByteSpan{ msg->Start(), msg->DataLength() }));
10091009

@@ -1016,11 +1016,8 @@ CHIP_ERROR CASESession::HandleSigma1(System::PacketBufferHandle && msg)
10161016

10171017
VerifyOrExit(mFabricsTable != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
10181018

1019-
// TODO: Added by Amine, taken from inside ParseSigma1
1020-
// This was removed to remove the non-parsing parts from ParseSigma1, decoupling it from higher levels
1021-
// TODO: Should i change it?
1022-
// Set the recieved MRP parameters included with Sigma1
1023-
if (parsedSigma1.InitiatorMRPParamsPresent == true)
1019+
// Set the MRP parameters provided in the Sigma1 message
1020+
if (parsedSigma1.InitiatorMRPParamsPresent)
10241021
{
10251022
mExchangeCtxt.Value()->GetSessionHandle()->AsUnauthenticatedSession()->SetRemoteSessionParameters(
10261023
GetRemoteSessionParameters());
@@ -2191,7 +2188,7 @@ CHIP_ERROR CASESession::OnFailureStatusReport(Protocols::SecureChannel::GeneralS
21912188
return err;
21922189
}
21932190

2194-
CHIP_ERROR CASESession::ParseSigma1(TLV::ContiguousBufferTLVReader & tlvReader, Sigma1Param & output)
2191+
CHIP_ERROR CASESession::ParseSigma1(TLV::ContiguousBufferTLVReader & tlvReader, ParseSigma1Param & output)
21952192
{
21962193
using namespace TLV;
21972194

‎src/protocols/secure_channel/CASESession.h

+25-7
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@
5151

5252
namespace chip {
5353

54+
namespace Testing {
55+
class FuzzCASESession;
56+
}
57+
5458
// TODO: temporary derive from Messaging::UnsolicitedMessageHandler, actually the CASEServer should be the umh, it will be fixed
5559
// when implementing concurrent CASE session.
5660
class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
@@ -118,19 +122,35 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
118122
*/
119123
void SetGroupDataProvider(Credentials::GroupDataProvider * groupDataProvider) { mGroupDataProvider = groupDataProvider; }
120124

125+
// This struct is only serves as a base struct for EncodeSigma1 and ParseSigma1
121126
struct Sigma1Param
122127
{
123128
ByteSpan initiatorRandom;
124129
uint16_t initiatorSessionId;
125130
ByteSpan destinationId;
126-
ByteSpan initiatorEphPubKey;
127131
bool sessionResumptionRequested = false;
128-
bool InitiatorMRPParamsPresent = false;
129132
ByteSpan resumptionId;
130133
ByteSpan initiatorResumeMICSpan;
134+
};
135+
136+
struct EncodeSigma1Param : Sigma1Param
137+
{
138+
const Crypto::P256PublicKey * pEphPubKey;
139+
const ReliableMessageProtocolConfig * initiatorMrpConfig;
131140
uint8_t initiatorResume1MIC[Crypto::CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES];
132141
};
133142

143+
struct ParseSigma1Param : Sigma1Param
144+
{
145+
ByteSpan initiatorEphPubKey;
146+
bool InitiatorMRPParamsPresent = false;
147+
};
148+
149+
/*
150+
* TLV Encodes a Sigma1 message into the payload handled by msg
151+
*/
152+
CHIP_ERROR EncodeSigma1(System::PacketBufferHandle & msg, EncodeSigma1Param & InputEncodeSigma1);
153+
134154
/**
135155
* Parse a sigma1 message. This function will return success only if the
136156
* message passes schema checks. Specifically:
@@ -149,11 +169,8 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
149169
* and the resumptionID and initiatorResumeMIC outparams will be set to
150170
* valid values, or the resumptionRequested outparam will be set to false.
151171
*/
152-
CHIP_ERROR ParseSigma1(TLV::ContiguousBufferTLVReader & tlvReader, Sigma1Param & OutputParseSigma1);
153-
154-
// TODO: Add message
155-
// TODO: should i keep it as public? why is ParseSigma1 public?
156-
CHIP_ERROR EncodeSigma1(System::PacketBufferHandle & msg, Sigma1Param & encodeSigma1);
172+
CHIP_ERROR
173+
ParseSigma1(TLV::ContiguousBufferTLVReader & tlvReader, ParseSigma1Param & OutputParseSigma1);
157174

158175
/**
159176
* @brief
@@ -229,6 +246,7 @@ class DLL_EXPORT CASESession : public Messaging::UnsolicitedMessageHandler,
229246

230247
private:
231248
friend class TestCASESession;
249+
friend class Testing::FuzzCASESession;
232250

233251
using AutoReleaseSessionKey = Crypto::AutoReleaseSymmetricKey<Crypto::Aes128KeyHandle>;
234252

‎src/protocols/secure_channel/tests/TestCASESession.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -745,7 +745,7 @@ static CHIP_ERROR EncodeSigma1(MutableByteSpan & buf)
745745
\
746746
TLV::ContiguousBufferTLVReader reader; \
747747
reader.Init(buf); \
748-
CASESession::Sigma1Param parsedSigma1; \
748+
CASESession::ParseSigma1Param parsedSigma1; \
749749
CASESession session; \
750750
\
751751
EXPECT_EQ(session.ParseSigma1(reader, parsedSigma1) == CHIP_NO_ERROR, params::expectSuccess); \

0 commit comments

Comments
 (0)