How do I ensure correctness of an IPv4 packet created for a native `ping` implementation?

Question

Overview

I have been working on a side project which is essentially a network troubleshooting tool. My intent is to deepen my understanding of networking fundamentals and get comfortable using troubleshooting tools offerred by the OS.

It’s a CLI app, that would take the hostname and try to diagnose issues(if any). The plan was to implement ping and traceroute first and progressively implement other tools depending on my level of comfort.

However, my ping implmentation is not accurate in the sense that, the IPv4 packets are malformed. That’s what wireshark had to say.

1   0.000000    192.168.0.100   142.250.195.132 ICMP    300 Unknown ICMP (obsolete or malformed?)

Code

Here’s how I have implemented ping

package ping

import (
    "encoding/json"
    "net"

    "github.com/pkg/errors"
)

var (
    IcmpProtocolNumber uint8 = 1
    IPv4Version        uint8 = 4
    IPv4IHL            uint8 = 5
    ICMPHeaderType     uint8 = 8
    ICMPHeaderSubtype  uint8 = 0
)

type NativePinger struct {
    SourceIP string
    DestIP   string
}

type ICMPHeader struct {
    Type     uint8
    Code     uint8
    Checksum uint16
}

type ICMPPacket struct {
    Header  ICMPHeader
    Payload interface{}
}

type IPv4Header struct {
    SourceIP       string
    DestinationIP  string
    Length         uint16
    Identification uint16
    FlagsAndOffset uint16
    Checksum       uint16
    VersionIHL     uint8
    DSCPAndECN     uint8
    TTL            uint8
    Protocol       uint8
}

type IPv4Packet struct {
    Header  IPv4Header
    Payload *ICMPPacket
}

func (p *NativePinger) createIPv4Packet() (*IPv4Packet, error) {
    versionIHL := (IPv4Version << 4) | IPv4IHL

    icmpPacket := &ICMPPacket{
        Header: ICMPHeader{
            Type: ICMPHeaderType,
            Code: ICMPHeaderSubtype,
        },
    }
    ipv4Packet := &IPv4Packet{
        Header: IPv4Header{
            VersionIHL:     versionIHL,
            DSCPAndECN:     0,
            Identification: 0,
            FlagsAndOffset: 0,
            TTL:            64,
            Protocol:       IcmpProtocolNumber,
            SourceIP:       p.SourceIP,
            DestinationIP:  p.DestIP,
        },
        Payload: icmpPacket,
    }
    ipv4Packet.Header.Length = 40

    bytes, err := json.Marshal(icmpPacket)
    if err != nil {
        return nil, errors.Wrapf(err, "error converting ICMP packet to bytes")
    }

    icmpPacket.Header.Checksum = calculateChecksum(bytes)

    bytes, err = json.Marshal(ipv4Packet)
    if err != nil {
        return nil, errors.Wrapf(err, "error converting IPv4 packet to bytes")
    }

    ipv4Packet.Header.Checksum = calculateChecksum(bytes)

    return ipv4Packet, nil
}

func calculateChecksum(data []byte) uint16 {
    sum := uint32(0)

    // creating 16 bit words
    for i := 0; i < len(data)-1; i++ {
        word := uint32(data[i])<<8 | uint32(data[i+1])
        sum += word
    }
    if len(data)%2 == 1 {
        sum += uint32(data[len(data)-1])
    }

    // adding carry bits with lower 16 bits
    for (sum >> 16) > 0 {
        sum = (sum & 0xffff) + (sum >> 16)
    }

    // taking one's compliment
    checksum := ^sum
    return uint16(checksum)
}

func (p *NativePinger) ResolveAddress(dest string) error {
    ips, err := net.LookupIP(dest)
    if err != nil {
        return errors.Wrapf(err, "error resolving address of remote host")
    }

    for _, ip := range ips {
        if ipv4 := ip.To4(); ipv4 != nil {
            p.DestIP = ipv4.String()
        }
    }

    // The destination address does not need to exist as unlike tcp, udp does not require a handshake.
    // The goal here is to retrieve the outbound IP. Source: https://stackoverflow.com/a/37382208/3728336
    //
    conn, err := net.Dial("udp", "8.8.8.8:80")
    if err != nil {
        return errors.Wrapf(err, "error resolving outbound ip address of local machine")
    }
    defer conn.Close()

    p.SourceIP = conn.LocalAddr().(*net.UDPAddr).IP.String()

    return nil
}

func (p *NativePinger) Ping(host string) error {
    if err := p.ResolveAddress(host); err != nil {
        return errors.Wrapf(err, "error resolving source/destination addresses")
    }

    packet, err := p.createIPv4Packet()
    if err != nil {
        return errors.Wrapf(err, "error creating IPv4Packet")
    }

    conn, err := net.Dial("ip4:icmp", packet.Header.DestinationIP)
    if err != nil {
        return errors.Wrapf(err, "error eshtablishing connection with %s", host)
    }
    defer conn.Close()

    bytes, err := json.Marshal(packet)
    if err != nil {
        return errors.Wrapf(err, "error converting IPv4 packet into bytes")
    }

    _, err = conn.Write(bytes)
    if err != nil {
        return errors.Wrapf(err, "error sending ICMP echo request")
    }

    buff := make([]byte, 2048)
    _, err = conn.Read(buff) // The implementation doesn't proceed beyond this point
    if err != nil {
        return errors.Wrapf(err, "error receiving ICMP echo response")
    }

    return nil
}

Upon introspection

I'm uncertain whether the packet's malformation is due to a single reason or multiple reasons. I feel the problem lies in either(or both?) of these two places:

1. Incorrect calculation of header length I have manually calculated the length to be 40 bytes (wordsize = 4 bytes). Wrote the struct fields in an order that would prevent a maligned struct. I referred this source to know the sizes of various types.

// 1 word (4 bytes)
type ICMPHeader struct {
    Type     uint8  // 8 bit
    Code     uint8  // 8 bit
    Checksum uint16 // 16 bit
}

// 3 words (3*4 = 12 bytes)
type ICMPPacket struct {
    Header  ICMPHeader  // 1 word
    Payload interface{} // 2 words
}

// 7 words (7*4 = 28 bytes)
type IPv4Header struct {
    // Below group takes 4 words (each string takes 2 words)
    SourceIP      string
    DestinationIP string

    // Following group takes 2 words (each 16 bits)
    Length         uint16
    Identification uint16
    FlagsAndOffset uint16
    Checksum       uint16

    // Below group takes 1 word (each takes 8 bits)
    VersionIHL uint8
    DSCPAndECN uint8
    TTL        uint8
    Protocol   uint8
}

// 10 words (40 bytes)
type IPv4Packet struct {
    Header  IPv4Header // 7 words as calculated above
    Payload ICMPPacket // 3 words as calculated above
}

2. Incorrect checksum calculation I implemented the internet checksum algorithm. Please let me know if that's not what I was supposed to do here.

There are missing parts in the implementation such as configuring count, assigning sequence numbers to packets, etc, but before that the basic implementation needs to be fixed i.e. receiving a response for the ICMP ECHO packet. It would be great to know where I'm making an error.

Thanks!

Update 24th Aug '23

I've updated the code considering the suggestions I got in the comments i.e. fixing byte ordering and using raw bytes for source, dest addresses. However, this alone does not solve the problem, the packet is still malformed so there must be other things going wrong.

Answer 1

I got this to work at last. I should talk about a couple of issues with the code.

Serialization issues

As rightly pointed out by Andy, I was sending JSON object instead of sending raw bytes in network byte order. This was fixed using binary.Write(buf, binary.BigEndian, field)

However, since this method works only for fixed-size values, I had to do this for each struct field, making the code repetitive and somewhat ugly.

Struct optimization and serialization are separate concerns.

I knew about this practice of fitting the Version and IHL field together to optimize memory which is why I had this single field VersionIHL in my struct. But while serializing, the field values (4 and 5 in this case) were to be serialized individually which I didn't do. Instead, I was converting the entire VersionIHL field's value to bytes.

As a result, I found myself sending an unexpected octet 69 in my byte stream which came from combining 4 and 5 together 0100 0101.

Incomplete ICMP packet

The ICMP struct I had didn't include the identifier and sequence number fields. The information provided in the ICMP datagram header section on Wikipedia felt a bit generic. However, I found the details on the RFC pages(page 14) to be much more insightful.

This feels quite peculiar, given the significance of the sequence number for the ping utility. During the implementation process, I frequently found myself wondering about the appropriate placement of the sequence number in the code. It wasn't until I stumbled upon the RFC pages that I gained clarity on when and where to incorporate the sequence number.

For anyone who might be interested, here's the functional code I've put together.