Skip to content

Commit

Permalink
Add interfaces to pass ipk and adminSubject from op creds delegate
Browse files Browse the repository at this point in the history
The operational credentials delegate provides interfaces to pass
certificates back to the commissioner, but cannot fully specify the
contents of the AddNOC command.  Missing are the IPK and AdminSubject.
Without these, those can only be managed locally.

This commit adds optional interfaces to pass these from the delegate.
Where AdminSubject is not passed, the commissioner uses its own node ID
as before.  And where IPK is not passed, the commissioner writes a
0-length IPK, which is also as before and is what we are using until
GroupKeyManagement and other dependencies are in place.

Fixes #13503
  • Loading branch information
msandstedt committed Jan 21, 2022
1 parent f765050 commit c8e4976
Show file tree
Hide file tree
Showing 8 changed files with 124 additions and 6 deletions.
14 changes: 11 additions & 3 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<AesCcm128KeySpan> ipk,
Optional<NodeId> adminSubject)
{
CHIP_ERROR err = CHIP_NO_ERROR;

Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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");

Expand Down
2 changes: 1 addition & 1 deletion src/controller/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<AesCcm128KeySpan> ipk, Optional<NodeId> adminSubject);

/**
* @brief
Expand Down
19 changes: 19 additions & 0 deletions src/controller/CommissioneeDeviceProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ void CommissioneeDeviceProxy::Reset()

ReleaseDAC();
ReleasePAI();
ClearIpk();
ClearAdminSubject();
}

CHIP_ERROR CommissioneeDeviceProxy::LoadSecureSessionParameters()
Expand Down Expand Up @@ -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<AesCcm128KeySpan> ipk)
{
if (!ipk.HasValue())
{
ClearIpk();
}
else
{
mIpk.Emplace(ipk.Value());
}
}

CommissioneeDeviceProxy::~CommissioneeDeviceProxy()
{
ReleaseDAC();
Expand Down
15 changes: 15 additions & 0 deletions src/controller/CommissioneeDeviceProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,15 @@ class CommissioneeDeviceProxy : public DeviceProxy, public SessionReleaseDelegat
CHIP_ERROR SetDAC(const ByteSpan & dac);
CHIP_ERROR SetPAI(const ByteSpan & pai);

Optional<AesCcm128KeySpan> GetIpk() const
{
return mIpk.HasValue() ? Optional<AesCcm128KeySpan>((mIpk.Value().Span())) : Optional<AesCcm128KeySpan>();
}
void SetIpk(Optional<AesCcm128KeySpan> ipk);

Optional<NodeId> GetAdminSubject() const { return mAdminSubject; }
void SetAdminSubject(Optional<NodeId> adminSubject) { mAdminSubject = adminSubject; }

MutableByteSpan GetMutableNOCCert() { return MutableByteSpan(mNOCCertBuffer, sizeof(mNOCCertBuffer)); }

CHIP_ERROR SetNOCCertBufferSize(size_t new_size);
Expand Down Expand Up @@ -332,6 +341,8 @@ class CommissioneeDeviceProxy : public DeviceProxy, public SessionReleaseDelegat

void ReleaseDAC();
void ReleasePAI();
void ClearIpk();
void ClearAdminSubject() { mAdminSubject = Optional<NodeId>(); }

FabricIndex mFabricIndex = kUndefinedFabricIndex;

Expand All @@ -350,6 +361,10 @@ class CommissioneeDeviceProxy : public DeviceProxy, public SessionReleaseDelegat
uint8_t mICACertBuffer[Credentials::kMaxCHIPCertLength];
size_t mICACertBufferSize = 0;

Optional<Crypto::AesCcm128Key> mIpk;

Optional<NodeId> mAdminSubject;

SessionIDAllocator * mIDAllocator = nullptr;
};

Expand Down
3 changes: 2 additions & 1 deletion src/controller/ExampleOperationalCredentialsIssuer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<AesCcm128KeySpan>(),
Optional<NodeId>());
return CHIP_NO_ERROR;
}

Expand Down
2 changes: 1 addition & 1 deletion src/controller/OperationalCredentialsDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<Crypto::AesCcm128KeySpan> ipk, Optional<NodeId> adminSubject);

constexpr uint32_t kMaxCHIPDERCertLength = 600;
constexpr size_t kOpCSRNonceLength = 32;
Expand Down
41 changes: 41 additions & 0 deletions src/crypto/CHIPCryptoPAL.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<Crypto::kAES_CCM128_Key_Length>;

class AesCcm128Key
{
public:
AesCcm128Key() {}

~AesCcm128Key()
{
// Sanitize after use
ClearSecretData(&bytes[0], Length());
}

template <size_t N>
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 <size_t N>
constexpr AesCcm128Key(const FixedByteSpan<N> & 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.
*
Expand Down
34 changes: 34 additions & 0 deletions src/crypto/tests/CHIPCryptoPALTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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),
Expand Down

0 comments on commit c8e4976

Please sign in to comment.