Skip to content

Commit

Permalink
Fix escaping in Linux app storage (#20289)
Browse files Browse the repository at this point in the history
* Fix escaping in Linux app storage

- Linux apps (like all-clusters-app) use a different storage backend
  than chip-tool, but the backend eventually goes to the same INI
  format.
- The Linux app did not properly manage keys with `=` and `\n` in the
  key, when attempting reload of value.

Fixes #20188

This PR:

- Adds the same escaping to Linux apps as for chip-tool
- Adds unescaping
- Adds unit tests for escaping and unescaping
- Adds debug dumping of all keys to chip-tool storage init

Testing done:

- Added unit tests pass
- Existing unit tests pass
- Cert tests pass
- Manual validation of restart with colliding keys in INI backend
  no longer shows collision after change

* Fix lint

* Fix CI

* Address review comment
  • Loading branch information
tcarmelveilleux authored and pull[bot] committed Sep 29, 2023
1 parent f27a4dd commit 1443906
Show file tree
Hide file tree
Showing 9 changed files with 385 additions and 69 deletions.
82 changes: 26 additions & 56 deletions examples/chip-tool/config/PersistentStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@
#include "PersistentStorage.h"

#include <lib/core/CHIPEncoding.h>
#include <lib/support/Base64.h>
#include <protocols/secure_channel/PASESession.h>
#include <lib/support/IniEscaping.h>

#include <fstream>
#include <memory>
Expand All @@ -30,6 +29,7 @@ using Sections = std::map<String, Section>;

using namespace ::chip;
using namespace ::chip::Controller;
using namespace ::chip::IniEscaping;
using namespace ::chip::Logging;

constexpr const char kDefaultSectionName[] = "Default";
Expand All @@ -48,60 +48,6 @@ std::string GetFilename(const char * name)
return "/tmp/chip_tool_config." + std::string(name) + ".ini";
}

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 the escape itself with hex-escaped (C-style) characters.
if ((c <= 0x20) || (c == '=') || (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())]);

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);
}

} // namespace

CHIP_ERROR PersistentStorage::Init(const char * name)
{
CHIP_ERROR err = CHIP_NO_ERROR;
Expand All @@ -118,6 +64,12 @@ CHIP_ERROR PersistentStorage::Init(const char * name)
mName = name;
mConfig.parse(ifs);
ifs.close();

// To audit the contents at init, uncomment the following:
#if 0
DumpKeys();
#endif

exit:
return err;
}
Expand Down Expand Up @@ -189,6 +141,24 @@ bool PersistentStorage::SyncDoesKeyExist(const char * key)
return (it != section.end());
}

void PersistentStorage::DumpKeys() const
{
#if CHIP_PROGRESS_LOGGING
for (const auto & section : mConfig.sections)
{
const std::string & sectionName = section.first;
const auto & sectionContent = section.second;

ChipLogProgress(chipTool, "[%s]", sectionName.c_str());
for (const auto & entry : sectionContent)
{
const std::string & keyName = entry.first;
ChipLogProgress(chipTool, " => %s", UnescapeKey(keyName).c_str());
}
}
#endif // CHIP_PROGRESS_LOGGING
}

CHIP_ERROR PersistentStorage::SyncClearAll()
{
ChipLogProgress(chipTool, "Clearing %s storage", kDefaultSectionName);
Expand Down
2 changes: 2 additions & 0 deletions examples/chip-tool/config/PersistentStorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ class PersistentStorage : public chip::PersistentStorageDelegate
CHIP_ERROR SyncDeleteKeyValue(const char * key) override;
bool SyncDoesKeyExist(const char * key) override;

void DumpKeys() const;

uint16_t GetListenPort();
chip::Logging::LogCategory GetLoggingLevel();

Expand Down
2 changes: 2 additions & 0 deletions scripts/tools/check_includes_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@
# Not intended for embedded clients (#11705).
'src/app/ClusterStateCache.h': {'list', 'map', 'set', 'vector', 'queue'},
'src/app/BufferedReadCallback.h': {'vector'},
'src/lib/support/IniEscaping.cpp': {'string'},
'src/lib/support/IniEscaping.h': {'string'},

# Itself in DENY.
'src/lib/support/CHIPListUtils.h': {'set'},
Expand Down
2 changes: 2 additions & 0 deletions src/lib/support/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ static_library("support") {
"FibonacciUtils.h",
"FixedBufferAllocator.cpp",
"FixedBufferAllocator.h",
"IniEscaping.cpp",
"IniEscaping.h",
"Iterators.h",
"LifetimePersistedCounter.h",
"ObjectLifeCycle.h",
Expand Down
148 changes: 148 additions & 0 deletions src/lib/support/IniEscaping.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
/*
*
* Copyright (c) 2022 Project CHIP Authors
* All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include <memory>
#include <string>

#include "IniEscaping.h"
#include <lib/support/Base64.h>
#include <lib/support/BytesToHex.h>

namespace chip {
namespace IniEscaping {

namespace {

constexpr size_t kEscapeChunkSize = 4; // "\x12" --> 4 chars

constexpr bool NeedsEscape(char c)
{
return (c <= 0x20) || (c == '=') || (c == '\\') || (c >= 0x7F);
}

constexpr bool IsLowercaseHex(char c)
{
return ((c >= '0') && (c <= '9')) || ((c >= 'a') && (c <= 'f'));
}

bool IsValidEscape(const std::string & s)
{
return (s.size() >= kEscapeChunkSize) && (s[0] == '\\') && (s[1] == 'x') && IsLowercaseHex(s[2]) && IsLowercaseHex(s[3]);
}

} // 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 the escape itself with hex-escaped (C-style) characters.
if (NeedsEscape(c))
{
char escaped[kEscapeChunkSize + 1] = { 0 };
snprintf(escaped, sizeof(escaped), "\\x%02x", (static_cast<unsigned>(c) & 0xff));
escapedKey += escaped;
}
else
{
escapedKey += c;
}
}

return escapedKey;
}

std::string UnescapeKey(const std::string & key)
{
std::string unescaped;
unescaped.reserve(key.size());

size_t idx = 0;
size_t remaining = key.size();
while (remaining > 0)
{
char c = key[idx];
if (c == '\\')
{
// Don't process invalid escapes.
if (remaining < kEscapeChunkSize)
{
return "";
}

auto escapeChunk = key.substr(idx, kEscapeChunkSize);
if (!IsValidEscape(escapeChunk))
{
return "";
}

// We validated format, now extract the last two chars as hex
auto hexDigits = escapeChunk.substr(2, 2);
uint8_t charByte = 0;
if ((chip::Encoding::HexToBytes(hexDigits.data(), 2, &charByte, 1) != 1) || !NeedsEscape(static_cast<char>(charByte)))
{
return "";
}

unescaped += static_cast<char>(charByte);
idx += kEscapeChunkSize;
}
else
{
unescaped += c;
idx += 1;
}

remaining = key.size() - idx;
}

return unescaped;
}

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);
}

} // namespace IniEscaping
} // namespace chip
69 changes: 69 additions & 0 deletions src/lib/support/IniEscaping.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
*
* Copyright (c) 2022 Project CHIP Authors
* All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#pragma once

#include <string>

namespace chip {
namespace IniEscaping {

/**
* @brief Escape a storage key to be INI-safe.
*
* All characters <= 0x20, >= 0x7F and `\` and `=` are
* escaped as `\xYY` where `YY` is a 2-digit lowercase hex value.
*
* @param key - key to escape
* @return the escaped key
*/
std::string EscapeKey(const std::string & key);

/**
* @brief Unescape a storage key escaped by `EscapeKey`
*
* If any character not expected to be escaped is found, or
* if any escape sequences are partial, or if uppercase hex is seen
* in an escape sequence, the empty string is returned.
*
* @param key - key to unescape
* @return the original key that was provided to EscapeKey or empty string on error.
*/
std::string UnescapeKey(const std::string & escapedKey);

/**
* @brief Takes an octet string passed into a std::string and converts it to base64
*
* There may be `\0` characters in the data of std::string.
*
* @param value - Value to convert to base64.
* @return the base64 encoding of the `value` input
*/
std::string StringToBase64(const std::string & value);

/**
* @brief Takes a base64 buffer and converts it to an octet string buffer
* within a std::string.
*
* @param b64Value - Buffer of base64 to decode
* @return an std::string with the bytes, or empty string on decoding errors
*/
std::string Base64ToString(const std::string & b64Value);

} // namespace IniEscaping
} // namespace chip
1 change: 1 addition & 0 deletions src/lib/support/tests/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ chip_test_suite("tests") {
"TestErrorStr.cpp",
"TestFixedBufferAllocator.cpp",
"TestFold.cpp",
"TestIniEscaping.cpp",
"TestIntrusiveList.cpp",
"TestOwnerOf.cpp",
"TestPersistedCounter.cpp",
Expand Down
Loading

0 comments on commit 1443906

Please sign in to comment.