Fix chip-tool PersistentStorageDelegate failing on some keys (#20239)

* Fix chip-tool PersistentStorageDelegate failing on some keys

- chip-tool's PersistentStorageDelegate failed the audit:

```
[1656721506.201383][688277:688277] CHIP:ATM: ../../../examples/chip-tool/third_party/connectedhomeip/src/lib/support/PersistentStorageAudit.cpp:248: assertion failed: "err == CHIP_NO_ERROR"
[1656721506.201409][688277:688277] CHIP:ATM: ../../../examples/chip-tool/third_party/connectedhomeip/src/lib/support/PersistentStorageAudit.cpp:249: assertion failed: "size == strlen(kBase64SymbolValues)"
[1656721506.201413][688277:688277] CHIP:ATM: ../../../examples/chip-tool/third_party/connectedhomeip/src/lib/support/PersistentStorageAudit.cpp:250: assertion failed: "0 == memcmp(&buf[0], kBase64SymbolValues, strlen(kBase64SymbolValues))"
[1656721506.201437][688277:688277] CHIP:ATM: ../../../examples/chip-tool/third_party/connectedhomeip/src/lib/support/PersistentStorageAudit.cpp:255: assertion failed: "err == CHIP_NO_ERROR"
```

- The audit also crashed on SyncSetKeyValue with nullptr argument

- To fail the audit, had to run the audit on a version that forcibly loaded what
  was stored again.
- Root cause was base64 padding has `=` which confuses INI parser into
  giving out keys that are too small

Issue #20188

This PR:
- Implements `SyncDoesKeyExist` natively
- Fixes handling of `nullptr` arguments and zero-size keys in the storage impl
- Uses `\x00` C-style hex escaping for any characters that could fool the INI parser,
  which retains human readability of keys in the INI files, but fixes the bugs
  found by the audit
- Removes unnecessary newliens in storage audit logging

Testing done:
- Unit tests still pass
- Cert tests still pass
- Storage audit passed

```
[1656722163.492413][696066:696066] CHIP:ATM: ==== PersistentStorageDelegate API audit: SUCCESS ====
```

* Restyled by clang-format

* Remove a stale temporary added during testing

Co-authored-by: Restyled.io <[email protected]>

examples/chip-tool/config/PersistentStorage.cpp

@@ -50,6 +50,29 @@ std::string GetFilename(const char * name)
 
 namespace {
 
+std::string EscapeKey(const std::string & key)
+{
+    std::string escapedKey;
+    escapedKey.reserve(key.size());
+
+    for (char c : key)
+    {
+        // Replace spaces, non-printable chars and `=` with hex-escaped (C-style) characters.
+        if ((c <= 0x20) || (c == '=') || (c >= 0x7F))
+        {
+            char escaped[5] = { 0 };
+            snprintf(escaped, sizeof(escaped), "\\x%02x", (static_cast<unsigned>(c) & 0xff));
+            escapedKey += escaped;
+        }
+        else
+        {
+            escapedKey += c;
+        }
+    }
+
+    return escapedKey;
+}
+
 std::string StringToBase64(const std::string & value)
 {
     std::unique_ptr<char[]> buffer(new char[BASE64_ENCODED_LEN(value.length())]);
@@ -106,10 +129,11 @@ CHIP_ERROR PersistentStorage::SyncGetKeyValue(const char * key, void * value, ui
     ReturnErrorCodeIf(((value == nullptr) && (size != 0)), CHIP_ERROR_INVALID_ARGUMENT);
 
     auto section = mConfig.sections[kDefaultSectionName];
-    auto it      = section.find(key);
-    ReturnErrorCodeIf(it == section.end(), CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND);
 
-    ReturnErrorCodeIf(!inipp::extract(section[key], iniValue), CHIP_ERROR_INVALID_ARGUMENT);
+    ReturnErrorCodeIf(!SyncDoesKeyExist(key), CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND);
+
+    std::string escapedKey = EscapeKey(key);
+    ReturnErrorCodeIf(!inipp::extract(section[escapedKey], iniValue), CHIP_ERROR_INVALID_ARGUMENT);
 
     iniValue = Base64ToString(iniValue);
 
@@ -126,8 +150,19 @@ CHIP_ERROR PersistentStorage::SyncGetKeyValue(const char * key, void * value, ui
 
 CHIP_ERROR PersistentStorage::SyncSetKeyValue(const char * key, const void * value, uint16_t size)
 {
+    ReturnErrorCodeIf((value == nullptr) && (size != 0), CHIP_ERROR_INVALID_ARGUMENT);
+
     auto section = mConfig.sections[kDefaultSectionName];
-    section[key] = StringToBase64(std::string(static_cast<const char *>(value), size));
+
+    std::string escapedKey = EscapeKey(key);
+    if (value == nullptr)
+    {
+        section[escapedKey] = "";
+    }
+    else
+    {
+        section[escapedKey] = StringToBase64(std::string(static_cast<const char *>(value), size));
+    }
 
     mConfig.sections[kDefaultSectionName] = section;
     return CommitConfig(mName);
@@ -136,15 +171,24 @@ CHIP_ERROR PersistentStorage::SyncSetKeyValue(const char * key, const void * val
 CHIP_ERROR PersistentStorage::SyncDeleteKeyValue(const char * key)
 {
     auto section = mConfig.sections[kDefaultSectionName];
-    auto it      = section.find(key);
-    ReturnErrorCodeIf(it == section.end(), CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND);
 
-    section.erase(key);
+    ReturnErrorCodeIf(!SyncDoesKeyExist(key), CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND);
+
+    std::string escapedKey = EscapeKey(key);
+    section.erase(escapedKey);
 
     mConfig.sections[kDefaultSectionName] = section;
     return CommitConfig(mName);
 }
 
+bool PersistentStorage::SyncDoesKeyExist(const char * key)
+{
+    std::string escapedKey = EscapeKey(key);
+    auto section           = mConfig.sections[kDefaultSectionName];
+    auto it                = section.find(escapedKey);
+    return (it != section.end());
+}
+
 CHIP_ERROR PersistentStorage::SyncClearAll()
 {
     ChipLogProgress(chipTool, "Clearing %s storage", kDefaultSectionName);

examples/chip-tool/config/PersistentStorage.h

@@ -32,6 +32,7 @@ class PersistentStorage : public chip::PersistentStorageDelegate
     CHIP_ERROR SyncGetKeyValue(const char * key, void * buffer, uint16_t & size) override;
     CHIP_ERROR SyncSetKeyValue(const char * key, const void * value, uint16_t size) override;
     CHIP_ERROR SyncDeleteKeyValue(const char * key) override;
+    bool SyncDoesKeyExist(const char * key) override;
 
     uint16_t GetListenPort();
     chip::Logging::LogCategory GetLoggingLevel();

src/lib/support/PersistentStorageAudit.cpp

@@ -34,7 +34,7 @@
                                                                                                                                    \
         if (!(inCondition))                                                                                                        \
         {                                                                                                                          \
-            ChipLogError(Automation, "%s:%u: assertion failed: \"%s\"\n", __FILE__, __LINE__, #inCondition);                       \
+            ChipLogError(Automation, "%s:%u: assertion failed: \"%s\"", __FILE__, __LINE__, #inCondition);                         \
             (inSuite)->failedAssertions += 1;                                                                                      \
             (inSuite)->flagError = true;                                                                                           \
         }                                                                                                                          \