bluecatengineering / dhcproto

A DHCP parser and encoder for DHCPv4/DHCPv6
MIT License
114 stars 26 forks source link

Encoder produces different results #75

Open f3rn0s opened 2 months ago

f3rn0s commented 2 months ago

I'm using this package to build a DHCP packet that gets sent over the wire. I was observing that the packet would be different over different executions. I'm trying to determine why this is the case. chaddr is always the same.

pub fn create_pxe_find_packet(chaddr: [u8; 6]) -> Vec<u8> {
    let message_type = v4::MessageType::Discover;
    let params = v4::DhcpOption::ParameterRequestList(vec![
        v4::OptionCode::SubnetMask,
        v4::OptionCode::Router,
        v4::OptionCode::DomainNameServer,
        v4::OptionCode::TFTPServerName,
        v4::OptionCode::BootfileName,
    ]);

    let mut msg = v4::Message::default();

    msg.set_flags(v4::Flags::default().set_broadcast())
        .set_chaddr(&chaddr);

    msg.opts_mut().insert(v4::DhcpOption::MessageType(v4::MessageType::Discover));

    msg.opts_mut().insert(params);

    msg.opts_mut().insert(v4::DhcpOption::End);

    let mut buf = Vec::new();
    let mut e = Encoder::new(&mut buf);
    msg.encode(&mut e).unwrap();

    buf
}

For example, if I do some testing:

let first_packet = create_pxe_find_packet(chaddr);

while true {
    let pxe_packet = create_pxe_find_packet(chaddr);

    for i in 0..pxe_packet.len() {
        if first_packet[i] != pxe_packet[i] {
            println!("Message mismatch at index {}: {} {}", i, first_packet[i], pxe_packet[i]);
        }
    }
}

I get:

Message mismatch at index 4: 254 187
Message mismatch at index 5: 67 5
Message mismatch at index 6: 97 197
Message mismatch at index 7: 64 51
Message mismatch at index 4: 254 69
Message mismatch at index 5: 67 156
Message mismatch at index 6: 97 35
Message mismatch at index 7: 64 107
Message mismatch at index 240: 55 255
Message mismatch at index 241: 5 53
Message mismatch at index 243: 3 1
Message mismatch at index 244: 6 55
Message mismatch at index 245: 66 5
Message mismatch at index 246: 67 1
Message mismatch at index 247: 53 3
Message mismatch at index 248: 1 6
Message mismatch at index 249: 1 66
Message mismatch at index 250: 255 67
Message mismatch at index 4: 254 157
Message mismatch at index 5: 67 177
Message mismatch at index 6: 97 244
Message mismatch at index 7: 64 82
Message mismatch at index 240: 55 53
Message mismatch at index 241: 5 1
Message mismatch at index 243: 3 255
Message mismatch at index 244: 6 55
Message mismatch at index 245: 66 5
Message mismatch at index 246: 67 1
Message mismatch at index 247: 53 3
Message mismatch at index 248: 1 6
Message mismatch at index 249: 1 66
Message mismatch at index 250: 255 67
Message mismatch at index 4: 254 22
Message mismatch at index 5: 67 152
Message mismatch at index 6: 97 102
Message mismatch at index 7: 64 249
Message mismatch at index 240: 55 255
Message mismatch at index 241: 5 55
Message mismatch at index 242: 1 5
Message mismatch at index 243: 3 1
Message mismatch at index 244: 6 3
Message mismatch at index 245: 66 6
Message mismatch at index 246: 67 66
Message mismatch at index 247: 53 67
Message mismatch at index 248: 1 53
Message mismatch at index 250: 255 1
Message mismatch at index 4: 254 218
Message mismatch at index 5: 67 121
Message mismatch at index 6: 97 1
Message mismatch at index 7: 64 98
Message mismatch at index 240: 55 53
Message mismatch at index 241: 5 1
Message mismatch at index 243: 3 255
Message mismatch at index 244: 6 55
Message mismatch at index 245: 66 5
Message mismatch at index 246: 67 1
Message mismatch at index 247: 53 3
Message mismatch at index 248: 1 6
Message mismatch at index 249: 1 66
Message mismatch at index 250: 255 67
Message mismatch at index 4: 254 90
Message mismatch at index 5: 67 36
Message mismatch at index 6: 97 58
Message mismatch at index 7: 64 236
Message mismatch at index 240: 55 255
Message mismatch at index 241: 5 53
Message mismatch at index 243: 3 1
Message mismatch at index 244: 6 55
Message mismatch at index 245: 66 5
Message mismatch at index 246: 67 1
Message mismatch at index 247: 53 3
Message mismatch at index 248: 1 6
Message mismatch at index 249: 1 66
Message mismatch at index 250: 255 67
Message mismatch at index 4: 254 21
Message mismatch at index 5: 67 234
Message mismatch at index 6: 97 230
Message mismatch at index 7: 64 8
Message mismatch at index 4: 254 56
Message mismatch at index 5: 67 207
Message mismatch at index 6: 97 134
Message mismatch at index 7: 64 255
Message mismatch at index 240: 55 255
Message mismatch at index 241: 5 55
Message mismatch at index 242: 1 5
Message mismatch at index 243: 3 1
Message mismatch at index 244: 6 3

In my case, I can observe this in wireshark because in some requests MessageType isn't being set:

image

leshow commented 2 months ago

the options are put into a hashmap https://github.com/bluecatengineering/dhcproto/blob/master/src/v4/options.rs#L158 which does not maintain any ordering of elements, so it's likely that when you write out to a buffer the result is not exactly the same byte for byte between encodes.

You don't need to explicitly add the End option, it will be written by the library. It's possible that by manually inserting it, the encode is short-circuiting because as I said, the options are iterated at random from the hashmap. If you could try not inserting the End option, the bytes will still not match but you should see all options written to the buffer. Perhaps we should add a guard so that End can't be inserted

Is the ordering of options important to you? We could switch to a B-Tree map, which maintains order (an order but not the insert order), but it would be a breaking change I think because some HashMap internals are exposed in various iterators.