diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 61350870fb1a0e..e3389be2d825fc 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -1274,7 +1274,8 @@ void DeviceCommissioner::OnOperationalCertificateSigningRequest(void * context, } void DeviceCommissioner::OnDeviceNOCChainGeneration(void * context, CHIP_ERROR status, const ByteSpan & noc, const ByteSpan & icac, - const ByteSpan & rcac) + const ByteSpan & rcac, Optional ipk, + Optional adminSubject) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -1324,6 +1325,9 @@ void DeviceCommissioner::OnDeviceNOCChainGeneration(void * context, CHIP_ERROR s SuccessOrExit(err); } + device->SetIpk(ipk); + device->SetAdminSubject(adminSubject); + exit: if (err != CHIP_NO_ERROR) { @@ -1373,8 +1377,12 @@ CHIP_ERROR DeviceCommissioner::SendOperationalCertificate(CommissioneeDeviceProx Callback::Cancelable * successCallback = mNOCResponseCallback.Cancel(); Callback::Cancelable * failureCallback = mOnCertFailureCallback.Cancel(); - ReturnErrorOnFailure(cluster.AddNOC(successCallback, failureCallback, nocCertBuf, icaCertBuf, ByteSpan(nullptr, 0), - mLocalId.GetNodeId(), mVendorId)); + ReturnErrorOnFailure( + cluster.AddNOC(successCallback, failureCallback, nocCertBuf, icaCertBuf, + // TODO(#13825): If not passed by the signer, the commissioner should + // provide its current IPK to the commissionee in the AddNOC command. + device->GetIpk().HasValue() ? device->GetIpk().Value() : ByteSpan(nullptr, 0), + device->GetAdminSubject().HasValue() ? device->GetAdminSubject().Value() : mLocalId.GetNodeId(), mVendorId)); ChipLogProgress(Controller, "Sent operational certificate to the device"); diff --git a/src/controller/CHIPDeviceController.h b/src/controller/CHIPDeviceController.h index 22a61010ae7167..dcb51f34a20407 100644 --- a/src/controller/CHIPDeviceController.h +++ b/src/controller/CHIPDeviceController.h @@ -749,7 +749,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController, static void OnDeviceAttestationInformationVerification(void * context, Credentials::AttestationVerificationResult result); static void OnDeviceNOCChainGeneration(void * context, CHIP_ERROR status, const ByteSpan & noc, const ByteSpan & icac, - const ByteSpan & rcac); + const ByteSpan & rcac, Optional ipk, Optional adminSubject); /** * @brief diff --git a/src/controller/CommissioneeDeviceProxy.cpp b/src/controller/CommissioneeDeviceProxy.cpp index 74b0231e3a7401..bab9b497cde349 100644 --- a/src/controller/CommissioneeDeviceProxy.cpp +++ b/src/controller/CommissioneeDeviceProxy.cpp @@ -162,6 +162,8 @@ void CommissioneeDeviceProxy::Reset() ReleaseDAC(); ReleasePAI(); + ClearIpk(); + ClearAdminSubject(); } CHIP_ERROR CommissioneeDeviceProxy::LoadSecureSessionParameters() @@ -274,6 +276,23 @@ CHIP_ERROR CommissioneeDeviceProxy::SetPAI(const chip::ByteSpan & pai) return CHIP_NO_ERROR; } +void CommissioneeDeviceProxy::ClearIpk() +{ + mIpk.ClearValue(); // AesCcm128Key destructor will clear secret data. +} + +void CommissioneeDeviceProxy::SetIpk(Optional ipk) +{ + if (!ipk.HasValue()) + { + ClearIpk(); + } + else + { + mIpk.Emplace(ipk.Value()); + } +} + CommissioneeDeviceProxy::~CommissioneeDeviceProxy() { ReleaseDAC(); diff --git a/src/controller/CommissioneeDeviceProxy.h b/src/controller/CommissioneeDeviceProxy.h index fa36aaf24b6f58..414dc3d12f8553 100644 --- a/src/controller/CommissioneeDeviceProxy.h +++ b/src/controller/CommissioneeDeviceProxy.h @@ -257,6 +257,15 @@ class CommissioneeDeviceProxy : public DeviceProxy, public SessionReleaseDelegat CHIP_ERROR SetDAC(const ByteSpan & dac); CHIP_ERROR SetPAI(const ByteSpan & pai); + Optional GetIpk() const + { + return mIpk.HasValue() ? Optional((mIpk.Value().Span())) : Optional(); + } + void SetIpk(Optional ipk); + + Optional GetAdminSubject() const { return mAdminSubject; } + void SetAdminSubject(Optional adminSubject) { mAdminSubject = adminSubject; } + MutableByteSpan GetMutableNOCCert() { return MutableByteSpan(mNOCCertBuffer, sizeof(mNOCCertBuffer)); } CHIP_ERROR SetNOCCertBufferSize(size_t new_size); @@ -332,6 +341,8 @@ class CommissioneeDeviceProxy : public DeviceProxy, public SessionReleaseDelegat void ReleaseDAC(); void ReleasePAI(); + void ClearIpk(); + void ClearAdminSubject() { mAdminSubject = Optional(); } FabricIndex mFabricIndex = kUndefinedFabricIndex; @@ -350,6 +361,10 @@ class CommissioneeDeviceProxy : public DeviceProxy, public SessionReleaseDelegat uint8_t mICACertBuffer[Credentials::kMaxCHIPCertLength]; size_t mICACertBufferSize = 0; + Optional mIpk; + + Optional mAdminSubject; + SessionIDAllocator * mIDAllocator = nullptr; }; diff --git a/src/controller/ExampleOperationalCredentialsIssuer.cpp b/src/controller/ExampleOperationalCredentialsIssuer.cpp index c06a4d5c56d5cd..c6b67b2f581b4d 100644 --- a/src/controller/ExampleOperationalCredentialsIssuer.cpp +++ b/src/controller/ExampleOperationalCredentialsIssuer.cpp @@ -180,7 +180,8 @@ CHIP_ERROR ExampleOperationalCredentialsIssuer::GenerateNOCChain(const ByteSpan ReturnErrorOnFailure(GenerateNOCChainAfterValidation(assignedId, mNextFabricId, pubkey, rcacSpan, icacSpan, nocSpan)); ChipLogProgress(Controller, "Providing certificate chain to the commissioner"); - onCompletion->mCall(onCompletion->mContext, CHIP_NO_ERROR, nocSpan, icacSpan, rcacSpan); + onCompletion->mCall(onCompletion->mContext, CHIP_NO_ERROR, nocSpan, icacSpan, rcacSpan, Optional(), + Optional()); return CHIP_NO_ERROR; } diff --git a/src/controller/OperationalCredentialsDelegate.h b/src/controller/OperationalCredentialsDelegate.h index cc82faec4f20d8..579bd9d985ecae 100644 --- a/src/controller/OperationalCredentialsDelegate.h +++ b/src/controller/OperationalCredentialsDelegate.h @@ -30,7 +30,7 @@ namespace chip { namespace Controller { typedef void (*OnNOCChainGeneration)(void * context, CHIP_ERROR status, const ByteSpan & noc, const ByteSpan & icac, - const ByteSpan & rcac); + const ByteSpan & rcac, Optional ipk, Optional adminSubject); constexpr uint32_t kMaxCHIPDERCertLength = 600; constexpr size_t kOpCSRNonceLength = 32; diff --git a/src/crypto/CHIPCryptoPAL.h b/src/crypto/CHIPCryptoPAL.h index ab6f3bcab20161..874ebf9b7db38a 100644 --- a/src/crypto/CHIPCryptoPAL.h +++ b/src/crypto/CHIPCryptoPAL.h @@ -434,6 +434,47 @@ class P256Keypair : public P256KeypairBase bool mInitialized = false; }; +/** + * @brief A data structure for holding an AES CCM128 symmetric key, without the ownership of it. + */ +using AesCcm128KeySpan = FixedByteSpan; + +class AesCcm128Key +{ +public: + AesCcm128Key() {} + + ~AesCcm128Key() + { + // Sanitize after use + ClearSecretData(&bytes[0], Length()); + } + + template + constexpr AesCcm128Key(const uint8_t (&raw_value)[N]) + { + static_assert(N == kAES_CCM128_Key_Length, "Can only array-initialize from proper bounds"); + memcpy(&bytes[0], &raw_value[0], N); + } + + template + constexpr AesCcm128Key(const FixedByteSpan & value) + { + static_assert(N == kAES_CCM128_Key_Length, "Can only initialize from proper sized byte span"); + memcpy(&bytes[0], value.data(), N); + } + + size_t Length() const { return sizeof(bytes); } + operator uint8_t *() { return bytes; } + operator const uint8_t *() const { return bytes; } + const uint8_t * ConstBytes() const { return &bytes[0]; } + AesCcm128KeySpan Span() const { return AesCcm128KeySpan(bytes); } + uint8_t * Bytes() { return &bytes[0]; } + +private: + uint8_t bytes[kAES_CCM128_Key_Length]; +}; + /** * @brief Convert a raw ECDSA signature to ASN.1 signature (per X9.62) as used by TLS libraries. * diff --git a/src/crypto/tests/CHIPCryptoPALTest.cpp b/src/crypto/tests/CHIPCryptoPALTest.cpp index 1dc409dc7838de..aedee0e554d6d4 100644 --- a/src/crypto/tests/CHIPCryptoPALTest.cpp +++ b/src/crypto/tests/CHIPCryptoPALTest.cpp @@ -594,6 +594,39 @@ static void TestAES_CCM_128DecryptInvalidIVLen(nlTestSuite * inSuite, void * inC NL_TEST_ASSERT(inSuite, numOfTestsRan > 0); } +static void TestAES_CCM_128Containers(nlTestSuite * inSuite, void * inContext) +{ + HeapChecker heapChecker(inSuite); + uint8_t testVector[kAES_CCM128_Key_Length]; + AesCcm128Key deepCopy; + AesCcm128KeySpan shallowCopy; + CHIP_ERROR err = CHIP_NO_ERROR; + + // Give us some data. + err = DRBG_get_bytes(testVector, sizeof(testVector)); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + // Test deep copy from array. + deepCopy = AesCcm128Key(testVector); + NL_TEST_ASSERT(inSuite, memcmp(deepCopy, testVector, sizeof(testVector)) == 0); + + // Test sanitization. + deepCopy = AesCcm128Key(); + NL_TEST_ASSERT(inSuite, memcmp(deepCopy, testVector, sizeof(testVector))); + + // Give us different data. + err = DRBG_get_bytes(testVector, sizeof(testVector)); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + // Test deep copy from KeySpan. + shallowCopy = AesCcm128KeySpan(testVector); + deepCopy = AesCcm128Key(shallowCopy); + NL_TEST_ASSERT(inSuite, memcmp(deepCopy, testVector, sizeof(testVector)) == 0); + + // Test Span getter. + NL_TEST_ASSERT(inSuite, memcmp(testVector, deepCopy.Span().data(), deepCopy.Span().size()) == 0); +} + static void TestAsn1Conversions(nlTestSuite * inSuite, void * inContext) { HeapChecker heapChecker(inSuite); @@ -2129,6 +2162,7 @@ static const nlTest sTests[] = { NL_TEST_DEF("Test encrypting AES-CCM-128 using invalid tag", TestAES_CCM_128EncryptInvalidTagLen), NL_TEST_DEF("Test decrypting AES-CCM-128 invalid key", TestAES_CCM_128DecryptInvalidKey), NL_TEST_DEF("Test decrypting AES-CCM-128 invalid IV", TestAES_CCM_128DecryptInvalidIVLen), + NL_TEST_DEF("Test decrypting AES-CCM-128 Containers", TestAES_CCM_128Containers), NL_TEST_DEF("Test encrypting AES-CCM-256 test vectors", TestAES_CCM_256EncryptTestVectors), NL_TEST_DEF("Test decrypting AES-CCM-256 test vectors", TestAES_CCM_256DecryptTestVectors), NL_TEST_DEF("Test encrypting AES-CCM-256 using nil key", TestAES_CCM_256EncryptNilKey),