From 382289611d99ff6f6bf80273bac3d8e31b31c3e8 Mon Sep 17 00:00:00 2001 From: Lars Toenning Date: Sun, 12 May 2024 21:26:51 +0200 Subject: [PATCH] fix: Check correct key length for AFV encryption --- .../afv/connection/clientconnectiondata.cpp | 9 ++++++++- src/blackcore/afv/crypto/cryptodtochannel.cpp | 13 +++++++++++++ src/blackcore/afv/crypto/cryptodtoserializer.cpp | 1 + src/blackcore/afv/crypto/cryptodtoserializer.h | 1 + src/blackcore/afv/dto.h | 1 + 5 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/blackcore/afv/connection/clientconnectiondata.cpp b/src/blackcore/afv/connection/clientconnectiondata.cpp index bdf41c55b..2de31e4e4 100644 --- a/src/blackcore/afv/connection/clientconnectiondata.cpp +++ b/src/blackcore/afv/connection/clientconnectiondata.cpp @@ -42,7 +42,14 @@ namespace BlackCore::Afv::Connection CLogMessage(this).warning(u"Tokens not set"); return; } - m_voiceCryptoChannel.reset(new CCryptoDtoChannel(m_tokens.VoiceServer.channelConfig)); + try + { + m_voiceCryptoChannel.reset(new CCryptoDtoChannel(m_tokens.VoiceServer.channelConfig)); + } + catch (const std::invalid_argument &) + { + m_voiceCryptoChannel.reset(); + } } void CClientConnectionData::setTsAuthenticatedToNow() diff --git a/src/blackcore/afv/crypto/cryptodtochannel.cpp b/src/blackcore/afv/crypto/cryptodtochannel.cpp index b947ebe9d..6baf4132f 100644 --- a/src/blackcore/afv/crypto/cryptodtochannel.cpp +++ b/src/blackcore/afv/crypto/cryptodtochannel.cpp @@ -3,6 +3,7 @@ #include "blackcore/afv/crypto/cryptodtochannel.h" #include "blackmisc/verify.h" +#include "sodium/crypto_aead_chacha20poly1305.h" using namespace BlackMisc; @@ -10,6 +11,18 @@ namespace BlackCore::Afv::Crypto { CCryptoDtoChannel::CCryptoDtoChannel(const CryptoDtoChannelConfigDto &channelConfig, int receiveSequenceHistorySize) : m_aeadTransmitKey(channelConfig.aeadTransmitKey), m_aeadReceiveKey(channelConfig.aeadReceiveKey), m_receiveSequenceSizeMaxSize(receiveSequenceHistorySize), m_hmacKey(channelConfig.hmacKey), m_channelTag(channelConfig.channelTag) { + if (m_aeadTransmitKey.size() != crypto_aead_chacha20poly1305_IETF_KEYBYTES) + { + BLACK_AUDIT_X(false, Q_FUNC_INFO, "wrong transmit key size"); + throw std::invalid_argument("wrong transmit key size"); + } + + if (m_aeadReceiveKey.size() != crypto_aead_chacha20poly1305_IETF_KEYBYTES) + { + BLACK_AUDIT_X(false, Q_FUNC_INFO, "wrong receive key size"); + throw std::invalid_argument("wrong receive key size"); + } + if (m_receiveSequenceSizeMaxSize < 1) { m_receiveSequenceSizeMaxSize = 1; } m_receiveSequenceHistory.fill(0, m_receiveSequenceSizeMaxSize); m_receiveSequenceHistoryDepth = 0; diff --git a/src/blackcore/afv/crypto/cryptodtoserializer.cpp b/src/blackcore/afv/crypto/cryptodtoserializer.cpp index 22e210d1d..8f273045f 100644 --- a/src/blackcore/afv/crypto/cryptodtoserializer.cpp +++ b/src/blackcore/afv/crypto/cryptodtoserializer.cpp @@ -48,6 +48,7 @@ namespace BlackCore::Afv::Crypto QByteArray key; if (loopback) { key = channel.getTransmitKey(CryptoDtoMode::AEAD_ChaCha20Poly1305); } else { key = channel.getReceiveKey(CryptoDtoMode::AEAD_ChaCha20Poly1305); } + Q_ASSERT_X(key.size() == crypto_aead_chacha20poly1305_IETF_KEYBYTES, Q_FUNC_INFO, ""); int result = crypto_aead_chacha20poly1305_ietf_decrypt(reinterpret_cast(decryptedPayload.data()), &mlen, nullptr, reinterpret_cast(aePayloadBuffer.constData()), aePayloadBuffer.size(), reinterpret_cast(adBuffer.constData()), adBuffer.size(), diff --git a/src/blackcore/afv/crypto/cryptodtoserializer.h b/src/blackcore/afv/crypto/cryptodtoserializer.h index 81d8a6e6d..611f475aa 100644 --- a/src/blackcore/afv/crypto/cryptodtoserializer.h +++ b/src/blackcore/afv/crypto/cryptodtoserializer.h @@ -35,6 +35,7 @@ namespace BlackCore::Afv::Crypto template static QByteArray serialize(const QString &channelTag, CryptoDtoMode mode, const QByteArray &transmitKey, uint sequenceToBeSent, T dto) { + Q_ASSERT_X(transmitKey.size() == crypto_aead_chacha20poly1305_IETF_KEYBYTES, Q_FUNC_INFO, ""); const CryptoDtoHeaderDto header = { channelTag.toStdString(), sequenceToBeSent, mode }; QBuffer headerBuffer; diff --git a/src/blackcore/afv/dto.h b/src/blackcore/afv/dto.h index fedcbb634..077572201 100644 --- a/src/blackcore/afv/dto.h +++ b/src/blackcore/afv/dto.h @@ -16,6 +16,7 @@ namespace BlackCore::Afv { //! Channel config DTO + //! \warning Data inside the DTO is taken from the network AS IS. No content verification is performed. struct CryptoDtoChannelConfigDto { //! @{