From 7358513a2a6fc44186402e0951a8db5c97495c51 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 21 Jul 2021 18:07:50 -0400 Subject: [PATCH] Add a NodeId header and some NodeId utilities (#8465) Fixes https://github.com/project-chip/connectedhomeip/issues/8461 The encoder.cpp changes are needed because it used to bootleg the PacketBuffer header via basic-types.h including MessageHeader.h, instead of doing include-what-you-use correctly. --- .../commands/pairing/PairingCommand.cpp | 14 +++-- examples/pump-app/pump-common/gen/encoder.cpp | 1 + .../pump-controller-common/gen/encoder.cpp | 1 + src/app/util/CHIPDeviceCallbacksMgr.cpp | 4 +- src/app/util/CHIPDeviceCallbacksMgr.h | 2 +- src/app/util/basic-types.h | 2 +- .../templates/app/encoder-src.zapt | 1 + src/controller/CHIPDeviceController.cpp | 6 +- .../ExampleOperationalCredentialsIssuer.cpp | 21 +++++++ .../ExampleOperationalCredentialsIssuer.h | 9 +++ src/controller/data_model/gen/encoder.cpp | 1 + src/lib/core/BUILD.gn | 1 + src/lib/core/NodeId.h | 62 +++++++++++++++++++ src/lib/core/PeerId.h | 7 +-- src/lib/support/CHIPArgParser.cpp | 36 ----------- src/lib/support/CHIPArgParser.hpp | 1 - src/messaging/ExchangeContext.cpp | 2 +- .../tests/TestReliableMessageProtocol.cpp | 4 +- .../secure_channel/tests/TestCASESession.cpp | 4 +- .../secure_channel/tests/TestPASESession.cpp | 4 +- src/transport/SecureSessionHandle.h | 2 +- 21 files changed, 125 insertions(+), 60 deletions(-) create mode 100644 src/lib/core/NodeId.h diff --git a/examples/chip-tool/commands/pairing/PairingCommand.cpp b/examples/chip-tool/commands/pairing/PairingCommand.cpp index bf9264c2ed44c8..ffbf6b7c9812f1 100644 --- a/examples/chip-tool/commands/pairing/PairingCommand.cpp +++ b/examples/chip-tool/commands/pairing/PairingCommand.cpp @@ -18,6 +18,7 @@ #include "PairingCommand.h" #include "platform/PlatformManager.h" +#include #include #include #include @@ -40,12 +41,17 @@ CHIP_ERROR PairingCommand::Run() #if CONFIG_PAIR_WITH_RANDOM_ID // Generate a random remote id so we don't end up reusing the same node id // for different nodes. + // + // TODO: Ideally we'd just ask for an operational cert for the commissionnee + // and get the node from that, but the APIs are not set up that way yet. NodeId randomId; - DRBG_get_bytes(reinterpret_cast(&randomId), sizeof(randomId)); - ChipLogProgress(Controller, "Generated random node id: 0x" ChipLogFormatX64, ChipLogValueX64(randomId)); - if (GetExecContext()->storage->SetRemoteNodeId(randomId) == CHIP_NO_ERROR) + if (Controller::ExampleOperationalCredentialsIssuer::GetRandomOperationalNodeId(&randomId) == CHIP_NO_ERROR) { - GetExecContext()->remoteId = randomId; + ChipLogProgress(Controller, "Generated random node id: 0x" ChipLogFormatX64, ChipLogValueX64(randomId)); + if (GetExecContext()->storage->SetRemoteNodeId(randomId) == CHIP_NO_ERROR) + { + GetExecContext()->remoteId = randomId; + } } #endif // CONFIG_PAIR_WITH_RANDOM_ID diff --git a/examples/pump-app/pump-common/gen/encoder.cpp b/examples/pump-app/pump-common/gen/encoder.cpp index 44fe94d51656ed..bbf2c6a8eef50d 100644 --- a/examples/pump-app/pump-common/gen/encoder.cpp +++ b/examples/pump-app/pump-common/gen/encoder.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include diff --git a/examples/pump-controller-app/pump-controller-common/gen/encoder.cpp b/examples/pump-controller-app/pump-controller-common/gen/encoder.cpp index be6ed47551a45e..402c52ea7d1126 100644 --- a/examples/pump-controller-app/pump-controller-common/gen/encoder.cpp +++ b/examples/pump-controller-app/pump-controller-common/gen/encoder.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include diff --git a/src/app/util/CHIPDeviceCallbacksMgr.cpp b/src/app/util/CHIPDeviceCallbacksMgr.cpp index 7fd694f1f68882..7a48e28d841a5b 100644 --- a/src/app/util/CHIPDeviceCallbacksMgr.cpp +++ b/src/app/util/CHIPDeviceCallbacksMgr.cpp @@ -87,7 +87,7 @@ CHIP_ERROR CHIPDeviceCallbacksMgr::CancelResponseCallback(NodeId nodeId, uint8_t CHIP_ERROR CHIPDeviceCallbacksMgr::AddResponseFilter(const ResponseCallbackInfo & info, TLVDataFilter filter) { - constexpr ResponseCallbackInfo kEmptyInfo{ kAnyNodeId, 0 }; + constexpr ResponseCallbackInfo kEmptyInfo{ kPlaceholderNodeId, 0 }; for (size_t i = 0; i < kTLVFilterPoolSize; i++) { @@ -112,7 +112,7 @@ CHIP_ERROR CHIPDeviceCallbacksMgr::PopResponseFilter(const ResponseCallbackInfo { *outFilter = mTLVFilterPool[i].filter; } - mTLVFilterPool[i].info = ResponseCallbackInfo{ kAnyNodeId, 0 }; + mTLVFilterPool[i].info = ResponseCallbackInfo{ kPlaceholderNodeId, 0 }; mTLVFilterPool[i].filter = nullptr; return CHIP_NO_ERROR; } diff --git a/src/app/util/CHIPDeviceCallbacksMgr.h b/src/app/util/CHIPDeviceCallbacksMgr.h index dbf4b262338c3c..ba2845049002f9 100644 --- a/src/app/util/CHIPDeviceCallbacksMgr.h +++ b/src/app/util/CHIPDeviceCallbacksMgr.h @@ -95,7 +95,7 @@ class DLL_EXPORT CHIPDeviceCallbacksMgr struct TLVFilterItem { - ResponseCallbackInfo info = { kAnyNodeId, 0 }; + ResponseCallbackInfo info = { kPlaceholderNodeId, 0 }; TLVDataFilter filter = nullptr; }; diff --git a/src/app/util/basic-types.h b/src/app/util/basic-types.h index 5f6e82111da5d5..fe03366487d37d 100644 --- a/src/app/util/basic-types.h +++ b/src/app/util/basic-types.h @@ -26,7 +26,7 @@ #include // Pull in NodeId -#include +#include // Pull in VendorId #include diff --git a/src/app/zap-templates/templates/app/encoder-src.zapt b/src/app/zap-templates/templates/app/encoder-src.zapt index 8de5c319ae8776..be48e0b2405efa 100644 --- a/src/app/zap-templates/templates/app/encoder-src.zapt +++ b/src/app/zap-templates/templates/app/encoder-src.zapt @@ -6,6 +6,7 @@ #include #include #include +#include #include #include diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index a41dec0c091a9e..c5a5a7a95b23fd 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -49,6 +49,8 @@ #include #include #include +#include +#include #include #include #include @@ -859,7 +861,7 @@ CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, RendezvousParam Transport::AdminPairingInfo * admin = mAdmins.FindAdminWithId(mAdminId); - VerifyOrExit(remoteDeviceId != kAnyNodeId && remoteDeviceId != kUndefinedNodeId, err = CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrExit(IsOperationalNodeId(remoteDeviceId), err = CHIP_ERROR_INVALID_ARGUMENT); VerifyOrExit(mState == State::Initialized, err = CHIP_ERROR_INCORRECT_STATE); VerifyOrExit(mDeviceBeingPaired == kNumMaxActiveDevices, err = CHIP_ERROR_INCORRECT_STATE); VerifyOrExit(admin != nullptr, err = CHIP_ERROR_INCORRECT_STATE); @@ -974,7 +976,7 @@ CHIP_ERROR DeviceCommissioner::PairTestDeviceWithoutSecurity(NodeId remoteDevice // Check that the caller has provided an IP address (instead of a BLE peer address) VerifyOrExit(peerAddress.GetTransportType() == Transport::Type::kUdp, err = CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrExit(remoteDeviceId != kUndefinedNodeId && remoteDeviceId != kAnyNodeId, err = CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrExit(IsOperationalNodeId(remoteDeviceId), err = CHIP_ERROR_INVALID_ARGUMENT); VerifyOrExit(mState == State::Initialized, err = CHIP_ERROR_INCORRECT_STATE); VerifyOrExit(mDeviceBeingPaired == kNumMaxActiveDevices, err = CHIP_ERROR_INCORRECT_STATE); diff --git a/src/controller/ExampleOperationalCredentialsIssuer.cpp b/src/controller/ExampleOperationalCredentialsIssuer.cpp index 4b1d979efce9af..a4ff37faa2f197 100644 --- a/src/controller/ExampleOperationalCredentialsIssuer.cpp +++ b/src/controller/ExampleOperationalCredentialsIssuer.cpp @@ -142,5 +142,26 @@ CHIP_ERROR ExampleOperationalCredentialsIssuer::GetRootCACertificate(FabricId fa return CHIP_NO_ERROR; } +CHIP_ERROR ExampleOperationalCredentialsIssuer::GetRandomOperationalNodeId(NodeId * aNodeId) +{ + for (int i = 0; i < 10; ++i) + { + CHIP_ERROR err = DRBG_get_bytes(reinterpret_cast(aNodeId), sizeof(*aNodeId)); + if (err != CHIP_NO_ERROR) + { + return err; + } + + if (IsOperationalNodeId(*aNodeId)) + { + return CHIP_NO_ERROR; + } + } + + // Terrible, universe-ending luck (chances are 1 in 2^280 or so here, if our + // DRBG is good). + return CHIP_ERROR_INTERNAL; +} + } // namespace Controller } // namespace chip diff --git a/src/controller/ExampleOperationalCredentialsIssuer.h b/src/controller/ExampleOperationalCredentialsIssuer.h index 4c331e7ec39d50..6bad06bed8d10e 100644 --- a/src/controller/ExampleOperationalCredentialsIssuer.h +++ b/src/controller/ExampleOperationalCredentialsIssuer.h @@ -69,6 +69,15 @@ class DLL_EXPORT ExampleOperationalCredentialsIssuer : public OperationalCredent void SetCertificateValidityPeriod(uint32_t validity) { mValidity = validity; } + /** + * Generate a random operational node id. + * + * @param[out] aNodeId where to place the generated id. + * + * On error no guarantees are made about the state of aNodeId. + */ + static CHIP_ERROR GetRandomOperationalNodeId(NodeId * aNodeId); + private: Crypto::P256Keypair mIssuer; Crypto::P256Keypair mIntermediateIssuer; diff --git a/src/controller/data_model/gen/encoder.cpp b/src/controller/data_model/gen/encoder.cpp index f307cca65b43b0..aa79a32b61d8ac 100644 --- a/src/controller/data_model/gen/encoder.cpp +++ b/src/controller/data_model/gen/encoder.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include diff --git a/src/lib/core/BUILD.gn b/src/lib/core/BUILD.gn index e60a34ddff2752..2d0937a8a1a6f8 100644 --- a/src/lib/core/BUILD.gn +++ b/src/lib/core/BUILD.gn @@ -101,6 +101,7 @@ static_library("core") { "CHIPTLVUpdater.cpp", "CHIPTLVUtilities.cpp", "CHIPTLVWriter.cpp", + "NodeId.h", "PeerId.h", ] diff --git a/src/lib/core/NodeId.h b/src/lib/core/NodeId.h new file mode 100644 index 00000000000000..07e79b01378fb0 --- /dev/null +++ b/src/lib/core/NodeId.h @@ -0,0 +1,62 @@ +/* + * + * Copyright (c) 2021 Project CHIP Authors + * + * 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 + +namespace chip { + +// TODO: Consider making this a class and the various utility methods static +// methods. +using NodeId = uint64_t; + +constexpr NodeId kUndefinedNodeId = 0ULL; + +// The range of possible NodeId values has some pieces carved out for special +// uses. +constexpr NodeId kMinGroupNodeId = 0xFFFF'FFFF'FFFF'0000ULL; +// The max group id is complicated, depending on how we want to count the +// various special group ids. Let's not define it for now, until we have use +// cases. + +constexpr NodeId kMinTemporaryLocalId = 0xFFFF'FFFE'0000'0000ULL; +// We use the largest available temporary local id to represent +// kPlaceholderNodeId, so the range is narrowed compared to the spec. +constexpr NodeId kMaxTemporaryLocalId = 0xFFFF'FFFE'FFFF'FFFEULL; +constexpr NodeId kPlaceholderNodeId = 0xFFFF'FFFE'FFFF'FFFFULL; + +constexpr NodeId kMinCASEAuthTag1 = 0xFFFF'FFFD'0000'0000ULL; +constexpr NodeId kMaxCASEAuthTag1 = 0xFFFF'FFFD'FFFF'FFFFULL; + +constexpr NodeId kMinCASEAuthTag2 = 0xFFFF'FFFC'0000'0000ULL; +constexpr NodeId kMaxCASEAuthTag2 = 0xFFFF'FFFC'FFFF'FFFFULL; + +constexpr NodeId kMinPAKEKeyId = 0xFFFF'FFFB'0000'0000ULL; +constexpr NodeId kMaxPAKEKeyId = 0xFFFF'FFFB'FFFF'FFFFULL; + +// There are more reserved ranges here, not assigned to anything yet, going down +// all the way to 0xFFFF'FFF0'0000'0000ULL + +constexpr NodeId kMaxOperationalNodeId = 0xFFFF'FFEF'FFFF'FFFFULL; + +constexpr bool IsOperationalNodeId(NodeId aNodeId) +{ + return (aNodeId != kUndefinedNodeId) && (aNodeId <= kMaxOperationalNodeId); +} + +} // namespace chip diff --git a/src/lib/core/PeerId.h b/src/lib/core/PeerId.h index 82f1fc7d361dd9..2df07c9c7d6345 100644 --- a/src/lib/core/PeerId.h +++ b/src/lib/core/PeerId.h @@ -17,15 +17,12 @@ #pragma once +#include + namespace chip { -/// Convenience types to make it clear what different number types mean -using NodeId = uint64_t; using FabricId = uint64_t; -constexpr NodeId kUndefinedNodeId = 0ULL; -constexpr NodeId kAnyNodeId = 0xFFFFFFFFFFFFFFFFULL; - constexpr FabricId kUndefinedFabricId = 0ULL; constexpr uint16_t kUndefinedVendorId = 0U; diff --git a/src/lib/support/CHIPArgParser.cpp b/src/lib/support/CHIPArgParser.cpp index c9ea9043e1f5c3..34473fffa4a5cb 100644 --- a/src/lib/support/CHIPArgParser.cpp +++ b/src/lib/support/CHIPArgParser.cpp @@ -57,10 +57,6 @@ #define CHIP_ARG_PARSER_PARSE_FABRIC_ID 0 #endif // CHIP_ARG_PARSER_PARSE_FABRIC_ID -#ifndef CHIP_ARG_PARSER_PARSE_NODE_ID -#define CHIP_ARG_PARSER_PARSE_NODE_ID 0 -#endif // CHIP_ARG_PARSER_PARSE_NODE_ID - namespace chip { namespace ArgParser { @@ -892,38 +888,6 @@ bool ParseInt(const char * str, uint8_t & output) return false; } -#if CHIP_ARG_PARSER_PARSE_NODE_ID -/** - * Parse a CHIP node id in text form. - * - * @param[in] str A pointer to a NULL-terminated C string containing - * the node id to parse. - * @param[out] output A reference to an uint64_t lvalue in which the parsed - * value will be stored on success. - * - * @return true if the value was successfully parsed; false if not. - * - * @details - * The ParseNodeId() function accepts either a 64-bit node id given in hex - * format (with or without a leading '0x'), or the words 'any' or 'all' which - * are interpreted as meaning the Any node id (0xFFFFFFFFFFFFFFFF). - */ -bool ParseNodeId(const char * str, uint64_t & nodeId) -{ - char * parseEnd; - - if (strcasecmp(str, "any") == 0 || strcasecmp(str, "all") == 0) - { - nodeId = kAnyNodeId; - return true; - } - - errno = 0; - nodeId = strtoull(str, &parseEnd, 16); - return parseEnd > str && *parseEnd == 0 && (nodeId != ULLONG_MAX || errno == 0); -} -#endif // CHIP_ARG_PARSER_PARSE_NODE_ID - #if CHIP_ARG_PARSER_PARSE_FABRIC_ID /** * Parse a CHIP fabric id in text form. diff --git a/src/lib/support/CHIPArgParser.hpp b/src/lib/support/CHIPArgParser.hpp index 909ac08878a5dd..0eab18419babaa 100644 --- a/src/lib/support/CHIPArgParser.hpp +++ b/src/lib/support/CHIPArgParser.hpp @@ -126,7 +126,6 @@ bool ParseInt(const char * str, uint64_t & output); bool ParseInt(const char * str, int32_t & output, int base); bool ParseInt(const char * str, uint32_t & output, int base); bool ParseInt(const char * str, uint64_t & output, int base); -bool ParseNodeId(const char * str, uint64_t & nodeId); bool ParseFabricId(const char * str, uint64_t & fabricId, bool allowReserved = false); bool ParseSubnetId(const char * str, uint16_t & subnetId); bool ParseHexString(const char * hexStr, uint32_t strLen, uint8_t * outBuf, uint32_t outBufSize, uint32_t & outDataLen); diff --git a/src/messaging/ExchangeContext.cpp b/src/messaging/ExchangeContext.cpp index b8f387258e93ea..dcabcc2b320fcf 100644 --- a/src/messaging/ExchangeContext.cpp +++ b/src/messaging/ExchangeContext.cpp @@ -303,7 +303,7 @@ bool ExchangeContext::MatchExchange(SecureSessionHandle session, const PacketHea // AND The message's source Node ID matches the peer Node ID associated with the exchange, or the peer Node ID of the // exchange is 'any'. - && ((mSecureSession.GetPeerNodeId() == kAnyNodeId) || + && ((mSecureSession.GetPeerNodeId() == kPlaceholderNodeId) || (packetHeader.GetSourceNodeId().HasValue() && mSecureSession.GetPeerNodeId() == packetHeader.GetSourceNodeId().Value())) // AND The message was sent by an initiator and the exchange context is a responder (IsInitiator==false) diff --git a/src/messaging/tests/TestReliableMessageProtocol.cpp b/src/messaging/tests/TestReliableMessageProtocol.cpp index e62658f8e11069..11ffc10c348fb5 100644 --- a/src/messaging/tests/TestReliableMessageProtocol.cpp +++ b/src/messaging/tests/TestReliableMessageProtocol.cpp @@ -568,8 +568,8 @@ void CheckResendSessionEstablishmentMessageWithPeerExchange(nlTestSuite * inSuit CHIP_ERROR err = ctx.Init(inSuite, &gTransportMgr); NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); - ctx.SetSourceNodeId(kAnyNodeId); - ctx.SetDestinationNodeId(kAnyNodeId); + ctx.SetSourceNodeId(kPlaceholderNodeId); + ctx.SetDestinationNodeId(kPlaceholderNodeId); ctx.SetLocalKeyId(0); ctx.SetPeerKeyId(0); ctx.SetAdminId(kUndefinedAdminId); diff --git a/src/protocols/secure_channel/tests/TestCASESession.cpp b/src/protocols/secure_channel/tests/TestCASESession.cpp index 1978c442cd8488..2820b60fc89c60 100644 --- a/src/protocols/secure_channel/tests/TestCASESession.cpp +++ b/src/protocols/secure_channel/tests/TestCASESession.cpp @@ -482,8 +482,8 @@ CHIP_ERROR CASETestSecurePairingSetup(void * inContext) ReturnErrorOnFailure(ctx.Init(&sSuite, &gTransportMgr)); - ctx.SetSourceNodeId(kAnyNodeId); - ctx.SetDestinationNodeId(kAnyNodeId); + ctx.SetSourceNodeId(kPlaceholderNodeId); + ctx.SetDestinationNodeId(kPlaceholderNodeId); ctx.SetLocalKeyId(0); ctx.SetPeerKeyId(0); ctx.SetAdminId(kUndefinedAdminId); diff --git a/src/protocols/secure_channel/tests/TestPASESession.cpp b/src/protocols/secure_channel/tests/TestPASESession.cpp index 4973ae7d4dea59..e6ab5a0725784a 100644 --- a/src/protocols/secure_channel/tests/TestPASESession.cpp +++ b/src/protocols/secure_channel/tests/TestPASESession.cpp @@ -388,8 +388,8 @@ int TestSecurePairing_Setup(void * inContext) if (err != CHIP_NO_ERROR) return FAILURE; - ctx.SetSourceNodeId(kAnyNodeId); - ctx.SetDestinationNodeId(kAnyNodeId); + ctx.SetSourceNodeId(kPlaceholderNodeId); + ctx.SetDestinationNodeId(kPlaceholderNodeId); ctx.SetLocalKeyId(0); ctx.SetPeerKeyId(0); ctx.SetAdminId(kUndefinedAdminId); diff --git a/src/transport/SecureSessionHandle.h b/src/transport/SecureSessionHandle.h index 167d982fa49cc8..a57f8dbbbbe1cc 100644 --- a/src/transport/SecureSessionHandle.h +++ b/src/transport/SecureSessionHandle.h @@ -24,7 +24,7 @@ class SecureSessionMgr; class SecureSessionHandle { public: - SecureSessionHandle() : mPeerNodeId(kAnyNodeId), mPeerKeyId(0), mAdmin(Transport::kUndefinedAdminId) {} + SecureSessionHandle() : mPeerNodeId(kPlaceholderNodeId), mPeerKeyId(0), mAdmin(Transport::kUndefinedAdminId) {} SecureSessionHandle(NodeId peerNodeId, uint16_t peerKeyId, Transport::AdminId admin) : mPeerNodeId(peerNodeId), mPeerKeyId(peerKeyId), mAdmin(admin) {}