From 10e681d87fc50a88c59234735bed1c9b9bdd9cc9 Mon Sep 17 00:00:00 2001 From: Bryan Biedenkapp Date: Wed, 7 Oct 2026 14:19:07 -0400 Subject: [PATCH] harden OTAR KMM handling and add bearer integration tests; --- src/common/p25/kmm/KMMNegativeAck.cpp | 10 +- src/common/p25/kmm/KMMNegativeAck.h | 2 +- src/common/p25/kmm/KMMRekeyAck.cpp | 8 +- src/common/p25/kmm/KMMRekeyAck.h | 2 +- src/fne/network/P25OTARService.cpp | 176 +++++++++++++++++++------- src/fne/network/P25OTARService.h | 4 +- tests/fne/FNETestHooks.cpp | 122 +++++++++++++++++- tests/fne/FNETestHooks.h | 49 ++++++- tests/p25/P25_KMM_Tests.cpp | 147 ++++++++++++++++++++- 9 files changed, 455 insertions(+), 65 deletions(-) diff --git a/src/common/p25/kmm/KMMNegativeAck.cpp b/src/common/p25/kmm/KMMNegativeAck.cpp index c480f5b9..86ea8389 100644 --- a/src/common/p25/kmm/KMMNegativeAck.cpp +++ b/src/common/p25/kmm/KMMNegativeAck.cpp @@ -25,11 +25,11 @@ using namespace p25::kmm; /* Initializes a new instance of the KMMNegativeAck class. */ KMMNegativeAck::KMMNegativeAck() : KMMFrame(), - m_messageId(0U), + m_nakMessageId(0U), m_messageNo(0U), m_status(KMM_Status::CMD_NOT_PERFORMED) { - KMMFrame::m_messageId = KMM_MessageType::NAK; + m_messageId = KMM_MessageType::NAK; m_respKind = KMM_ResponseKind::NONE; } @@ -53,7 +53,7 @@ bool KMMNegativeAck::decode(const uint8_t* data) KMMFrame::decodeHeader(data); - m_messageId = data[10U + m_bodyOffset]; // Message ID + m_nakMessageId = data[10U + m_bodyOffset]; // Message ID m_messageNo = GET_UINT16(data, 11U + m_bodyOffset); // Message Number m_status = data[13U + m_bodyOffset]; // Status @@ -69,7 +69,7 @@ void KMMNegativeAck::encode(uint8_t* data) KMMFrame::encodeHeader(data); - data[10U + m_bodyOffset] = m_messageId; // Message ID + data[10U + m_bodyOffset] = m_nakMessageId; // Message ID SET_UINT16(m_messageNo, data, 11U + m_bodyOffset); // Message Number data[13U + m_bodyOffset] = m_status; // Status } @@ -91,7 +91,7 @@ void KMMNegativeAck::copy(const KMMNegativeAck& data) { KMMFrame::copy(data); - m_messageId = data.m_messageId; + m_nakMessageId = data.m_nakMessageId; m_messageNo = data.m_messageNo; m_status = data.m_status; } diff --git a/src/common/p25/kmm/KMMNegativeAck.h b/src/common/p25/kmm/KMMNegativeAck.h index 7002f680..f678c9da 100644 --- a/src/common/p25/kmm/KMMNegativeAck.h +++ b/src/common/p25/kmm/KMMNegativeAck.h @@ -83,7 +83,7 @@ namespace p25 /** * @brief */ - DECLARE_PROPERTY(uint8_t, messageId, MessageId); + DECLARE_PROPERTY(uint8_t, nakMessageId, NakMessageId); /** * @brief */ diff --git a/src/common/p25/kmm/KMMRekeyAck.cpp b/src/common/p25/kmm/KMMRekeyAck.cpp index 61a321f5..f574742d 100644 --- a/src/common/p25/kmm/KMMRekeyAck.cpp +++ b/src/common/p25/kmm/KMMRekeyAck.cpp @@ -25,7 +25,7 @@ using namespace p25::kmm; /* Initializes a new instance of the KMMRekeyAck class. */ KMMRekeyAck::KMMRekeyAck() : KMMFrame(), - m_messageId(0U), + m_ackMessageId(0U), m_numberOfKeyStatus(0U), m_keystatus() { @@ -55,7 +55,7 @@ bool KMMRekeyAck::decode(const uint8_t* data) KMMFrame::decodeHeader(data); - m_messageId = data[10U + m_bodyOffset]; // Message ID + m_ackMessageId = data[10U + m_bodyOffset]; // Message ID m_numberOfKeyStatus = data[11U + m_bodyOffset]; // Number of Key Status uint16_t offset = 0U; @@ -85,7 +85,7 @@ void KMMRekeyAck::encode(uint8_t* data) KMMFrame::encodeHeader(data); - data[10U + m_bodyOffset] = m_messageId; // Message ID + data[10U + m_bodyOffset] = m_ackMessageId; // Message ID data[11U + m_bodyOffset] = m_numberOfKeyStatus; // Number of Key Status uint16_t offset = 0U; @@ -115,7 +115,7 @@ void KMMRekeyAck::copy(const KMMRekeyAck& data) { KMMFrame::copy(data); - m_messageId = data.m_messageId; + m_ackMessageId = data.m_ackMessageId; m_numberOfKeyStatus = data.m_numberOfKeyStatus; m_keystatus = data.m_keystatus; diff --git a/src/common/p25/kmm/KMMRekeyAck.h b/src/common/p25/kmm/KMMRekeyAck.h index 3c359ba9..580dff55 100644 --- a/src/common/p25/kmm/KMMRekeyAck.h +++ b/src/common/p25/kmm/KMMRekeyAck.h @@ -84,7 +84,7 @@ namespace p25 /** * @brief */ - DECLARE_PROPERTY(uint8_t, messageId, MessageId); + DECLARE_PROPERTY(uint8_t, ackMessageId, AckMessageId); /** * @brief */ diff --git a/src/fne/network/P25OTARService.cpp b/src/fne/network/P25OTARService.cpp index b01f403d..1b89c82c 100644 --- a/src/fne/network/P25OTARService.cpp +++ b/src/fne/network/P25OTARService.cpp @@ -100,6 +100,27 @@ bool allowsUnauthenticatedKMM(uint8_t messageId) } } +/** + * @brief Determines if a KMM message type requires outer encryption. + * @param messageId The KMM message type identifier. + * @return true if the message type requires outer encryption, false otherwise. + */ +bool requiresEncryptedKMM(uint8_t messageId) +{ + switch (messageId) { + case KMM_MessageType::HELLO: + case KMM_MessageType::NO_SERVICE: + case KMM_MessageType::DEREG_CMD: + case KMM_MessageType::DEREG_RSP: + case KMM_MessageType::REG_CMD: + case KMM_MessageType::REG_RSP: + case KMM_MessageType::UNABLE_TO_DECRYPT: + return false; + default: + return true; + } +} + // --------------------------------------------------------------------------- // Class Declaration // --------------------------------------------------------------------------- @@ -137,6 +158,7 @@ P25OTARService::P25OTARService(TrafficNetwork* network, P25PacketData* packetDat m_rsiMessageNumber(), m_rsiInboundMessageNumber(), m_rsiInboundFingerprint(), + m_dliRegistered(), m_allowNoUKEKRekey(false), m_debug(debug), m_verbose(verbose) @@ -182,19 +204,11 @@ bool P25OTARService::processDLD(const uint8_t* data, uint32_t len, uint32_t llId ::memcpy(resolvedMI, mi, MI_LENGTH_BYTES); } else { - // legacy fallback for payload-embedded KMM encryption parameters - if (len < 11U) { - LogError(LOG_P25, P25_KMM_STR ", encrypted KMM payload too short, len = %u", len); - sendNack(PDUAckType::NACK_ILLEGAL); - return false; - } - - for (uint8_t i = 0; i < MI_LENGTH_BYTES; i++) { - resolvedMI[i] = data[i]; - } - - resolvedAlgoId = data[9U]; - resolvedKId = GET_UINT16(data, 10U); + // AACA-D 12.2 encrypted DLD requires the Auxiliary ES context. + // Do not guess at a non-standard payload-embedded layout. + LogError(LOG_P25, P25_KMM_STR ", encrypted DLD missing Auxiliary ES metadata"); + sendNack(PDUAckType::NACK_ILLEGAL); + return false; } kmmPayload = cryptKMM(resolvedAlgoId, resolvedKId, resolvedMI, data, len, false); @@ -205,9 +219,8 @@ bool P25OTARService::processDLD(const uint8_t* data, uint32_t len, uint32_t llId } } - std::unique_ptr frame = KMMFactory::create(kmmPayload.get(), len); - if (frame == nullptr) { - LogWarning(LOG_P25, P25_KMM_STR ", undecodable KMM packet"); + if (len < 10U || len > 512U || ((((uint32_t)kmmPayload[1U] << 8U) | kmmPayload[2U]) + 3U) != len) { + LogWarning(LOG_P25, P25_KMM_STR ", invalid KMM length, len = %u", len); sendNack(PDUAckType::NACK_ILLEGAL); return false; } @@ -225,8 +238,22 @@ bool P25OTARService::processDLD(const uint8_t* data, uint32_t len, uint32_t llId return true; } - // handle DLD encrypted KMM frame - if (encrypted) { + bool responseEncrypted = encrypted; + if (!responseEncrypted) { + std::unique_ptr response = KMMFactory::create(pduUserData.get(), payloadSize); + if (response != nullptr && requiresEncryptedKMM(response->getMessageId())) { + if (response->getMACType() != KMM_MAC::ENH_MAC || response->getMACAlgId() != ALGO_AES_256) { + LogError(LOG_P25, P25_KMM_STR ", secured response has no usable outer TEK context"); + sendNack(PDUAckType::NACK_UNDELIVERABLE); + return false; + } + resolvedAlgoId = response->getMACAlgId(); + resolvedKId = response->getMACKId(); + responseEncrypted = true; + } + } + + if (responseEncrypted) { // re-encrypt the KMM response pduUserData = cryptKMM(resolvedAlgoId, resolvedKId, resolvedMI, pduUserData.get(), payloadSize, true); if (pduUserData == nullptr) { @@ -236,7 +263,8 @@ bool P25OTARService::processDLD(const uint8_t* data, uint32_t len, uint32_t llId } } - m_packetData->write_PDU_KMM(pduUserData.get(), payloadSize, llId, encrypted, resolvedAlgoId, resolvedKId, resolvedMI); + m_packetData->write_PDU_KMM(pduUserData.get(), payloadSize, llId, responseEncrypted, + resolvedAlgoId, resolvedKId, resolvedMI); m_packetData->write_PDU_Ack_Response(PDUAckClass::ACK, PDUAckType::ACK, n, llId, false); return true; } @@ -369,7 +397,7 @@ void P25OTARService::taskNetworkRx(OTARPacketRequest* req) uint32_t payloadSize = 0U; UInt8Array pduUserData = network->processKMM(buffer.get(), kmmLength, 0U, false, &payloadSize, - encrypted ? algoId : ALGO_UNENCRYPT, encrypted ? kid : 0U, encrypted ? mi : nullptr); + encrypted ? algoId : ALGO_UNENCRYPT, encrypted ? kid : 0U, encrypted ? mi : nullptr, true); if (pduUserData == nullptr || payloadSize == 0U) { if (network->m_debug) LogDebug(LOG_P25, P25_KMM_STR ", no KMM response generated for network request"); @@ -380,7 +408,23 @@ void P25OTARService::taskNetworkRx(OTARPacketRequest* req) return; } - if (encrypted) { + bool responseEncrypted = encrypted; + if (!responseEncrypted) { + std::unique_ptr response = KMMFactory::create(pduUserData.get(), payloadSize); + if (response != nullptr && requiresEncryptedKMM(response->getMessageId())) { + if (response->getMACType() != KMM_MAC::ENH_MAC || response->getMACAlgId() != ALGO_AES_256) { + LogError(LOG_P25, P25_KMM_STR ", secured DLI response has no usable outer TEK context"); + if (req->buffer != nullptr) delete[] req->buffer; + delete req; + return; + } + algoId = response->getMACAlgId(); + kid = response->getMACKId(); + responseEncrypted = true; + } + } + + if (responseEncrypted) { // re-encrypt the KMM response pduUserData = network->cryptKMM(algoId, kid, mi, pduUserData.get(), payloadSize, true); if (pduUserData == nullptr) { @@ -397,10 +441,10 @@ void P25OTARService::taskNetworkRx(OTARPacketRequest* req) datagram[0U] = 0U; // Version-0 preamble format datagram[1U] = MFG_STANDARD; - datagram[2U] = encrypted ? algoId : ALGO_UNENCRYPT; - SET_UINT16(encrypted ? kid : 0U, datagram.get(), 3U); + datagram[2U] = responseEncrypted ? algoId : ALGO_UNENCRYPT; + SET_UINT16(responseEncrypted ? kid : 0U, datagram.get(), 3U); - if (encrypted) + if (responseEncrypted) ::memcpy(datagram.get() + 5U, mi, MI_LENGTH_BYTES); ::memcpy(datagram.get() + 14U, pduUserData.get(), payloadSize); @@ -447,6 +491,8 @@ UInt8Array P25OTARService::cryptKMM(uint8_t algoId, uint16_t kid, uint8_t* mi, c */ ::EKCKeyItem keyItem = m_network->m_cryptoLookup->find(kid); + // find() searches the existing EKC collection only. UKEKs are kept + // in the separate collection and are available only via findUKEK(). if (!keyItem.isInvalid()) { uint8_t key[P25DEF::MAX_ENC_KEY_LENGTH_BYTES]; ::memset(key, 0x00U, P25DEF::MAX_ENC_KEY_LENGTH_BYTES); @@ -476,11 +522,17 @@ UInt8Array P25OTARService::cryptKMM(uint8_t algoId, uint16_t kid, uint8_t* mi, c /* Helper used to process KMM frames. */ UInt8Array P25OTARService::processKMM(const uint8_t* data, uint32_t len, uint32_t llId, bool encrypted, uint32_t* payloadSize, - uint8_t algoId, uint16_t kid, const uint8_t* mi) + uint8_t algoId, uint16_t kid, const uint8_t* mi, bool dataLinkIndependent) { if (payloadSize != nullptr) *payloadSize = 0U; + if (data == nullptr || len < 10U || len > 512U || + ((((uint32_t)data[1U] << 8U) | data[2U]) + 3U) != len) { + LogWarning(LOG_P25, P25_KMM_STR ", invalid KMM framing length, len = %u", len); + return nullptr; + } + UInt8Array buffer = std::make_unique(len); ::memset(buffer.get(), 0x00U, len); ::memcpy(buffer.get(), data, len); @@ -498,22 +550,8 @@ UInt8Array P25OTARService::processKMM(const uint8_t* data, uint32_t len, uint32_ ::memcpy(resolvedMI, mi, MI_LENGTH_BYTES); buffer = cryptKMM(algoId, kid, resolvedMI, data, len, false); } - else { - // legacy fallback for payload-embedded KMM encryption parameters - if (len < 11U) { - LogError(LOG_P25, P25_KMM_STR ", encrypted KMM payload too short, len = %u", len); - return nullptr; - } - - for (uint8_t i = 0; i < MI_LENGTH_BYTES; i++) { - resolvedMI[i] = data[i]; - } - - resolvedAlgoId = data[9U]; - resolvedKId = GET_UINT16(data, 10U); - - buffer = cryptKMM(resolvedAlgoId, resolvedKId, resolvedMI, data + 10U, len - 10U, false); - } + else + return nullptr; if (buffer == nullptr) { LogError(LOG_P25, P25_KMM_STR ", unable to decrypt KMM, algoId = $%02X, kID = $%04X", resolvedAlgoId, resolvedKId); @@ -566,7 +604,6 @@ UInt8Array P25OTARService::processKMM(const uint8_t* data, uint32_t len, uint32_ return nullptr; } - // AACA-D 7.4: validate addressing, MN/MAC coupling, authentication and // anti-replay state before executing message-specific behavior if (frame->getDstLLId() != WUID_FNE || frame->getSrcLLId() == 0U || @@ -575,6 +612,14 @@ UInt8Array P25OTARService::processKMM(const uint8_t* data, uint32_t len, uint32_ return nullptr; } + const bool outerEncrypted = algoId != ALGO_UNENCRYPT; + if (requiresEncryptedKMM(frame->getMessageId()) && !outerEncrypted) { + LogWarning(LOG_P25, P25_KMM_STR ", encrypted KMM required, messageId = $%02X, RSI = %u", + frame->getMessageId(), frame->getSrcLLId()); + + return nullptr; + } + // initialize the authentication context based on the frame's MAC information KMMAuthContext auth; auth.hasMessageNumber = frame->getHasMessageNumber(); @@ -583,6 +628,18 @@ UInt8Array P25OTARService::processKMM(const uint8_t* data, uint32_t len, uint32_ auth.keyId = frame->getMACKId(); auth.format = frame->getMACFormat(); + // AACA-D Table 79 requires DLI registration -- registration itself and a + // repeated deregistration are allowed before registered service exists + if (dataLinkIndependent && frame->getMessageId() != KMM_MessageType::REG_CMD && + frame->getMessageId() != KMM_MessageType::DEREG_CMD && + m_dliRegistered.find(frame->getSrcLLId()) == m_dliRegistered.end()) { + LogWarning(LOG_P25, P25_KMM_STR ", rejecting unregistered DLI RSI = %u", frame->getSrcLLId()); + if (frame->getResponseKind() == KMM_ResponseKind::IMMEDIATE) + return write_KMM_NoService(llId, frame->getSrcLLId(), payloadSize, auth); + + return nullptr; + } + // bryanb: yay CPP lambdas! auto makeNack = [&](uint8_t status) -> UInt8Array { // table 55 requires a NACK to be encrypted and authenticated - prefer @@ -749,8 +806,8 @@ UInt8Array P25OTARService::processKMM(const uint8_t* data, uint32_t len, uint32_ case KMM_MessageType::NAK: { KMMNegativeAck* kmm = static_cast(frame.get()); - LogInfoEx(LOG_P25, P25_KMM_STR ", %s, llId = %u, messageId = $%02X, messageNo = %u, status = $%02X", kmm->toString().c_str(), - llId, kmm->getMessageId(), kmm->getMessageNumber(), kmm->getStatus()); + LogInfoEx(LOG_P25, P25_KMM_STR ", %s, llId = %u, nackMessageId = $%02X, messageNo = %u, status = $%02X", kmm->toString().c_str(), + llId, kmm->getNakMessageId(), kmm->getMessageNumber(), kmm->getStatus()); logResponseStatus(llId, kmm->toString(), kmm->getStatus()); } break; @@ -758,8 +815,8 @@ UInt8Array P25OTARService::processKMM(const uint8_t* data, uint32_t len, uint32_ case KMM_MessageType::REKEY_ACK: { KMMRekeyAck* kmm = static_cast(frame.get()); - LogInfoEx(LOG_P25, P25_KMM_STR ", %s, llId = %u, messageId = $%02X, numOfStatus = %u", kmm->toString().c_str(), - llId, kmm->getMessageId(), kmm->getNumberOfKeyStatus()); + LogInfoEx(LOG_P25, P25_KMM_STR ", %s, llId = %u, ackMessageId = $%02X, numOfStatus = %u", kmm->toString().c_str(), + llId, kmm->getAckMessageId(), kmm->getNumberOfKeyStatus()); if (kmm->getNumberOfKeyStatus() > 0U) { for (auto entry : kmm->getKeyStatus()) { @@ -795,8 +852,25 @@ UInt8Array P25OTARService::processKMM(const uint8_t* data, uint32_t len, uint32_ // respond with No-Service if KMF services are disabled if (!m_network->m_kmfServicesEnabled) return write_KMM_NoService(llId, kmm->getSrcLLId(), payloadSize, auth); - else + else { + m_dliRegistered.erase(kmm->getSrcLLId()); return write_KMM_Dereg_Response(llId, kmm->getSrcLLId(), payloadSize, auth); + } + } + break; + case KMM_MessageType::REG_CMD: + { + KMMRegistrationCommand* kmm = static_cast(frame.get()); + if (kmm->getKMFRSI() != WUID_FNE || !m_network->m_kmfServicesEnabled) + return write_KMM_NoService(llId, kmm->getSrcLLId(), payloadSize, auth); + + m_dliRegistered[kmm->getSrcLLId()] = true; + KMMRegistrationResponse response; + response.setSrcLLId(WUID_FNE); + response.setDstLLId(kmm->getSrcLLId()); + response.setResponseKind(KMM_ResponseKind::NONE); + response.setStatus(KMM_Status::CMD_PERFORMED); + return encode_KMM_Response(response, payloadSize, auth); } break; case KMM_MessageType::REG_RSP: @@ -815,8 +889,13 @@ UInt8Array P25OTARService::processKMM(const uint8_t* data, uint32_t len, uint32_ llId, kmm->getStatus()); logResponseStatus(llId, kmm->toString(), kmm->getStatus()); } + break; default: + { + LogWarning(LOG_P25, P25_KMM_STR ", unsupported inbound messageId = $%02X, RSI = %u", frame->getMessageId(), frame->getSrcLLId()); + return makeNack(KMM_Status::INVALID_MSG_ID); + } break; } // switch (frame->getMessageId()) @@ -1078,7 +1157,7 @@ UInt8Array P25OTARService::write_KMM_NegativeAck(uint32_t kmmRSI, uint8_t messag KMMNegativeAck outKmm; outKmm.setSrcLLId(WUID_FNE); outKmm.setDstLLId(kmmRSI); - outKmm.setMessageId(messageId); + outKmm.setNakMessageId(messageId); outKmm.setMessageNumber(messageNumber); outKmm.setStatus(status); @@ -1121,6 +1200,7 @@ UInt8Array P25OTARService::encode_KMM_Response(KMMFrame& frame, uint32_t* payloa frame.encode(encoded.get()); if (auth.authenticated) frame.generateMAC(macTek, encoded.get()); + return encoded; } diff --git a/src/fne/network/P25OTARService.h b/src/fne/network/P25OTARService.h index b491f5cc..338d9fb3 100644 --- a/src/fne/network/P25OTARService.h +++ b/src/fne/network/P25OTARService.h @@ -129,6 +129,7 @@ namespace network concurrent::unordered_map m_rsiMessageNumber; concurrent::unordered_map m_rsiInboundMessageNumber; concurrent::unordered_map m_rsiInboundFingerprint; + concurrent::unordered_map m_dliRegistered; bool m_allowNoUKEKRekey; @@ -163,7 +164,8 @@ namespace network * @returns UInt8Array Buffer containing the processed KMM frame (if any). */ UInt8Array processKMM(const uint8_t* data, uint32_t len, uint32_t llId, bool encrypted, uint32_t* payloadSize, - uint8_t algoId = P25DEF::ALGO_UNENCRYPT, uint16_t kid = 0U, const uint8_t* mi = nullptr); + uint8_t algoId = P25DEF::ALGO_UNENCRYPT, uint16_t kid = 0U, const uint8_t* mi = nullptr, + bool dataLinkIndependent = false); /** * @brief Helper used to return a Rekey-Command KMM to the calling SU. diff --git a/tests/fne/FNETestHooks.cpp b/tests/fne/FNETestHooks.cpp index 29f31e94..ddf7311b 100644 --- a/tests/fne/FNETestHooks.cpp +++ b/tests/fne/FNETestHooks.cpp @@ -10,12 +10,16 @@ #include "fne/FNETestHooks.h" #include "fne/HostFNE.h" #include "fne/network/P25OTARService.h" +#include "common/p25/data/Assembler.h" +#include "common/Utils.h" #include #include #include using namespace network; +using namespace p25::defines; +using namespace p25::data; // --------------------------------------------------------------------------- // Global Variables @@ -80,14 +84,14 @@ FNEPeerConnection& FNETestHooks::addPeer(TrafficNetwork& network, uint32_t peerI */ std::unique_ptr FNETestHooks::processOTARKMM(TrafficNetwork& network, const std::vector& packet, uint32_t llId, uint32_t& payloadSize, - uint8_t outerAlgoId, uint16_t outerKId) + uint8_t outerAlgoId, uint16_t outerKId, bool dataLinkIndependent) { payloadSize = 0U; if (packet.empty() || network.m_p25OTARService == nullptr) return nullptr; return network.m_p25OTARService->processKMM(packet.data(), (uint32_t)packet.size(), llId, - false, &payloadSize, outerAlgoId, outerKId); + false, &payloadSize, outerAlgoId, outerKId, nullptr, dataLinkIndependent); } /** @@ -112,6 +116,64 @@ bool FNETestHooks::processOTARDLD(TrafficNetwork& network, const std::vector& packet, + uint32_t llId, bool encrypted, uint8_t algoId, uint16_t kid, const uint8_t* mi) +{ + if (packet.empty() || network.m_tagP25 == nullptr) + return false; + + p25::data::DataHeader header; + header.setFormat(PDUFormatType::CONFIRMED); + header.setMFId(MFG_STANDARD); + header.setAckNeeded(true); + header.setOutbound(false); + header.setSAP(encrypted ? PDUSAP::ENC_USER_DATA : PDUSAP::UNENC_KMM); + header.setLLId(llId); + header.setFullMessage(true); + header.setBlocksToFollow(1U); + if (encrypted) { + if (mi == nullptr) + return false; + header.setEXSAP(PDUSAP::UNENC_KMM); + header.setAlgId(algoId); + header.setKId(kid); + header.setMI(mi); + } + header.calculateLength((uint32_t)packet.size()); + + p25::data::Assembler assembler; + uint32_t bitLength = 0U; + UInt8Array assembled = assembler.assemble(header, false, encrypted, packet.data(), &bitLength); + if (assembled == nullptr || bitLength <= P25_PREAMBLE_LENGTH_BITS) + return false; + + bitLength -= header.getPadLength() * 8U; + const uint32_t blockCount = (bitLength - P25_PREAMBLE_LENGTH_BITS) / P25_PDU_FEC_LENGTH_BITS; + auto* packetData = network.m_tagP25->packetData(); + for (uint32_t block = 0U; block < blockCount; ++block) { + uint8_t envelope[24U + P25_PDU_FEC_LENGTH_BYTES] = { 0U }; + SET_UINT24(P25_PDU_FEC_LENGTH_BYTES, envelope, 8U); + envelope[20U] = (uint8_t)(blockCount - 1U); + envelope[21U] = (uint8_t)block; + Utils::getBitRange(assembled.get(), envelope + 24U, + P25_PREAMBLE_LENGTH_BITS + block * P25_PDU_FEC_LENGTH_BITS, P25_PDU_FEC_LENGTH_BITS); + if (!packetData->processFrame(envelope, sizeof(envelope), 1U, (uint16_t)block, 1U)) + return false; + } + return true; +} + /** * @brief Passes a complete Version-0 DLI datagram through the network receive task. * @param network The TrafficNetwork instance. @@ -134,6 +196,62 @@ void FNETestHooks::processOTARDLI(TrafficNetwork& network, const std::vectoropen(address, port); +} + +/** + * @brief Polls the real DLI UDP endpoint. + * @param network The TrafficNetwork instance. + * @param ms The number of milliseconds to wait. + */ +void FNETestHooks::clockOTARDLI(TrafficNetwork& network, uint32_t ms) +{ + if (network.m_p25OTARService != nullptr) + network.m_p25OTARService->clock(ms); +} + +/** + * @brief Closes the real DLI UDP endpoint. + * @param network The TrafficNetwork instance. + */ +void FNETestHooks::closeOTARDLI(TrafficNetwork& network) +{ + if (network.m_p25OTARService != nullptr) + network.m_p25OTARService->close(); +} + +/** + * @brief Applies the OTAR outer encryption/decryption implementation. + * @param network The TrafficNetwork instance. + * @param algoId The algorithm ID used for encryption/decryption. + * @param kid The key ID used for encryption/decryption. + * @param mi The message integrity value. + * @param packet The packet data to be encrypted/decrypted. + * @param encrypt True to encrypt, false to decrypt. + * @return The resulting encrypted/decrypted packet. + */ +std::vector FNETestHooks::cryptOTARKMM(TrafficNetwork& network, uint8_t algoId, + uint16_t kid, uint8_t* mi, const std::vector& packet, bool encrypt) +{ + if (packet.empty() || network.m_p25OTARService == nullptr) + return {}; + + UInt8Array result = network.m_p25OTARService->cryptKMM(algoId, kid, mi, + packet.data(), (uint32_t)packet.size(), encrypt); + if (result == nullptr) + return {}; + return std::vector(result.get(), result.get() + packet.size()); +} + /** * @brief Checks if the specified inbound message number exists for the given RSI. * @param network The TrafficNetwork instance. diff --git a/tests/fne/FNETestHooks.h b/tests/fne/FNETestHooks.h index e600c43e..d87e39ea 100644 --- a/tests/fne/FNETestHooks.h +++ b/tests/fne/FNETestHooks.h @@ -15,6 +15,7 @@ #include #include +#include #include // --------------------------------------------------------------------------- @@ -95,7 +96,8 @@ public: */ static std::unique_ptr processOTARKMM(network::TrafficNetwork& network, const std::vector& packet, uint32_t llId, uint32_t& payloadSize, - uint8_t outerAlgoId = P25DEF::ALGO_UNENCRYPT, uint16_t outerKId = 0U); + uint8_t outerAlgoId = P25DEF::ALGO_UNENCRYPT, uint16_t outerKId = 0U, + bool dataLinkIndependent = false); /** * @brief Passes a DLD KMM through the public P25 OTAR bearer entry point. * @param network The TrafficNetwork that owns the OTAR service. @@ -111,12 +113,57 @@ public: static bool processOTARDLD(network::TrafficNetwork& network, const std::vector& packet, uint32_t llId, uint8_t n, bool encrypted = false, uint8_t algoId = P25DEF::ALGO_UNENCRYPT, uint16_t kid = 0U, const uint8_t* mi = nullptr); + /** + * @brief Encodes and injects a complete DLD KMM through the P25 PDU assembler path. + * @param network The TrafficNetwork that owns the OTAR service. + * @param packet Encoded DLD bytes. + * @param llId Logical Link ID associated with the message. + * @param encrypted Whether the packet is encrypted. + * @param algoId The algorithm ID used for encryption. + * @param kid The key ID used for encryption. + * @param mi The message integrity value. + * @return True if the packet was successfully processed, false otherwise. + */ + static bool processOTARDLDPDU(network::TrafficNetwork& network, const std::vector& packet, + uint32_t llId, bool encrypted = false, uint8_t algoId = P25DEF::ALGO_UNENCRYPT, + uint16_t kid = 0U, const uint8_t* mi = nullptr); /** * @brief Passes a complete Version-0 DLI datagram through the network receive task. * @param network The TrafficNetwork that owns the OTAR service. * @param datagram Encoded DLI bytes. */ static void processOTARDLI(network::TrafficNetwork& network, const std::vector& datagram); + /** + * @brief Opens the real DLI UDP endpoint for an integration test. + * @param network The TrafficNetwork that owns the OTAR service. + * @param address The IP address of the DLI UDP endpoint. + * @param port The port number of the DLI UDP endpoint. + * @return True if the endpoint was successfully opened, false otherwise. + */ + static bool openOTARDLI(network::TrafficNetwork& network, const std::string& address, uint16_t port); + /** + * @brief Polls the real DLI UDP endpoint. + * @param network The TrafficNetwork that owns the OTAR service. + * @param ms The number of milliseconds to wait. + */ + static void clockOTARDLI(network::TrafficNetwork& network, uint32_t ms = 1U); + /** + * @brief Closes the real DLI UDP endpoint. + * @param network The TrafficNetwork that owns the OTAR service. + */ + static void closeOTARDLI(network::TrafficNetwork& network); + /** + * @brief Applies the OTAR outer encryption/decryption implementation. + * @param network The TrafficNetwork that owns the OTAR service. + * @param algoId The algorithm ID used for encryption/decryption. + * @param kid The key ID used for encryption/decryption. + * @param mi The message integrity value. + * @param packet The packet data to be encrypted/decrypted. + * @param encrypt True to encrypt, false to decrypt. + * @return The resulting encrypted/decrypted packet. + */ + static std::vector cryptOTARKMM(network::TrafficNetwork& network, uint8_t algoId, + uint16_t kid, uint8_t* mi, const std::vector& packet, bool encrypt); /** * @brief Checks if the specified inbound message number exists for the given RSI. * @param network The TrafficNetwork that owns the OTAR service. diff --git a/tests/p25/P25_KMM_Tests.cpp b/tests/p25/P25_KMM_Tests.cpp index 32d8410e..d064e90a 100644 --- a/tests/p25/P25_KMM_Tests.cpp +++ b/tests/p25/P25_KMM_Tests.cpp @@ -13,16 +13,27 @@ #include "common/p25/kmm/KMMDeregistrationCommand.h" #include "common/p25/kmm/KMMDeregistrationResponse.h" #include "common/p25/kmm/KMMHello.h" +#include "common/p25/kmm/KMMInventoryCommand.h" #include "common/p25/kmm/KMMNoService.h" #include "common/p25/kmm/KMMNegativeAck.h" +#include "common/p25/kmm/KMMRekeyAck.h" #include "fne/FNETestHooks.h" #include "fne/HostFNE.h" +#include "common/lookups/RadioIdLookup.h" #include #include +#include +#include #include +#if !defined(_WIN32) +#include +#include +#include +#endif + using namespace p25; using namespace p25::defines; using namespace p25::kmm; @@ -38,15 +49,17 @@ public: KMMFNEHarness() : host("fne-kmm-test.yml"), crypto("", "", "", false, 0U, true), + rid("", 0U, false, false), traffic(&host, "127.0.0.1", 62031U, 999999U, "test-password", "test-fne", false, false, false, false, true, true, true, true, true, 0U, false, true, true, 5U, 10U, 2U) { - traffic.setLookups(nullptr, nullptr, nullptr, nullptr, &crypto, nullptr); + traffic.setLookups(&rid, nullptr, nullptr, nullptr, &crypto, nullptr); } HostFNE host; CryptoContainer crypto; + lookups::RadioIdLookup rid; network::TrafficNetwork traffic; }; @@ -216,6 +229,21 @@ TEST_CASE("FNE OTAR dispatcher follows AACA response-kind procedures", "[p25][km REQUIRE(decoded->getDstLLId() == SU_RSI); REQUIRE(decoded->getSrcLLId() == WUID_FNE); } + + SECTION("security-sensitive KMM received without outer encryption is discarded") { + KMMRekeyAck ack; + ack.setDstLLId(KMF_RSI); + ack.setSrcLLId(SU_RSI); + ack.setResponseKind(KMM_ResponseKind::NONE); + ack.setMessageId(KMM_MessageType::REKEY_CMD); + ack.setNumberOfKeyStatus(0U); + + uint32_t payloadSize = 99U; + UInt8Array response = FNETestHooks::processOTARKMM(harness.traffic, + encodeKMM(ack), SU_RSI, payloadSize); + REQUIRE(response == nullptr); + REQUIRE(payloadSize == 0U); + } } TEST_CASE("FNE OTAR authenticates and replay-checks KMM before dispatch", "[p25][kmm][otar][security]") @@ -310,7 +338,7 @@ TEST_CASE("FNE OTAR returns secured RK3 KMM NACKs for validation failures", "[p2 KMMNegativeAck* nack = dynamic_cast(decoded.get()); REQUIRE(nack != nullptr); REQUIRE(nack->getResponseKind() == KMM_ResponseKind::NONE); - REQUIRE(nack->getMessageId() == rejectedMessageId); + REQUIRE(nack->getNakMessageId() == rejectedMessageId); REQUIRE(nack->getMessageNumber() == mn); REQUIRE(nack->getStatus() == status); REQUIRE(nack->getMACType() == KMM_MAC::ENH_MAC); @@ -349,6 +377,17 @@ TEST_CASE("FNE OTAR returns secured RK3 KMM NACKs for validation failures", "[p2 std::vector missingMacKey = signedHello(104U); SET_UINT16(0x4321U, missingMacKey.data(), missingMacKey.size() - 3U); requireNack(missingMacKey, KMM_Status::ITEM_NOT_EXIST, 104U); + + KMMInventoryCommand unsupportedProcedure; + unsupportedProcedure.setDstLLId(WUID_FNE); unsupportedProcedure.setSrcLLId(SU_RSI); + unsupportedProcedure.setResponseKind(KMM_ResponseKind::IMMEDIATE); + unsupportedProcedure.setHasMessageNumber(true); unsupportedProcedure.setMessageNumber(105U); + unsupportedProcedure.setMACType(KMM_MAC::ENH_MAC); unsupportedProcedure.setMACAlgId(ALGO_AES_256); + unsupportedProcedure.setMACKId(TEK_KID); unsupportedProcedure.setMACFormat(KMM_MAC_FORMAT_CBC); + unsupportedProcedure.setInventoryType(KMM_InventoryType::LIST_ACTIVE_KEYSET_IDS); + std::vector unsupportedBytes = encodeKMM(unsupportedProcedure); + unsupportedProcedure.generateMAC(tek, unsupportedBytes.data()); + requireNack(unsupportedBytes, KMM_Status::INVALID_MSG_ID, 105U, KMM_MessageType::INVENTORY_CMD); } TEST_CASE("P25 OTAR DLD processes KMM through the packet-data service entry point", "[p25][kmm][otar][dld]") @@ -379,6 +418,26 @@ TEST_CASE("P25 OTAR DLD processes KMM through the packet-data service entry poin REQUIRE(FNETestHooks::processOTARDLD(harness.traffic, encoded, SU_RSI, 7U)); REQUIRE(FNETestHooks::hasOTARInboundMessageNumber(harness.traffic, SU_RSI, MN)); + SECTION("encrypted DLD decrypts through the Auxiliary ES context") { + hello.setMessageNumber(MN + 1U); + std::vector next = encodeKMM(hello); + hello.generateMAC(tek, next.data()); + uint8_t mi[MI_LENGTH_BYTES] = { 0x10U, 0x21U, 0x32U, 0x43U, 0x54U, + 0x65U, 0x76U, 0x87U, 0x98U }; + std::vector encrypted = FNETestHooks::cryptOTARKMM(harness.traffic, + ALGO_AES_256, 0x1234U, mi, next, true); + REQUIRE(encrypted.size() == next.size()); + REQUIRE(encrypted != next); + REQUIRE(FNETestHooks::processOTARDLDPDU(harness.traffic, encrypted, SU_RSI, + true, ALGO_AES_256, 0x1234U, mi)); + REQUIRE(FNETestHooks::hasOTARInboundMessageNumber(harness.traffic, SU_RSI, MN + 1U)); + } + + SECTION("encrypted DLD without Auxiliary ES metadata is rejected") { + REQUIRE_FALSE(FNETestHooks::processOTARDLD(harness.traffic, encoded, SU_RSI, 10U, + true, ALGO_AES_256, 0x1234U, nullptr)); + } + std::vector truncated(encoded.begin(), encoded.end() - 1U); REQUIRE_FALSE(FNETestHooks::processOTARDLD(harness.traffic, truncated, SU_RSI, 8U)); @@ -426,6 +485,23 @@ TEST_CASE("P25 OTAR DLI validates its Version-0 preamble and dispatches KMM", "[ SECTION("valid clear DLI reaches the authenticated KMM dispatcher") { KMMFNEHarness harness; makeHarnessKey(harness); + FNETestHooks::setKMFServicesEnabled(harness.traffic, true); + + KMMRegistrationCommand registration; + registration.setDstLLId(WUID_FNE); + registration.setSrcLLId(SU_RSI); + registration.setResponseKind(KMM_ResponseKind::IMMEDIATE); + registration.setKMFRSI(WUID_FNE); + uint32_t registrationResponseSize = 0U; + UInt8Array registrationResponse = FNETestHooks::processOTARKMM(harness.traffic, + encodeKMM(registration), SU_RSI, registrationResponseSize, + ALGO_UNENCRYPT, 0U, true); + REQUIRE(registrationResponse != nullptr); + std::unique_ptr decodedRegistration = KMMFactory::create( + registrationResponse.get(), registrationResponseSize); + REQUIRE(decodedRegistration != nullptr); + REQUIRE(decodedRegistration->getMessageId() == KMM_MessageType::REG_RSP); + FNETestHooks::processOTARDLI(harness.traffic, datagram); REQUIRE(FNETestHooks::hasOTARInboundMessageNumber(harness.traffic, SU_RSI, MN)); } @@ -447,6 +523,73 @@ TEST_CASE("P25 OTAR DLI validates its Version-0 preamble and dispatches KMM", "[ } } +#if !defined(_WIN32) +TEST_CASE("P25 OTAR DLI traverses the real UDP endpoint", "[p25][kmm][otar][dli][integration]") +{ + KMMFNEHarness harness; + constexpr uint32_t SU_RSI = 0x654321U; + FNETestHooks::setKMFServicesEnabled(harness.traffic, true); + + int probe = ::socket(AF_INET, SOCK_DGRAM, 0); + REQUIRE(probe >= 0); + sockaddr_in probeAddress = {}; + probeAddress.sin_family = AF_INET; + probeAddress.sin_addr.s_addr = htonl(INADDR_LOOPBACK); + probeAddress.sin_port = 0U; + REQUIRE(::bind(probe, reinterpret_cast(&probeAddress), sizeof(probeAddress)) == 0); + socklen_t probeLength = sizeof(probeAddress); + REQUIRE(::getsockname(probe, reinterpret_cast(&probeAddress), &probeLength) == 0); + const uint16_t serverPort = ntohs(probeAddress.sin_port); + ::close(probe); + + REQUIRE(FNETestHooks::openOTARDLI(harness.traffic, "127.0.0.1", serverPort)); + + int client = ::socket(AF_INET, SOCK_DGRAM, 0); + REQUIRE(client >= 0); + sockaddr_in serverAddress = {}; + serverAddress.sin_family = AF_INET; + serverAddress.sin_addr.s_addr = htonl(INADDR_LOOPBACK); + serverAddress.sin_port = htons(serverPort); + + KMMRegistrationCommand registration; + registration.setDstLLId(WUID_FNE); + registration.setSrcLLId(SU_RSI); + registration.setResponseKind(KMM_ResponseKind::IMMEDIATE); + registration.setKMFRSI(WUID_FNE); + std::vector registrationKMM = encodeKMM(registration); + std::vector datagram(14U + registrationKMM.size(), 0U); + datagram[0U] = 0U; + datagram[1U] = MFG_STANDARD; + datagram[2U] = ALGO_UNENCRYPT; + ::memcpy(datagram.data() + 14U, registrationKMM.data(), registrationKMM.size()); + REQUIRE(::sendto(client, datagram.data(), datagram.size(), 0, + reinterpret_cast(&serverAddress), sizeof(serverAddress)) == (ssize_t)datagram.size()); + + std::vector response(512U, 0U); + ssize_t responseLength = -1; + for (uint32_t attempt = 0U; attempt < 100U && responseLength < 0; ++attempt) { + FNETestHooks::clockOTARDLI(harness.traffic); + responseLength = ::recvfrom(client, response.data(), response.size(), MSG_DONTWAIT, nullptr, nullptr); + if (responseLength < 0) + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + } + + REQUIRE(responseLength >= 24); + response.resize((size_t)responseLength); + REQUIRE(response[0U] == 0U); + REQUIRE(response[1U] == MFG_STANDARD); + REQUIRE(response[2U] == ALGO_UNENCRYPT); + std::unique_ptr decoded = KMMFactory::create(response.data() + 14U, + (uint32_t)response.size() - 14U); + REQUIRE(decoded != nullptr); + REQUIRE(decoded->getMessageId() == KMM_MessageType::REG_RSP); + REQUIRE(decoded->getDstLLId() == SU_RSI); + + ::close(client); + FNETestHooks::closeOTARDLI(harness.traffic); +} +#endif + TEST_CASE("KMM Key Format follows AACA-D Table 70", "[p25][kmm][key-format]") { REQUIRE(KMM_KEY_FORMAT_TEK == 0x00U);