diff --git a/src/common/p25/data/Assembler.cpp b/src/common/p25/data/Assembler.cpp index 899433e2..8df3a9f5 100644 --- a/src/common/p25/data/Assembler.cpp +++ b/src/common/p25/data/Assembler.cpp @@ -218,9 +218,9 @@ bool Assembler::disassemble(const uint8_t* pduBlock, uint32_t blockLength, bool dataHeader.decodeAuxES(secondHeader); if (s_verbose) { - LogInfoEx(LOG_P25, P25_PDU_STR ", ISP, block %u, fmt = $%02X, lastBlock = %u, sap = $%02X, algoId = $%02X, kId = $%04X", - dataBlocks[i].getSerialNo(), dataBlocks[i].getFormat(), dataBlocks[i].getLastBlock(), dataHeader.getEXSAP(), - dataHeader.getAlgId(), dataHeader.getKId()); + LogInfoEx(LOG_P25, P25_PDU_STR ", ISP, block %u, fmt = $%02X, lastBlock = %u, sap = $%02X, exSAP = $%02X, algoId = $%02X, kId = $%04X", + dataBlocks[i].getSerialNo(), dataBlocks[i].getFormat(), dataBlocks[i].getLastBlock(), dataHeader.getSAP(), + dataHeader.getEXSAP(), dataHeader.getAlgId(), dataHeader.getKId()); if (dataHeader.getAlgId() != ALGO_UNENCRYPT) { uint8_t mi[MI_LENGTH_BYTES]; @@ -432,8 +432,8 @@ UInt8Array Assembler::assemble(data::DataHeader& dataHeader, bool extendedAddres dataHeader.encodeAuxES(packetData); if (s_verbose) { - LogInfoEx(LOG_P25, P25_PDU_STR ", OSP, auxiliary ES, algId = $%02X, kId = $%04X", - dataHeader.getAlgId(), dataHeader.getKId()); + LogInfoEx(LOG_P25, P25_PDU_STR ", OSP, auxiliary ES, exSAP = $%02X, algId = $%02X, kId = $%04X", + dataHeader.getEXSAP(), dataHeader.getAlgId(), dataHeader.getKId()); if (dataHeader.getAlgId() != ALGO_UNENCRYPT) { uint8_t mi[MI_LENGTH_BYTES]; diff --git a/src/fne/network/P25OTARService.cpp b/src/fne/network/P25OTARService.cpp index 11672aaa..168d218f 100644 --- a/src/fne/network/P25OTARService.cpp +++ b/src/fne/network/P25OTARService.cpp @@ -177,6 +177,62 @@ P25OTARService::~P25OTARService() delete m_socket; } +/* Resolves the required outer-encryption context for a generated response. */ + +bool P25OTARService::resolveResponseSecurity(const uint8_t* data, uint32_t len, bool& encrypted, + uint8_t& algoId, uint16_t& kid) const +{ + if (data == nullptr) { + LogError(LOG_P25, P25_KMM_STR ", cannot resolve outer encryption for a null generated response"); + return false; + } + + const uint8_t messageId = len > 0U ? data[0U] : KMM_MessageType::NULL_CMD; + const uint32_t declaredLength = len >= 3U ? + ((((uint32_t)data[1U] << 8U) | data[2U]) + 3U) : 0U; + if (len < 10U || len > 512U || declaredLength != len) { + LogError(LOG_P25, P25_KMM_STR ", cannot outer-encrypt generated KMM: invalid length, messageId = $%02X, declared/actual = %u/%u", + messageId, declaredLength, len); + return false; + } + + if (encrypted || !requiresEncryptedKMM(messageId)) + return true; + + // This is locally generated and its total length has already been checked. + // Decode only the common header: the bounded air-facing factory intentionally + // rejects nested-count messages such as RK3 until it has a structural validator. + KMMOpaqueFrame response; + if (!response.decode(data) || response.getMACType() != KMM_MAC::ENH_MAC || + response.getMACAlgId() != ALGO_AES_256 || response.getMACKId() == 0U) { + LogError(LOG_P25, P25_KMM_STR ", generated KMM requires outer encryption but has no usable enhanced-MAC TEK context, messageId = $%02X, macType = $%02X, macAlgId = $%02X, macKId = $%04X", + messageId, response.getMACType(), response.getMACAlgId(), response.getMACKId()); + return false; + } + + EKCKeyItem outerTek = m_network->m_cryptoLookup->find(response.getMACKId()); + if (outerTek.isInvalid()) { + LogError(LOG_P25, P25_KMM_STR ", generated KMM requires outer encryption but MAC TEK was not found, messageId = $%02X, algId = $%02X, kId = $%04X", + messageId, response.getMACAlgId(), response.getMACKId()); + return false; + } + if (outerTek.algId() != response.getMACAlgId()) { + LogError(LOG_P25, P25_KMM_STR ", generated KMM outer TEK algorithm mismatch, messageId = $%02X, required/found algId = $%02X/$%02X, kId = $%04X", + messageId, response.getMACAlgId(), outerTek.algId(), response.getMACKId()); + return false; + } + + algoId = response.getMACAlgId(); + kid = response.getMACKId(); + encrypted = true; + if (m_debug) { + LogDebugEx(LOG_P25, "P25OTARService::resolveResponseSecurity()", + "selected outer encryption for generated KMM, messageId = $%02X, algId = $%02X, kId = $%04X", + messageId, algoId, kid); + } + return true; +} + /* Helper used to process KMM frames from PDU data. */ bool P25OTARService::processDLD(const uint8_t* data, uint32_t len, uint32_t llId, uint8_t n, bool encrypted, @@ -240,20 +296,18 @@ bool P25OTARService::processDLD(const uint8_t* data, uint32_t len, uint32_t llId } 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 (!resolveResponseSecurity(pduUserData.get(), payloadSize, responseEncrypted, + resolvedAlgoId, resolvedKId)) { + LogError(LOG_P25, P25_KMM_STR ", secured response has no usable outer TEK context"); + sendNack(PDUAckType::NACK_UNDELIVERABLE); + return false; } + // Complete the inbound confirmed-delivery transaction before starting the + // independent outbound KMM transaction. The SU is still in stop-and-wait + // for this N(R) and may discard an RK3 transmitted ahead of it. + m_packetData->write_PDU_Ack_Response(PDUAckClass::ACK, PDUAckType::ACK, n, llId, false); + // lambda function to handle dispatching of KMM responses, including encryption if required auto dispatchResponse = [&](const uint8_t* response, uint32_t responseLength) -> bool { uint8_t responseMI[MI_LENGTH_BYTES]; @@ -262,9 +316,6 @@ bool P25OTARService::processDLD(const uint8_t* data, uint32_t len, uint32_t llId UInt8Array encryptedResponse; if (responseEncrypted) { - P25Crypto responseCrypto; - responseCrypto.generateMI(); - responseCrypto.getMI(responseMI); encryptedResponse = cryptKMM(resolvedAlgoId, resolvedKId, responseMI, response, responseLength, true); if (encryptedResponse == nullptr) return false; @@ -285,7 +336,6 @@ bool P25OTARService::processDLD(const uint8_t* data, uint32_t len, uint32_t llId LogError(LOG_P25, P25_KMM_STR ", unable to dispatch KMM response, llId = %u", llId); } - sendNack(PDUAckType::NACK_UNDELIVERABLE); return false; } @@ -293,16 +343,10 @@ bool P25OTARService::processDLD(const uint8_t* data, uint32_t len, uint32_t llId for (const std::vector& response : additionalResponses) { if (!dispatchResponse(response.data(), (uint32_t)response.size())) { LogError(LOG_P25, P25_KMM_STR ", unable to dispatch additional Rekey Command, llId = %u", llId); - sendNack(PDUAckType::NACK_UNDELIVERABLE); return false; } } - /** - * The CAI data-link ACK covers receipt of the triggering request. Each RK3 - * Rekey Command is acknowledged separately by its Rekey-Acknowledgment. - */ - m_packetData->write_PDU_Ack_Response(PDUAckClass::ACK, PDUAckType::ACK, n, llId, false); return true; } @@ -447,19 +491,12 @@ void P25OTARService::taskNetworkRx(OTARPacketRequest* req) } 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 (!network->resolveResponseSecurity(pduUserData.get(), payloadSize, + responseEncrypted, algoId, kid)) { + 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; } // lambda function to send KMM responses, handling encryption if necessary @@ -569,6 +606,10 @@ UInt8Array P25OTARService::cryptKMM(uint8_t algoId, uint16_t kid, uint8_t* mi, c break; } } + else { + LogError(LOG_P25, P25_KMM_STR ", unable to %s outer KMM, KEK not found, algId = $%02X, kId = $%04X", + encrypt ? "encrypt" : "decrypt", algoId, kid); + } return nullptr; } diff --git a/src/fne/network/P25OTARService.h b/src/fne/network/P25OTARService.h index 32ce3891..b68ef006 100644 --- a/src/fne/network/P25OTARService.h +++ b/src/fne/network/P25OTARService.h @@ -154,6 +154,18 @@ namespace network */ UInt8Array cryptKMM(uint8_t algoId, uint16_t kid, uint8_t* mi, const uint8_t* buffer, uint32_t len, bool encrypt = false); + /** + * @brief Resolves the required outer-encryption context for a locally generated KMM response. + * @param[in] data Encoded plaintext KMM response. + * @param len Length of the encoded response. + * @param[in,out] encrypted Whether outer encryption is already in use or is required. + * @param[in,out] algoId Outer-encryption algorithm ID. + * @param[in,out] kid Outer-encryption key ID. + * @return True when the response has a valid usable security context. + */ + bool resolveResponseSecurity(const uint8_t* data, uint32_t len, bool& encrypted, + uint8_t& algoId, uint16_t& kid) const; + /** * @brief Helper used to process KMM frames. * @param[in] data Network data buffer. diff --git a/src/fne/network/callhandler/packetdata/P25PacketData.cpp b/src/fne/network/callhandler/packetdata/P25PacketData.cpp index 0ff8f842..23b75dc7 100644 --- a/src/fne/network/callhandler/packetdata/P25PacketData.cpp +++ b/src/fne/network/callhandler/packetdata/P25PacketData.cpp @@ -186,13 +186,6 @@ bool P25PacketData::processFrame(const uint8_t* data, uint32_t len, uint32_t pee return false; } - // check for inconsistent total block count - if (status->dataBlockCnt > 0U && status->totalBlocks != totalBlocks) { - LogError(LOG_P25, P25_PDU_STR ", inconsistent total block count, received/expected = %u/%u", - totalBlocks, status->totalBlocks); - return false; - } - LogInfoEx(LOG_NET, P25_PDU_STR ", received block %u, peerId = %u, len = %u", currentBlock, peerId, blockLength); diff --git a/src/host/p25/packet/Data.cpp b/src/host/p25/packet/Data.cpp index 1ad1f372..b419c58f 100644 --- a/src/host/p25/packet/Data.cpp +++ b/src/host/p25/packet/Data.cpp @@ -559,15 +559,15 @@ bool Data::processNetwork(uint8_t* data, uint32_t len, uint8_t currentBlock, uin // handle standard P25 service access points switch (sap) { - case PDUSAP::ARP: - { - if (m_netPduUserDataLength < P25_PDU_ARP_PCKT_LENGTH) { - LogWarning(LOG_NET, P25_PDU_STR ", ignoring truncated ARP payload, length = %u", - m_netPduUserDataLength); - break; - } + case PDUSAP::ARP: + { + if (m_netPduUserDataLength < P25_PDU_ARP_PCKT_LENGTH) { + LogWarning(LOG_NET, P25_PDU_STR ", ignoring truncated ARP payload, length = %u", + m_netPduUserDataLength); + break; + } - /* bryanb: quick and dirty ARP logging */ + /* bryanb: quick and dirty ARP logging */ uint8_t arpPacket[P25_PDU_ARP_PCKT_LENGTH]; ::memset(arpPacket, 0x00U, P25_PDU_ARP_PCKT_LENGTH); ::memcpy(arpPacket, m_netPduUserData, P25_PDU_ARP_PCKT_LENGTH); diff --git a/tests/fne/FNETestHooks.cpp b/tests/fne/FNETestHooks.cpp index 6d486868..8bcff95a 100644 --- a/tests/fne/FNETestHooks.cpp +++ b/tests/fne/FNETestHooks.cpp @@ -125,6 +125,25 @@ std::vector> FNETestHooks::buildOTARRekey(TrafficNetwork& n return frames; } +/** + * @brief Resolves the security parameters of an OTAR response packet. + * @param network The TrafficNetwork instance. + * @param packet The OTAR response packet data. + * @param encrypted Output flag indicating if the packet is encrypted. + * @param algoId Output parameter for the algorithm ID used for encryption. + * @param kid Output parameter for the key ID used for encryption. + * @return True if the security parameters were successfully resolved, false otherwise. + */ +bool FNETestHooks::resolveOTARResponseSecurity(TrafficNetwork& network, + const std::vector& packet, bool& encrypted, uint8_t& algoId, uint16_t& kid) +{ + if (packet.empty() || network.m_p25OTARService == nullptr) + return false; + + return network.m_p25OTARService->resolveResponseSecurity(packet.data(), + (uint32_t)packet.size(), encrypted, algoId, kid); +} + /** * @brief Passes a DLD KMM through the public P25 OTAR bearer entry point. * @param network The TrafficNetwork instance. diff --git a/tests/fne/FNETestHooks.h b/tests/fne/FNETestHooks.h index 7e6df3ce..abaeb706 100644 --- a/tests/fne/FNETestHooks.h +++ b/tests/fne/FNETestHooks.h @@ -107,6 +107,17 @@ public: */ static std::vector> buildOTARRekey(network::TrafficNetwork& network, uint32_t llId, uint32_t kmmRSI); + /** + * @brief Resolves the outer security context selected for a generated response. + * @param network The TrafficNetwork instance. + * @param packet The OTAR response packet data. + * @param encrypted Output flag indicating if the packet is encrypted. + * @param algoId Output parameter for the algorithm ID used for encryption. + * @param kid Output parameter for the key ID used for encryption. + * @return True if the security parameters were successfully resolved, false otherwise. + */ + static bool resolveOTARResponseSecurity(network::TrafficNetwork& network, + const std::vector& packet, bool& encrypted, uint8_t& algoId, uint16_t& kid); /** * @brief Passes a DLD KMM through the public P25 OTAR bearer entry point. * @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 9dcb954b..ba7a09fe 100644 --- a/tests/p25/P25_KMM_Tests.cpp +++ b/tests/p25/P25_KMM_Tests.cpp @@ -555,6 +555,15 @@ TEST_CASE("P25 OTAR Rekey Commands batch four keys per KMM", "[p25][kmm][otar][r FNETestHooks::buildOTARRekey(harness.traffic, SU_RSI, SU_RSI); REQUIRE(frames.size() == 3U); + bool outerEncrypted = false; + uint8_t outerAlgId = ALGO_UNENCRYPT; + uint16_t outerKId = 0U; + REQUIRE(FNETestHooks::resolveOTARResponseSecurity(harness.traffic, frames[0U], + outerEncrypted, outerAlgId, outerKId)); + CHECK(outerEncrypted); + CHECK(outerAlgId == ALGO_AES_256); + CHECK(outerKId == allowed[0U]); + const uint32_t expectedCounts[] = { 4U, 4U, 1U }; for (size_t i = 0U; i < frames.size(); i++) { REQUIRE(frames[i].size() >= 3U);