2
\$\begingroup\$

I could not find an implementation of the IPv4 header checksum calculation. So, I decided to made one as defined in RFC 791.

I made use of the "newer" ReadOnlySpan<T> classes to optimize things a bit. Appreciate any feedback!

Extension Methods

Note: These are two extension methods to illustrate the only two use cases that a calculation of the checksum would be need in the context of reading bytes contained within a IPv4 packet. They produce the API to expose the private method (below). They differ only by ignoring the 10th byte (Header Checksum).

public static ushort GetInternetChecksum(this ReadOnlySpan<byte> bytes)
    => CalculateChecksum(bytes, ignoreHeaderChecksum: true);
public static bool IsValidChecksum(this ReadOnlySpan<byte> bytes)
    // Should equal zero (valid)
    => CalculateChecksum(bytes, ignoreHeaderChecksum: false) == 0;

The Checksum Calculation

using System.Buffers.Binary;

/// <summary>
/// An implementation of the IPv4 header checksum calculation, 
/// as defined in RFC 791.
/// </summary>
private static ushort CalculateChecksum(
    ReadOnlySpan<byte> bytes, 
    bool ignoreHeaderChecksum)
{
    ushort checksum = 0;
    for (int i = 0; i <= 18; i += 2)
    {
        // Ignore the Header Checksum?
        if (ignoreHeaderChecksum && i == 10) continue;

        // Use network byte order
        ushort value = BinaryPrimitives.ReadUInt16BigEndian(bytes[i..(i + 2)]);

        // Each time a carry occurs, we must add a 1 to the sum
        if (checksum + value > ushort.MaxValue)
        {
            checksum++;
        }

        checksum += value;
    }
    
    // One’s complement
    return (ushort)~checksum;
}

For a detailed explanation of the loop above:

i = 0   e.g. bytes[0..2]   // Version and Internal Header Length
i = 2   e.g. bytes[2..4]   // Total Length
i = 4   e.g. bytes[4..6]   // Identification
i = 6   e.g. bytes[6..8]   // Flags and Fragmentation Offset
i = 8   e.g. bytes[8..10]  // TTL and Protocol
i = 10  e.g. bytes[10..12] // ***Header Checksum***
i = 12  e.g. bytes[12..14] // Source Address #1
i = 14  e.g. bytes[14..16] // Source Address #2
i = 16  e.g. bytes[16..18] // Destination Address #1
i = 18  e.g. bytes[18..20] // Destination Address #2

// Note: We should ignore the Header Checksum value
//       when calculating the checksum, and include
//       it back in when verifying.

EDIT: I added some additional context surrounding the Extension Methods section (above) and I also added an example use case (below).

Example Usage

CapturePacketsAsync() snippet

PipeOptions options = new(
   pauseWriterThreshold: 65535, // 64kB
   resumeWriterThreshold: 1400); // or MTU - TCP/IP Headers
Pipe pipe = new(options);
Task writing = WriteToPipeAsync(pipe.Writer, _socket, cancellationToken);
Task reading = ReadFromPipeAsync(pipe.Reader, cancellationToken);
await Task.WhenAll(reading, writing).ConfigureAwait(true);

WriteToPipeAsync() snippet

while(!cancellationToken.IsCancellationRequested)
{
   const int minimumBufferSize = 65535;
   Memory<byte> buffer = writer.GetMemory(minimumBufferSize);
   _ = await socket.ReceiveAsync(buffer, socketFlags, cancellationToken);

   // We can perform either of the checksum methods here
   //    - use GetInternetChecksum() if we are sending
   //    - use IsValidChecksum() if we are receiving
   // Then handle any invalid packets

   writer.Advance(bytesReceived);

   // try/finally, mark pipe complete via CompleteAsync(), etc.
}

ReadFromPipeAsync() snippet

while(!cancellationToken.IsCancellationRequested)
{
   ReadResult result = await reader.ReadAsync(cancellationToken).ConfigureAwait(true);
   ReadOnlySequence<byte> buffer = result.Buffer;

   // We can perform either of the checksum methods here
   //    - use GetInternetChecksum() if we are sending
   //    - use IsValidChecksum() if we are receiving
   // Then handle any invalid packets

   reader.AdvanceTo(consumed, examined);

   // try/finally, mark pipe complete via CompleteAsync(), etc.
}

In the above, the choice of doing the checksum calculations would depend on how you would want to manage the pressure on either side of the pipe.

\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

This is interesting code and I'm happy it works for you in your specific usage, but on a general level I find it lacking.

For starters, you have ZERO checks to validate the input bytes form a valid IPv4 header. All someone has to do is pass in a ReadOnlySpan<byte> of ANY length, including 0 length.

I also find it somewhat restrictive because you limit the input to ReadOnlySpan<byte>. If someone has an array or a list, they are out of luck. I tend towards the philosophy that public input parameters should be as broadly available. This would suggest an input of IEnumerable<byte> would have the broadest appeal. But again, this input would need to be validated that it forms a proper IPv4 header regardless of whether it was a ReadOnlySpan<byte>, byte[], List<byte>, or any enumerable collection of byte.

While the main calculation is a lot of math operations and should be quick, I find something inefficient about IsValidChecksum. I am trying to figure out how you would use it. Here are 2 possible ways:

Branch 1:

var checksum = GetInternetChecksum(bytes);
if (IsValidChecksum(bytes))  // <== calculated twice
{
   // do something
}

Branch 2:

if (IsValidChecksum(bytes))  
{
    var checksum = GetInternetChecksum(bytes);  // <== calculated twice
   // do something
}

What you see is that in these 2 possible usages that you will have to perform the calculations twice, which just just does not seem right.

\$\endgroup\$
4
  • 1
    \$\begingroup\$ These functions would not be used the way you have suggested. GetInternetChecksum would be called when sending a packet, and the resulting ushort would be stored in the 10th and 11th bytes of the header. IsValidChecksum would be called when reading a packet, and it would only be processed if the checksum is correct. Hence, it does make sense to have these 2 different signatures. \$\endgroup\$ Commented Jul 3, 2022 at 11:10
  • \$\begingroup\$ I guess it might have been better to provide full context, as these extension methods are very specific to dealing with packets over a socket. So, typically these extension methods would be held within it's own namespace -- e.g., in an internal class. There are very specific uses for each extension method as well, as @MiguelAlorda noted, he is accurate on this matter. \$\endgroup\$
    – Svek
    Commented Jul 4, 2022 at 6:24
  • \$\begingroup\$ I added some context to my original post. \$\endgroup\$
    – Svek
    Commented Jul 4, 2022 at 6:57
  • \$\begingroup\$ Wanted to extend some clarity, that your feedback was very helpful and greatly appreciated. \$\endgroup\$
    – Svek
    Commented Jul 4, 2022 at 17:05
2
\$\begingroup\$

In addition of @Rick-Davin,

  • too broad method names : GetInternetChecksum and IsValidChecksum and CalculateChecksum, they are not telling which type of data is going to validate or calculate its checksum.
  • targeting a very general type instead of specific type.

If you only make them private and only shipped public extension methods for IPAddress that will use them internally, then it would probably be better, because of narrowing the scope to a specific type, and also save efforts and time on validating IP, you probably just need to validate against a specific IP scheme, and it will also open a wide range of other .NET supported schemes to adopt if needed.

Also, since the bytes size and positions are fixed, maybe using a class that represents the address in readable form would make it much easier to work with, and it would be only validated once.

something like this :

public class IPAddressChecksumValidator
{
    private readonly IPAddressEntry _entry; 
    
    private class IPAddressEntry
    {
        public byte Version { get; set; }
        public byte TotalLength { get; set; }
        public byte Identification { get; set; }
        // ..etc.
        public IPAddressEntry(byte[] entry)
        {
            /// assign bytes to each property 
        }
    }
    
    public IPAddressChecksumValidator(IPAddress ipAddress)
    {
        // initialize and validate IPAddress version
    }
    
    public ushort GetInternetChecksum() { 
        // use _entry to get the result
    }
    
    public bool IsValid() { 
        // use _entry to get the result     
    }
}


public static class IPAddressExtension
{
    public static IPAddressClass GetValidator(this IPAddress ipAddress)
        => new IPAddressChecksumValidator(ipAddress);
}

usage example :

var ipAddress = new IPAddress(bytes);

var checksumValidator = ipAddress.GetValidator(); 
// or new IPAddressChecksumValidator(ipAddress);

if(checksumValidator.IsValid())
{
    // do stuff
}
\$\endgroup\$
5
  • \$\begingroup\$ Though it is true that maybe some validation should be done on data input (at least checking the size to be exactly 18), I think creating a class for this purpose might be a bit overkill. Specially since it would make the implementation of the CalculateChecksum a bit cumbersome. After all, it's just summing the bytes in pairs. It is true that the names are kind of broad, so maybe using a namespace could be more appropriate. Also, it doesn't make sense to initialize an IP packet header just with an IPAddress. The header needs source and dest address', id, TTL, fragmentation data... \$\endgroup\$ Commented Jul 3, 2022 at 11:21
  • \$\begingroup\$ Also, if this was to be used in the real IPv4 protocol implementation, the method would be called for each packet being sent through each network interface. The overhead of copying all of the header bytes to named fields in a class may be unnecessary. \$\endgroup\$ Commented Jul 3, 2022 at 11:24
  • \$\begingroup\$ The project that I am working on does in fact have a class, which is used when parsing packets. However, instantiation of every packet is not something is "mandatory" or at least I assumed that would be the most efficient. When dealing with packets at this low-level, I think that we "should" use the generic type, because there are specific scenarios where allocation of resources (either compute or memory) isn't a desirable side-effect. For example, if we used the Pipe, PipeReader and PipeWriter then this kind of method can drop right into it. \$\endgroup\$
    – Svek
    Commented Jul 4, 2022 at 6:36
  • \$\begingroup\$ I added an example use case, to help illustrate how the methods might be used. \$\endgroup\$
    – Svek
    Commented Jul 4, 2022 at 6:56
  • \$\begingroup\$ Wanted to extend some clarity, that your feedback was very helpful and greatly appreciated. \$\endgroup\$
    – Svek
    Commented Jul 4, 2022 at 17:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.