Skip to content

Commit

Permalink
Fix Use of PasscodeId in the PASESession Class (#15318)
Browse files Browse the repository at this point in the history
-- The PasscodeId should be provided by the PASE Session Initiator.
  -- The PASE Session Responder should verify that PasscodeId provided
     by Initiator matches its local record.
  • Loading branch information
emargolis authored and pull[bot] committed Feb 21, 2022
1 parent a5efac6 commit 7105708
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 19 deletions.
3 changes: 2 additions & 1 deletion src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -843,7 +843,8 @@ CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, Re
exchangeCtxt = mSystemState->ExchangeMgr()->NewContext(session.Value(), &device->GetPairing());
VerifyOrExit(exchangeCtxt != nullptr, err = CHIP_ERROR_INTERNAL);

err = device->GetPairing().Pair(params.GetPeerAddress(), params.GetSetupPINCode(), keyID,
// TODO: Need to determine how PasscodeId is provided for a non-default case. i.e. ECM
err = device->GetPairing().Pair(params.GetPeerAddress(), params.GetSetupPINCode(), kDefaultCommissioningPasscodeId, keyID,
Optional<ReliableMessageProtocolConfig>::Value(GetLocalMRPConfig()), exchangeCtxt, this);
SuccessOrExit(err);

Expand Down
15 changes: 9 additions & 6 deletions src/protocols/secure_channel/PASESession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ CHIP_ERROR PASESession::WaitForPairing(const PASEVerifier & verifier, uint32_t p
mIterationCount = pbkdf2IterCount;
mNextExpectedMsg = MsgType::PBKDFParamRequest;
mPairingComplete = false;
mPasscodeID = kDefaultCommissioningPasscodeId;
mPasscodeID = passcodeID;
mLocalMRPConfig = mrpConfig;

SetPeerNodeId(NodeIdFromPAKEKeyId(mPasscodeID));
Expand All @@ -321,9 +321,9 @@ CHIP_ERROR PASESession::WaitForPairing(const PASEVerifier & verifier, uint32_t p
return err;
}

CHIP_ERROR PASESession::Pair(const Transport::PeerAddress peerAddress, uint32_t peerSetUpPINCode, uint16_t mySessionId,
Optional<ReliableMessageProtocolConfig> mrpConfig, Messaging::ExchangeContext * exchangeCtxt,
SessionEstablishmentDelegate * delegate)
CHIP_ERROR PASESession::Pair(const Transport::PeerAddress peerAddress, uint32_t peerSetUpPINCode, PasscodeId passcodeID,
uint16_t mySessionId, Optional<ReliableMessageProtocolConfig> mrpConfig,
Messaging::ExchangeContext * exchangeCtxt, SessionEstablishmentDelegate * delegate)
{
TRACE_EVENT_SCOPE("Pair", "PASESession");
ReturnErrorCodeIf(exchangeCtxt == nullptr, CHIP_ERROR_INVALID_ARGUMENT);
Expand All @@ -334,9 +334,10 @@ CHIP_ERROR PASESession::Pair(const Transport::PeerAddress peerAddress, uint32_t
mExchangeCtxt->SetResponseTimeout(kSpake2p_Response_Timeout + mExchangeCtxt->GetSessionHandle()->GetAckTimeout());

SetPeerAddress(peerAddress);
SetPeerNodeId(NodeIdFromPAKEKeyId(mPasscodeID));

mLocalMRPConfig = mrpConfig;
mPasscodeID = passcodeID;
SetPeerNodeId(NodeIdFromPAKEKeyId(mPasscodeID));

err = SendPBKDFParamRequest();
SuccessOrExit(err);
Expand Down Expand Up @@ -431,6 +432,7 @@ CHIP_ERROR PASESession::HandlePBKDFParamRequest(System::PacketBufferHandle && ms
uint8_t initiatorRandom[kPBKDFParamRandomNumberSize];

uint32_t decodeTagIdSeq = 0;
PasscodeId passcodeId = kDefaultCommissioningPasscodeId;
bool hasPBKDFParameters = false;

ChipLogDetail(SecureChannel, "Received PBKDF param request");
Expand All @@ -454,7 +456,8 @@ CHIP_ERROR PASESession::HandlePBKDFParamRequest(System::PacketBufferHandle && ms

SuccessOrExit(err = tlvReader.Next());
VerifyOrExit(TLV::TagNumFromTag(tlvReader.GetTag()) == ++decodeTagIdSeq, err = CHIP_ERROR_INVALID_TLV_TAG);
SuccessOrExit(err = tlvReader.Get(mPasscodeID));
SuccessOrExit(err = tlvReader.Get(passcodeId));
VerifyOrExit(passcodeId == mPasscodeID, err = CHIP_ERROR_INVALID_PASE_PARAMETER);

SuccessOrExit(err = tlvReader.Next());
VerifyOrExit(TLV::TagNumFromTag(tlvReader.GetTag()) == ++decodeTagIdSeq, err = CHIP_ERROR_INVALID_TLV_TAG);
Expand Down
7 changes: 4 additions & 3 deletions src/protocols/secure_channel/PASESession.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ class DLL_EXPORT PASESession : public Messaging::ExchangeDelegate, public Pairin
*
* @param peerAddress Address of peer to pair
* @param peerSetUpPINCode Setup PIN code of the peer device
* @param passcodeID Passcode ID assigned by the administrator to this PASE verifier
* @param mySessionId Session ID to be assigned to the secure session on the peer node
* @param exchangeCtxt The exchange context to send and receive messages with the peer
* Note: It's expected that the caller of this API hands over the
Expand All @@ -124,9 +125,9 @@ class DLL_EXPORT PASESession : public Messaging::ExchangeDelegate, public Pairin
*
* @return CHIP_ERROR The result of initialization
*/
CHIP_ERROR Pair(const Transport::PeerAddress peerAddress, uint32_t peerSetUpPINCode, uint16_t mySessionId,
Optional<ReliableMessageProtocolConfig> mrpConfig, Messaging::ExchangeContext * exchangeCtxt,
SessionEstablishmentDelegate * delegate);
CHIP_ERROR Pair(const Transport::PeerAddress peerAddress, uint32_t peerSetUpPINCode, PasscodeId passcodeID,
uint16_t mySessionId, Optional<ReliableMessageProtocolConfig> mrpConfig,
Messaging::ExchangeContext * exchangeCtxt, SessionEstablishmentDelegate * delegate);

/**
* @brief
Expand Down
21 changes: 12 additions & 9 deletions src/protocols/secure_channel/tests/TestPASESession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,13 +163,15 @@ void SecurePairingStartTest(nlTestSuite * inSuite, void * inContext)
ExchangeContext * context = ctx.NewUnauthenticatedExchangeToBob(&pairing);

NL_TEST_ASSERT(inSuite,
pairing.Pair(Transport::PeerAddress(Transport::Type::kBle), sTestSpake2p01_PinCode, 0,
Optional<ReliableMessageProtocolConfig>::Missing(), nullptr, nullptr) != CHIP_NO_ERROR);
pairing.Pair(Transport::PeerAddress(Transport::Type::kBle), sTestSpake2p01_PinCode,
kDefaultCommissioningPasscodeId, 0, Optional<ReliableMessageProtocolConfig>::Missing(), nullptr,
nullptr) != CHIP_NO_ERROR);

gLoopback.Reset();
NL_TEST_ASSERT(inSuite,
pairing.Pair(Transport::PeerAddress(Transport::Type::kBle), sTestSpake2p01_PinCode, 0,
Optional<ReliableMessageProtocolConfig>::Missing(), context, &delegate) == CHIP_NO_ERROR);
pairing.Pair(Transport::PeerAddress(Transport::Type::kBle), sTestSpake2p01_PinCode,
kDefaultCommissioningPasscodeId, 0, Optional<ReliableMessageProtocolConfig>::Missing(), context,
&delegate) == CHIP_NO_ERROR);
ctx.DrainAndServiceIO();

NL_TEST_ASSERT(inSuite, gLoopback.mSentMessageCount == 1);
Expand All @@ -186,8 +188,8 @@ void SecurePairingStartTest(nlTestSuite * inSuite, void * inContext)
PASESession pairing1;
ExchangeContext * context1 = ctx.NewUnauthenticatedExchangeToBob(&pairing1);
NL_TEST_ASSERT(inSuite,
pairing1.Pair(Transport::PeerAddress(Transport::Type::kBle), sTestSpake2p01_PinCode, 0,
Optional<ReliableMessageProtocolConfig>::Missing(), context1,
pairing1.Pair(Transport::PeerAddress(Transport::Type::kBle), sTestSpake2p01_PinCode,
kDefaultCommissioningPasscodeId, 0, Optional<ReliableMessageProtocolConfig>::Missing(), context1,
&delegate) == CHIP_ERROR_BAD_REQUEST);
ctx.DrainAndServiceIO();

Expand Down Expand Up @@ -232,8 +234,9 @@ void SecurePairingHandshakeTestCommon(nlTestSuite * inSuite, void * inContext, P
ctx.DrainAndServiceIO();

NL_TEST_ASSERT(inSuite,
pairingCommissioner.Pair(Transport::PeerAddress(Transport::Type::kBle), sTestSpake2p01_PinCode, 0,
mrpCommissionerConfig, contextCommissioner, &delegateCommissioner) == CHIP_NO_ERROR);
pairingCommissioner.Pair(Transport::PeerAddress(Transport::Type::kBle), sTestSpake2p01_PinCode,
kDefaultCommissioningPasscodeId, 0, mrpCommissionerConfig, contextCommissioner,
&delegateCommissioner) == CHIP_NO_ERROR);
ctx.DrainAndServiceIO();

while (gLoopback.mMessageDropped)
Expand Down Expand Up @@ -360,7 +363,7 @@ void SecurePairingFailedHandshake(nlTestSuite * inSuite, void * inContext)
ctx.DrainAndServiceIO();

NL_TEST_ASSERT(inSuite,
pairingCommissioner.Pair(Transport::PeerAddress(Transport::Type::kBle), 4321, 0,
pairingCommissioner.Pair(Transport::PeerAddress(Transport::Type::kBle), 4321, kDefaultCommissioningPasscodeId, 0,
Optional<ReliableMessageProtocolConfig>::Missing(), contextCommissioner,
&delegateCommissioner) == CHIP_NO_ERROR);
ctx.DrainAndServiceIO();
Expand Down

0 comments on commit 7105708

Please sign in to comment.