Skip to content

Commit

Permalink
[crypto] Replace AesCcm128Key with sensitive data buffer (#24357)
Browse files Browse the repository at this point in the history
AesCcm128Key, despite its name, is currently only used for
Identity Protection Key. Additionally, it has very similar
interface to CapacityBoundBuffer.

Rename CapacityBoundBuffer to SensitiveDataBuffer to better
describe its purpose and add SensitiveDataFixedBuffer for
fixed-size contents. Align interfaces of these two and
implicit cast operators for better type safety.

Finally, replace AesCcm128Key with new IdentityProtectionKey
type alias defined as SensitiveDataFixedBuffer<
CRYPTO_SYMMETRIC_KEY_LENGTH_BYTES>.

Signed-off-by: Damian Krolik <[email protected]>

Signed-off-by: Damian Krolik <[email protected]>
  • Loading branch information
Damian-Nordic authored and pull[bot] committed Sep 29, 2023
1 parent 757131a commit 3427224
Show file tree
Hide file tree
Showing 33 changed files with 264 additions and 201 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ CHIP_ERROR CustomCSRResponseOperationalKeyStore::ReuseOpKeypair(FabricIndex fabr

// Scope 1: Load up the keypair data from storage
{
// Use a CapacityBoundBuffer to get RAII secret data clearing on scope exit.
Crypto::CapacityBoundBuffer<OpKeyTLVMaxSize()> buf;
// Use a SensitiveDataBuffer to get RAII secret data clearing on scope exit.
Crypto::SensitiveDataBuffer<OpKeyTLVMaxSize()> buf;

// Load up the operational key structure from storage
uint16_t size = static_cast<uint16_t>(buf.Capacity());
Expand Down
2 changes: 1 addition & 1 deletion src/controller/AutoCommissioner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ Optional<System::Clock::Timeout> AutoCommissioner::GetCommandTimeout(DeviceProxy
return MakeOptional(timeout);
}

CHIP_ERROR AutoCommissioner::NOCChainGenerated(ByteSpan noc, ByteSpan icac, ByteSpan rcac, AesCcm128KeySpan ipk,
CHIP_ERROR AutoCommissioner::NOCChainGenerated(ByteSpan noc, ByteSpan icac, ByteSpan rcac, IdentityProtectionKeySpan ipk,
NodeId adminSubject)
{
// Reuse ICA Cert buffer for temporary store Root Cert.
Expand Down
2 changes: 1 addition & 1 deletion src/controller/AutoCommissioner.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class AutoCommissioner : public CommissioningDelegate
ByteSpan GetDAC() const { return ByteSpan(mDAC, mDACLen); }
ByteSpan GetPAI() const { return ByteSpan(mPAI, mPAILen); }

CHIP_ERROR NOCChainGenerated(ByteSpan noc, ByteSpan icac, ByteSpan rcac, AesCcm128KeySpan ipk, NodeId adminSubject);
CHIP_ERROR NOCChainGenerated(ByteSpan noc, ByteSpan icac, ByteSpan rcac, IdentityProtectionKeySpan ipk, NodeId adminSubject);
/**
* The device argument to GetCommandTimeout is the device whose session will
* be used for sending the relevant command.
Expand Down
9 changes: 5 additions & 4 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1244,7 +1244,7 @@ void DeviceCommissioner::OnOperationalCertificateSigningRequest(
}

void DeviceCommissioner::OnDeviceNOCChainGeneration(void * context, CHIP_ERROR status, const ByteSpan & noc, const ByteSpan & icac,
const ByteSpan & rcac, Optional<AesCcm128KeySpan> ipk,
const ByteSpan & rcac, Optional<IdentityProtectionKeySpan> ipk,
Optional<NodeId> adminSubject)
{
MATTER_TRACE_EVENT_SCOPE("OnDeviceNOCChainGeneration", "DeviceCommissioner");
Expand All @@ -1271,7 +1271,7 @@ void DeviceCommissioner::OnDeviceNOCChainGeneration(void * context, CHIP_ERROR s

// TODO - Verify that the generated root cert matches with commissioner's root cert
CommissioningDelegate::CommissioningReport report;
report.Set<NocChain>(NocChain(noc, icac, rcac, ipk.HasValue() ? ipk.Value() : AesCcm128KeySpan(placeHolderIpk),
report.Set<NocChain>(NocChain(noc, icac, rcac, ipk.HasValue() ? ipk.Value() : IdentityProtectionKeySpan(placeHolderIpk),
adminSubject.HasValue() ? adminSubject.Value() : commissioner->GetNodeId()));
commissioner->CommissioningStageComplete(status, report);
}
Expand Down Expand Up @@ -1325,8 +1325,9 @@ CHIP_ERROR DeviceCommissioner::ProcessCSR(DeviceProxy * proxy, const ByteSpan &
}

CHIP_ERROR DeviceCommissioner::SendOperationalCertificate(DeviceProxy * device, const ByteSpan & nocCertBuf,
const Optional<ByteSpan> & icaCertBuf, const AesCcm128KeySpan ipk,
const NodeId adminSubject, Optional<System::Clock::Timeout> timeout)
const Optional<ByteSpan> & icaCertBuf,
const IdentityProtectionKeySpan ipk, const NodeId adminSubject,
Optional<System::Clock::Timeout> timeout)
{
MATTER_TRACE_EVENT_SCOPE("SendOperationalCertificate", "DeviceCommissioner");

Expand Down
6 changes: 4 additions & 2 deletions src/controller/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,8 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
The function does not hold a reference to the device object.
*/
CHIP_ERROR SendOperationalCertificate(DeviceProxy * device, const ByteSpan & nocCertBuf, const Optional<ByteSpan> & icaCertBuf,
AesCcm128KeySpan ipk, NodeId adminSubject, Optional<System::Clock::Timeout> timeout);
IdentityProtectionKeySpan ipk, NodeId adminSubject,
Optional<System::Clock::Timeout> timeout);
/* This function sends the trusted root certificate to the device.
The function does not hold a reference to the device object.
*/
Expand Down Expand Up @@ -792,7 +793,8 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
Credentials::AttestationVerificationResult result);

static void OnDeviceNOCChainGeneration(void * context, CHIP_ERROR status, const ByteSpan & noc, const ByteSpan & icac,
const ByteSpan & rcac, Optional<AesCcm128KeySpan> ipk, Optional<NodeId> adminSubject);
const ByteSpan & rcac, Optional<IdentityProtectionKeySpan> ipk,
Optional<NodeId> adminSubject);
static void OnArmFailSafe(void * context,
const chip::app::Clusters::GeneralCommissioning::Commands::ArmFailSafeResponse::DecodableType & data);
static void OnSetRegulatoryConfigResponse(
Expand Down
14 changes: 7 additions & 7 deletions src/controller/CommissioningDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,9 @@ class CommissioningParameters
// Epoch key for the identity protection key for the node being commissioned. In the AutoCommissioner, this is set by by the
// kGenerateNOCChain stage through the OperationalCredentialsDelegate.
// This value must be set before calling PerformCommissioningStep for the kSendNOC step.
const Optional<AesCcm128KeySpan> GetIpk() const
const Optional<IdentityProtectionKeySpan> GetIpk() const
{
return mIpk.HasValue() ? Optional<AesCcm128KeySpan>(mIpk.Value().Span()) : Optional<AesCcm128KeySpan>();
return mIpk.HasValue() ? Optional<IdentityProtectionKeySpan>(mIpk.Value().Span()) : Optional<IdentityProtectionKeySpan>();
}

// Admin subject id used for the case access control entry created if the AddNOC command succeeds. In the AutoCommissioner, this
Expand Down Expand Up @@ -321,9 +321,9 @@ class CommissioningParameters
mIcac.SetValue(icac);
return *this;
}
CommissioningParameters & SetIpk(const AesCcm128KeySpan ipk)
CommissioningParameters & SetIpk(const IdentityProtectionKeySpan ipk)
{
mIpk.SetValue(AesCcm128Key(ipk));
mIpk.SetValue(IdentityProtectionKey(ipk));
return *this;
}
CommissioningParameters & SetAdminSubject(const NodeId adminSubject)
Expand Down Expand Up @@ -439,7 +439,7 @@ class CommissioningParameters
Optional<ByteSpan> mRootCert;
Optional<ByteSpan> mNoc;
Optional<ByteSpan> mIcac;
Optional<AesCcm128Key> mIpk;
Optional<IdentityProtectionKey> mIpk;
Optional<NodeId> mAdminSubject;
// Items that come from the device in commissioning steps
Optional<ByteSpan> mAttestationElements;
Expand Down Expand Up @@ -484,13 +484,13 @@ struct CSRResponse

struct NocChain
{
NocChain(ByteSpan newNoc, ByteSpan newIcac, ByteSpan newRcac, AesCcm128KeySpan newIpk, NodeId newAdminSubject) :
NocChain(ByteSpan newNoc, ByteSpan newIcac, ByteSpan newRcac, IdentityProtectionKeySpan newIpk, NodeId newAdminSubject) :
noc(newNoc), icac(newIcac), rcac(newRcac), ipk(newIpk), adminSubject(newAdminSubject)
{}
ByteSpan noc;
ByteSpan icac;
ByteSpan rcac;
AesCcm128KeySpan ipk;
IdentityProtectionKeySpan ipk;
NodeId adminSubject;
};

Expand Down
2 changes: 1 addition & 1 deletion src/controller/ExampleOperationalCredentialsIssuer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ CHIP_ERROR ExampleOperationalCredentialsIssuer::GenerateNOCChain(const ByteSpan
// Prepare IPK to be sent back. A more fully-fledged operational credentials delegate
// would obtain a suitable key per fabric.
uint8_t ipkValue[CHIP_CRYPTO_SYMMETRIC_KEY_LENGTH_BYTES];
Crypto::AesCcm128KeySpan ipkSpan(ipkValue);
Crypto::IdentityProtectionKeySpan ipkSpan(ipkValue);

ReturnErrorCodeIf(defaultIpkSpan.size() != sizeof(ipkValue), CHIP_ERROR_INTERNAL);
memcpy(&ipkValue[0], defaultIpkSpan.data(), defaultIpkSpan.size());
Expand Down
3 changes: 2 additions & 1 deletion src/controller/OperationalCredentialsDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ namespace chip {
namespace Controller {

typedef void (*OnNOCChainGeneration)(void * context, CHIP_ERROR status, const ByteSpan & noc, const ByteSpan & icac,
const ByteSpan & rcac, Optional<Crypto::AesCcm128KeySpan> ipk, Optional<NodeId> adminSubject);
const ByteSpan & rcac, Optional<Crypto::IdentityProtectionKeySpan> ipk,
Optional<NodeId> adminSubject);

constexpr uint32_t kMaxCHIPDERCertLength = 600;
constexpr size_t kCSRNonceLength = 32;
Expand Down
5 changes: 3 additions & 2 deletions src/controller/java/AndroidOperationalCredentialsIssuer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,8 @@ CHIP_ERROR AndroidOperationalCredentialsIssuer::CallbackGenerateNOCChain(const B
}

CHIP_ERROR AndroidOperationalCredentialsIssuer::NOCChainGenerated(CHIP_ERROR status, const ByteSpan & noc, const ByteSpan & icac,
const ByteSpan & rcac, Optional<Crypto::AesCcm128KeySpan> ipk,
const ByteSpan & rcac,
Optional<Crypto::IdentityProtectionKeySpan> ipk,
Optional<NodeId> adminSubject)
{
ReturnErrorCodeIf(mOnNOCCompletionCallback == nullptr, CHIP_ERROR_INCORRECT_STATE);
Expand Down Expand Up @@ -384,7 +385,7 @@ CHIP_ERROR AndroidOperationalCredentialsIssuer::LocalGenerateNOCChain(const Byte
// Prepare IPK to be sent back. A more fully-fledged operational credentials delegate
// would obtain a suitable key per fabric.
uint8_t ipkValue[CHIP_CRYPTO_SYMMETRIC_KEY_LENGTH_BYTES];
Crypto::AesCcm128KeySpan ipkSpan(ipkValue);
Crypto::IdentityProtectionKeySpan ipkSpan(ipkValue);

ReturnErrorCodeIf(defaultIpkSpan.size() != sizeof(ipkValue), CHIP_ERROR_INTERNAL);

Expand Down
2 changes: 1 addition & 1 deletion src/controller/java/AndroidOperationalCredentialsIssuer.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class DLL_EXPORT AndroidOperationalCredentialsIssuer : public OperationalCredent
Callback::Callback<OnNOCChainGeneration> * onCompletion) override;

CHIP_ERROR NOCChainGenerated(CHIP_ERROR status, const ByteSpan & noc, const ByteSpan & icac, const ByteSpan & rcac,
Optional<Crypto::AesCcm128KeySpan> ipk, Optional<NodeId> adminSubject);
Optional<Crypto::IdentityProtectionKeySpan> ipk, Optional<NodeId> adminSubject);

void SetUseJavaCallbackForNOCRequest(bool useJavaCallbackForNOCRequest)
{
Expand Down
4 changes: 2 additions & 2 deletions src/controller/java/CHIPDeviceController-JNI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,9 @@ JNI_METHOD(jint, onNOCChainGeneration)
// use ipk and adminSubject from CommissioningParameters if not set in ControllerParams
CommissioningParameters commissioningParams = wrapper->GetCommissioningParameters();

Optional<Crypto::AesCcm128KeySpan> ipkOptional;
Optional<Crypto::IdentityProtectionKeySpan> ipkOptional;
uint8_t ipkValue[CHIP_CRYPTO_SYMMETRIC_KEY_LENGTH_BYTES];
Crypto::AesCcm128KeySpan ipkTempSpan(ipkValue);
Crypto::IdentityProtectionKeySpan ipkTempSpan(ipkValue);

jbyteArray ipk = (jbyteArray) env->CallObjectMethod(controllerParams, getIpk);
if (ipk != nullptr)
Expand Down
6 changes: 3 additions & 3 deletions src/controller/python/ChipDeviceController-IssueNocChain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ PyChipError pychip_DeviceController_IssueNOCChain(chip::Controller::DeviceCommis
}

void pychip_DeviceController_IssueNOCChainCallback(void * context, CHIP_ERROR status, const ByteSpan & noc, const ByteSpan & icac,
const ByteSpan & rcac, Optional<Crypto::AesCcm128KeySpan> ipk,
const ByteSpan & rcac, Optional<Crypto::IdentityProtectionKeySpan> ipk,
Optional<NodeId> adminSubject)
{
if (pychip_DeviceController_IssueNOCChainCallbackPythonCallbackFunct == nullptr)
Expand All @@ -65,8 +65,8 @@ void pychip_DeviceController_IssueNOCChainCallback(void * context, CHIP_ERROR st
MutableByteSpan chipIcacSpan;
MutableByteSpan chipRcacSpan;

Crypto::AesCcm128KeySpan ipkData;
ipkData = ipk.ValueOr(Crypto::AesCcm128KeySpan());
Crypto::IdentityProtectionKeySpan ipkData;
ipkData = ipk.ValueOr(Crypto::IdentityProtectionKeySpan());

CHIP_ERROR err = status;
if (err != CHIP_NO_ERROR)
Expand Down
2 changes: 1 addition & 1 deletion src/credentials/CHIPCert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ CHIP_ERROR ChipCertificateSet::VerifySignature(const ChipCertificateData * cert,

VerifyOrReturnError((cert != nullptr) && (caCert != nullptr), CHIP_ERROR_INVALID_ARGUMENT);
ReturnErrorOnFailure(signature.SetLength(cert->mSignature.size()));
memcpy(signature, cert->mSignature.data(), cert->mSignature.size());
memcpy(signature.Bytes(), cert->mSignature.data(), cert->mSignature.size());

memcpy(caPublicKey, caCert->mPublicKey.data(), caCert->mPublicKey.size());

Expand Down
4 changes: 2 additions & 2 deletions src/credentials/CertificationDeclaration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ CHIP_ERROR EncodeSignerInfo(const ByteSpan & signerKeyId, const P256ECDSASignatu

uint8_t asn1SignatureBuf[kMax_ECDSA_Signature_Length_Der];
MutableByteSpan asn1Signature(asn1SignatureBuf);
ReturnErrorOnFailure(EcdsaRawSignatureToAsn1(kP256_FE_Length, ByteSpan(signature, signature.Length()), asn1Signature));
ReturnErrorOnFailure(EcdsaRawSignatureToAsn1(kP256_FE_Length, signature.Span(), asn1Signature));

// signature OCTET STRING
ReturnErrorOnFailure(writer.PutOctetString(asn1Signature.data(), static_cast<uint16_t>(asn1Signature.size())));
Expand Down Expand Up @@ -539,7 +539,7 @@ CHIP_ERROR DecodeSignerInfo(ASN1Reader & reader, ByteSpan & signerKeyId, P256ECD
// signature OCTET STRING
ASN1_PARSE_ELEMENT(kASN1TagClass_Universal, kASN1UniversalTag_OctetString);

MutableByteSpan signatureSpan(signature, signature.Capacity());
MutableByteSpan signatureSpan(signature.Bytes(), signature.Capacity());
ReturnErrorOnFailure(
EcdsaAsn1SignatureToRaw(kP256_FE_Length, ByteSpan(reader.GetValue(), reader.GetValueLen()), signatureSpan));
ReturnErrorOnFailure(signature.SetLength(signatureSpan.size()));
Expand Down
2 changes: 1 addition & 1 deletion src/credentials/tests/TestCertificationDeclaration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ static void TestCD_CMSSignAndVerify(nlTestSuite * inSuite, void * inContext)
// Test with known key
P256Keypair keypair2;
P256SerializedKeypair serializedKeypair;
memcpy(serializedKeypair, sTestCMS_SignerSerializedKeypair, sizeof(sTestCMS_SignerSerializedKeypair));
memcpy(serializedKeypair.Bytes(), sTestCMS_SignerSerializedKeypair, sizeof(sTestCMS_SignerSerializedKeypair));
serializedKeypair.SetLength(sizeof(sTestCMS_SignerSerializedKeypair));
cdContentIn = ByteSpan(sTestCMS_CDContent02);
signedMessage = MutableByteSpan(signedMessageBuf);
Expand Down
12 changes: 6 additions & 6 deletions src/credentials/tests/TestFabricTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ static CHIP_ERROR LoadTestFabric_Node01_01(nlTestSuite * inSuite, FabricTable &
static Crypto::P256Keypair opKey_Node01_01;

FabricIndex fabricIndex;
memcpy((uint8_t *) (opKeysSerialized), TestCerts::sTestCert_Node01_01_PublicKey, TestCerts::sTestCert_Node01_01_PublicKey_Len);
memcpy((uint8_t *) (opKeysSerialized) + TestCerts::sTestCert_Node01_01_PublicKey_Len, TestCerts::sTestCert_Node01_01_PrivateKey,
memcpy(opKeysSerialized.Bytes(), TestCerts::sTestCert_Node01_01_PublicKey, TestCerts::sTestCert_Node01_01_PublicKey_Len);
memcpy(opKeysSerialized.Bytes() + TestCerts::sTestCert_Node01_01_PublicKey_Len, TestCerts::sTestCert_Node01_01_PrivateKey,
TestCerts::sTestCert_Node01_01_PrivateKey_Len);

ByteSpan rcacSpan(TestCerts::sTestCert_Root01_Chip, TestCerts::sTestCert_Root01_Chip_Len);
Expand Down Expand Up @@ -119,8 +119,8 @@ static CHIP_ERROR LoadTestFabric_Node01_02(nlTestSuite * inSuite, FabricTable &
FabricIndex fabricIndex;
static Crypto::P256Keypair opKey_Node01_02;

memcpy((uint8_t *) (opKeysSerialized), TestCerts::sTestCert_Node01_02_PublicKey, TestCerts::sTestCert_Node01_02_PublicKey_Len);
memcpy((uint8_t *) (opKeysSerialized) + TestCerts::sTestCert_Node01_02_PublicKey_Len, TestCerts::sTestCert_Node01_02_PrivateKey,
memcpy(opKeysSerialized.Bytes(), TestCerts::sTestCert_Node01_02_PublicKey, TestCerts::sTestCert_Node01_02_PublicKey_Len);
memcpy(opKeysSerialized.Bytes() + TestCerts::sTestCert_Node01_02_PublicKey_Len, TestCerts::sTestCert_Node01_02_PrivateKey,
TestCerts::sTestCert_Node01_02_PrivateKey_Len);

ByteSpan rcacSpan(TestCerts::sTestCert_Root01_Chip, TestCerts::sTestCert_Root01_Chip_Len);
Expand Down Expand Up @@ -151,8 +151,8 @@ static CHIP_ERROR LoadTestFabric_Node02_01(nlTestSuite * inSuite, FabricTable &
FabricIndex fabricIndex;
static Crypto::P256Keypair opKey_Node02_01;

memcpy((uint8_t *) (opKeysSerialized), TestCerts::sTestCert_Node02_01_PublicKey, TestCerts::sTestCert_Node02_01_PublicKey_Len);
memcpy((uint8_t *) (opKeysSerialized) + TestCerts::sTestCert_Node02_01_PublicKey_Len, TestCerts::sTestCert_Node02_01_PrivateKey,
memcpy(opKeysSerialized.Bytes(), TestCerts::sTestCert_Node02_01_PublicKey, TestCerts::sTestCert_Node02_01_PublicKey_Len);
memcpy(opKeysSerialized.Bytes() + TestCerts::sTestCert_Node02_01_PublicKey_Len, TestCerts::sTestCert_Node02_01_PrivateKey,
TestCerts::sTestCert_Node02_01_PrivateKey_Len);

ByteSpan rcacSpan(TestCerts::sTestCert_Root02_Chip, TestCerts::sTestCert_Root02_Chip_Len);
Expand Down
Loading

0 comments on commit 3427224

Please sign in to comment.