Stop converting values to base64 for Darwin storage. (#18404)

* Stop converting values to base64 for Darwin storage. We can just store NSData directly. * Address review comments. * Address review comments about API naming.
project-chip · Nov 30, 2023 · 2543884 · 2543884
1 parent 30a26a2
commit 2543884
Show file tree

Hide file tree

Showing 9 changed files with 113 additions and 160 deletions.
diff --git a/examples/chip-tool-darwin/commands/common/CHIPCommandStorageDelegate.h b/examples/chip-tool-darwin/commands/common/CHIPCommandStorageDelegate.h
@@ -1,8 +1,12 @@
 #import <CHIP/CHIP.h>
 #import <Foundation/Foundation.h>
 
+NS_ASSUME_NONNULL_BEGIN
+
 @interface CHIPToolPersistentStorageDelegate : NSObject <CHIPPersistentStorageDelegate>
-- (NSString *)CHIPGetKeyValue:(NSString *)key;
-- (void)CHIPSetKeyValue:(NSString *)key value:(NSString *)value;
-- (void)CHIPDeleteKeyValue:(NSString *)key;
+- (nullable NSData *)storageDataForKey:(NSString *)key;
+- (BOOL)setStorageData:(NSData *)value forKey:(NSString *)key;
+- (BOOL)removeStorageDataForKey:(NSString *)key;
 @end
+
+NS_ASSUME_NONNULL_END
diff --git a/examples/chip-tool-darwin/commands/common/CHIPCommandStorageDelegate.mm b/examples/chip-tool-darwin/commands/common/CHIPCommandStorageDelegate.mm
@@ -11,10 +11,10 @@ id CHIPGetDomainValueForKey(NSString * domain, NSString * key)
     return nil;
 }
 
-void CHIPSetDomainValueForKey(NSString * domain, NSString * key, id value)
+BOOL CHIPSetDomainValueForKey(NSString * domain, NSString * key, id value)
 {
     CFPreferencesSetAppValue((CFStringRef) key, (__bridge CFPropertyListRef _Nullable)(value), (CFStringRef) domain);
-    CFPreferencesAppSynchronize((CFStringRef) domain);
+    return CFPreferencesAppSynchronize((CFStringRef) domain) == true;
 }
 
 void CHIPRemoveDomainValueForKey(NSString * domain, NSString * key)
@@ -27,21 +27,25 @@ @implementation CHIPToolPersistentStorageDelegate
 
 // MARK: CHIPPersistentStorageDelegate
 
-- (NSString *)CHIPGetKeyValue:(NSString *)key
+- (nullable NSData *)storageDataForKey:(NSString *)key
 {
-    NSString * value = CHIPGetDomainValueForKey(kCHIPToolDefaultsDomain, key);
+    NSData * value = CHIPGetDomainValueForKey(kCHIPToolDefaultsDomain, key);
     NSLog(@"CHIPPersistentStorageDelegate Get Value for Key: %@, value %@", key, value);
     return value;
 }
 
-- (void)CHIPSetKeyValue:(NSString *)key value:(NSString *)value
+- (BOOL)setStorageData:(NSData *)value forKey:(NSString *)key
 {
-    CHIPSetDomainValueForKey(kCHIPToolDefaultsDomain, key, value);
+    return CHIPSetDomainValueForKey(kCHIPToolDefaultsDomain, key, value);
 }
 
-- (void)CHIPDeleteKeyValue:(NSString *)key
+- (BOOL)removeStorageDataForKey:(NSString *)key
 {
+    if (CHIPGetDomainValueForKey(kCHIPToolDefaultsDomain, key) == nil) {
+        return NO;
+    }
     CHIPRemoveDomainValueForKey(kCHIPToolDefaultsDomain, key);
+    return YES;
 }
 
 @end
diff --git a/examples/chip-tool-darwin/commands/common/CHIPToolKeypair.mm b/examples/chip-tool-darwin/commands/common/CHIPToolKeypair.mm
@@ -3,10 +3,7 @@
 #include <credentials/CHIPCert.h>
 #include <crypto/CHIPCryptoPAL.h>
 #include <lib/asn1/ASN1.h>
-#include <lib/core/CHIPSafeCasts.h>
-#include <lib/support/Base64.h>
 #include <stddef.h>
-#include <string>
 
 #define CHIPPlugin_CAKeyTag "com.apple.matter.commissioner.ca.issuer.id"
 #define Public_KeySize "256"
@@ -15,31 +12,6 @@
 static NSString * const kOperationalCredentialsIssuerKeypairStorage = @"ChipToolOpCredsCAKey";
 static NSString * const kOperationalCredentialsIPK = @"ChipToolOpCredsIPK";
 
-std::string StringToBase64(const std::string & value)
-{
-    std::unique_ptr<char[]> buffer(new char[BASE64_ENCODED_LEN(value.length())]);
-
-    uint32_t len = chip::Base64Encode32(
-        reinterpret_cast<const uint8_t *>(value.data()), static_cast<uint32_t>(value.length()), buffer.get());
-    if (len == UINT32_MAX) {
-        return "";
-    }
-
-    return std::string(buffer.get(), len);
-}
-
-std::string Base64ToString(const std::string & b64Value)
-{
-    std::unique_ptr<uint8_t[]> buffer(new uint8_t[BASE64_MAX_DECODED_LEN(b64Value.length())]);
-
-    uint32_t len = chip::Base64Decode32(b64Value.data(), static_cast<uint32_t>(b64Value.length()), buffer.get());
-    if (len == UINT32_MAX) {
-        return "";
-    }
-
-    return std::string(reinterpret_cast<const char *>(buffer.get()), len);
-}
-
 @interface CHIPToolKeypair ()
 @property (nonatomic) chip::Crypto::P256Keypair mKeyPair;
 @property (nonatomic) chip::Crypto::P256Keypair mIssuer;
@@ -105,7 +77,7 @@ - (CHIP_ERROR)createOrLoadKeys:(CHIPToolPersistentStorageDelegate *)storage
 {
     chip::ASN1::ASN1UniversalTime effectiveTime;
     chip::Crypto::P256SerializedKeypair serializedKey;
-    NSString * value;
+    NSData * value;
     CHIP_ERROR err = CHIP_NO_ERROR;
 
     // Initializing the default start validity to start of 2021. The default validity duration is 10 years.
@@ -115,8 +87,8 @@ - (CHIP_ERROR)createOrLoadKeys:(CHIPToolPersistentStorageDelegate *)storage
     effectiveTime.Day = 1;
     ReturnErrorOnFailure(chip::Credentials::ASN1ToChipEpochTime(effectiveTime, _mNow));
 
-    value = [storage CHIPGetKeyValue:kOperationalCredentialsIssuerKeypairStorage];
-    err = [self decodeNSStringWithValue:value serializedKey:serializedKey];
+    value = [storage storageDataForKey:kOperationalCredentialsIssuerKeypairStorage];
+    err = [self initSerializedKeyFromValue:value serializedKey:serializedKey];
 
     if (err != CHIP_NO_ERROR) {
         // Storage doesn't have an existing keypair. Let's create one and add it to the storage.
@@ -125,56 +97,43 @@ - (CHIP_ERROR)createOrLoadKeys:(CHIPToolPersistentStorageDelegate *)storage
         }
         ReturnErrorOnFailure([self Serialize:serializedKey]);
 
-        std::string serializedString(chip::Uint8::to_const_char(serializedKey.Bytes()), serializedKey.Length());
-        std::string base64Value = StringToBase64(serializedString);
-        NSString * valueString = [NSString stringWithUTF8String:base64Value.c_str()];
-        [storage CHIPSetKeyValue:kOperationalCredentialsIssuerKeypairStorage value:valueString];
+        NSData * valueData = [NSData dataWithBytes:serializedKey.Bytes() length:serializedKey.Length()];
+        [storage setStorageData:valueData forKey:kOperationalCredentialsIssuerKeypairStorage];
     } else {
         ReturnErrorOnFailure([self Deserialize:serializedKey]);
     }
 
-    NSData * ipk;
-    value = [storage CHIPGetKeyValue:kOperationalCredentialsIPK];
-    err = [self decodeNSStringToNSData:value serializedKey:&ipk];
+    NSData * ipk = [storage storageDataForKey:kOperationalCredentialsIPK];
+    if (ipk == nil) {
+        err = CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND;
+    }
     if (err != CHIP_NO_ERROR) {
         uint8_t tempIPK[chip::Crypto::CHIP_CRYPTO_SYMMETRIC_KEY_LENGTH_BYTES];
 
         ReturnLogErrorOnFailure(chip::Crypto::DRBG_get_bytes(tempIPK, sizeof(tempIPK)));
 
         _ipk = [NSData dataWithBytes:tempIPK length:sizeof(tempIPK)];
-        NSString * valueString = [_ipk base64EncodedStringWithOptions:0];
-        [storage CHIPSetKeyValue:kOperationalCredentialsIPK value:valueString];
+        [storage setStorageData:_ipk forKey:kOperationalCredentialsIPK];
     } else {
         _ipk = ipk;
     }
 
     return CHIP_NO_ERROR;
 }
 
-- (CHIP_ERROR)decodeNSStringWithValue:(NSString *)value serializedKey:(chip::Crypto::P256SerializedKeypair &)serializedKey
+- (CHIP_ERROR)initSerializedKeyFromValue:(NSData *)value serializedKey:(chip::Crypto::P256SerializedKeypair &)serializedKey
 {
     if (value == nil) {
         return CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND;
     }
 
-    std::string decoded = Base64ToString([value UTF8String]);
-
-    if (serializedKey.Capacity() < decoded.length()) {
+    if (serializedKey.Capacity() < [value length]) {
         return CHIP_ERROR_BUFFER_TOO_SMALL;
     }
 
-    memcpy(serializedKey.Bytes(), decoded.data(), decoded.length());
-    serializedKey.SetLength(decoded.length());
+    memcpy(serializedKey.Bytes(), [value bytes], [value length]);
+    serializedKey.SetLength([value length]);
     return CHIP_NO_ERROR;
 }
 
-- (CHIP_ERROR)decodeNSStringToNSData:(NSString *)value serializedKey:(NSData **)decodedData
-{
-    if (value == nil) {
-        return CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND;
-    }
-
-    *decodedData = [[NSData alloc] initWithBase64EncodedString:value options:0];
-    return CHIP_NO_ERROR;
-}
 @end
diff --git a/src/darwin/CHIPTool/CHIPTool/Framework Helpers/DefaultsUtils.h b/src/darwin/CHIPTool/CHIPTool/Framework Helpers/DefaultsUtils.h
@@ -26,7 +26,7 @@ extern NSString * const kFabricIdKey;
 CHIPDeviceController * InitializeCHIP(void);
 CHIPDeviceController * CHIPRestartController(CHIPDeviceController * controller);
 id CHIPGetDomainValueForKey(NSString * domain, NSString * key);
-void CHIPSetDomainValueForKey(NSString * domain, NSString * key, id value);
+BOOL CHIPSetDomainValueForKey(NSString * domain, NSString * key, id value);
 void CHIPRemoveDomainValueForKey(NSString * domain, NSString * key);
 uint64_t CHIPGetNextAvailableDeviceID(void);
 NSString * KeyForPairedDevice(uint64_t id);
@@ -39,6 +39,12 @@ BOOL CHIPGetConnectedDeviceWithID(uint64_t deviceId, CHIPDeviceConnectionCallbac
 void CHIPUnpairDeviceWithID(uint64_t deviceId);
 CHIPDevice * CHIPGetDeviceBeingCommissioned(void);
 
-@interface CHIPToolPersistentStorageDelegate : NSObject <CHIPPersistentStorageDelegate>
+NS_ASSUME_NONNULL_BEGIN
 
+@interface CHIPToolPersistentStorageDelegate : NSObject <CHIPPersistentStorageDelegate>
+- (NSData *)storageDataForKey:(NSString *)key;
+- (BOOL)setStorageData:(NSData *)value forKey:(NSString *)key;
+- (BOOL)removeStorageDataForKey:(NSString *)key;
 @end
+
+NS_ASSUME_NONNULL_END
diff --git a/src/darwin/CHIPTool/CHIPTool/Framework Helpers/DefaultsUtils.m b/src/darwin/CHIPTool/CHIPTool/Framework Helpers/DefaultsUtils.m
@@ -34,10 +34,10 @@ id CHIPGetDomainValueForKey(NSString * domain, NSString * key)
     return nil;
 }
 
-void CHIPSetDomainValueForKey(NSString * domain, NSString * key, id value)
+BOOL CHIPSetDomainValueForKey(NSString * domain, NSString * key, id value)
 {
     CFPreferencesSetAppValue((CFStringRef) key, (__bridge CFPropertyListRef _Nullable)(value), (CFStringRef) domain);
-    CFPreferencesAppSynchronize((CFStringRef) domain);
+    return CFPreferencesAppSynchronize((CFStringRef) domain) == true;
 }
 
 void CHIPRemoveDomainValueForKey(NSString * domain, NSString * key)
@@ -209,21 +209,25 @@ @implementation CHIPToolPersistentStorageDelegate
 
 // MARK: CHIPPersistentStorageDelegate
 
-- (NSString *)CHIPGetKeyValue:(NSString *)key
+- (nullable NSData *)storageDataForKey:(NSString *)key
 {
-    NSString * value = CHIPGetDomainValueForKey(kCHIPToolDefaultsDomain, key);
+    NSData * value = CHIPGetDomainValueForKey(kCHIPToolDefaultsDomain, key);
     NSLog(@"CHIPPersistentStorageDelegate Get Value for Key: %@, value %@", key, value);
     return value;
 }
 
-- (void)CHIPSetKeyValue:(NSString *)key value:(NSString *)value
+- (BOOL)setStorageData:(NSData *)value forKey:(NSString *)key
 {
-    CHIPSetDomainValueForKey(kCHIPToolDefaultsDomain, key, value);
+    return CHIPSetDomainValueForKey(kCHIPToolDefaultsDomain, key, value);
 }
 
-- (void)CHIPDeleteKeyValue:(NSString *)key
+- (BOOL)removeStorageDataForKey:(NSString *)key
 {
+    if (CHIPGetDomainValueForKey(kCHIPToolDefaultsDomain, key) == nil) {
+        return NO;
+    }
     CHIPRemoveDomainValueForKey(kCHIPToolDefaultsDomain, key);
+    return YES;
 }
 
 @end
diff --git a/src/darwin/Framework/CHIP/CHIPPersistentStorageDelegate.h b/src/darwin/Framework/CHIP/CHIPPersistentStorageDelegate.h
@@ -28,22 +28,22 @@ NS_ASSUME_NONNULL_BEGIN
 @required
 
 /**
- * Get the value for the given key
- *
+ * Get the data for the given key.  Returns nil if there is no data for the
+ * key.
  */
-- (nullable NSString *)CHIPGetKeyValue:(NSString *)key;
+- (nullable NSData *)storageDataForKey:(NSString *)key;
 
 /**
- * Set the value of the key to the given value
- *
+ * Set the data for the viven key to the given value.  Returns YES if the key
+ * was set successfully, NO otherwise.
  */
-- (void)CHIPSetKeyValue:(NSString *)key value:(NSString *)value;
+- (BOOL)setStorageData:(NSData *)value forKey:(NSString *)key;
 
 /**
- * Delete the key and corresponding value
- *
+ * Delete the key and corresponding data.  Returns YES if the key was present,
+ * NO if the key was not present.
  */
-- (void)CHIPDeleteKeyValue:(NSString *)key;
+- (BOOL)removeStorageDataForKey:(NSString *)key;
 
 @end