Skip to content

Commit

Permalink
Crypto: Fix node id in nonce
Browse files Browse the repository at this point in the history
  • Loading branch information
kghost committed Mar 24, 2022
1 parent 025e0bf commit 25bc9a0
Show file tree
Hide file tree
Showing 10 changed files with 278 additions and 136 deletions.
1 change: 1 addition & 0 deletions src/credentials/tests/CHIPCert_test_vectors.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

#pragma once

#include <credentials/CHIPCert.h>
#include <lib/support/CodeUtils.h>

namespace chip {
Expand Down
9 changes: 9 additions & 0 deletions src/lib/support/Span.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#pragma once

#include <array>
#include <cstdint>
#include <cstdlib>
#include <string.h>
Expand Down Expand Up @@ -46,6 +47,10 @@ class Span
constexpr explicit Span(T (&databuf)[N]) : Span(databuf, N)
{}

template <class U, size_t N, typename = std::enable_if_t<std::is_same<std::remove_const_t<T>, std::remove_const_t<U>>::value>>
constexpr Span(std::array<U, N> & arr) : mDataBuf(arr.data()), mDataLen(N)
{}

template <size_t N>
constexpr Span & operator=(T (&databuf)[N])
{
Expand Down Expand Up @@ -169,6 +174,10 @@ class FixedSpan
static_assert(M >= N, "Passed-in buffer too small for FixedSpan");
}

template <class U, typename = std::enable_if_t<std::is_same<std::remove_const_t<T>, std::remove_const_t<U>>::value>>
constexpr FixedSpan(std::array<U, N> & arr) : mDataBuf(arr.data())
{}

// Allow implicit construction from a FixedSpan of sufficient size over a
// type that matches our type, up to const-ness.
template <class U, size_t M, typename = std::enable_if_t<std::is_same<std::remove_const_t<T>, std::remove_const_t<U>>::value>>
Expand Down
8 changes: 6 additions & 2 deletions src/protocols/secure_channel/tests/TestPASESession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,9 @@ void SecurePairingSerializeTest(nlTestSuite * inSuite, void * inContext)

NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

err = session1.Encrypt(plain_text, sizeof(plain_text), encrypted, header, mac);
CryptoContext::NonceStorage nonce;
CryptoContext::BuildNonce(nonce, header.GetSecurityFlags(), header.GetMessageCounter(), kUndefinedNodeId);
err = session1.Encrypt(plain_text, sizeof(plain_text), encrypted, nonce, header, mac);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
}

Expand All @@ -426,7 +428,9 @@ void SecurePairingSerializeTest(nlTestSuite * inSuite, void * inContext)
testPairingSession2->DeriveSecureSession(session2, CryptoContext::SessionRole::kResponder) == CHIP_NO_ERROR);

uint8_t decrypted[64];
NL_TEST_ASSERT(inSuite, session2.Decrypt(encrypted, sizeof(plain_text), decrypted, header, mac) == CHIP_NO_ERROR);
CryptoContext::NonceStorage nonce;
CryptoContext::BuildNonce(nonce, header.GetSecurityFlags(), header.GetMessageCounter(), kUndefinedNodeId);
NL_TEST_ASSERT(inSuite, session2.Decrypt(encrypted, sizeof(plain_text), decrypted, nonce, header, mac) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, memcmp(plain_text, decrypted, sizeof(plain_text)) == 0);
}

Expand Down
38 changes: 15 additions & 23 deletions src/transport/CryptoContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ namespace chip {

namespace {

constexpr size_t kAESCCMNonceLen = 13;
constexpr size_t kMaxAADLen = 128;
constexpr size_t kMaxAADLen = 128;

/* Session Establish Key Info */
constexpr uint8_t SEKeysInfo[] = { 0x53, 0x65, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x4b, 0x65, 0x79, 0x73 };
Expand Down Expand Up @@ -131,16 +130,13 @@ CHIP_ERROR CryptoContext::InitFromKeyPair(const Crypto::P256Keypair & local_keyp
return InitFromSecret(ByteSpan(secret, secret.Length()), salt, infoType, role);
}

CHIP_ERROR CryptoContext::GetNonce(const PacketHeader & header, uint8_t * nonce, size_t len)
CHIP_ERROR CryptoContext::BuildNonce(NonceView nonce, uint8_t securityFlags, uint32_t messageCounter, NodeId nodeId)
{
Encoding::LittleEndian::BufferWriter bbuf(nonce.data(), nonce.size());

VerifyOrReturnError(len == kAESCCMNonceLen, CHIP_ERROR_INVALID_ARGUMENT);

Encoding::LittleEndian::BufferWriter bbuf(nonce, len);

bbuf.Put8(header.GetSecurityFlags());
bbuf.Put32(header.GetMessageCounter());
bbuf.Put64(header.GetSourceNodeId().ValueOr(0));
bbuf.Put8(securityFlags);
bbuf.Put32(messageCounter);
bbuf.Put64(nodeId);

return bbuf.Fit() ? CHIP_NO_ERROR : CHIP_ERROR_NO_MEMORY;
}
Expand All @@ -161,8 +157,8 @@ CHIP_ERROR CryptoContext::GetAdditionalAuthData(const PacketHeader & header, uin
return CHIP_NO_ERROR;
}

CHIP_ERROR CryptoContext::Encrypt(const uint8_t * input, size_t input_length, uint8_t * output, PacketHeader & header,
MessageAuthenticationCode & mac) const
CHIP_ERROR CryptoContext::Encrypt(const uint8_t * input, size_t input_length, uint8_t * output, ConstNonceView nonce,
PacketHeader & header, MessageAuthenticationCode & mac) const
{

const size_t taglen = header.MICTagLength();
Expand All @@ -174,11 +170,9 @@ CHIP_ERROR CryptoContext::Encrypt(const uint8_t * input, size_t input_length, ui
VerifyOrReturnError(output != nullptr, CHIP_ERROR_INVALID_ARGUMENT);

uint8_t AAD[kMaxAADLen];
uint8_t nonce[kAESCCMNonceLen];
uint16_t aadLen = sizeof(AAD);
uint8_t tag[kMaxTagLen];

ReturnErrorOnFailure(GetNonce(header, nonce, sizeof(nonce)));
ReturnErrorOnFailure(GetAdditionalAuthData(header, AAD, aadLen));

if (mKeyContext)
Expand All @@ -187,7 +181,7 @@ CHIP_ERROR CryptoContext::Encrypt(const uint8_t * input, size_t input_length, ui
MutableByteSpan ciphertext(output, input_length);
MutableByteSpan mic(tag, taglen);

ReturnErrorOnFailure(mKeyContext->EncryptMessage(plaintext, ByteSpan(AAD, aadLen), ByteSpan(nonce), mic, ciphertext));
ReturnErrorOnFailure(mKeyContext->EncryptMessage(plaintext, ByteSpan(AAD, aadLen), nonce, mic, ciphertext));
}
else
{
Expand All @@ -202,29 +196,27 @@ CHIP_ERROR CryptoContext::Encrypt(const uint8_t * input, size_t input_length, ui
usage = kI2RKey;
}

ReturnErrorOnFailure(AES_CCM_encrypt(input, input_length, AAD, aadLen, mKeys[usage], Crypto::kAES_CCM128_Key_Length, nonce,
sizeof(nonce), output, tag, taglen));
ReturnErrorOnFailure(AES_CCM_encrypt(input, input_length, AAD, aadLen, mKeys[usage], Crypto::kAES_CCM128_Key_Length,
nonce.data(), nonce.size(), output, tag, taglen));
}

mac.SetTag(&header, tag, taglen);

return CHIP_NO_ERROR;
}

CHIP_ERROR CryptoContext::Decrypt(const uint8_t * input, size_t input_length, uint8_t * output, const PacketHeader & header,
const MessageAuthenticationCode & mac) const
CHIP_ERROR CryptoContext::Decrypt(const uint8_t * input, size_t input_length, uint8_t * output, ConstNonceView nonce,
const PacketHeader & header, const MessageAuthenticationCode & mac) const
{
const size_t taglen = header.MICTagLength();
const uint8_t * tag = mac.GetTag();
uint8_t nonce[kAESCCMNonceLen];
uint8_t AAD[kMaxAADLen];
uint16_t aadLen = sizeof(AAD);

VerifyOrReturnError(input != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(input_length > 0, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(output != nullptr, CHIP_ERROR_INVALID_ARGUMENT);

ReturnErrorOnFailure(GetNonce(header, nonce, sizeof(nonce)));
ReturnErrorOnFailure(GetAdditionalAuthData(header, AAD, aadLen));

if (nullptr != mKeyContext)
Expand All @@ -233,7 +225,7 @@ CHIP_ERROR CryptoContext::Decrypt(const uint8_t * input, size_t input_length, ui
MutableByteSpan plaintext(output, input_length);
ByteSpan mic(tag, taglen);

CHIP_ERROR err = mKeyContext->DecryptMessage(ciphertext, ByteSpan(AAD, aadLen), ByteSpan(nonce), mic, plaintext);
CHIP_ERROR err = mKeyContext->DecryptMessage(ciphertext, ByteSpan(AAD, aadLen), nonce, mic, plaintext);
ReturnErrorOnFailure(err);
}
else
Expand All @@ -250,7 +242,7 @@ CHIP_ERROR CryptoContext::Decrypt(const uint8_t * input, size_t input_length, ui
}

ReturnErrorOnFailure(AES_CCM_decrypt(input, input_length, AAD, aadLen, tag, taglen, mKeys[usage],
Crypto::kAES_CCM128_Key_Length, nonce, sizeof(nonce), output));
Crypto::kAES_CCM128_Key_Length, nonce.data(), nonce.size(), output));
}
return CHIP_NO_ERROR;
}
Expand Down
18 changes: 13 additions & 5 deletions src/transport/CryptoContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ namespace chip {
class DLL_EXPORT CryptoContext
{
public:
static constexpr size_t kAESCCMNonceLen = 13;
using NonceStorage = std::array<uint8_t, kAESCCMNonceLen>;
using NonceView = FixedSpan<uint8_t, kAESCCMNonceLen>;
using ConstNonceView = FixedSpan<const uint8_t, kAESCCMNonceLen>;

CryptoContext();
~CryptoContext();
CryptoContext(CryptoContext &&) = default;
Expand Down Expand Up @@ -86,19 +91,23 @@ class DLL_EXPORT CryptoContext
*/
CHIP_ERROR InitFromSecret(const ByteSpan & secret, const ByteSpan & salt, SessionInfoType infoType, SessionRole role);

/** @brief Build a Nonce buffer using given parameters for encrypt or decrypt. */
static CHIP_ERROR BuildNonce(NonceView nonce, uint8_t securityFlags, uint32_t messageCounter, NodeId nodeId);

/**
* @brief
* Encrypt the input data using keys established in the secure channel
*
* @param input Unencrypted input data
* @param input_length Length of the input data
* @param output Output buffer for encrypted data
* @param nonce Nonce buffer for encrypt
* @param header message header structure. Encryption type will be set on the header.
* @param mac - output the resulting mac
*
* @return CHIP_ERROR The result of encryption
*/
CHIP_ERROR Encrypt(const uint8_t * input, size_t input_length, uint8_t * output, PacketHeader & header,
CHIP_ERROR Encrypt(const uint8_t * input, size_t input_length, uint8_t * output, ConstNonceView nonce, PacketHeader & header,
MessageAuthenticationCode & mac) const;

/**
Expand All @@ -108,12 +117,13 @@ class DLL_EXPORT CryptoContext
* @param input Encrypted input data
* @param input_length Length of the input data
* @param output Output buffer for decrypted data
* @param nonce Nonce buffer for decrypt
* @param header message header structure
* @return CHIP_ERROR The result of decryption
* @param mac Input mac
*/
CHIP_ERROR Decrypt(const uint8_t * input, size_t input_length, uint8_t * output, const PacketHeader & header,
const MessageAuthenticationCode & mac) const;
CHIP_ERROR Decrypt(const uint8_t * input, size_t input_length, uint8_t * output, ConstNonceView nonce,
const PacketHeader & header, const MessageAuthenticationCode & mac) const;

ByteSpan GetAttestationChallenge() const { return ByteSpan(mKeys[kAttestationChallengeKey], Crypto::kAES_CCM128_Key_Length); }

Expand Down Expand Up @@ -143,8 +153,6 @@ class DLL_EXPORT CryptoContext
CryptoKey mKeys[KeyUsage::kNumCryptoKeys];
Crypto::SymmetricKeyContext * mKeyContext = nullptr;

static CHIP_ERROR GetNonce(const PacketHeader & header, uint8_t * nonce, size_t len);

// Use unencrypted header as additional authenticated data (AAD) during encryption and decryption.
// The encryption operations includes AAD when message authentication tag is generated. This tag
// is used at the time of decryption to integrity check the received data.
Expand Down
12 changes: 6 additions & 6 deletions src/transport/SecureMessageCodec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ using System::PacketBufferHandle;

namespace SecureMessageCodec {

CHIP_ERROR Encrypt(const CryptoContext & context, PayloadHeader & payloadHeader, PacketHeader & packetHeader,
System::PacketBufferHandle & msgBuf)
CHIP_ERROR Encrypt(const CryptoContext & context, CryptoContext::ConstNonceView nonce, PayloadHeader & payloadHeader,
PacketHeader & packetHeader, System::PacketBufferHandle & msgBuf)
{
VerifyOrReturnError(!msgBuf.IsNull(), CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(!msgBuf->HasChainedBuffer(), CHIP_ERROR_INVALID_MESSAGE_LENGTH);
Expand All @@ -52,7 +52,7 @@ CHIP_ERROR Encrypt(const CryptoContext & context, PayloadHeader & payloadHeader,
uint16_t totalLen = msgBuf->TotalLength();

MessageAuthenticationCode mac;
ReturnErrorOnFailure(context.Encrypt(data, totalLen, data, packetHeader, mac));
ReturnErrorOnFailure(context.Encrypt(data, totalLen, data, nonce, packetHeader, mac));

uint16_t taglen = 0;
ReturnErrorOnFailure(mac.Encode(packetHeader, &data[totalLen], msgBuf->AvailableDataLength(), &taglen));
Expand All @@ -63,8 +63,8 @@ CHIP_ERROR Encrypt(const CryptoContext & context, PayloadHeader & payloadHeader,
return CHIP_NO_ERROR;
}

CHIP_ERROR Decrypt(const CryptoContext & context, PayloadHeader & payloadHeader, const PacketHeader & packetHeader,
System::PacketBufferHandle & msg)
CHIP_ERROR Decrypt(const CryptoContext & context, CryptoContext::ConstNonceView nonce, PayloadHeader & payloadHeader,
const PacketHeader & packetHeader, System::PacketBufferHandle & msg)
{
ReturnErrorCodeIf(msg.IsNull(), CHIP_ERROR_INVALID_ARGUMENT);

Expand Down Expand Up @@ -93,7 +93,7 @@ CHIP_ERROR Decrypt(const CryptoContext & context, PayloadHeader & payloadHeader,
msg->SetDataLength(len);

uint8_t * plainText = msg->Start();
ReturnErrorOnFailure(context.Decrypt(data, len, plainText, packetHeader, mac));
ReturnErrorOnFailure(context.Decrypt(data, len, plainText, nonce, packetHeader, mac));

ReturnErrorOnFailure(payloadHeader.DecodeAndConsume(msg));
return CHIP_NO_ERROR;
Expand Down
8 changes: 4 additions & 4 deletions src/transport/SecureMessageCodec.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ namespace SecureMessageCodec {
* the encrypted message.
* @return A CHIP_ERROR value consistent with the result of the encryption operation
*/
CHIP_ERROR Encrypt(const CryptoContext & context, PayloadHeader & payloadHeader, PacketHeader & packetHeader,
System::PacketBufferHandle & msgBuf);
CHIP_ERROR Encrypt(const CryptoContext & context, CryptoContext::ConstNonceView nonce, PayloadHeader & payloadHeader,
PacketHeader & packetHeader, System::PacketBufferHandle & msgBuf);

/**
* @brief
Expand All @@ -66,8 +66,8 @@ CHIP_ERROR Encrypt(const CryptoContext & context, PayloadHeader & payloadHeader,
* the decrypted message.
* @return A CHIP_ERROR value consistent with the result of the decryption operation
*/
CHIP_ERROR Decrypt(const CryptoContext & context, PayloadHeader & payloadHeader, const PacketHeader & packetHeader,
System::PacketBufferHandle & msgBuf);
CHIP_ERROR Decrypt(const CryptoContext & context, CryptoContext::ConstNonceView nonce, PayloadHeader & payloadHeader,
const PacketHeader & packetHeader, System::PacketBufferHandle & msgBuf);

} // namespace SecureMessageCodec

Expand Down
31 changes: 26 additions & 5 deletions src/transport/SessionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,10 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P
VerifyOrReturnError(nullptr != keyContext, CHIP_ERROR_INTERNAL);

packetHeader.SetSessionId(keyContext->GetKeyHash());
CHIP_ERROR err = SecureMessageCodec::Encrypt(CryptoContext(keyContext), payloadHeader, packetHeader, message);
CryptoContext::NonceStorage nonce;
CryptoContext::BuildNonce(nonce, packetHeader.GetSecurityFlags(), packetHeader.GetMessageCounter(),
groupSession->GetSourceNodeId());
CHIP_ERROR err = SecureMessageCodec::Encrypt(CryptoContext(keyContext), nonce, payloadHeader, packetHeader, message);
keyContext->Release();
ReturnErrorOnFailure(err);

Expand All @@ -197,7 +200,18 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P
// Trace before any encryption
CHIP_TRACE_MESSAGE_SENT(payloadHeader, packetHeader, message->Start(), message->TotalLength());

ReturnErrorOnFailure(SecureMessageCodec::Encrypt(session->GetCryptoContext(), payloadHeader, packetHeader, message));
CryptoContext::NonceStorage nonce;
if (session->GetSecureSessionType() == SecureSession::Type::kCASE)
{
FabricInfo * fabric = mFabricTable->FindFabricWithIndex(session->GetFabricIndex());
VerifyOrDie(fabric != nullptr);
CryptoContext::BuildNonce(nonce, packetHeader.GetSecurityFlags(), messageCounter, fabric->GetNodeId());
}
else
{
CryptoContext::BuildNonce(nonce, packetHeader.GetSecurityFlags(), messageCounter, kUndefinedNodeId);
}
ReturnErrorOnFailure(SecureMessageCodec::Encrypt(session->GetCryptoContext(), nonce, payloadHeader, packetHeader, message));
ReturnErrorOnFailure(counter.Advance());

#if CHIP_PROGRESS_LOGGING
Expand Down Expand Up @@ -563,7 +577,11 @@ void SessionManager::SecureUnicastMessageDispatch(const PacketHeader & packetHea

Transport::SecureSession * secureSession = session.Value()->AsSecureSession();
// Decrypt and verify the message before message counter verification or any further processing.
if (SecureMessageCodec::Decrypt(secureSession->GetCryptoContext(), payloadHeader, packetHeader, msg) != CHIP_NO_ERROR)
CryptoContext::NonceStorage nonce;
CryptoContext::BuildNonce(nonce, packetHeader.GetSecurityFlags(), packetHeader.GetMessageCounter(),
secureSession->GetSecureSessionType() == SecureSession::Type::kCASE ? secureSession->GetPeerNodeId()
: kUndefinedNodeId);
if (SecureMessageCodec::Decrypt(secureSession->GetCryptoContext(), nonce, payloadHeader, packetHeader, msg) != CHIP_NO_ERROR)
{
ChipLogError(Inet, "Secure transport received message, but failed to decode/authenticate it, discarding");
return;
Expand Down Expand Up @@ -662,8 +680,11 @@ void SessionManager::SecureGroupMessageDispatch(const PacketHeader & packetHeade
continue;
}
msgCopy = msg.CloneData();
decrypted =
(CHIP_NO_ERROR == SecureMessageCodec::Decrypt(CryptoContext(groupContext.key), payloadHeader, packetHeader, msgCopy));
CryptoContext::NonceStorage nonce;
CryptoContext::BuildNonce(nonce, packetHeader.GetSecurityFlags(), packetHeader.GetMessageCounter(),
packetHeader.GetSourceNodeId().Value());
decrypted = (CHIP_NO_ERROR ==
SecureMessageCodec::Decrypt(CryptoContext(groupContext.key), nonce, payloadHeader, packetHeader, msgCopy));
}
iter->Release();
if (!decrypted)
Expand Down
Loading

0 comments on commit 25bc9a0

Please sign in to comment.