Skip to content

Commit

Permalink
Add a way to send a CloseSession StatusReport via chip-tool. (#19641)
Browse files Browse the repository at this point in the history
Specific changes:

1) Add a way to create a Protocols::Id from a 32-bit spec representation.
2) Change StatusReport to take Protocols::Id, not uint32_t.
3) Fix chip-tool interactive mode to log failures when a command fails to run.
4) Add a chip-tool command to send a CloseSession StatusReport.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Jun 27, 2022
1 parent 5748770 commit d3107fb
Show file tree
Hide file tree
Showing 12 changed files with 185 additions and 21 deletions.
2 changes: 2 additions & 0 deletions examples/chip-tool/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ static_library("chip-tool-utils") {
# TODO - enable CommissionedListCommand once DNS Cache is implemented
# "commands/pairing/CommissionedListCommand.cpp",
# "commands/pairing/CommissionedListCommand.h",
"commands/pairing/CloseSessionCommand.cpp",
"commands/pairing/CloseSessionCommand.h",
"commands/pairing/OpenCommissioningWindowCommand.cpp",
"commands/pairing/OpenCommissioningWindowCommand.h",
"commands/pairing/PairingCommand.cpp",
Expand Down
7 changes: 6 additions & 1 deletion examples/chip-tool/commands/common/Commands.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,12 @@ int Commands::Run(int argc, char ** argv)
int Commands::RunInteractive(int argc, char ** argv)
{
CHIP_ERROR err = RunCommand(argc, argv, true);
return (err == CHIP_NO_ERROR) ? EXIT_SUCCESS : EXIT_FAILURE;
if (err == CHIP_NO_ERROR)
{
return EXIT_SUCCESS;
}
ChipLogError(chipTool, "Run command failure: %s", chip::ErrorStr(err));
return EXIT_FAILURE;
}

CHIP_ERROR Commands::RunCommand(int argc, char ** argv, bool interactive)
Expand Down
88 changes: 88 additions & 0 deletions examples/chip-tool/commands/pairing/CloseSessionCommand.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
* Copyright (c) 2022 Project CHIP Authors
* All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/

#include "CloseSessionCommand.h"
#include <protocols/secure_channel/StatusReport.h>
#include <system/SystemPacketBuffer.h>

using namespace chip;
using namespace chip::Protocols;

CHIP_ERROR CloseSessionCommand::RunCommand()
{
CommissioneeDeviceProxy * commissioneeDeviceProxy = nullptr;
if (CHIP_NO_ERROR == CurrentCommissioner().GetDeviceBeingCommissioned(mDestinationId, &commissioneeDeviceProxy))
{
return CloseSession(commissioneeDeviceProxy);
}

return CurrentCommissioner().GetConnectedDevice(mDestinationId, &mOnDeviceConnectedCallback,
&mOnDeviceConnectionFailureCallback);
}

CHIP_ERROR CloseSessionCommand::CloseSession(DeviceProxy * device)
{
VerifyOrReturnError(device->GetSecureSession().HasValue(), CHIP_ERROR_INCORRECT_STATE);

// TODO perhaps factor out this code into something on StatusReport that
// takes an exchange and maybe a SendMessageFlags?
SecureChannel::StatusReport statusReport(SecureChannel::GeneralStatusCode::kSuccess, SecureChannel::Id,
SecureChannel::kProtocolCodeCloseSession);

size_t reportSize = statusReport.Size();
Encoding::LittleEndian::PacketBufferWriter bbuf(MessagePacketBuffer::New(reportSize), reportSize);
statusReport.WriteToBuffer(bbuf);

System::PacketBufferHandle msg = bbuf.Finalize();
VerifyOrReturnError(!msg.IsNull(), CHIP_ERROR_NO_MEMORY);

auto * exchange = device->GetExchangeManager()->NewContext(device->GetSecureSession().Value(), nullptr);
VerifyOrReturnError(exchange != nullptr, CHIP_ERROR_NO_MEMORY);

// Per spec, CloseSession reports are always sent with MRP disabled.
CHIP_ERROR err =
exchange->SendMessage(SecureChannel::MsgType::StatusReport, std::move(msg), Messaging::SendMessageFlags::kNoAutoRequestAck);
if (err == CHIP_NO_ERROR)
{
SetCommandExitStatus(CHIP_NO_ERROR);
}
else
{
exchange->Close();
}

return err;
}

void CloseSessionCommand::OnDeviceConnectedFn(void * context, OperationalDeviceProxy * device)
{
auto * command = reinterpret_cast<CloseSessionCommand *>(context);
VerifyOrReturn(command != nullptr, ChipLogError(chipTool, "OnDeviceConnectedFn: context is null"));

CHIP_ERROR err = command->CloseSession(device);
VerifyOrReturn(CHIP_NO_ERROR == err, command->SetCommandExitStatus(err));
}

void CloseSessionCommand::OnDeviceConnectionFailureFn(void * context, PeerId peerId, CHIP_ERROR err)
{
LogErrorOnFailure(err);

auto * command = reinterpret_cast<CloseSessionCommand *>(context);
VerifyOrReturn(command != nullptr, ChipLogError(chipTool, "OnDeviceConnectionFailureFn: context is null"));
command->SetCommandExitStatus(err);
}
57 changes: 57 additions & 0 deletions examples/chip-tool/commands/pairing/CloseSessionCommand.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* Copyright (c) 2022 Project CHIP Authors
* All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/

#pragma once

#include "../common/CHIPCommand.h"
#include <app/OperationalDeviceProxy.h>
#include <lib/core/CHIPCallback.h>
#include <lib/core/DataModelTypes.h>

class CloseSessionCommand : public CHIPCommand
{
public:
CloseSessionCommand(CredentialIssuerCommands * credIssuerCommands) :
CHIPCommand("close-session", credIssuerCommands), mOnDeviceConnectedCallback(OnDeviceConnectedFn, this),
mOnDeviceConnectionFailureCallback(OnDeviceConnectionFailureFn, this)
{
AddArgument("destination-id", 0, UINT64_MAX, &mDestinationId);
AddArgument("timeout", 0, UINT64_MAX, &mTimeoutSecs,
"Time, in seconds, before this command is considered to have timed out.");
}

/////////// CHIPCommand Interface /////////
CHIP_ERROR RunCommand() override;
chip::System::Clock::Timeout GetWaitDuration() const override
{
return chip::System::Clock::Seconds16(mTimeoutSecs.ValueOr(10));
}

private:
chip::NodeId mDestinationId;
chip::Optional<uint16_t> mTimeoutSecs;

static void OnDeviceConnectedFn(void * context, chip::OperationalDeviceProxy * device);
static void OnDeviceConnectionFailureFn(void * context, PeerId peerId, CHIP_ERROR error);

// Try to send the action CloseSession status report.
CHIP_ERROR CloseSession(chip::DeviceProxy * device);

chip::Callback::Callback<chip::OnDeviceConnected> mOnDeviceConnectedCallback;
chip::Callback::Callback<chip::OnDeviceConnectionFailure> mOnDeviceConnectionFailureCallback;
};
2 changes: 2 additions & 0 deletions examples/chip-tool/commands/pairing/Commands.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#pragma once

#include "CloseSessionCommand.h"
#include "CommissionedListCommand.h"
#include "OpenCommissioningWindowCommand.h"
#include "PairingCommand.h"
Expand Down Expand Up @@ -218,6 +219,7 @@ void registerCommandsPairing(Commands & commands, CredentialIssuerCommands * cre
// make_unique<CommissionedListCommand>(),
make_unique<StartUdcServerCommand>(credsIssuerConfig),
make_unique<OpenCommissioningWindowCommand>(credsIssuerConfig),
make_unique<CloseSessionCommand>(credsIssuerConfig),
};

commands.Register(clusterName, clusterCommands);
Expand Down
10 changes: 9 additions & 1 deletion src/protocols/Protocols.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,16 @@ class Id
constexpr VendorId GetVendorId() const { return mVendorId; }
constexpr uint16_t GetProtocolId() const { return mProtocolId; }

static constexpr uint32_t kVendorIdShift = 16;

static Id FromFullyQualifiedSpecForm(uint32_t aSpecForm)
{
return Id(static_cast<VendorId>(aSpecForm >> kVendorIdShift),
static_cast<uint16_t>(aSpecForm & ((1 << kVendorIdShift) - 1)));
}

private:
constexpr uint32_t ToUint32() const { return (static_cast<uint32_t>(mVendorId) << 16) | mProtocolId; }
constexpr uint32_t ToUint32() const { return (static_cast<uint32_t>(mVendorId) << kVendorIdShift) | mProtocolId; }

chip::VendorId mVendorId;
uint16_t mProtocolId;
Expand Down
6 changes: 3 additions & 3 deletions src/protocols/bdx/BdxTransferSession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ CHIP_ERROR TransferSession::HandleStatusReportMessage(const PayloadHeader & head

Protocols::SecureChannel::StatusReport report;
ReturnErrorOnFailure(report.Parse(std::move(msg)));
VerifyOrReturnError((report.GetProtocolId() == Protocols::BDX::Id.ToFullyQualifiedSpecForm()), CHIP_ERROR_INVALID_MESSAGE_TYPE);
VerifyOrReturnError((report.GetProtocolId() == Protocols::BDX::Id), CHIP_ERROR_INVALID_MESSAGE_TYPE);

mStatusReportData.statusCode = static_cast<StatusCode>(report.GetProtocolCode());

Expand Down Expand Up @@ -871,8 +871,8 @@ void TransferSession::PrepareStatusReport(StatusCode code)
{
mStatusReportData.statusCode = code;

Protocols::SecureChannel::StatusReport report(Protocols::SecureChannel::GeneralStatusCode::kFailure,
Protocols::BDX::Id.ToFullyQualifiedSpecForm(), to_underlying(code));
Protocols::SecureChannel::StatusReport report(Protocols::SecureChannel::GeneralStatusCode::kFailure, Protocols::BDX::Id,
to_underlying(code));
size_t msgSize = report.Size();
Encoding::LittleEndian::PacketBufferWriter bbuf(chip::MessagePacketBuffer::New(msgSize), msgSize);
VerifyOrExit(!bbuf.IsNull(), mPendingOutput = OutputEventType::kInternalError);
Expand Down
2 changes: 1 addition & 1 deletion src/protocols/bdx/tests/TestBdxTransferSession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ void VerifyStatusReport(nlTestSuite * inSuite, void * inContext, const System::P
err = report.Parse(std::move(msgCopy));
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, report.GetGeneralCode() == SecureChannel::GeneralStatusCode::kFailure);
NL_TEST_ASSERT(inSuite, report.GetProtocolId() == Protocols::BDX::Id.ToFullyQualifiedSpecForm());
NL_TEST_ASSERT(inSuite, report.GetProtocolId() == Protocols::BDX::Id);
NL_TEST_ASSERT(inSuite, report.GetProtocolCode() == static_cast<uint16_t>(expectedCode));
}

Expand Down
6 changes: 2 additions & 4 deletions src/protocols/secure_channel/PairingSession.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,11 @@ class DLL_EXPORT PairingSession : public SessionDelegate
Protocols::SecureChannel::GeneralStatusCode generalCode = (protocolCode == Protocols::SecureChannel::kProtocolCodeSuccess)
? Protocols::SecureChannel::GeneralStatusCode::kSuccess
: Protocols::SecureChannel::GeneralStatusCode::kFailure;
uint32_t protocolId = Protocols::SecureChannel::Id.ToFullyQualifiedSpecForm();

ChipLogDetail(SecureChannel, "Sending status report. Protocol code %d, exchange %d", protocolCode,
exchangeCtxt->GetExchangeId());

Protocols::SecureChannel::StatusReport statusReport(generalCode, protocolId, protocolCode);
Protocols::SecureChannel::StatusReport statusReport(generalCode, Protocols::SecureChannel::Id, protocolCode);

Encoding::LittleEndian::PacketBufferWriter bbuf(System::PacketBufferHandle::New(statusReport.Size()));
statusReport.WriteToBuffer(bbuf);
Expand All @@ -161,8 +160,7 @@ class DLL_EXPORT PairingSession : public SessionDelegate
Protocols::SecureChannel::StatusReport report;
CHIP_ERROR err = report.Parse(std::move(msg));
ReturnErrorOnFailure(err);
VerifyOrReturnError(report.GetProtocolId() == Protocols::SecureChannel::Id.ToFullyQualifiedSpecForm(),
CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(report.GetProtocolId() == Protocols::SecureChannel::Id, CHIP_ERROR_INVALID_ARGUMENT);

if (report.GetGeneralCode() == Protocols::SecureChannel::GeneralStatusCode::kSuccess &&
report.GetProtocolCode() == Protocols::SecureChannel::kProtocolCodeSuccess && successExpected)
Expand Down
13 changes: 8 additions & 5 deletions src/protocols/secure_channel/StatusReport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,15 @@ namespace chip {
namespace Protocols {
namespace SecureChannel {

StatusReport::StatusReport() : mGeneralCode(GeneralStatusCode::kSuccess), mProtocolId(0), mProtocolCode(0), mProtocolData(nullptr)
StatusReport::StatusReport() :
mGeneralCode(GeneralStatusCode::kSuccess), mProtocolId(SecureChannel::Id), mProtocolCode(0), mProtocolData(nullptr)
{}

StatusReport::StatusReport(GeneralStatusCode generalCode, uint32_t protocolId, uint16_t protocolCode) :
StatusReport::StatusReport(GeneralStatusCode generalCode, Protocols::Id protocolId, uint16_t protocolCode) :
mGeneralCode(generalCode), mProtocolId(protocolId), mProtocolCode(protocolCode), mProtocolData(nullptr)
{}

StatusReport::StatusReport(GeneralStatusCode generalCode, uint32_t protocolId, uint16_t protocolCode,
StatusReport::StatusReport(GeneralStatusCode generalCode, Protocols::Id protocolId, uint16_t protocolCode,
System::PacketBufferHandle protocolData) :
mGeneralCode(generalCode),
mProtocolId(protocolId), mProtocolCode(protocolCode), mProtocolData(std::move(protocolData))
Expand All @@ -54,7 +55,9 @@ CHIP_ERROR StatusReport::Parse(System::PacketBufferHandle buf)
uint8_t * bufStart = buf->Start();
LittleEndian::Reader bufReader(bufStart, buf->DataLength());

ReturnErrorOnFailure(bufReader.Read16(&tempGeneralCode).Read32(&mProtocolId).Read16(&mProtocolCode).StatusCode());
uint32_t protocolId;
ReturnErrorOnFailure(bufReader.Read16(&tempGeneralCode).Read32(&protocolId).Read16(&mProtocolCode).StatusCode());
mProtocolId = Protocols::Id::FromFullyQualifiedSpecForm(protocolId);
mGeneralCode = static_cast<GeneralStatusCode>(tempGeneralCode);

// Any data that exists after the required fields is considered protocol-specific data.
Expand All @@ -78,7 +81,7 @@ CHIP_ERROR StatusReport::Parse(System::PacketBufferHandle buf)

Encoding::LittleEndian::BufferWriter & StatusReport::WriteToBuffer(Encoding::LittleEndian::BufferWriter & buf) const
{
buf.Put16(to_underlying(mGeneralCode)).Put32(mProtocolId).Put16(mProtocolCode);
buf.Put16(to_underlying(mGeneralCode)).Put32(mProtocolId.ToFullyQualifiedSpecForm()).Put16(mProtocolCode);
if (!mProtocolData.IsNull())
{
buf.Put(mProtocolData->Start(), mProtocolData->DataLength());
Expand Down
9 changes: 5 additions & 4 deletions src/protocols/secure_channel/StatusReport.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#pragma once

#include <lib/support/BufferWriter.h>
#include <protocols/Protocols.h>
#include <system/SystemPacketBuffer.h>

namespace chip {
Expand All @@ -44,7 +45,7 @@ class DLL_EXPORT StatusReport
* @param protocolId Must specify a ProtocolId which consists of Vendor Id (upper 16 bits) and ProtocolId (lower 16 bits)
* @param protocolCode A code defined by the specified protocol which provides more information about the status
*/
StatusReport(GeneralStatusCode generalCode, uint32_t protocolId, uint16_t protocolCode);
StatusReport(GeneralStatusCode generalCode, Protocols::Id protocolId, uint16_t protocolCode);

//
/**
Expand All @@ -55,7 +56,7 @@ class DLL_EXPORT StatusReport
* @param protocolCode A code defined by the specified protocol which provides more information about the status
* @param protocolData A \c PacketBufferHandle containing the protocol-specific data
*/
StatusReport(GeneralStatusCode generalCode, uint32_t protocolId, uint16_t protocolCode,
StatusReport(GeneralStatusCode generalCode, Protocols::Id protocolId, uint16_t protocolCode,
System::PacketBufferHandle protocolData);

/**
Expand Down Expand Up @@ -88,13 +89,13 @@ class DLL_EXPORT StatusReport
size_t Size() const;

GeneralStatusCode GetGeneralCode() const { return mGeneralCode; }
uint32_t GetProtocolId() const { return mProtocolId; }
Protocols::Id GetProtocolId() const { return mProtocolId; }
uint16_t GetProtocolCode() const { return mProtocolCode; }
const System::PacketBufferHandle & GetProtocolData() const { return mProtocolData; }

private:
GeneralStatusCode mGeneralCode;
uint32_t mProtocolId;
Protocols::Id mProtocolId;
uint16_t mProtocolCode;

System::PacketBufferHandle mProtocolData;
Expand Down
4 changes: 2 additions & 2 deletions src/protocols/secure_channel/tests/TestStatusReport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ using namespace chip::Protocols::SecureChannel;
void TestStatusReport_NoData(nlTestSuite * inSuite, void * inContext)
{
GeneralStatusCode generalCode = GeneralStatusCode::kSuccess;
uint32_t protocolId = SecureChannel::Id.ToFullyQualifiedSpecForm();
auto protocolId = SecureChannel::Id;
uint16_t protocolCode = kProtocolCodeSuccess;

StatusReport testReport(generalCode, protocolId, protocolCode);
Expand All @@ -59,7 +59,7 @@ void TestStatusReport_NoData(nlTestSuite * inSuite, void * inContext)
void TestStatusReport_WithData(nlTestSuite * inSuite, void * inContext)
{
GeneralStatusCode generalCode = GeneralStatusCode::kFailure;
uint32_t protocolId = SecureChannel::Id.ToFullyQualifiedSpecForm();
auto protocolId = SecureChannel::Id;
uint16_t protocolCode = static_cast<uint16_t>(StatusCode::InvalidFabricConfig);
uint8_t data[6] = { 42, 19, 3, 1, 3, 0 };
const uint16_t dataLen = 6;
Expand Down

0 comments on commit d3107fb

Please sign in to comment.