Skip to content

Commit

Permalink
Address review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
@yufengwangca
yufengwangca committed Feb 8, 2021
1 parent 16fd9bb commit b949ac5
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 48 deletions.
6 changes: 3 additions & 3 deletions src/messaging/ExchangeContext.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,9 @@ class DLL_EXPORT ExchangeContext : public ReferenceCounted<ExchangeContext, Exch

uint16_t GetExchangeId() const { return mExchangeId; }

void SetChallenge(const uint8_t * value) { memcpy(&mChallenage[0], value, kMsgCounterChallengeSize); }
void SetChallenge(const uint8_t * value) { memcpy(&mChallenge[0], value, kMsgCounterChallengeSize); }

const uint8_t * GetChallenge() const { return mChallenage; }
const uint8_t * GetChallenge() const { return mChallenge; }

SecureSessionHandle GetSecureSessionHandle() const { return mSecureSession; }

Expand Down Expand Up @@ -190,7 +190,7 @@ class DLL_EXPORT ExchangeContext : public ReferenceCounted<ExchangeContext, Exch

// [TODO: #4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual
// actions with a delegate pattern.
uint8_t mChallenage[kMsgCounterChallengeSize]; // Challenge number to identify the sychronization request cryptographically.
uint8_t mChallenge[kMsgCounterChallengeSize]; // Challenge number to identify the sychronization request cryptographically.

BitFlags<uint16_t, ExFlagValues> mFlags; // Internal state flags

Expand Down
4 changes: 2 additions & 2 deletions src/messaging/ExchangeMgr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ class DLL_EXPORT ExchangeManager : public SecureSessionMgrDelegate

ReliableMessageMgr * GetReliableMessageMgr() { return &mReliableMessageMgr; };

SecureChannel::SecureChannelMgr * GetSecureChannelMgr() { return &mSecureChannelMgr; };
Protocols::SecureChannel::SecureChannelMgr * GetSecureChannelMgr() { return &mSecureChannelMgr; };

size_t GetContextsInUse() const { return mContextsInUse; }

Expand All @@ -200,7 +200,7 @@ class DLL_EXPORT ExchangeManager : public SecureSessionMgrDelegate
State mState;
SecureSessionMgr * mSessionMgr;
ReliableMessageMgr mReliableMessageMgr;
SecureChannel::SecureChannelMgr mSecureChannelMgr;
Protocols::SecureChannel::SecureChannelMgr mSecureChannelMgr;

std::array<ExchangeContext, CHIP_CONFIG_MAX_EXCHANGE_CONTEXTS> mContextPool;
size_t mContextsInUse;
Expand Down
64 changes: 31 additions & 33 deletions src/messaging/SecureChannelMgr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,9 @@
#include <support/logging/CHIPLogging.h>

namespace chip {
namespace Protocols {
namespace SecureChannel {

SecureChannelMgr::SecureChannelMgr()
{
mExchangeMgr = nullptr;
}

CHIP_ERROR SecureChannelMgr::Init(Messaging::ExchangeManager * exchangeMgr)
{
CHIP_ERROR err = CHIP_NO_ERROR;
Expand All @@ -66,52 +62,52 @@ void SecureChannelMgr::Shutdown()
}
}

void SecureChannelMgr::OnMessageReceived(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader,
void SecureChannelMgr::OnMessageReceived(Messaging::ExchangeContext * exchangeContext, const PacketHeader & packetHeader,
const PayloadHeader & payloadHeader, System::PacketBufferHandle msgBuf)
{
if (payloadHeader.HasMessageType(Protocols::SecureChannel::MsgType::MsgCounterSyncReq))
{
HandleMsgCounterSyncReq(ec, packetHeader, std::move(msgBuf));
HandleMsgCounterSyncReq(exchangeContext, packetHeader, std::move(msgBuf));
}
else if (payloadHeader.HasMessageType(Protocols::SecureChannel::MsgType::MsgCounterSyncRsp))
{
HandleMsgCounterSyncResp(ec, packetHeader, std::move(msgBuf));
HandleMsgCounterSyncResp(exchangeContext, packetHeader, std::move(msgBuf));
}
}

void SecureChannelMgr::OnResponseTimeout(Messaging::ExchangeContext * ec)
void SecureChannelMgr::OnResponseTimeout(Messaging::ExchangeContext * exchangeContext)
{
// Close the exchange if MsgCounterSyncRsp is not received before kMsgCounterSyncTimeout.
if (ec != nullptr)
ec->Close();
if (exchangeContext != nullptr)
exchangeContext->Close();
}

// Create and initialize new exchange for the message counter synchronization request/response messages.
CHIP_ERROR SecureChannelMgr::NewMsgCounterSyncExchange(SecureSessionHandle session, Messaging::ExchangeContext *& ec)
CHIP_ERROR SecureChannelMgr::NewMsgCounterSyncExchange(SecureSessionHandle session, Messaging::ExchangeContext *& exchangeContext)
{
CHIP_ERROR err = CHIP_NO_ERROR;

// Message counter synchronization protocol is only applicable for application group keys.
VerifyOrExit(ChipKeyId::IsAppGroupKey(session.GetPeerKeyId()), err = CHIP_ERROR_INVALID_ARGUMENT);

// Create new exchange context.
ec = mExchangeMgr->NewContext(session, this);
VerifyOrExit(ec != nullptr, err = CHIP_ERROR_NO_MEMORY);
exchangeContext = mExchangeMgr->NewContext(session, this);
VerifyOrExit(exchangeContext != nullptr, err = CHIP_ERROR_NO_MEMORY);

exit:
return err;
}

CHIP_ERROR SecureChannelMgr::SendMsgCounterSyncReq(SecureSessionHandle session)
{
CHIP_ERROR err = CHIP_NO_ERROR;
Messaging::ExchangeContext * ec = nullptr;
CHIP_ERROR err = CHIP_NO_ERROR;
Messaging::ExchangeContext * exchangeContext = nullptr;
System::PacketBufferHandle msgBuf;
Messaging::SendFlags sendFlags;
uint8_t challenge[kMsgCounterChallengeSize];

// Create and initialize new exchange.
err = NewMsgCounterSyncExchange(session, ec);
err = NewMsgCounterSyncExchange(session, exchangeContext);
SuccessOrExit(err);

// Allocate a buffer for the null message.
Expand All @@ -123,7 +119,7 @@ CHIP_ERROR SecureChannelMgr::SendMsgCounterSyncReq(SecureSessionHandle session)
SuccessOrExit(err);

// Store generated Challenge value to ExchangeContext to resolve synchronization response.
ec->SetChallenge(challenge);
exchangeContext->SetChallenge(challenge);

memcpy(msgBuf->Start(), challenge, kMsgCounterChallengeSize);
msgBuf->SetDataLength(kMsgCounterChallengeSize);
Expand All @@ -132,10 +128,10 @@ CHIP_ERROR SecureChannelMgr::SendMsgCounterSyncReq(SecureSessionHandle session)
sendFlags.Set(Messaging::SendMessageFlags::kNoAutoRequestAck, true).Set(Messaging::SendMessageFlags::kExpectResponse, true);

// Arm a timer to enforce that a MsgCounterSyncRsp is received before kMsgCounterSyncTimeout.
ec->SetResponseTimeout(kMsgCounterSyncTimeout);
exchangeContext->SetResponseTimeout(kMsgCounterSyncTimeout);

// Send the message counter synchronization request in a Secure Channel Protocol::MsgCounterSyncReq message.
err = ec->SendMessage(Protocols::SecureChannel::MsgType::MsgCounterSyncReq, std::move(msgBuf), sendFlags);
err = exchangeContext->SendMessage(Protocols::SecureChannel::MsgType::MsgCounterSyncReq, std::move(msgBuf), sendFlags);
SuccessOrExit(err);

exit:
Expand All @@ -147,7 +143,7 @@ CHIP_ERROR SecureChannelMgr::SendMsgCounterSyncReq(SecureSessionHandle session)
return err;
}

CHIP_ERROR SecureChannelMgr::SendMsgCounterSyncResp(Messaging::ExchangeContext * ec, SecureSessionHandle session)
CHIP_ERROR SecureChannelMgr::SendMsgCounterSyncResp(Messaging::ExchangeContext * exchangeContext, SecureSessionHandle session)
{
CHIP_ERROR err = CHIP_NO_ERROR;
Transport::PeerConnectionState * state = nullptr;
Expand All @@ -171,7 +167,7 @@ CHIP_ERROR SecureChannelMgr::SendMsgCounterSyncResp(Messaging::ExchangeContext *
bbuf.Put32(state->GetSendMessageIndex());

// Fill in the random value
bbuf.Put(ec->GetChallenge(), kMsgCounterChallengeSize);
bbuf.Put(exchangeContext->GetChallenge(), kMsgCounterChallengeSize);

VerifyOrExit(bbuf.Fit(), err = CHIP_ERROR_NO_MEMORY);
}
Expand All @@ -180,8 +176,8 @@ CHIP_ERROR SecureChannelMgr::SendMsgCounterSyncResp(Messaging::ExchangeContext *
msgBuf->SetDataLength(kMsgCounterSyncRespMsgSize);

// Send message counter synchronization response message.
err = ec->SendMessage(Protocols::SecureChannel::MsgType::MsgCounterSyncRsp, std::move(msgBuf),
Messaging::SendFlags(Messaging::SendMessageFlags::kNoAutoRequestAck));
err = exchangeContext->SendMessage(Protocols::SecureChannel::MsgType::MsgCounterSyncRsp, std::move(msgBuf),
Messaging::SendFlags(Messaging::SendMessageFlags::kNoAutoRequestAck));

exit:
if (err != CHIP_NO_ERROR)
Expand All @@ -192,7 +188,7 @@ CHIP_ERROR SecureChannelMgr::SendMsgCounterSyncResp(Messaging::ExchangeContext *
return err;
}

void SecureChannelMgr::HandleMsgCounterSyncReq(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader,
void SecureChannelMgr::HandleMsgCounterSyncReq(Messaging::ExchangeContext * exchangeContext, const PacketHeader & packetHeader,
System::PacketBufferHandle msgBuf)
{
CHIP_ERROR err = CHIP_NO_ERROR;
Expand All @@ -208,24 +204,24 @@ void SecureChannelMgr::HandleMsgCounterSyncReq(Messaging::ExchangeContext * ec,
VerifyOrExit(reqlen == kMsgCounterChallengeSize, err = CHIP_ERROR_INVALID_MESSAGE_LENGTH);

// Store the 64-bit value sent in the Challenge filed of the MsgCounterSyncReq.
ec->SetChallenge(req);
exchangeContext->SetChallenge(req);

// Respond with MsgCounterSyncResp
err = SendMsgCounterSyncResp(ec, { packetHeader.GetSourceNodeId().Value(), packetHeader.GetEncryptionKeyID() });
err = SendMsgCounterSyncResp(exchangeContext, { packetHeader.GetSourceNodeId().Value(), packetHeader.GetEncryptionKeyID() });

exit:
if (err != CHIP_NO_ERROR)
{
ChipLogError(ExchangeManager, "Failed to handle MsgCounterSyncReq message with error:%s", ErrorStr(err));
}

if (ec != nullptr)
ec->Close();
if (exchangeContext != nullptr)
exchangeContext->Close();

return;
}

void SecureChannelMgr::HandleMsgCounterSyncResp(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader,
void SecureChannelMgr::HandleMsgCounterSyncResp(Messaging::ExchangeContext * exchangeContext, const PacketHeader & packetHeader,
System::PacketBufferHandle msgBuf)
{
CHIP_ERROR err = CHIP_NO_ERROR;
Expand All @@ -249,7 +245,8 @@ void SecureChannelMgr::HandleMsgCounterSyncResp(Messaging::ExchangeContext * ec,
memcpy(challenge, resp, kMsgCounterChallengeSize);

// Verify that the response field matches the expected Challenge field for the exchange.
VerifyOrExit(memcmp(ec->GetChallenge(), challenge, kMsgCounterChallengeSize) == 0, err = CHIP_ERROR_INVALID_SIGNATURE);
VerifyOrExit(memcmp(exchangeContext->GetChallenge(), challenge, kMsgCounterChallengeSize) == 0,
err = CHIP_ERROR_INVALID_SIGNATURE);

// ToDo:(#4628)Initialize/synchronize peer's message counter to FabricState.
VerifyOrExit(syncCounter != 0, err = CHIP_ERROR_READ_FAILED);
Expand All @@ -260,11 +257,12 @@ void SecureChannelMgr::HandleMsgCounterSyncResp(Messaging::ExchangeContext * ec,
ChipLogError(ExchangeManager, "Failed to handle MsgCounterSyncResp message with error:%s", ErrorStr(err));
}

if (ec != nullptr)
ec->Close();
if (exchangeContext != nullptr)
exchangeContext->Close();

return;
}

} // namespace SecureChannel
} // namespace Protocols
} // namespace chip
18 changes: 10 additions & 8 deletions src/messaging/SecureChannelMgr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <system/SystemPacketBuffer.h>

namespace chip {
namespace Protocols {
namespace SecureChannel {

constexpr uint16_t kMsgCounterSyncRespMsgSize = 12; // The size of the message counter synchronization response message.
Expand All @@ -36,7 +37,7 @@ class ExchangeManager;
class SecureChannelMgr : public Messaging::ExchangeDelegate
{
public:
SecureChannelMgr();
SecureChannelMgr() : mExchangeMgr(nullptr) {}

CHIP_ERROR Init(Messaging::ExchangeManager * exchangeMgr);
void Shutdown();
Expand All @@ -58,21 +59,22 @@ class SecureChannelMgr : public Messaging::ExchangeDelegate
private:
Messaging::ExchangeManager * mExchangeMgr; // [READ ONLY] Associated Exchange Manager object.

CHIP_ERROR NewMsgCounterSyncExchange(SecureSessionHandle session, Messaging::ExchangeContext *& ec);
CHIP_ERROR NewMsgCounterSyncExchange(SecureSessionHandle session, Messaging::ExchangeContext *& exchangeContext);

CHIP_ERROR SendMsgCounterSyncResp(Messaging::ExchangeContext * ec, SecureSessionHandle session);
CHIP_ERROR SendMsgCounterSyncResp(Messaging::ExchangeContext * exchangeContext, SecureSessionHandle session);

void HandleMsgCounterSyncReq(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader,
void HandleMsgCounterSyncReq(Messaging::ExchangeContext * exchangeContext, const PacketHeader & packetHeader,
System::PacketBufferHandle msgBuf);

void HandleMsgCounterSyncResp(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader,
void HandleMsgCounterSyncResp(Messaging::ExchangeContext * exchangeContext, const PacketHeader & packetHeader,
System::PacketBufferHandle msgBuf);

void OnMessageReceived(Messaging::ExchangeContext * ec, const PacketHeader & packetHeader, const PayloadHeader & payloadHeader,
System::PacketBufferHandle payload) override;
void OnMessageReceived(Messaging::ExchangeContext * exchangeContext, const PacketHeader & packetHeader,
const PayloadHeader & payloadHeader, System::PacketBufferHandle payload) override;

void OnResponseTimeout(Messaging::ExchangeContext * ec) override;
void OnResponseTimeout(Messaging::ExchangeContext * exchangeContext) override;
};

} // namespace SecureChannel
} // namespace Protocols
} // namespace chip
4 changes: 2 additions & 2 deletions src/messaging/tests/TestSecureChannelMgr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ void CheckSendMsgCounterSyncReq(nlTestSuite * inSuite, void * inContext)
testExchangeMgr.mSuite = inSuite;
ctx.GetSecureSessionManager().SetDelegate(&testExchangeMgr);

chip::SecureChannel::SecureChannelMgr * sm = ctx.GetExchangeManager().GetSecureChannelMgr();
chip::Protocols::SecureChannel::SecureChannelMgr * sm = ctx.GetExchangeManager().GetSecureChannelMgr();
NL_TEST_ASSERT(inSuite, sm != nullptr);

Optional<Transport::PeerAddress> peer(Transport::PeerAddress::UDP(addr, CHIP_PORT));
Expand Down Expand Up @@ -180,7 +180,7 @@ void CheckReceiveMsgCounterSyncReq(nlTestSuite * inSuite, void * inContext)

mockAppDelegate.mSuite = inSuite;

chip::SecureChannel::SecureChannelMgr * sm = ctx.GetExchangeManager().GetSecureChannelMgr();
chip::Protocols::SecureChannel::SecureChannelMgr * sm = ctx.GetExchangeManager().GetSecureChannelMgr();
NL_TEST_ASSERT(inSuite, sm != nullptr);

// Register to receive unsolicited Secure Channel Request messages from the exchange manager.
Expand Down

0 comments on commit b949ac5

Please sign in to comment.