Skip to content

Commit

Permalink
Remove Peer/local nodeid dependency for PASE (project-chip#4945)
Browse files Browse the repository at this point in the history 
* Removed nodeid (local and remote) fromo PASE connection - PASE should have no concept of nodeids as at that time the nodes do not yet belong to the fabric

* Fix test suite compilation

* Cleanup some more node id logic

* Do not send an undefined key ID as part of the destination for secure message headers

* Updated todo a bit

* Update rendezvous session to keep using nodeid in messages for admininfo. Fixes cirque but now logic is highlighted as subject to change
  • Loading branch information
@andy31415
andy31415 authored Feb 23, 2021
1 parent 9059556 commit 7c09658
Show file tree
Hide file tree
Showing 13 changed files with 109 additions and 166 deletions.
3 changes: 1 addition & 2 deletions src/app/tests/integration/chip_im_initiator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,7 @@ CHIP_ERROR EstablishSecureSession()
{
CHIP_ERROR err = CHIP_NO_ERROR;

chip::SecurePairingUsingTestSecret * testSecurePairingSecret = chip::Platform::New<chip::SecurePairingUsingTestSecret>(
chip::Optional<chip::NodeId>::Value(chip::kTestDeviceNodeId), static_cast<uint16_t>(0), static_cast<uint16_t>(0));
chip::SecurePairingUsingTestSecret * testSecurePairingSecret = chip::Platform::New<chip::SecurePairingUsingTestSecret>();
VerifyOrExit(testSecurePairingSecret != nullptr, err = CHIP_ERROR_NO_MEMORY);

// Attempt to connect to the peer.
Expand Down
3 changes: 1 addition & 2 deletions src/controller/CHIPDeviceController.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -652,8 +652,7 @@ CHIP_ERROR DeviceCommissioner::PairTestDeviceWithoutSecurity(NodeId remoteDevice
VerifyOrExit(mState == State::Initialized, err = CHIP_ERROR_INCORRECT_STATE);
VerifyOrExit(mDeviceBeingPaired == kNumMaxActiveDevices, err = CHIP_ERROR_INCORRECT_STATE);

testSecurePairingSecret = chip::Platform::New<SecurePairingUsingTestSecret>(Optional<NodeId>::Value(remoteDeviceId),
static_cast<uint16_t>(0), static_cast<uint16_t>(0));
testSecurePairingSecret = chip::Platform::New<SecurePairingUsingTestSecret>();
VerifyOrExit(testSecurePairingSecret != nullptr, err = CHIP_ERROR_NO_MEMORY);

mDeviceBeingPaired = GetInactiveDeviceIndex();
Expand Down
5 changes: 2 additions & 3 deletions src/messaging/tests/MessagingContext.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,8 @@ class MessagingContext : public IOContext
{
public:
MessagingContext() :
mPeer(Transport::PeerAddress::UDP(GetAddress(), CHIP_PORT)),
mPairingPeerToLocal(Optional<NodeId>::Value(GetSourceNodeId()), GetLocalKeyId(), GetPeerKeyId()),
mPairingLocalToPeer(Optional<NodeId>::Value(GetDestinationNodeId()), GetPeerKeyId(), GetLocalKeyId())
mPeer(Transport::PeerAddress::UDP(GetAddress(), CHIP_PORT)), mPairingPeerToLocal(GetLocalKeyId(), GetPeerKeyId()),
mPairingLocalToPeer(GetPeerKeyId(), GetLocalKeyId())
{}

/// Initialize the underlying layers and test suite pointer
Expand Down
12 changes: 4 additions & 8 deletions src/messaging/tests/TestMessageCounterSyncMgr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,15 +144,13 @@ void CheckSendMsgCounterSyncReq(nlTestSuite * inSuite, void * inContext)

Optional<Transport::PeerAddress> peer(Transport::PeerAddress::UDP(addr, CHIP_PORT));

SecurePairingUsingTestSecret pairingLocalToPeer(Optional<NodeId>::Value(kDestinationNodeId), kTestPeerGroupKeyId,
kTestLocalGroupKeyId);
SecurePairingUsingTestSecret pairingLocalToPeer(kTestPeerGroupKeyId, kTestLocalGroupKeyId);

err = ctx.GetSecureSessionManager().NewPairing(peer, kDestinationNodeId, &pairingLocalToPeer,
SecureSessionMgr::PairingDirection::kInitiator, 0);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

SecurePairingUsingTestSecret pairingPeerToLocal(Optional<NodeId>::Value(kSourceNodeId), kTestLocalGroupKeyId,
kTestPeerGroupKeyId);
SecurePairingUsingTestSecret pairingPeerToLocal(kTestLocalGroupKeyId, kTestPeerGroupKeyId);

err = ctx.GetSecureSessionManager().NewPairing(peer, kSourceNodeId, &pairingPeerToLocal,
SecureSessionMgr::PairingDirection::kResponder, 1);
Expand Down Expand Up @@ -192,15 +190,13 @@ void CheckReceiveMsgCounterSyncReq(nlTestSuite * inSuite, void * inContext)

Optional<Transport::PeerAddress> peer(Transport::PeerAddress::UDP(addr, CHIP_PORT));

SecurePairingUsingTestSecret pairingLocalToPeer(Optional<NodeId>::Value(kDestinationNodeId), kTestPeerGroupKeyId,
kTestLocalGroupKeyId);
SecurePairingUsingTestSecret pairingLocalToPeer(kTestPeerGroupKeyId, kTestLocalGroupKeyId);

err = ctx.GetSecureSessionManager().NewPairing(peer, kDestinationNodeId, &pairingLocalToPeer,
SecureSessionMgr::PairingDirection::kInitiator, 0);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

SecurePairingUsingTestSecret pairingPeerToLocal(Optional<NodeId>::Value(kSourceNodeId), kTestLocalGroupKeyId,
kTestPeerGroupKeyId);
SecurePairingUsingTestSecret pairingPeerToLocal(kTestLocalGroupKeyId, kTestPeerGroupKeyId);

err = ctx.GetSecureSessionManager().NewPairing(peer, kSourceNodeId, &pairingPeerToLocal,
SecureSessionMgr::PairingDirection::kResponder, 1);
Expand Down
3 changes: 1 addition & 2 deletions src/messaging/tests/echo/echo_requester.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,7 @@ CHIP_ERROR EstablishSecureSession()
CHIP_ERROR err = CHIP_NO_ERROR;

chip::Optional<chip::Transport::PeerAddress> peerAddr;
chip::SecurePairingUsingTestSecret * testSecurePairingSecret = chip::Platform::New<chip::SecurePairingUsingTestSecret>(
chip::Optional<chip::NodeId>::Value(chip::kTestDeviceNodeId), static_cast<uint16_t>(0), static_cast<uint16_t>(0));
chip::SecurePairingUsingTestSecret * testSecurePairingSecret = chip::Platform::New<chip::SecurePairingUsingTestSecret>();
VerifyOrExit(testSecurePairingSecret != nullptr, err = CHIP_ERROR_NO_MEMORY);

if (gUseTCP)
Expand Down
3 changes: 1 addition & 2 deletions src/messaging/tests/echo/echo_responder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ chip::Protocols::Echo::EchoServer gEchoServer;
chip::TransportMgr<chip::Transport::UDP> gUDPManager;
chip::TransportMgr<chip::Transport::TCP<kMaxTcpActiveConnectionCount, kMaxTcpPendingPackets>> gTCPManager;
chip::SecureSessionMgr gSessionManager;
chip::SecurePairingUsingTestSecret gTestPairing(chip::Optional<chip::NodeId>::Value(chip::kUndefinedNodeId),
static_cast<uint16_t>(0), static_cast<uint16_t>(0));
chip::SecurePairingUsingTestSecret gTestPairing;

// Callback handler when a CHIP EchoRequest is received.
void HandleEchoRequestReceived(chip::Messaging::ExchangeContext * ec, chip::System::PacketBufferHandle payload)
Expand Down
72 changes: 13 additions & 59 deletions src/transport/PASESession.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ void PASESession::Clear()
chip::Platform::MemoryFree(mSalt);
mSalt = nullptr;
}
mLocalNodeId = kUndefinedNodeId;
mKeLen = sizeof(mKe);
mPairingComplete = false;
mComputeVerifier = true;
Expand Down Expand Up @@ -138,16 +137,11 @@ CHIP_ERROR PASESession::ToSerializable(PASESessionSerializable & serializable)
{
CHIP_ERROR error = CHIP_NO_ERROR;

const NodeId peerNodeId = mConnectionState.GetPeerNodeId();
VerifyOrExit(CanCastTo<uint16_t>(mKeLen), error = CHIP_ERROR_INTERNAL);
VerifyOrExit(CanCastTo<uint64_t>(mLocalNodeId), error = CHIP_ERROR_INTERNAL);
VerifyOrExit(CanCastTo<uint64_t>(peerNodeId), error = CHIP_ERROR_INTERNAL);

memset(&serializable, 0, sizeof(serializable));
serializable.mKeLen = static_cast<uint16_t>(mKeLen);
serializable.mPairingComplete = (mPairingComplete) ? 1 : 0;
serializable.mLocalNodeId = mLocalNodeId;
serializable.mPeerNodeId = peerNodeId;
serializable.mLocalKeyId = mConnectionState.GetLocalKeyID();
serializable.mPeerKeyId = mConnectionState.GetPeerKeyID();

Expand All @@ -168,17 +162,14 @@ CHIP_ERROR PASESession::FromSerializable(const PASESessionSerializable & seriali
memset(mKe, 0, sizeof(mKe));
memcpy(mKe, serializable.mKe, mKeLen);

mLocalNodeId = serializable.mLocalNodeId;
mConnectionState.SetPeerNodeId(serializable.mPeerNodeId);
mConnectionState.SetLocalKeyID(serializable.mLocalKeyId);
mConnectionState.SetPeerKeyID(serializable.mPeerKeyId);

exit:
return error;
}

CHIP_ERROR PASESession::Init(Optional<NodeId> myNodeId, uint16_t myKeyId, uint32_t setupCode,
SessionEstablishmentDelegate * delegate)
CHIP_ERROR PASESession::Init(uint16_t myKeyId, uint32_t setupCode, SessionEstablishmentDelegate * delegate)
{
CHIP_ERROR err = CHIP_NO_ERROR;

Expand All @@ -193,8 +184,7 @@ CHIP_ERROR PASESession::Init(Optional<NodeId> myNodeId, uint16_t myKeyId, uint32
err = mCommissioningHash.AddData(Uint8::from_const_char(kSpake2pContext), strlen(kSpake2pContext));
SuccessOrExit(err);

mDelegate = delegate;
mLocalNodeId = myNodeId.ValueOr(kUndefinedNodeId);
mDelegate = delegate;
mConnectionState.SetLocalKeyID(myKeyId);
mSetupPINCode = setupCode;
mComputeVerifier = true;
Expand Down Expand Up @@ -256,14 +246,14 @@ CHIP_ERROR PASESession::SetupSpake2p(uint32_t pbkdf2IterCount, const uint8_t * s
}

CHIP_ERROR PASESession::WaitForPairing(uint32_t mySetUpPINCode, uint32_t pbkdf2IterCount, const uint8_t * salt, size_t saltLen,
Optional<NodeId> myNodeId, uint16_t myKeyId, SessionEstablishmentDelegate * delegate)
uint16_t myKeyId, SessionEstablishmentDelegate * delegate)
{
CHIP_ERROR err = CHIP_NO_ERROR;

VerifyOrExit(salt != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrExit(saltLen > 0, err = CHIP_ERROR_INVALID_ARGUMENT);

err = Init(myNodeId, myKeyId, mySetUpPINCode, delegate);
err = Init(myKeyId, mySetUpPINCode, delegate);
SuccessOrExit(err);

VerifyOrExit(CanCastTo<uint16_t>(saltLen), err = CHIP_ERROR_INVALID_ARGUMENT);
Expand Down Expand Up @@ -295,11 +285,10 @@ CHIP_ERROR PASESession::WaitForPairing(uint32_t mySetUpPINCode, uint32_t pbkdf2I
return err;
}

CHIP_ERROR PASESession::WaitForPairing(const PASEVerifier & verifier, Optional<NodeId> myNodeId, uint16_t myKeyId,
SessionEstablishmentDelegate * delegate)
CHIP_ERROR PASESession::WaitForPairing(const PASEVerifier & verifier, uint16_t myKeyId, SessionEstablishmentDelegate * delegate)
{
CHIP_ERROR err = WaitForPairing(0, kSpake2p_Iteration_Count, reinterpret_cast<const unsigned char *>(kSpake2pKeyExchangeSalt),
strlen(kSpake2pKeyExchangeSalt), myNodeId, myKeyId, delegate);
strlen(kSpake2pKeyExchangeSalt), myKeyId, delegate);
SuccessOrExit(err);

memmove(&mPASEVerifier, verifier, sizeof(verifier));
Expand All @@ -322,25 +311,21 @@ CHIP_ERROR PASESession::AttachHeaderAndSend(Protocols::SecureChannel::MsgType ms
CHIP_ERROR err = payloadHeader.EncodeBeforeData(msgBuf);
SuccessOrExit(err);

err = mDelegate->SendSessionEstablishmentMessage(PacketHeader()
.SetSourceNodeId(mLocalNodeId)
.SetDestinationNodeId(mConnectionState.GetPeerNodeId())
.SetEncryptionKeyID(mConnectionState.GetLocalKeyID()),
err = mDelegate->SendSessionEstablishmentMessage(PacketHeader().SetEncryptionKeyID(mConnectionState.GetLocalKeyID()),
mConnectionState.GetPeerAddress(), std::move(msgBuf));
SuccessOrExit(err);

exit:
return err;
}

CHIP_ERROR PASESession::Pair(const Transport::PeerAddress peerAddress, uint32_t peerSetUpPINCode, Optional<NodeId> myNodeId,
NodeId peerNodeId, uint16_t myKeyId, SessionEstablishmentDelegate * delegate)
CHIP_ERROR PASESession::Pair(const Transport::PeerAddress peerAddress, uint32_t peerSetUpPINCode, uint16_t myKeyId,
SessionEstablishmentDelegate * delegate)
{
CHIP_ERROR err = Init(myNodeId, myKeyId, peerSetUpPINCode, delegate);
CHIP_ERROR err = Init(myKeyId, peerSetUpPINCode, delegate);
SuccessOrExit(err);

mConnectionState.SetPeerAddress(peerAddress);
mConnectionState.SetPeerNodeId(peerNodeId);

err = SendPBKDFParamRequest();
SuccessOrExit(err);
Expand All @@ -353,14 +338,13 @@ CHIP_ERROR PASESession::Pair(const Transport::PeerAddress peerAddress, uint32_t
return err;
}

CHIP_ERROR PASESession::Pair(const Transport::PeerAddress peerAddress, const PASEVerifier & verifier, Optional<NodeId> myNodeId,
NodeId peerNodeId, uint16_t myKeyId, SessionEstablishmentDelegate * delegate)
CHIP_ERROR PASESession::Pair(const Transport::PeerAddress peerAddress, const PASEVerifier & verifier, uint16_t myKeyId,
SessionEstablishmentDelegate * delegate)
{
CHIP_ERROR err = Init(myNodeId, myKeyId, 0, delegate);
CHIP_ERROR err = Init(myKeyId, 0, delegate);
SuccessOrExit(err);

mConnectionState.SetPeerAddress(peerAddress);
mConnectionState.SetPeerNodeId(peerNodeId);

memmove(&mPASEVerifier, verifier, sizeof(verifier));
mComputeVerifier = false;
Expand Down Expand Up @@ -440,11 +424,6 @@ CHIP_ERROR PASESession::HandlePBKDFParamRequest(const PacketHeader & header, con
err = mCommissioningHash.AddData(req, reqlen);
SuccessOrExit(err);

if (header.GetSourceNodeId().HasValue() && mConnectionState.GetPeerNodeId() == kUndefinedNodeId)
{
mConnectionState.SetPeerNodeId(header.GetSourceNodeId().Value());
}

err = SendPBKDFParamResponse();
SuccessOrExit(err);

Expand Down Expand Up @@ -548,11 +527,6 @@ CHIP_ERROR PASESession::HandlePBKDFParamResponse(const PacketHeader & header, co
SuccessOrExit(err);
}

if (header.GetSourceNodeId().HasValue() && mConnectionState.GetPeerNodeId() == kUndefinedNodeId)
{
mConnectionState.SetPeerNodeId(header.GetSourceNodeId().Value());
}

err = SendMsg1();
SuccessOrExit(err);

Expand Down Expand Up @@ -624,10 +598,6 @@ CHIP_ERROR PASESession::HandleMsg1_and_SendMsg2(const PacketHeader & header, con
SuccessOrExit(err);

mConnectionState.SetPeerKeyID(header.GetEncryptionKeyID());
if (header.GetSourceNodeId().HasValue() && mConnectionState.GetPeerNodeId() == kUndefinedNodeId)
{
mConnectionState.SetPeerNodeId(header.GetSourceNodeId().Value());
}

// Make sure our addition doesn't overflow.
VerifyOrExit(UINTMAX_MAX - verifier_len >= Y_len, err = CHIP_ERROR_INVALID_MESSAGE_LENGTH);
Expand Down Expand Up @@ -685,10 +655,6 @@ CHIP_ERROR PASESession::HandleMsg2_and_SendMsg3(const PacketHeader & header, con
verifier_len = static_cast<uint16_t>(verifier_len_raw);

mConnectionState.SetPeerKeyID(header.GetEncryptionKeyID());
if (header.GetSourceNodeId().HasValue() && mConnectionState.GetPeerNodeId() == kUndefinedNodeId)
{
mConnectionState.SetPeerNodeId(header.GetSourceNodeId().Value());
}

{
Encoding::PacketBufferWriter bbuf(System::PacketBufferHandle::New(verifier_len));
Expand Down Expand Up @@ -745,9 +711,6 @@ CHIP_ERROR PASESession::HandleMsg3(const PacketHeader & header, const System::Pa

VerifyOrExit(hash != nullptr, err = CHIP_ERROR_MESSAGE_INCOMPLETE);
VerifyOrExit(msg->DataLength() == kMAX_Hash_Length, err = CHIP_ERROR_INVALID_MESSAGE_LENGTH);

VerifyOrExit(header.GetSourceNodeId().ValueOr(kUndefinedNodeId) == mConnectionState.GetPeerNodeId(),
err = CHIP_ERROR_WRONG_NODE_ID);
VerifyOrExit(header.GetEncryptionKeyID() == mConnectionState.GetPeerKeyID(), err = CHIP_ERROR_INVALID_KEY_ID);

err = mSpake2p.KeyConfirm(hash, kMAX_Hash_Length);
Expand Down Expand Up @@ -830,15 +793,6 @@ CHIP_ERROR PASESession::HandlePeerMessage(const PacketHeader & packetHeader, con

mConnectionState.SetPeerAddress(peerAddress);

if (mLocalNodeId == kUndefinedNodeId)
{
mLocalNodeId = packetHeader.GetDestinationNodeId().ValueOr(kUndefinedNodeId);
}
else if (packetHeader.GetDestinationNodeId().HasValue())
{
VerifyOrExit(mLocalNodeId == packetHeader.GetDestinationNodeId().Value(), err = CHIP_ERROR_WRONG_NODE_ID);
}

switch (static_cast<Protocols::SecureChannel::MsgType>(payloadHeader.GetMessageType()))
{
case Protocols::SecureChannel::MsgType::PBKDFParamRequest:
Expand Down
Loading

0 comments on commit 7c09658

Please sign in to comment.