meh / rust-packet

Network packet handling for Rust.
90 stars 27 forks source link

fix: UDP packet checksum computation #11

Closed noirotm closed 4 years ago

noirotm commented 4 years ago

Hello, while trying to solve one of the layers in Tom Dalling's Data Onion challenge involving IP packet decoding, I decided to use this crate in order to verify UDP packet checksums. However, I realized it yielded incorrect results.

After reading RFC 768, and having a look at several implementations, I found out that there were two bugs that needed to be fixed for the computation to be correct.

  1. RFC 768 states that the checksum is the 16-bit one's complement of the one's complement sum of pseudo-header, UDP header, and data, therefore the initial value of 0xffffu32 is erroneous, and should be 0 instead.

  2. If payload data length is odd, the last call to buffer.read_u16::<BigEndian>() will fail entirely, causing the last byte of the payload to be ignored. Since that failure does not advance the cursor's pointer, we can try again and read a u8 instead, then pad it to 16-bit and use it in the checksum.

Apart from these issues, this code has been a great help to me, so thank you for releasing this 😃.

Regards, Marc

PS: examples of implementations I looked at: http://www.rohitab.com/discuss/topic/35831-udp-header-checksum-calculation/ https://github.com/shard013/DataOnion/blob/master/DataOnion/Layers/Layer4.cs#L75

noirotm commented 4 years ago

Just realized this repository uses tabs for indentation.

To be honest, I believe you might want to try relying on rustfmt, as it has really spoiled me regarding code formatting and the lack of need for a coding convention to be enforced when contributing to a Rust project.

If you are really intent on using tabs vs. spaces, you may provide a .rustfmt.toml file at the root of your project and use the following contents (reference here):

hard_tabs = true

This may also be enforced by the CI as explained here, for example.

meh commented 4 years ago

Thanks! And sorry for the rookie mistake.

Is rustfmt decent now? When I was using it ~3 years ago it was more trouble than help so I stopped using it, how's the situation with formatting of macros and such?

noirotm commented 4 years ago

My use of macros has been pretty limited so far, so I can't really tell how rustfmt works in that regard.

It would appear as though there are flags to control macro formatting which were added in 2018.

My experience with rustfmt is that it's doing mostly ok. A notable exception is when using libraries such as clap. It then looks like this, a little bit too spacious for my taste 😃.

ssrlive commented 9 months ago

After I use this crate, when I send an even-length UDP packet, the data is correct. But when I send an odd-length UDP packet, the data packet is wrong. What are your test results?