Add ClearSecretData calls for IPK bits on Darwin. (#17179)

Keypairs do this automatically, but we're using a raw buffer for the IPK.

src/crypto/CHIPCryptoPAL.h

@@ -178,6 +178,15 @@ enum class SupportedECPKeyTypes : uint8_t
  **/
 void ClearSecretData(uint8_t * buf, size_t len);
 
+/**
+ * Helper for clearing a C array which auto-deduces the size.
+ */
+template <size_t N>
+void ClearSecretData(uint8_t (&buf)[N])
+{
+    ClearSecretData(buf, N);
+}
+
 template <typename Sig>
 class ECPKey
 {

src/darwin/Framework/CHIP/CHIPOperationalCredentialsDelegate.h

@@ -65,7 +65,7 @@ class CHIPOperationalCredentialsDelegate : public chip::Controller::OperationalC
         const chip::Crypto::P256PublicKey & pubkey, chip::MutableByteSpan & rcac, chip::MutableByteSpan & icac,
         chip::MutableByteSpan & noc);
 
-    const chip::Crypto::AesCcm128KeySpan GetIPK() { return chip::Crypto::AesCcm128KeySpan(mIPK); }
+    const chip::Crypto::AesCcm128KeySpan GetIPK() { return mIPK.Span(); }
 
 private:
     CHIP_ERROR GenerateRootCertKeys();
@@ -83,7 +83,7 @@ class CHIPOperationalCredentialsDelegate : public chip::Controller::OperationalC
     ChipP256KeypairPtr mIssuerKey;
     uint64_t mIssuerId = 1234;
 
-    uint8_t mIPK[chip::Crypto::CHIP_CRYPTO_SYMMETRIC_KEY_LENGTH_BYTES];
+    chip::Crypto::AesCcm128Key mIPK;
 
     const uint32_t kCertificateValiditySecs = 365 * 24 * 60 * 60;
     const NSString * kCHIPCAKeyChainLabel = @"matter.nodeopcerts.CA:0";

src/darwin/Framework/CHIP/CHIPOperationalCredentialsDelegate.mm

@@ -46,6 +46,11 @@ static BOOL isRunningTests(void)
     return (environment[@"XCTestConfigurationFilePath"] != nil);
 }
 
+static void ClearSecretData(NSMutableData * data)
+{
+    Crypto::ClearSecretData(static_cast<uint8_t *>([data mutableBytes]), [data length]);
+}
+
 CHIP_ERROR CHIPOperationalCredentialsDelegate::init(
     CHIPPersistentStorageDelegateBridge * storage, ChipP256KeypairPtr nocSigner, NSData * _Nullable ipk)
 {
@@ -80,11 +85,11 @@ static BOOL isRunningTests(void)
     }
 
     if (ipk) {
-        if ([ipk length] != sizeof(mIPK)) {
+        if ([ipk length] != mIPK.Length()) {
             CHIP_LOG_ERROR("CHIPOperationalCredentialsDelegate::init provided IPK is wrong size");
             return CHIP_ERROR_INVALID_ARGUMENT;
         }
-        memcpy(mIPK, [ipk bytes], [ipk length]);
+        memcpy(mIPK.Bytes(), [ipk bytes], [ipk length]);
     } else {
         CHIP_ERROR err = LoadIPKFromKeyChain();
 
@@ -141,6 +146,8 @@ static BOOL isRunningTests(void)
         (id) kSecReturnData : @YES,
     };
 
+    // The CFDataRef we get from SecItemCopyMatching allocates its
+    // buffer in a way that zeroes it when deallocated.
     CFDataRef keyDataRef;
     OSStatus status = SecItemCopyMatching((CFDictionaryRef) query, (CFTypeRef *) &keyDataRef);
     if (status == errSecItemNotFound || keyDataRef == nil) {
@@ -151,17 +158,20 @@ static BOOL isRunningTests(void)
     CHIP_LOG_ERROR("Found an existing self managed keypair in the keychain");
     NSData * keyData = CFBridgingRelease(keyDataRef);
 
-    NSData * keypairData = [[NSData alloc] initWithBase64EncodedData:keyData options:0];
+    NSMutableData * keypairData = [[NSMutableData alloc] initWithBase64EncodedData:keyData options:0];
 
     chip::Crypto::P256SerializedKeypair serialized;
     if ([keypairData length] != serialized.Capacity()) {
         NSLog(@"Keypair length %zu does not match expected length %zu", [keypairData length], serialized.Capacity());
+        ClearSecretData(keypairData);
         return CHIP_ERROR_INTERNAL;
     }
 
     std::memmove((uint8_t *) serialized, [keypairData bytes], [keypairData length]);
     serialized.SetLength([keypairData length]);
 
+    ClearSecretData(keypairData);
+
     CHIP_LOG_ERROR("Deserializing the key");
     return mIssuerKey->Deserialize(serialized);
 }
@@ -175,6 +185,8 @@ static BOOL isRunningTests(void)
         (id) kSecReturnData : @YES,
     };
 
+    // The CFDataRef we get from SecItemCopyMatching allocates its
+    // buffer in a way that zeroes it when deallocated.
     CFDataRef keyDataRef;
     OSStatus status = SecItemCopyMatching((CFDictionaryRef) query, (CFTypeRef *) &keyDataRef);
     if (status == errSecItemNotFound || keyDataRef == nil) {
@@ -185,13 +197,15 @@ static BOOL isRunningTests(void)
     CHIP_LOG_ERROR("Found an existing IPK in the keychain");
     NSData * keyData = CFBridgingRelease(keyDataRef);
 
-    NSData * ipkData = [[NSData alloc] initWithBase64EncodedData:keyData options:0];
-    if ([ipkData length] != sizeof(mIPK)) {
-        NSLog(@"IPK length %zu does not match expected length %zu", [ipkData length], sizeof(mIPK));
+    NSMutableData * ipkData = [[NSMutableData alloc] initWithBase64EncodedData:keyData options:0];
+    if ([ipkData length] != mIPK.Length()) {
+        NSLog(@"IPK length %zu does not match expected length %zu", [ipkData length], mIPK.Length());
+        ClearSecretData(ipkData);
         return CHIP_ERROR_INTERNAL;
     }
 
-    memcpy(mIPK, [ipkData bytes], [ipkData length]);
+    memcpy(mIPK.Bytes(), [ipkData bytes], [ipkData length]);
+    ClearSecretData(ipkData);
     return CHIP_NO_ERROR;
 }
 
@@ -209,15 +223,18 @@ static BOOL isRunningTests(void)
         return errorCode;
     }
 
-    NSData * keypairData = [NSData dataWithBytes:serializedKey.Bytes() length:serializedKey.Length()];
+    NSMutableData * keypairData = [NSMutableData dataWithBytes:serializedKey.Bytes() length:serializedKey.Length()];
 
     const NSDictionary * addParams = @{
         (id) kSecClass : (id) kSecClassGenericPassword,
         (id) kSecAttrService : kCHIPCAKeyChainLabel,
         (id) kSecAttrSynchronizable : @YES,
+        // TODO: Figure out how to ClearSecretData on the base-64 encoded data?
         (id) kSecValueData : [keypairData base64EncodedDataWithOptions:0],
     };
 
+    ClearSecretData(keypairData);
+
     OSStatus status = SecItemAdd((__bridge CFDictionaryRef) addParams, nullptr);
     // TODO: Enable SecItemAdd for Darwin unit tests
     if (status != errSecSuccess && !isRunningTests()) {
@@ -232,20 +249,23 @@ static BOOL isRunningTests(void)
 
 CHIP_ERROR CHIPOperationalCredentialsDelegate::GenerateIPK()
 {
-    CHIP_ERROR errorCode = DRBG_get_bytes(mIPK, sizeof(mIPK));
+    CHIP_ERROR errorCode = DRBG_get_bytes(mIPK.Bytes(), mIPK.Length());
     if (errorCode != CHIP_NO_ERROR) {
         return errorCode;
     }
 
-    NSData * ipkAdata = [NSData dataWithBytes:mIPK length:sizeof(mIPK)];
+    NSMutableData * ipkData = [NSMutableData dataWithBytes:mIPK.Bytes() length:mIPK.Length()];
 
     const NSDictionary * addParams = @{
         (id) kSecClass : (id) kSecClassGenericPassword,
         (id) kSecAttrService : kCHIPIPKKeyChainLabel,
         (id) kSecAttrSynchronizable : @YES,
-        (id) kSecValueData : [ipkAdata base64EncodedDataWithOptions:0],
+        // TODO: Figure out how to ClearSecretData on the base-64 encoded data?
+        (id) kSecValueData : [ipkData base64EncodedDataWithOptions:0],
     };
 
+    ClearSecretData(ipkData);
+
     OSStatus status = SecItemAdd((__bridge CFDictionaryRef) addParams, nullptr);
     // TODO: Enable SecItemAdd for Darwin unit tests
     if (status != errSecSuccess && !isRunningTests()) {

src/protocols/secure_channel/CASESession.cpp

@@ -108,7 +108,7 @@ void CASESession::Clear()
     PairingSession::Clear();
 
     mState = kInitialized;
-    Crypto::ClearSecretData(&mIPK[0], sizeof(mIPK));
+    Crypto::ClearSecretData(mIPK);
 
     AbortExchange();
 }