Skip to content

Commit

Permalink
Add ClearSecretData calls for IPK bits on Darwin.
Browse files Browse the repository at this point in the history
Keypairs do this automatically, but we're using a raw buffer for the IPK.
  • Loading branch information
bzbarsky-apple committed Apr 7, 2022
1 parent 0c06ba5 commit 707e748
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 7 deletions.
9 changes: 9 additions & 0 deletions src/crypto/CHIPCryptoPAL.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class CHIPOperationalCredentialsDelegate : public chip::Controller::OperationalC
public:
using ChipP256KeypairPtr = std::unique_ptr<chip::Crypto::P256Keypair>;

~CHIPOperationalCredentialsDelegate() {}
~CHIPOperationalCredentialsDelegate() { chip::Crypto::ClearSecretData(mIPK); }

/**
* If nocSigner is not provided (is null), a keypair will be loaded from the
Expand Down
28 changes: 23 additions & 5 deletions src/darwin/Framework/CHIP/CHIPOperationalCredentialsDelegate.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -141,6 +146,7 @@ static BOOL isRunningTests(void)
(id) kSecReturnData : @YES,
};

// TODO: Figure out a way to ClearSecretData for this keyDataRef?
CFDataRef keyDataRef;
OSStatus status = SecItemCopyMatching((CFDictionaryRef) query, (CFTypeRef *) &keyDataRef);
if (status == errSecItemNotFound || keyDataRef == nil) {
Expand All @@ -151,17 +157,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);
}
Expand All @@ -175,6 +184,7 @@ static BOOL isRunningTests(void)
(id) kSecReturnData : @YES,
};

// TODO: Figure out a way to ClearSecretData for this keyDataRef?
CFDataRef keyDataRef;
OSStatus status = SecItemCopyMatching((CFDictionaryRef) query, (CFTypeRef *) &keyDataRef);
if (status == errSecItemNotFound || keyDataRef == nil) {
Expand All @@ -185,13 +195,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];
NSMutableData * ipkData = [[NSMutableData alloc] initWithBase64EncodedData:keyData options:0];
if ([ipkData length] != sizeof(mIPK)) {
NSLog(@"IPK length %zu does not match expected length %zu", [ipkData length], sizeof(mIPK));
ClearSecretData(ipkData);
return CHIP_ERROR_INTERNAL;
}

memcpy(mIPK, [ipkData bytes], [ipkData length]);
ClearSecretData(ipkData);
return CHIP_NO_ERROR;
}

Expand All @@ -209,15 +221,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()) {
Expand All @@ -237,15 +252,18 @@ static BOOL isRunningTests(void)
return errorCode;
}

NSData * ipkAdata = [NSData dataWithBytes:mIPK length:sizeof(mIPK)];
NSMutableData * ipkData = [NSMutableData dataWithBytes:mIPK length:sizeof(mIPK)];

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()) {
Expand Down
2 changes: 1 addition & 1 deletion src/protocols/secure_channel/CASESession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ void CASESession::Clear()
PairingSession::Clear();

mState = kInitialized;
Crypto::ClearSecretData(&mIPK[0], sizeof(mIPK));
Crypto::ClearSecretData(mIPK);

AbortExchange();
}
Expand Down

0 comments on commit 707e748

Please sign in to comment.