Skip to content

Commit

Permalink
fix issue with DataBlock destructor trying to free a unallocated arra…
Browse files Browse the repository at this point in the history 
…y; fix issue with dispatchUserFrameToFNE allowing local FNE network packets to cross external peer boundary; allow peers to connect if the peer list ACL is enabled, but the peer list itself is empty;
  • Loading branch information
@gatekeep
gatekeep committed Nov 8, 2024
1 parent 0304b98 commit 8a04125
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 34 deletions.
2 changes: 2 additions & 0 deletions configs/fne-config.example.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ master:
disablePacketData: false
# Flag indicating whether verbose dumping of data packets is enabled.
dumpPacketData: false
# Flag indicating whether verbose logging of data packet operations is enabled.
verbosePacketData: false

# Delay from when a call on a parrot TG ends to when the playback starts (in milliseconds).
parrotDelay: 2000
Expand Down
6 changes: 6 additions & 0 deletions src/common/lookups/PeerListLookup.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,12 @@ namespace lookups
*/
bool isPeerAllowed(uint32_t id) const;

/**
* @brief Checks if the peer list is empty.
* @returns bool True, if list is empty, otherwise false.
*/
bool isPeerListEmpty() const { return m_table.size() == 0U; }

/**
* @brief Sets the mode to either WHITELIST or BLACKLIST.
* @param mode The mode to set.
Expand Down
3 changes: 2 additions & 1 deletion src/common/p25/data/DataBlock.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ DataBlock::DataBlock() :

DataBlock::~DataBlock()
{
delete[] m_data;
if (m_data != nullptr)
delete[] m_data;
}

/* Decodes P25 PDU data block. */
Expand Down
21 changes: 18 additions & 3 deletions src/fne/network/FNENetwork.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ FNENetwork::FNENetwork(HostFNE* host, const std::string& address, uint16_t port,
m_influxLogRawData(false),
m_disablePacketData(false),
m_dumpPacketData(false),
m_verbosePacketData(false),
m_reportPeerPing(reportPeerPing),
m_verbose(verbose)
{
Expand Down Expand Up @@ -154,6 +155,7 @@ void FNENetwork::setOptions(yaml::Node& conf, bool printOptions)

m_disablePacketData = conf["disablePacketData"].as<bool>(false);
m_dumpPacketData = conf["dumpPacketData"].as<bool>(false);
m_verbosePacketData = conf["verbosePacketData"].as<bool>(false);

/*
** Drop Unit to Unit Peers
Expand Down Expand Up @@ -585,7 +587,11 @@ void* FNENetwork::threadedNetworkRx(void* arg)

// check if the peer is in the peer ACL list
if (network->m_peerListLookup->getACL()) {
if (!network->m_peerListLookup->isPeerAllowed(peerId)) {
if (network->m_peerListLookup->isPeerListEmpty()) {
LogWarning(LOG_NET, "Peer List ACL enabled, but we have an empty peer list? Passing all peers.");
}

if (!network->m_peerListLookup->isPeerAllowed(peerId) && !network->m_peerListLookup->isPeerListEmpty()) {
if (network->m_peerListLookup->getMode() == lookups::PeerListLookup::BLACKLIST) {
LogWarning(LOG_NET, "PEER %u RPTL, blacklisted from access", peerId);
} else {
Expand Down Expand Up @@ -619,7 +625,11 @@ void* FNENetwork::threadedNetworkRx(void* arg)

// check if the peer is in the peer ACL list
if (network->m_peerListLookup->getACL()) {
if (!network->m_peerListLookup->isPeerAllowed(peerId)) {
if (network->m_peerListLookup->isPeerListEmpty()) {
LogWarning(LOG_NET, "Peer List ACL enabled, but we have an empty peer list? Passing all peers.");
}

if (!network->m_peerListLookup->isPeerAllowed(peerId) && !network->m_peerListLookup->isPeerListEmpty()) {
if (network->m_peerListLookup->getMode() == lookups::PeerListLookup::BLACKLIST) {
LogWarning(LOG_NET, "PEER %u RPTL, blacklisted from access", peerId);
} else {
Expand Down Expand Up @@ -675,7 +685,7 @@ void* FNENetwork::threadedNetworkRx(void* arg)
// check if the peer is in the peer ACL list
bool validAcl = true;
if (network->m_peerListLookup->getACL()) {
if (!network->m_peerListLookup->isPeerAllowed(peerId)) {
if (!network->m_peerListLookup->isPeerAllowed(peerId) && !network->m_peerListLookup->isPeerListEmpty()) {
if (network->m_peerListLookup->getMode() == lookups::PeerListLookup::BLACKLIST) {
LogWarning(LOG_NET, "PEER %u RPTK, blacklisted from access", peerId);
} else {
Expand All @@ -694,6 +704,11 @@ void* FNENetwork::threadedNetworkRx(void* arg)
}
}
}

if (network->m_peerListLookup->isPeerListEmpty()) {
LogWarning(LOG_NET, "Peer List ACL enabled, but we have an empty peer list? Passing all peers.");
validAcl = true;
}
}

if (validAcl) {
Expand Down
1 change: 1 addition & 0 deletions src/fne/network/FNENetwork.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,7 @@ namespace network

bool m_disablePacketData;
bool m_dumpPacketData;
bool m_verbosePacketData;

bool m_reportPeerPing;
bool m_verbose;
Expand Down
49 changes: 20 additions & 29 deletions src/fne/network/callhandler/packetdata/P25PacketData.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -683,10 +683,10 @@ void P25PacketData::dispatchToFNE(uint32_t peerId)
}

write_PDU_User(peer.first, nullptr, status->header, status->extendedAddress, status->pduUserData, true);
if (m_network->m_debug) {
// if (m_network->m_debug) {
LogDebug(LOG_NET, "P25, srcPeer = %u, dstPeer = %u, duid = $%02X, srcId = %u, dstId = %u",
peerId, peer.first, DUID::PDU, srcId, dstId);
}
// }

i++;
}
Expand All @@ -713,16 +713,16 @@ void P25PacketData::dispatchToFNE(uint32_t peerId)
}

write_PDU_User(dstPeerId, peer.second, status->header, status->extendedAddress, status->pduUserData);
if (m_network->m_debug) {
// if (m_network->m_debug) {
LogDebug(LOG_NET, "P25, srcPeer = %u, dstPeer = %u, duid = $%02X, srcId = %u, dstId = %u",
peerId, dstPeerId, DUID::PDU, srcId, dstId);
}
// }
}
}
}
}

/* Helper to dispatch PDU user data back to the FNE network. */
/* Helper to dispatch PDU user data back to the local FNE network. (Will not transmit to external peers.) */

void P25PacketData::dispatchUserFrameToFNE(p25::data::DataHeader& dataHeader, bool extendedAddress, uint8_t* pduUserData)
{
Expand Down Expand Up @@ -759,19 +759,6 @@ void P25PacketData::dispatchUserFrameToFNE(p25::data::DataHeader& dataHeader, bo
}
m_network->m_frameQueue->flushQueue();
}

// repeat traffic to external peers
if (m_network->m_host->m_peerNetworks.size() > 0U) {
for (auto peer : m_network->m_host->m_peerNetworks) {
uint32_t dstPeerId = peer.second->getPeerId();

write_PDU_User(dstPeerId, peer.second, dataHeader, extendedAddress, pduUserData);
if (m_network->m_debug) {
LogDebug(LOG_NET, "P25, dstPeer = %u, duid = $%02X, srcId = %u, dstId = %u",
dstPeerId, DUID::PDU, srcId, dstId);
}
}
}
}

/* Helper used to process SNDCP control data from PDU data. */
Expand Down Expand Up @@ -962,10 +949,11 @@ void P25PacketData::write_PDU_User(uint32_t peerId, network::PeerNetwork* peerNe

uint32_t blocksToFollow = dataHeader.getBlocksToFollow();

LogMessage(LOG_NET, P25_PDU_STR ", OSP, peerId = %u, ack = %u, outbound = %u, fmt = $%02X, mfId = $%02X, sap = $%02X, fullMessage = %u, blocksToFollow = %u, padLength = %u, packetLength = %u, S = %u, n = %u, seqNo = %u, lastFragment = %u, hdrOffset = %u, llId = %u",
peerId, dataHeader.getAckNeeded(), dataHeader.getOutbound(), dataHeader.getFormat(), dataHeader.getMFId(), dataHeader.getSAP(), dataHeader.getFullMessage(),
dataHeader.getBlocksToFollow(), dataHeader.getPadLength(), dataHeader.getPacketLength(), dataHeader.getSynchronize(), dataHeader.getNs(), dataHeader.getFSN(), dataHeader.getLastFragment(),
dataHeader.getHeaderOffset(), dataHeader.getLLId());
if (m_network->m_verbosePacketData)
LogMessage(LOG_NET, P25_PDU_STR ", OSP, peerId = %u, ack = %u, outbound = %u, fmt = $%02X, mfId = $%02X, sap = $%02X, fullMessage = %u, blocksToFollow = %u, padLength = %u, packetLength = %u, S = %u, n = %u, seqNo = %u, lastFragment = %u, hdrOffset = %u, llId = %u",
peerId, dataHeader.getAckNeeded(), dataHeader.getOutbound(), dataHeader.getFormat(), dataHeader.getMFId(), dataHeader.getSAP(), dataHeader.getFullMessage(),
dataHeader.getBlocksToFollow(), dataHeader.getPadLength(), dataHeader.getPacketLength(), dataHeader.getSynchronize(), dataHeader.getNs(), dataHeader.getFSN(), dataHeader.getLastFragment(),
dataHeader.getHeaderOffset(), dataHeader.getLLId());

// generate the PDU header and 1/2 rate Trellis
dataHeader.encode(buffer);
Expand Down Expand Up @@ -995,16 +983,18 @@ void P25PacketData::write_PDU_User(uint32_t peerId, network::PeerNetwork* peerNe
blocksToFollow--;
networkBlock++;

LogMessage(LOG_NET, P25_PDU_STR ", OSP, extended address, sap = $%02X, srcLlId = %u",
dataHeader.getEXSAP(), dataHeader.getSrcLLId());
if (m_network->m_verbosePacketData)
LogMessage(LOG_NET, P25_PDU_STR ", OSP, extended address, sap = $%02X, srcLlId = %u",
dataHeader.getEXSAP(), dataHeader.getSrcLLId());
}

// are we processing extended address data from the first block?
if ((dataHeader.getFormat() == PDUFormatType::CONFIRMED) && (dataHeader.getSAP() == PDUSAP::EXT_ADDR) && extendedAddress) {
dataHeader.encodeExtAddr(pduUserData);

LogMessage(LOG_NET, P25_PDU_STR ", OSP, sap = $%02X, srcLlId = %u",
dataHeader.getEXSAP(), dataHeader.getSrcLLId());
if (m_network->m_verbosePacketData)
LogMessage(LOG_NET, P25_PDU_STR ", OSP, sap = $%02X, srcLlId = %u",
dataHeader.getEXSAP(), dataHeader.getSrcLLId());
}

if (dataHeader.getFormat() != PDUFormatType::AMBT) {
Expand All @@ -1022,9 +1012,10 @@ void P25PacketData::write_PDU_User(uint32_t peerId, network::PeerNetwork* peerNe
dataBlock.setSerialNo(i);
dataBlock.setData(pduUserData + dataOffset);

LogMessage(LOG_NET, P25_PDU_STR ", OSP, peerId = %u, block %u, fmt = $%02X, lastBlock = %u",
peerId, (dataHeader.getFormat() == PDUFormatType::CONFIRMED) ? dataBlock.getSerialNo() : i, dataBlock.getFormat(),
dataBlock.getLastBlock());
if (m_network->m_verbosePacketData)
LogMessage(LOG_NET, P25_PDU_STR ", OSP, peerId = %u, block %u, fmt = $%02X, lastBlock = %u",
peerId, (dataHeader.getFormat() == PDUFormatType::CONFIRMED) ? dataBlock.getSerialNo() : i, dataBlock.getFormat(),
dataBlock.getLastBlock());

::memset(buffer, 0x00U, P25_PDU_FEC_LENGTH_BYTES);
dataBlock.encode(buffer);
Expand Down
2 changes: 1 addition & 1 deletion src/fne/network/callhandler/packetdata/P25PacketData.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ namespace network
*/
void dispatchToFNE(uint32_t peerId);
/**
* @brief Helper to dispatch PDU user data back to the FNE network.
* @brief Helper to dispatch PDU user data back to the local FNE network. (Will not transmit to external peers.)
* @param dataHeader Instance of a PDU data header.
* @param extendedAddress Flag indicating whether or not to extended addressing is in use.
* @param pduUserData Buffer containing user data to transmit.
Expand Down

0 comments on commit 8a04125

Please sign in to comment.