Skip to content

Commit

Permalink
Fix status report message when destination id is unknown. (#23858)
Browse files Browse the repository at this point in the history
* Fix status report message when destination id is unknown.

We were ignoring the fact that we failed to match the destination id, pressing
on to SendSigma2, and failing out, but that clobbered the "err" variable with a
diferent error, so we ended up sending an error status report with the wrong
protocol code.

* Address review comments
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Sep 12, 2023
1 parent f19f59c commit 17dd036
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 0 deletions.
1 change: 1 addition & 0 deletions src/protocols/secure_channel/CASESession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,7 @@ CHIP_ERROR CASESession::HandleSigma1(System::PacketBufferHandle && msg)
ChipLogError(SecureChannel, "CASE failed to match destination ID with local fabrics");
ChipLogByteSpan(SecureChannel, destinationIdentifier);
}
SuccessOrExit(err);

// ParseSigma1 ensures that:
// mRemotePubKey.Length() == initiatorPubKey.size() == kP256_PublicKey_Length.
Expand Down
84 changes: 84 additions & 0 deletions src/protocols/secure_channel/tests/TestCASESession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ class TestCASESession
#if CONFIG_BUILD_FOR_HOST_UNIT_TEST
static void SimulateUpdateNOCInvalidatePendingEstablishment(nlTestSuite * inSuite, void * inContext);
#endif // CONFIG_BUILD_FOR_HOST_UNIT_TEST
static void Sigma1BadDestinationIdTest(nlTestSuite * inSuite, void * inContext);
};

void TestCASESession::SecurePairingWaitTest(nlTestSuite * inSuite, void * inContext)
Expand Down Expand Up @@ -1005,6 +1006,88 @@ void TestCASESession::SimulateUpdateNOCInvalidatePendingEstablishment(nlTestSuit
}
#endif // CONFIG_BUILD_FOR_HOST_UNIT_TEST

namespace {
class ExpectErrorExchangeDelegate : public ExchangeDelegate
{
public:
ExpectErrorExchangeDelegate(nlTestSuite * suite, uint16_t expectedProtocolCode) :
mSuite(suite), mExpectedProtocolCode(expectedProtocolCode)
{}

private:
CHIP_ERROR OnMessageReceived(ExchangeContext * ec, const PayloadHeader & payloadHeader,
System::PacketBufferHandle && buf) override
{
using namespace SecureChannel;

NL_TEST_ASSERT(mSuite, payloadHeader.HasMessageType(MsgType::StatusReport));

SecureChannel::StatusReport statusReport;
CHIP_ERROR err = statusReport.Parse(std::move(buf));
NL_TEST_ASSERT(mSuite, err == CHIP_NO_ERROR);

NL_TEST_ASSERT(mSuite, statusReport.GetProtocolId() == SecureChannel::Id);
NL_TEST_ASSERT(mSuite, statusReport.GetGeneralCode() == GeneralStatusCode::kFailure);
NL_TEST_ASSERT(mSuite, statusReport.GetProtocolCode() == mExpectedProtocolCode);
return CHIP_NO_ERROR;
}

void OnResponseTimeout(ExchangeContext * ec) override {}

Messaging::ExchangeMessageDispatch & GetMessageDispatch() override { return SessionEstablishmentExchangeDispatch::Instance(); }

nlTestSuite * mSuite;
uint16_t mExpectedProtocolCode;
};
} // anonymous namespace

void TestCASESession::Sigma1BadDestinationIdTest(nlTestSuite * inSuite, void * inContext)
{
using SecureChannel::MsgType;

TestContext & ctx = *reinterpret_cast<TestContext *>(inContext);

SessionManager & sessionManager = ctx.GetSecureSessionManager();

constexpr size_t bufferSize = 600;
System::PacketBufferHandle data = chip::System::PacketBufferHandle::New(bufferSize);
NL_TEST_ASSERT(inSuite, !data.IsNull());

MutableByteSpan buf(data->Start(), data->AvailableDataLength());
// This uses a bogus destination id that is not going to match anything in practice.
CHIP_ERROR err = EncodeSigma1<Sigma1Params>(buf);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
data->SetDataLength(static_cast<uint16_t>(buf.size()));

Optional<SessionHandle> session = sessionManager.CreateUnauthenticatedSession(ctx.GetAliceAddress(), GetDefaultMRPConfig());
NL_TEST_ASSERT(inSuite, session.HasValue());

TestCASESecurePairingDelegate caseDelegate;
CASESession caseSession;
caseSession.SetGroupDataProvider(&gDeviceGroupDataProvider);
err = caseSession.PrepareForSessionEstablishment(sessionManager, &gDeviceFabrics, nullptr, nullptr, &caseDelegate,
ScopedNodeId(), NullOptional);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

err = ctx.GetExchangeManager().RegisterUnsolicitedMessageHandlerForType(MsgType::CASE_Sigma1, &caseSession);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

ExpectErrorExchangeDelegate delegate(inSuite, SecureChannel::kProtocolCodeNoSharedRoot);
ExchangeContext * exchange = ctx.GetExchangeManager().NewContext(session.Value(), &delegate);
NL_TEST_ASSERT(inSuite, exchange != nullptr);

err = exchange->SendMessage(MsgType::CASE_Sigma1, std::move(data), SendMessageFlags::kExpectResponse);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

ctx.DrainAndServiceIO();

NL_TEST_ASSERT(inSuite, caseDelegate.mNumPairingErrors == 1);
NL_TEST_ASSERT(inSuite, caseDelegate.mNumPairingComplete == 0);

ctx.GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(MsgType::CASE_Sigma1);
caseSession.Clear();
}

} // namespace chip

// Test Suite
Expand All @@ -1027,6 +1110,7 @@ static const nlTest sTests[] =
// CASESession that are in the process of establishing.
NL_TEST_DEF("InvalidatePendingSessionEstablishment", chip::TestCASESession::SimulateUpdateNOCInvalidatePendingEstablishment),
#endif // CONFIG_BUILD_FOR_HOST_UNIT_TEST
NL_TEST_DEF("Sigma1BadDestinationId", chip::TestCASESession::Sigma1BadDestinationIdTest),

NL_TEST_SENTINEL()
};
Expand Down

0 comments on commit 17dd036

Please sign in to comment.