refactor arp handling

src/map/natmap.cpp

@@ -24,13 +24,14 @@ namespace otonat {
     }
 
     void NatMap::handlePdu(const Tins::PDU * pdu) {
-        Tins::PDU * pduCopy = pdu->clone();
+        if (pdu == nullptr) {
-        if (pdu == NULL) {
             return;
         }
 
+        Tins::PDU * pduCopy = pdu->clone();
+        delete pdu;
         Tins::ARP * arp = pduCopy->find_pdu<Tins::ARP>();
-        if (arp != 0) {
+        if (arp != nullptr) {
             if (handleArp(arp)) {
                 outgoingPduQueue.push(pduCopy);
             }
@@ -38,7 +39,7 @@ namespace otonat {
         }
 
         Tins::IP * ip = pduCopy->find_pdu<Tins::IP>();
-        if (ip != 0) {
+        if (ip != nullptr) {
             if (handleIp(ip, pduCopy)) {
                 outgoingPduQueue.push(pduCopy);
             }
@@ -71,11 +72,11 @@ namespace otonat {
             return true;
         } else {
             const Tins::EthernetII * eth = originPDU->find_pdu<Tins::EthernetII>();
-            if (eth != 0){
+            if (eth != nullptr) {
                 Tins::EthernetII fakeArp = Tins::ARP::make_arp_request(ip->dst_addr(), ip->src_addr(), eth->src_addr());
-                SendTranslatedArpRequest(originPDU->find_pdu<Tins::ARP>());
+                SendTranslatedArpRequest(fakeArp.find_pdu<Tins::ARP>());
             }
-            
+
             return false;
         }
     }
@@ -143,7 +144,7 @@ namespace otonat {
         if (isForMeOrFromMeArp(arp)) {
             return false;
         }
-        
+
         switch (arp->opcode()) {
             case Tins::ARP::REQUEST:
                 return this->handleArpReq(arp);
@@ -181,7 +182,7 @@ namespace otonat {
 
     bool NatMap::isIpInMyRanges(const Tins::IPv4Address & ipAddr, const NatRangeList & rangeList) {
         for (NatRange range : rangeList) {
-            if (range.calcIpRange(true).contains(ipAddr)) {
+            if (range.calcIpRange(false).contains(ipAddr)) {
                 return true;
             }
         }
@@ -190,53 +191,57 @@ namespace otonat {
     }
 
     bool NatMap::handleArpReq(Tins::ARP* arp) {
-        Tins::IPv4Address targetIp = arp->target_ip_addr();
+        Tins::IPv4Address targetIp = arp->target_ip_addr();        
-        Tins::IPv4Address senderIp = arp->sender_ip_addr();
         IpAdressMap::const_iterator transTargetIpIter = this->transMap.find(targetIp);
-        if (transTargetIpIter != transMap.end()) {
+        if (transTargetIpIter == transMap.end()) {
-            Tins::IPv4Address transTargetIp = transTargetIpIter->second;
+            SendTranslatedArpRequest(arp);
-            arp->target_ip_addr(transTargetIp);
-            IpAdressMap::const_iterator transSenderIpIter = this->transMap.find(senderIp);
-            if (transSenderIpIter != transMap.end()) {
-                arp->sender_ip_addr(transSenderIpIter->second);
-                return true;
-            } else {
-
-                for (NatRange & range : this->ranges) {
-                    const Tins::NetworkInterface::Info & interfaceInfo = range.interface.info();
-                    if (!interfaceInfo.is_up) {
-                        continue;
-                    }
-
-                    Tins::IPv4Range ipRange = range.calcIpRange(true);
-                    if (!ipRange.contains(transTargetIp)) {
-                        continue;
-                    }
-
-                    Tins::IPv4Address transSenderIp = range.mapIPv4Addres(senderIp, true);
-                    arp->sender_ip_addr(transSenderIp);
-
-                    IpAdressMap::const_iterator transSenderIpReqIter = this->reqIpMap.find(transSenderIp);
-                    if (transSenderIpReqIter == reqIpMap.end()) {
-                        this->reqIpMap.insert(IPv4AddressEntry(transSenderIp, senderIp));
-                    }
-                    return true;
-                }
-            }
-
             return false;
         }
 
-        SendTranslatedArpRequest(arp);
+        Tins::IPv4Address transTargetIp = transTargetIpIter->second;
-        return false;
+        arp->target_ip_addr(transTargetIp);
+        IpAdressMap::const_iterator transSenderIpIter = this->transMap.find(arp->sender_ip_addr());
+        if (transSenderIpIter != transMap.end()) {
+            arp->sender_ip_addr(transSenderIpIter->second);
+            return false;
+        }
+        
+        return handleArpAndTranslateSenderIp(arp);
     }
 
     bool NatMap::handleArpReply(Tins::ARP* arp) {
         return false;
     }
     
-    void NatMap::SendTranslatedArpRequest(const Tins::ARP * arp){
+    bool NatMap::handleArpAndTranslateSenderIp(Tins::ARP* arp) {
-        Tins::IPv4Address targetIp = arp->target_ip_addr();        
+        for (NatRange & range : this->ranges) {
+                const Tins::NetworkInterface::Info & interfaceInfo = range.interface.info();
+                if (!interfaceInfo.is_up) {
+                    continue;
+                }
+
+                Tins::IPv4Range ipRange = range.calcIpRange(true);
+                if (!ipRange.contains(arp->target_ip_addr())) {
+                    continue;
+                }
+                
+                Tins::IPv4Address senderIp = arp->sender_ip_addr();
+                Tins::IPv4Address transSenderIp = range.mapIPv4Addres(senderIp, true);
+                arp->sender_ip_addr(transSenderIp);
+
+                IpAdressMap::const_iterator transSenderIpReqIter = this->reqIpMap.find(transSenderIp);
+                if (transSenderIpReqIter == reqIpMap.end()) {
+                    this->reqIpMap.insert(IPv4AddressEntry(transSenderIp, senderIp));
+                }
+                
+                return true;
+            }
+        
+        return false;
+    }
+
+    void NatMap::SendTranslatedArpRequest(const Tins::ARP * arp) {
+        Tins::IPv4Address targetIp = arp->target_ip_addr();
         for (NatRange & range : this->ranges) {
             const Tins::NetworkInterface::Info & interfaceInfo = range.interface.info();
             if (!interfaceInfo.is_up) {
@@ -260,7 +265,7 @@ namespace otonat {
             if (transTargetIpReqIter == reqIpMap.end()) {
                 this->reqIpMap.insert(IPv4AddressEntry(transTargetIp, targetIp));
             }
-            
+
             Tins::EthernetII transArp = Tins::ARP::make_arp_request(transTargetIp, transSenderIp, arp->sender_hw_addr());
             outgoingPduQueue.push(transArp.clone());
         }

src/map/natmap.h

@@ -38,6 +38,7 @@ namespace otonat {
         bool handleArp(Tins::ARP * arp);
         bool handleArpReq(Tins::ARP * arp);
         bool handleArpReply(Tins::ARP * arp);
+        bool handleArpAndTranslateSenderIp(Tins::ARP* arp);
         Tins::IPv4Address InsertOrUdpateTranslateIpAddress(const Tins::IPv4Address & originIp, const NatRange & range);
         Tins::IPv4Address InsertOrUdpateTranslateIpAddress(const Tins::IPv4Address & originIp, const Tins::IPv4Address & transIp, NatRangeList & rangeList);
         void TranslateIpPacket(Tins::IP * ip, const Tins::IPv4Address & transIp);

tests/nattest.cpp

@@ -84,64 +84,65 @@ void nattest::testTranslateIp() {
     Tins::EthernetII eth4 = Tins::EthernetII("00:00:00:00:00:02", "00:00:00:00:00:05") / Tins::IP("10.0.3.55", "10.0.1.41") / Tins::TCP();
     Tins::EthernetII eth4Ack = Tins::EthernetII("00:00:00:00:00:05", "00:00:00:00:00:02") / Tins::IP("172.27.1.41", "172.17.3.55") / Tins::TCP();
 
-    natMap.handlePdu(&eth);
+    natMap.handlePdu(eth.clone());
-    CPPUNIT_ASSERT(natMap.outgoingPduQueue.empty());
+    CPPUNIT_ASSERT(natMap.outgoingPduQueue.size() == 1);
+    natMap.outgoingPduQueue.pop();            
 
     natMap.transMap.insert(otonat::NatMap::IPv4AddressEntry(Tins::IPv4Address("172.27.0.20"), Tins::IPv4Address("10.0.0.20")));
 
-    natMap.handlePdu(&ethW);
+    natMap.handlePdu(ethW.clone());
     CPPUNIT_ASSERT(natMap.outgoingPduQueue.empty());
 
-    natMap.handlePdu(&eth);
+    natMap.handlePdu(eth.clone());
     CPPUNIT_ASSERT(natMap.outgoingPduQueue.size() == 1);
     const Tins::PDU * result = natMap.outgoingPduQueue.front();
     checkEth(result->rfind_pdu<Tins::EthernetII>(), "00:00:00:00:00:01", "00:00:00:00:00:02", "10.0.0.20", "10.0.3.55");
     natMap.outgoingPduQueue.pop();
     CPPUNIT_ASSERT(natMap.outgoingPduQueue.empty());
 
-    natMap.handlePdu(&ethAck);
+    natMap.handlePdu(ethAck.clone());
     CPPUNIT_ASSERT(natMap.outgoingPduQueue.size() == 1);
     const Tins::PDU * resultAck = natMap.outgoingPduQueue.front();
     checkEth(resultAck->rfind_pdu<Tins::EthernetII>(), "00:00:00:00:00:02", "00:00:00:00:00:01", "172.16.3.55", "172.27.0.20");
     natMap.outgoingPduQueue.pop();
     CPPUNIT_ASSERT(natMap.outgoingPduQueue.empty());
 
-    natMap.handlePdu(&eth2);
+    natMap.handlePdu(eth2.clone());
     CPPUNIT_ASSERT(natMap.outgoingPduQueue.size() == 1);
     const Tins::PDU * result2 = natMap.outgoingPduQueue.front();
     checkEth(result2->rfind_pdu<Tins::EthernetII>(), "00:00:00:00:00:01", "00:00:00:00:00:03", "10.0.0.20", "10.0.3.55");
     natMap.outgoingPduQueue.pop();
     CPPUNIT_ASSERT(natMap.outgoingPduQueue.empty());
 
-    natMap.handlePdu(&eth2Ack);
+    natMap.handlePdu(eth2Ack.clone());
     CPPUNIT_ASSERT(natMap.outgoingPduQueue.size() == 1);
     const Tins::PDU * result2Ack = natMap.outgoingPduQueue.front();
     checkEth(result2Ack->rfind_pdu<Tins::EthernetII>(), "00:00:00:00:00:03", "00:00:00:00:00:01", "172.17.3.55", "172.27.0.20");
     natMap.outgoingPduQueue.pop();
     CPPUNIT_ASSERT(natMap.outgoingPduQueue.empty());
 
-    natMap.handlePdu(&eth3);
+    natMap.handlePdu(eth3.clone());
     CPPUNIT_ASSERT(natMap.outgoingPduQueue.size() == 1);
     const Tins::PDU * result3 = natMap.outgoingPduQueue.front();
     checkEth(result3->rfind_pdu<Tins::EthernetII>(), "00:00:00:00:00:01", "00:00:00:00:00:04", "10.0.0.20", "10.0.1.40");
     natMap.outgoingPduQueue.pop();
     CPPUNIT_ASSERT(natMap.outgoingPduQueue.empty());
 
-    natMap.handlePdu(&eth3Ack);
+    natMap.handlePdu(eth3Ack.clone());
     CPPUNIT_ASSERT(natMap.outgoingPduQueue.size() == 1);
     const Tins::PDU * result3Ack = natMap.outgoingPduQueue.front();
     checkEth(result3Ack->rfind_pdu<Tins::EthernetII>(), "00:00:00:00:00:04", "00:00:00:00:00:01", "172.18.1.40", "172.27.0.20");
     natMap.outgoingPduQueue.pop();
     CPPUNIT_ASSERT(natMap.outgoingPduQueue.empty());
 
-    natMap.handlePdu(&eth4);
+    natMap.handlePdu(eth4.clone());
     CPPUNIT_ASSERT(natMap.outgoingPduQueue.size() == 1);
     const Tins::PDU * result4 = natMap.outgoingPduQueue.front();
     checkEth(result4->rfind_pdu<Tins::EthernetII>(), "00:00:00:00:00:02", "00:00:00:00:00:05", "172.17.3.55", "172.27.1.41");
     natMap.outgoingPduQueue.pop();
     CPPUNIT_ASSERT(natMap.outgoingPduQueue.empty());
 
-    natMap.handlePdu(&eth4Ack);
+    natMap.handlePdu(eth4Ack.clone());
     CPPUNIT_ASSERT(natMap.outgoingPduQueue.size() == 1);
     const Tins::PDU * result4Ack = natMap.outgoingPduQueue.front();
     checkEth(result4Ack->rfind_pdu<Tins::EthernetII>(), "00:00:00:00:00:05", "00:00:00:00:00:02", "10.0.1.41", "10.0.3.55");
@@ -154,10 +155,10 @@ void nattest::testForMeFromMe() {
     Tins::EthernetII FromMe = Tins::EthernetII("00:00:00:00:00:01", "00:00:00:00:00:03") / Tins::IP("172.27.0.20", "172.16.0.1") / Tins::TCP();
     natMap.transMap.insert(otonat::NatMap::IPv4AddressEntry(Tins::IPv4Address("172.27.0.20"), Tins::IPv4Address("10.0.0.20")));
 
-    natMap.handlePdu(&forMe);
+    natMap.handlePdu(forMe.clone());
     CPPUNIT_ASSERT(natMap.outgoingPduQueue.empty());
 
-    natMap.handlePdu(&FromMe);
+    natMap.handlePdu(FromMe.clone());
     CPPUNIT_ASSERT(natMap.outgoingPduQueue.empty());
 }