Skip to content

Commit ff5b64d

Browse files
mkardous-silabsrestyled-commitsbzbarsky-appletcarmelveilleux
authored
[ICD] Add Hmac Key handle to ICDCheckInSender to support PSA backend (#30926)
* refactor CheckInMessage impl * finish ICD Check-In cleanup * Restyled by whitespace * Restyled by clang-format * Remove unused includes * Apply suggestions from code review Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> Co-authored-by: Tennessee Carmel-Veilleux <tennessee.carmelveilleux@gmail.com> * update test comments * Restyle ICDCheckInSender * Update error and associated comments * refactor Check-In message impl * rename gargantua to veryLarge * add missing include * Add buffer writer when encoding ActiveModeThreshold * Refactor subspan logic to use a cusorIndex * Apply suggestions from code review Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Fix function comments rename min payload size * Update src/app/icd/ICDCheckInSender.cpp Co-authored-by: Tennessee Carmel-Veilleux <tennessee.carmelveilleux@gmail.com> --------- Co-authored-by: Restyled.io <commits@restyled.io> Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> Co-authored-by: Tennessee Carmel-Veilleux <tennessee.carmelveilleux@gmail.com>
1 parent 5e6db9c commit ff5b64d

File tree

9 files changed

+244
-119
lines changed

9 files changed

+244
-119
lines changed

examples/lit-icd-app/silabs/src/ShellCommands.cpp

-3
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,6 @@
1818
#if defined(ENABLE_CHIP_SHELL)
1919

2020
#include "ShellCommands.h"
21-
#include "BindingHandler.h"
22-
23-
#include <app/clusters/bindings/bindings.h>
2421
#include <lib/shell/Engine.h>
2522
#include <lib/shell/commands/Help.h>
2623
#include <platform/CHIPDeviceLayer.h>

src/app/icd/BUILD.gn

+1
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ source_set("sender") {
8181
]
8282

8383
public_deps = [
84+
":configuration-data",
8485
":monitoring-table",
8586
":notifier",
8687
"${chip_root}/src/credentials:credentials",

src/app/icd/ICDCheckInSender.cpp

+25-11
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,12 @@
1515
* limitations under the License.
1616
*/
1717

18-
#include "ICDCheckInSender.h"
19-
20-
#include "ICDNotifier.h"
21-
22-
#include <system/SystemPacketBuffer.h>
23-
24-
#include <protocols/secure_channel/CheckinMessage.h>
25-
18+
#include <app/icd/ICDCheckInSender.h>
19+
#include <app/icd/ICDConfigurationData.h>
20+
#include <app/icd/ICDNotifier.h>
2621
#include <lib/dnssd/Resolver.h>
22+
#include <protocols/secure_channel/CheckinMessage.h>
23+
#include <system/SystemPacketBuffer.h>
2724

2825
namespace chip {
2926
namespace app {
@@ -56,12 +53,26 @@ void ICDCheckInSender::OnNodeAddressResolutionFailed(const PeerId & peerId, CHIP
5653

5754
CHIP_ERROR ICDCheckInSender::SendCheckInMsg(const Transport::PeerAddress & addr)
5855
{
59-
System::PacketBufferHandle buffer = MessagePacketBuffer::New(CheckinMessage::sMinPayloadSize);
56+
System::PacketBufferHandle buffer = MessagePacketBuffer::New(CheckinMessage::kMinPayloadSize);
6057

6158
VerifyOrReturnError(!buffer.IsNull(), CHIP_ERROR_NO_MEMORY);
6259
MutableByteSpan output{ buffer->Start(), buffer->MaxDataLength() };
6360

64-
ReturnErrorOnFailure(CheckinMessage::GenerateCheckinMessagePayload(mAesKeyHandle, mICDCounter, ByteSpan(), output));
61+
// Encoded ActiveModeThreshold in littleEndian for Check-In message application data
62+
{
63+
uint8_t activeModeThresholdBuffer[kApplicationDataSize] = { 0 };
64+
size_t writtenBytes = 0;
65+
Encoding::LittleEndian::BufferWriter writer(activeModeThresholdBuffer, sizeof(activeModeThresholdBuffer));
66+
67+
writer.Put16(ICDConfigurationData::GetInstance().GetActiveModeThresholdMs());
68+
VerifyOrReturnError(writer.Fit(writtenBytes), CHIP_ERROR_INTERNAL);
69+
70+
ByteSpan activeModeThresholdByteSpan(writer.Buffer(), writtenBytes);
71+
72+
ReturnErrorOnFailure(CheckinMessage::GenerateCheckinMessagePayload(mAes128KeyHandle, mHmac128KeyHandle, mICDCounter,
73+
activeModeThresholdByteSpan, output));
74+
}
75+
6576
buffer->SetDataLength(static_cast<uint16_t>(output.size()));
6677

6778
VerifyOrReturnError(mExchangeManager->GetSessionManager() != nullptr, CHIP_ERROR_INTERNAL);
@@ -89,9 +100,12 @@ CHIP_ERROR ICDCheckInSender::RequestResolve(ICDMonitoringEntry & entry, FabricTa
89100

90101
AddressResolve::NodeLookupRequest request(peerId);
91102

92-
memcpy(mAesKeyHandle.AsMutable<Crypto::Symmetric128BitsKeyByteArray>(),
103+
memcpy(mAes128KeyHandle.AsMutable<Crypto::Symmetric128BitsKeyByteArray>(),
93104
entry.aesKeyHandle.As<Crypto::Symmetric128BitsKeyByteArray>(), sizeof(Crypto::Symmetric128BitsKeyByteArray));
94105

106+
memcpy(mHmac128KeyHandle.AsMutable<Crypto::Symmetric128BitsKeyByteArray>(),
107+
entry.hmacKeyHandle.As<Crypto::Symmetric128BitsKeyByteArray>(), sizeof(Crypto::Symmetric128BitsKeyByteArray));
108+
95109
CHIP_ERROR err = AddressResolve::Resolver::Instance().LookupNode(request, mAddressLookupHandle);
96110

97111
if (err == CHIP_NO_ERROR)

src/app/icd/ICDCheckInSender.h

+4-1
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,17 @@ class ICDCheckInSender : public AddressResolve::NodeListener
4343
bool mResolveInProgress = false;
4444

4545
private:
46+
static constexpr uint8_t kApplicationDataSize = 2; // ActiveModeThreshold is 2 bytes
47+
4648
CHIP_ERROR SendCheckInMsg(const Transport::PeerAddress & addr);
4749

4850
// This is used when a node address is required.
4951
AddressResolve::NodeLookupHandle mAddressLookupHandle;
5052

5153
Messaging::ExchangeManager * mExchangeManager = nullptr;
5254

53-
Crypto::Aes128KeyHandle mAesKeyHandle = Crypto::Aes128KeyHandle();
55+
Crypto::Aes128KeyHandle mAes128KeyHandle = Crypto::Aes128KeyHandle();
56+
Crypto::Hmac128KeyHandle mHmac128KeyHandle = Crypto::Hmac128KeyHandle();
5457

5558
uint32_t mICDCounter = 0;
5659
};

src/app/icd/ICDConfigurationData.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ System::Clock::Milliseconds32 ICDConfigurationData::GetSlowPollingInterval()
2828
// When in SIT mode, the slow poll interval SHOULDN'T be greater than the SIT mode polling threshold, per spec.
2929
// This is important for ICD device configured for LIT operation but currently operating as a SIT
3030
// due to a lack of client registration
31-
if (mICDMode == ICDMode::SIT && GetSlowPollingInterval() > GetSITPollingThreshold())
31+
if (mICDMode == ICDMode::SIT && mSlowPollingInterval > kSITPollingThreshold)
3232
{
33-
return GetSITPollingThreshold();
33+
return kSITPollingThreshold;
3434
}
3535
#endif
3636
return mSlowPollingInterval;

src/app/icd/ICDManager.cpp

-9
Original file line numberDiff line numberDiff line change
@@ -275,15 +275,6 @@ void ICDManager::UpdateOperationState(OperationalState state)
275275

276276
System::Clock::Milliseconds32 slowPollInterval = ICDConfigurationData::GetInstance().GetSlowPollingInterval();
277277

278-
#if ICD_ENFORCE_SIT_SLOW_POLL_LIMIT
279-
// When in SIT mode, the slow poll interval SHOULDN'T be greater than the SIT mode polling threshold, per spec.
280-
if (ICDConfigurationData::GetInstance().GetICDMode() == ICDConfigurationData::ICDMode::SIT &&
281-
GetSlowPollingInterval() > GetSITPollingThreshold())
282-
{
283-
slowPollInterval = GetSITPollingThreshold();
284-
}
285-
#endif
286-
287278
// Going back to Idle, all Check-In messages are sent
288279
mICDSenderPool.ReleaseAll();
289280

src/protocols/secure_channel/CheckinMessage.cpp

+85-38
Original file line numberDiff line numberDiff line change
@@ -30,71 +30,118 @@ namespace chip {
3030
namespace Protocols {
3131
namespace SecureChannel {
3232

33-
CHIP_ERROR CheckinMessage::GenerateCheckinMessagePayload(Crypto::Aes128KeyHandle & key, CounterType counter,
34-
const ByteSpan & appData, MutableByteSpan & output)
33+
CHIP_ERROR CheckinMessage::GenerateCheckinMessagePayload(const Crypto::Aes128KeyHandle & aes128KeyHandle,
34+
const Crypto::Hmac128KeyHandle & hmacKeyHandle,
35+
const CounterType & counter, const ByteSpan & appData,
36+
MutableByteSpan & output)
3537
{
36-
VerifyOrReturnError(appData.size() <= sMaxAppDataSize, CHIP_ERROR_INVALID_ARGUMENT);
37-
VerifyOrReturnError(output.size() >= (appData.size() + sMinPayloadSize), CHIP_ERROR_INVALID_ARGUMENT);
38+
VerifyOrReturnError(output.size() >= (appData.size() + kMinPayloadSize), CHIP_ERROR_BUFFER_TOO_SMALL);
39+
size_t cursorIndex = 0;
3840

39-
CHIP_ERROR err = CHIP_NO_ERROR;
40-
uint8_t * appDataStartPtr = output.data() + CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES;
41-
Encoding::LittleEndian::Put32(appDataStartPtr, counter);
41+
// Generate Nonce from Key and counter value
42+
{
43+
MutableByteSpan nonce = output.SubSpan(0, CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES);
44+
cursorIndex += nonce.size();
4245

43-
chip::Crypto::HMAC_sha shaHandler;
44-
uint8_t nonceWorkBuffer[CHIP_CRYPTO_HASH_LEN_BYTES] = { 0 };
46+
Encoding::LittleEndian::BufferWriter writer(nonce);
47+
ReturnErrorOnFailure(GenerateCheckInMessageNonce(hmacKeyHandle, counter, writer));
48+
}
49+
50+
// Encrypt Counter and Application Data
51+
{
52+
MutableByteSpan payloadByteSpan = output.SubSpan(cursorIndex, sizeof(CounterType) + appData.size());
53+
cursorIndex += payloadByteSpan.size();
4554

46-
ReturnErrorOnFailure(shaHandler.HMAC_SHA256(key.As<Symmetric128BitsKeyByteArray>(), sizeof(Symmetric128BitsKeyByteArray),
47-
appDataStartPtr, sizeof(CounterType), nonceWorkBuffer, CHIP_CRYPTO_HASH_LEN_BYTES));
55+
Encoding::LittleEndian::BufferWriter payloadWriter(payloadByteSpan);
4856

49-
static_assert(sizeof(nonceWorkBuffer) >= CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES, "We're reading off the end of our buffer.");
50-
memcpy(output.data(), nonceWorkBuffer, CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES);
57+
payloadWriter.EndianPut(counter, sizeof(counter));
58+
payloadWriter.Put(appData.data(), appData.size());
59+
VerifyOrReturnError(payloadWriter.Fit(), CHIP_ERROR_INTERNAL);
5160

52-
// In place encryption to save some RAM
53-
memcpy(appDataStartPtr + sizeof(CounterType), appData.data(), appData.size());
61+
MutableByteSpan mic = output.SubSpan(cursorIndex, CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES);
62+
cursorIndex += mic.size();
5463

55-
uint8_t * micPtr = appDataStartPtr + sizeof(CounterType) + appData.size();
56-
ReturnErrorOnFailure(Crypto::AES_CCM_encrypt(appDataStartPtr, sizeof(CounterType) + appData.size(), nullptr, 0, key,
57-
output.data(), CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES, appDataStartPtr, micPtr,
58-
CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES));
64+
// Validate that the cursorIndex is within the available output space
65+
VerifyOrReturnError(cursorIndex <= output.size(), CHIP_ERROR_BUFFER_TOO_SMALL);
66+
// Validate that the cursorIndex matchs the message length
67+
VerifyOrReturnError(cursorIndex == appData.size() + kMinPayloadSize, CHIP_ERROR_INTERNAL);
5968

60-
output.reduce_size(appData.size() + sMinPayloadSize);
69+
ReturnErrorOnFailure(Crypto::AES_CCM_encrypt(payloadByteSpan.data(), payloadByteSpan.size(), nullptr, 0, aes128KeyHandle,
70+
output.data(), CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES, payloadByteSpan.data(),
71+
mic.data(), mic.size()));
72+
}
6173

62-
return err;
74+
output.reduce_size(appData.size() + kMinPayloadSize);
75+
return CHIP_NO_ERROR;
6376
}
6477

65-
CHIP_ERROR CheckinMessage::ParseCheckinMessagePayload(Crypto::Aes128KeyHandle & key, ByteSpan & payload, CounterType & counter,
66-
MutableByteSpan & appData)
78+
CHIP_ERROR CheckinMessage::ParseCheckinMessagePayload(const Crypto::Aes128KeyHandle & aes128KeyHandle,
79+
const Crypto::Hmac128KeyHandle & hmacKeyHandle, ByteSpan & payload,
80+
CounterType & counter, MutableByteSpan & appData)
6781
{
68-
VerifyOrReturnError(payload.size() >= sMinPayloadSize, CHIP_ERROR_INVALID_ARGUMENT);
69-
VerifyOrReturnError(payload.size() <= (sMinPayloadSize + sMaxAppDataSize), CHIP_ERROR_INVALID_ARGUMENT);
70-
71-
CHIP_ERROR err = CHIP_NO_ERROR;
7282
size_t appDataSize = GetAppDataSize(payload);
7383

84+
VerifyOrReturnError(payload.size() >= kMinPayloadSize, CHIP_ERROR_INVALID_MESSAGE_LENGTH);
7485
// To prevent workbuffer usage, appData size needs to be large enough to hold both the appData and the counter
75-
VerifyOrReturnError(appData.size() >= sizeof(CounterType) + appDataSize, CHIP_ERROR_INVALID_ARGUMENT);
86+
VerifyOrReturnError(appData.size() >= sizeof(CounterType) + appDataSize, CHIP_ERROR_BUFFER_TOO_SMALL);
87+
88+
// Decrypt received data
89+
{
90+
size_t cursorIndex = 0;
91+
92+
ByteSpan nonce = payload.SubSpan(cursorIndex, CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES);
93+
cursorIndex += nonce.size();
7694

77-
ByteSpan nonce = payload.SubSpan(0, CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES);
78-
ByteSpan encryptedData = payload.SubSpan(CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES, sizeof(CounterType) + appDataSize);
79-
ByteSpan mic =
80-
payload.SubSpan(CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES + sizeof(CounterType) + appDataSize, CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES);
95+
ByteSpan encryptedData = payload.SubSpan(cursorIndex, sizeof(CounterType) + appDataSize);
96+
cursorIndex += encryptedData.size();
8197

82-
err = Crypto::AES_CCM_decrypt(encryptedData.data(), encryptedData.size(), nullptr, 0, mic.data(), mic.size(), key, nonce.data(),
83-
nonce.size(), appData.data());
98+
ByteSpan mic = payload.SubSpan(cursorIndex, CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES);
99+
cursorIndex += mic.size();
84100

85-
ReturnErrorOnFailure(err);
101+
// Return Invalid message length since the payload isn't the right size
102+
VerifyOrReturnError(cursorIndex == payload.size(), CHIP_ERROR_INVALID_MESSAGE_LENGTH);
86103

104+
ReturnErrorOnFailure(Crypto::AES_CCM_decrypt(encryptedData.data(), encryptedData.size(), nullptr, 0, mic.data(), mic.size(),
105+
aes128KeyHandle, nonce.data(), nonce.size(), appData.data()));
106+
}
107+
108+
// Read decrypted counter and application data
87109
counter = Encoding::LittleEndian::Get32(appData.data());
110+
111+
// TODO : Validate received nonce by calculating it with the hmacKeyHandle and received Counter value
112+
88113
// Shift to remove the counter from the appData
89114
memmove(appData.data(), sizeof(CounterType) + appData.data(), appDataSize);
90-
91115
appData.reduce_size(appDataSize);
92-
return err;
116+
117+
return CHIP_NO_ERROR;
118+
}
119+
120+
CHIP_ERROR CheckinMessage::GenerateCheckInMessageNonce(const Crypto::Hmac128KeyHandle & hmacKeyHandle, CounterType counter,
121+
Encoding::LittleEndian::BufferWriter & writer)
122+
{
123+
VerifyOrReturnError(writer.Available() >= CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES, CHIP_ERROR_BUFFER_TOO_SMALL);
124+
125+
uint8_t hashWorkBuffer[CHIP_CRYPTO_HASH_LEN_BYTES] = { 0 };
126+
uint8_t counterBuffer[sizeof(CounterType)];
127+
128+
// validate that Check-In counter is a uint32_t
129+
static_assert(sizeof(CounterType) == sizeof(uint32_t), "Expect counter to be 32 bits for correct encoding");
130+
Encoding::LittleEndian::Put32(counterBuffer, counter);
131+
132+
chip::Crypto::HMAC_sha shaHandler;
133+
ReturnErrorOnFailure(
134+
shaHandler.HMAC_SHA256(hmacKeyHandle, counterBuffer, sizeof(CounterType), hashWorkBuffer, CHIP_CRYPTO_HASH_LEN_BYTES));
135+
136+
writer.Put(hashWorkBuffer, CHIP_CRYPTO_AEAD_NONCE_LENGTH_BYTES);
137+
VerifyOrReturnError(writer.Fit(), CHIP_ERROR_BUFFER_TOO_SMALL);
138+
139+
return CHIP_NO_ERROR;
93140
}
94141

95142
size_t CheckinMessage::GetAppDataSize(ByteSpan & payload)
96143
{
97-
return (payload.size() <= sMinPayloadSize) ? 0 : payload.size() - sMinPayloadSize;
144+
return (payload.size() <= kMinPayloadSize) ? 0 : payload.size() - kMinPayloadSize;
98145
}
99146

100147
} // namespace SecureChannel

0 commit comments

Comments
 (0)