Set proper value for length field

include/tins/icmp.h

@@ -396,13 +396,30 @@ namespace Tins {
          */
         bool has_extensions() const { return !extensions_.extensions().empty(); }
 
+        /**
+         * \brief Sets whether the length field will be set for packets that use it
+         *
+         * As defined in RFC 4884, some ICMP packet types can have a length field. This
+         * method controlers whether the length field is set or not.
+         *
+         * Note that this only indicates that the packet should use this field. The 
+         * actual value will be set during the packet's serialization.
+         *
+         * Note that, in order to br RFC compliant, if the size of the encapsulated
+         * PDU is greater than 128, the length field will always be set, regardless
+         * of whether this method was called or not.
+         *
+         * /param value true iff the length field should be set appropriately
+         */
+        void use_length_field(bool value);
+
         /**
          * \brief Getter for the PDU's type.
          *
          * \sa PDU::pdu_type
          */
         PDUType pdu_type() const { return PDU::ICMP; }
-        
+
         /**
          * \sa PDU::clone
          */
@@ -445,6 +462,8 @@ namespace Tins {
         void write_serialization(uint8_t *buffer, uint32_t total_sz, const PDU *parent);
 
         void try_parse_extensions(const uint8_t* buffer, uint32_t& total_sz);
+        bool are_extensions_allowed() const;
+        uint32_t get_adjusted_inner_pdu_size() const;
 
         icmphdr _icmp;
         uint32_t _orig_timestamp_or_address_mask, _recv_timestamp, _trans_timestamp;

include/tins/icmp_extension.h

@@ -30,6 +30,14 @@ public:
      */
     ICMPExtension();
 
+    /**
+     * \brief Constructor taking class and type
+     *
+     * \param ext_class The extension class
+     * \param ext_type The extension sub-type
+     */
+    ICMPExtension(uint8_t ext_class, uint8_t ext_type);
+
     /**
      * \brief Constructs an ICMP extension from a buffer
      *

src/icmp.cpp

@@ -149,7 +149,12 @@ uint32_t ICMP::trailer_size() const {
     if (has_extensions()) {
         output += extensions_.size();
         if (inner_pdu()) {
-            output += 128 - std::min(inner_pdu()->size(), 128U);
+            // This gets how much padding we'll use. 
+            // If the next pdu size is lower than 128 bytes, then padding = 128 - pdu size
+            // If the next pdu size is greater than 128 bytes, 
+            // then padding = pdu size padded to next 32 bit boundary - pdu size
+            const uint32_t upper_bound = std::max(get_adjusted_inner_pdu_size(), 128U);
+            output += upper_bound - inner_pdu()->size();
         }
     }
     return output;
@@ -210,6 +215,12 @@ void ICMP::set_redirect(uint8_t icode, address_type address) {
     gateway(address);
 }
 
+void ICMP::use_length_field(bool value) {
+    // We just need a non 0 value here, we'll use the right value on 
+    // write_serialization
+    _icmp.un.rfc4884.length = value ? 1 : 0;
+}
+
 void ICMP::write_serialization(uint8_t *buffer, uint32_t total_sz, const PDU *) {
     #ifdef TINS_DEBUG
     assert(total_sz >= sizeof(icmphdr));
@@ -229,16 +240,36 @@ void ICMP::write_serialization(uint8_t *buffer, uint32_t total_sz, const PDU *)
         memcpy(buffer + sizeof(icmphdr), &uint32_t_buffer, sizeof(uint32_t));
     }
 
+    // If extensions are allowed and we have to set the length field
+    if (are_extensions_allowed()) {
+        uint32_t length_value = get_adjusted_inner_pdu_size();
+        // If the next pdu size is greater than 128, we are forced to set the length field
+        if (length() != 0 || length_value > 128) {
+            length_value = length_value ? std::max(length_value, 128U) : 0;
+            // This field uses 32 bit words as the unit
+            _icmp.un.rfc4884.length = length_value / sizeof(uint32_t);
+        }
+    }
+
     if (has_extensions()) {
         uint8_t* extensions_ptr = buffer + sizeof(icmphdr);
         if (inner_pdu()) {
-            uint32_t inner_pdu_size = inner_pdu()->size();
+            // Get the size of the next pdu, padded to the next 32 bit boundary
+            uint32_t inner_pdu_size = get_adjusted_inner_pdu_size();
+            // If it's lower than 128, we need to padd enough zeroes to make it 128 bytes long
             if (inner_pdu_size < 128) {
                 memset(buffer + sizeof(icmphdr) + inner_pdu_size, 0, 128 - inner_pdu_size);
                 inner_pdu_size = 128;
             }
+            else {
+                // If the packet has to be padded to 32 bits, append the amount 
+                // of zeroes we need
+                uint32_t diff = inner_pdu_size - inner_pdu()->size();
+                memset(buffer + sizeof(icmphdr) + inner_pdu_size, 0, diff);
+            }
             extensions_ptr += inner_pdu_size;
         }
+        // Now serialize the exensions where they should be
         extensions_.serialize(extensions_ptr, total_sz - (extensions_ptr - buffer));
     }
 
@@ -254,12 +285,25 @@ void ICMP::write_serialization(uint8_t *buffer, uint32_t total_sz, const PDU *)
     memcpy(buffer + 2, &_icmp.check, sizeof(uint16_t));
 }
 
+uint32_t ICMP::get_adjusted_inner_pdu_size() const {
+    // This gets the size of the next pdu, padded to the next 32 bit word boundary
+    if (inner_pdu()) {
+        uint32_t inner_pdu_size = inner_pdu()->size();
+        uint32_t padding = inner_pdu_size % 4;
+        inner_pdu_size = padding ? (inner_pdu_size - padding + 4) : inner_pdu_size;
+        return inner_pdu_size;
+    }
+    else {
+        return 0;
+    }
+}
+
 void ICMP::try_parse_extensions(const uint8_t* buffer, uint32_t& total_sz) {
     if (total_sz == 0) {
         return;
     }
     // Check if this is one of the types defined in RFC 4884
-    if (type() == DEST_UNREACHABLE || type() == TIME_EXCEEDED || type() == PARAM_PROBLEM) {
+    if (are_extensions_allowed()) {
         uint32_t actual_length = length() * sizeof(uint32_t);
         // Check if we actually have this amount of data and whether it's more than
         // the minimum encapsulated packet size
@@ -286,6 +330,10 @@ void ICMP::try_parse_extensions(const uint8_t* buffer, uint32_t& total_sz) {
     }
 }
 
+bool ICMP::are_extensions_allowed() const {
+    return type() == DEST_UNREACHABLE || type() == TIME_EXCEEDED || type() == PARAM_PROBLEM;
+}
+
 bool ICMP::matches_response(const uint8_t *ptr, uint32_t total_sz) const {
     if(total_sz < sizeof(icmphdr))
         return false;

src/icmp_extension.cpp

@@ -17,6 +17,12 @@ ICMPExtension::ICMPExtension()
 
 } 
 
+ICMPExtension::ICMPExtension(uint8_t ext_class, uint8_t ext_type)
+: extension_class_(ext_class), extension_type_(ext_type) {
+
+}
+
+
 ICMPExtension::ICMPExtension(const uint8_t* buffer, uint32_t total_sz) {
     // Check for the base header (u16 length + u8 clss + u8 type)
     if (total_sz < BASE_HEADER_SIZE) {
@@ -76,7 +82,7 @@ ICMPExtension::serialization_type ICMPExtension::serialize() const {
 const uint32_t ICMPExtensionsStructure::BASE_HEADER_SIZE = sizeof(uint16_t) * 2;
 
 ICMPExtensionsStructure::ICMPExtensionsStructure() 
-: version_and_reserved_(), checksum_(0) {
+: version_and_reserved_(0), checksum_(0) {
     version(2);
 }
 
@@ -98,7 +104,7 @@ ICMPExtensionsStructure::ICMPExtensionsStructure(const uint8_t* buffer, uint32_t
 }
 
 void ICMPExtensionsStructure::reserved(small_uint<12> value) {
-    uint16_t current_value = version_and_reserved_;
+    uint16_t current_value = Endian::be_to_host(version_and_reserved_);
     current_value &= 0xf000;
     current_value |= value;
     version_and_reserved_ = Endian::host_to_be(current_value);
@@ -142,6 +148,7 @@ void ICMPExtensionsStructure::add_extension(const ICMPExtension& extension) {
 void ICMPExtensionsStructure::serialize(uint8_t* buffer, uint32_t buffer_size) {
     const uint32_t structure_size = size();
     if (buffer_size < structure_size) {
+        std::cout << buffer_size << " vs " << structure_size << std::endl;
         throw malformed_packet();
     }
     uint8_t* original_ptr = buffer;

tests/src/icmp.cpp

@@ -14,7 +14,8 @@ using namespace Tins;
 class ICMPTest : public testing::Test {
 public:
     static const uint8_t expected_packets[][8];
-    static const uint8_t ts_request[], ts_reply[], packet_with_extensions[];
+    static const uint8_t ts_request[], ts_reply[], packet_with_extensions[], 
+                         packet_with_extensions_and_length[];
     static const uint32_t expected_packet_count;
     
     void test_equals(const ICMP &icmp1, const ICMP &icmp2);
@@ -44,6 +45,14 @@ const uint8_t ICMPTest::packet_with_extensions[] = {
     8, 1, 1, 24, 150, 1, 1
 };
 
+const uint8_t ICMPTest::packet_with_extensions_and_length[] = { 
+    11, 0, 204, 228, 0, 32, 0, 0, 69, 0, 0, 40, 165, 76, 0, 0, 1, 17, 247, 111, 12, 4, 4, 4, 12, 
+    1, 1, 1, 165, 75, 130, 155, 0, 20, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 32, 0, 197, 95, 0, 
+    8, 1, 1, 24, 150, 1, 1
+};
 
 
 TEST_F(ICMPTest, DefaultConstructor) {
@@ -322,3 +331,42 @@ TEST_F(ICMPTest, ExtensionsParsingWithoutALengthField) {
         PDU::serialization_type(packet_with_extensions, packet_with_extensions + sizeof(packet_with_extensions))
     );
 }
+
+TEST_F(ICMPTest, ExtensionsParsingWithALengthField) {
+    const uint8_t encapsulated[] = { 69, 0, 0, 40, 165, 76, 0, 0, 1, 17, 247, 111, 12, 4, 4, 4, 12, 1, 1, 1, 165, 75, 130, 155, 0, 20, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
+    const uint8_t ext[] = { 0, 8, 1, 1, 24, 150, 1, 1 };
+    ICMP icmp(packet_with_extensions_and_length, sizeof(packet_with_extensions_and_length));
+    ICMPExtensionsStructure extensions = icmp.extensions();
+    ASSERT_EQ(1, extensions.extensions().size());
+    EXPECT_EQ(
+        ICMPExtension::payload_type(ext, ext + sizeof(ext)), 
+        extensions.extensions().begin()->serialize()
+    );
+    const RawPDU* raw = icmp.find_pdu<RawPDU>();
+    ASSERT_TRUE(raw != 0);
+    EXPECT_EQ(
+        RawPDU::payload_type(encapsulated, encapsulated + sizeof(encapsulated)),
+        raw->payload()
+    );
+
+    // Dump this packet without a length field
+    icmp.use_length_field(false);
+    PDU::serialization_type buffer = icmp.serialize();
+    
+    EXPECT_EQ(sizeof(packet_with_extensions), buffer.size());
+    EXPECT_EQ(
+        buffer, 
+        PDU::serialization_type(packet_with_extensions, packet_with_extensions + sizeof(packet_with_extensions))
+    );
+
+    // Dump this packet using a length field
+    icmp.use_length_field(true);
+    buffer = icmp.serialize();
+
+    EXPECT_EQ(sizeof(packet_with_extensions_and_length), buffer.size());
+    EXPECT_EQ(
+        buffer, 
+        PDU::serialization_type(packet_with_extensions_and_length, 
+                                packet_with_extensions_and_length + sizeof(packet_with_extensions_and_length))
+    );
+}

tests/src/ip.cpp

@@ -6,6 +6,7 @@
 #include "tcp.h"
 #include "udp.h"
 #include "icmp.h"
+#include "icmp_extension.h"
 #include "rawpdu.h"
 #include "ip_address.h"
 #include "utils.h"
@@ -783,3 +784,53 @@ TEST_F(IPTest, FragmentOffset) {
     ip.flags(IP::DONT_FRAGMENT);
     EXPECT_FALSE(ip.is_fragmented());
 }
+
+// Use a large buffer. This wil set the length field
+TEST_F(IPTest, SerializePacketHavingICMPExtensionsWithLengthAndLotsOfPayload) {
+    IP encapsulated = IP(TINS_DEFAULT_TEST_IP) / UDP(99, 12) / RawPDU(std::string(250, 'A'));
+    EthernetII pkt = EthernetII() / IP() / ICMP(ICMP::TIME_EXCEEDED) / encapsulated;
+    const uint8_t payload[] = { 24, 150, 1, 1 }; 
+    ICMPExtension extension(1, 1);
+    ICMPExtension::payload_type ext_payload(payload, payload + sizeof(payload));
+    extension.payload(ext_payload);
+    pkt.rfind_pdu<ICMP>().extensions().add_extension(extension);
+
+    PDU::serialization_type buffer = pkt.serialize();
+    EthernetII serialized(&buffer[0], buffer.size());
+    ASSERT_EQ(1, serialized.rfind_pdu<ICMP>().extensions().extensions().size());
+    EXPECT_EQ(ext_payload, serialized.rfind_pdu<ICMP>().extensions().extensions().begin()->payload());
+}
+
+// Use a short buffer and set the length field
+TEST_F(IPTest, SerializePacketHavingICMPExtensionsWithLengthAndShortPayload) {
+    IP encapsulated = IP(TINS_DEFAULT_TEST_IP) / UDP(99, 12) / RawPDU(std::string(40, 'A'));
+    EthernetII pkt = EthernetII() / IP() / ICMP(ICMP::TIME_EXCEEDED) / encapsulated;
+    const uint8_t payload[] = { 24, 150, 1, 1 }; 
+    ICMPExtension extension(1, 1);
+    ICMPExtension::payload_type ext_payload(payload, payload + sizeof(payload));
+    extension.payload(ext_payload);
+    pkt.rfind_pdu<ICMP>().extensions().add_extension(extension);
+    pkt.rfind_pdu<ICMP>().use_length_field(true);
+
+    PDU::serialization_type buffer = pkt.serialize();
+    EthernetII serialized(&buffer[0], buffer.size());
+    ASSERT_EQ(1, serialized.rfind_pdu<ICMP>().extensions().extensions().size());
+    EXPECT_EQ(ext_payload, serialized.rfind_pdu<ICMP>().extensions().extensions().begin()->payload());
+}
+
+// Use a short buffer and don't set the length field
+TEST_F(IPTest, SerializePacketHavingICMPExtensionsWithoutLengthAndShortPayload) {
+    IP encapsulated = IP(TINS_DEFAULT_TEST_IP) / UDP(99, 12) / RawPDU(std::string(40, 'A'));
+    EthernetII pkt = EthernetII() / IP() / ICMP(ICMP::TIME_EXCEEDED) / encapsulated;
+    const uint8_t payload[] = { 24, 150, 1, 1 }; 
+    ICMPExtension extension(1, 1);
+    ICMPExtension::payload_type ext_payload(payload, payload + sizeof(payload));
+    extension.payload(ext_payload);
+    pkt.rfind_pdu<ICMP>().extensions().add_extension(extension);
+    pkt.rfind_pdu<ICMP>().use_length_field(false);
+
+    PDU::serialization_type buffer = pkt.serialize();
+    EthernetII serialized(&buffer[0], buffer.size());
+    ASSERT_EQ(1, serialized.rfind_pdu<ICMP>().extensions().extensions().size());
+    EXPECT_EQ(ext_payload, serialized.rfind_pdu<ICMP>().extensions().extensions().begin()->payload());
+}