This crate assumes the sname field of a v4::Message will be sent as UTF-8, but as far as I can tell in the RFC there is no mention at all of encoding. In addition, the pre-ipv6 RFCs for DHCP predate the UTF-8 RFC.

Perhaps most importantly, there is no encoding requirement for domain names in DNS, so I'd be a little surprised if that non-requirement doesn't extend to DHCP. This might sound a bit pedantic, but I'm aware of an organization that stores binary data in domain names within DNS, and a quick Google turned up this person who found Windows encoding in DHCP: https://stackoverflow.com/questions/31215085/windows-dhcp-client-hostname-encoding

Can the UTF-8 assumption/requirement in this crate be lifted?

This PR adds support for DHCP option 121 (Classless Static Routes) for DHCPv4 as described in RFC3442.

This option allows a DHCP server to send a whole routing table to a client. It is also sometimes used instead of DHCP option 3 (Router) in some environments.

Related to this option, this PR adds support for merging some consecutive DHCP options that contains lists of items, option 121 being a prime example. The previous behavior was to replace the previously parsed option when encountering it multiple times in a packet; however it may be useful to specify options multiple times when they are pretty large (e.g. large routing tables).

Moving the lifetime into Decodable makes it much more restrictive when actually using <'a, T: Decodable<'a>. Opt is an internal type so it doesn't need to implement the trait.

Hello,

It seems right now, the dhcproto does not provide any way to create DhcpOption such as IAAddr, or IANA, from scratch. Structures have private fields, and no getter / setter / new methods. I fixed that in a fork by setting all the structure fields to public ( https://github.com/rgrunbla/dhcproto/commit/3ac5f0419d97acfd64360aa9603e09fcb8cbc4ac ) but that might not be what you want for this crate. Could this be solved upstream ?

Thanks, Rémy

closes #16

Exposed some of the pub fields here, other types I just removed the wrapper for so it looks more like the v4 opts. Before merging I want to take a second to make sure we've got the correct type for VendorClass and UserClass though, the rfc looks like we can't count on it being utf-8 encoded, so I think String is maybe not the correct type to have here. For example, the StatusCode option explicitly says the message field is utf-8, so I think that still needs to change...

@rgrunbla any other thoughts on this?

I have a number of optimizations for you too, but I'd like these to be accepted first so we can see a before-and-after.

The coverage isn't spectacular, but these benchmarks are enough to point out some pretty clear opportunities.

(I'm /u/Saefroch on Reddit)

Hello,

This nice looking library has nearly no examples. In documentation comment I found:

use dhcproto::v4::{Decodable, Decoder, Encodable, Encoder, Message};
fn main() -> Result<(), Box<dyn std::error::Error>> {
    // decode
    let bytes = dhcp_offer();
    let msg = Message::decode(&mut Decoder::new(&bytes))?;
    println!("{:#?}", msg);
    // now encode
    let mut buf = Vec::new();
    let mut e = Encoder::new(&mut buf);
    msg.encode(&mut e)?;
    Ok(())
}
fn dhcp_offer() -> Vec<u8> {
    vec![
        0x02, 0x01, 0x06, 0x00, 0x00, 0x00, 0x15, 0x5c, 0x00, 0x00, 0x80, 0x00, 0x00, 0x00, 0x00,
        0x00, 0xc0, 0xa8, 0x00, 0x03, ..... 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    ]
}

Thing I looking for is how to create ( how to encode ) byte content for a DHCP packet such a DHCP Discover.

Seeing output as

Message {
    opcode: BootReply,
    htype: Eth,
    hlen: 6,
    hops: 0,
    xid: 5468,
    secs: 0,
    flags: Flags {
        broadcast: true,
    },
    ciaddr: 0.0.0.0,
    yiaddr: 192.168.0.3,
    siaddr: 0.0.0.0,
    giaddr: 0.0.0.0,
    chaddr: [
        204,
        0,
        10,
        196,
        0,
        0,
        0,
        0,
        0,
        0,
        0,
        0,
        0,
        0,
        0,
        0,
    ],
    sname: None,
    file: None,
    magic: [
        99,
        130,
        83,
        99,
    ],
    opts: DhcpOptions(
        {
            Router: Router(
                [
                    192.168.0.1,
                ],
            ),
            MessageType: MessageType(
                Offer,
            ),
            Rebinding: Rebinding(
                52,
            ),
            DomainNameServer: DomainNameServer(
                [
                    192.168.0.1,
                    192.168.1.1,
                ],
            ),
            AddressLeaseTime: AddressLeaseTime(
                60,
            ),
            SubnetMask: SubnetMask(
                255.255.255.0,
            ),
            Renewal: Renewal(
                30,
            ),
            ServerIdentifier: ServerIdentifier(
                192.168.0.1,
            ),
        },
    ),
}

does make me think there is a struct involved.

At https://docs.rs/dhcproto/0.2.1/dhcproto/encoder/struct.Encoder.html is even said

pub struct Encoder<'a> { /* fields omitted */ }

but the text

Encoder type, holds a mut ref to a buffer that it will write data to and an offset of the next position to write

with especial an offset of the next position to write makes me think there is some serialiazation, that I must be carefull in which sequence what is set.

Hence my wish: Have usage examples

Had initially kept the bound on decode fn, but it's strictly less expressive because you can't impl Decodable on types that contain references (like the internal Opt<'a>). Realized that HRTBs will help when using the type generically, so dropped an example in there.

I waffled on this a bit, master has removed then added the lifetime bound in the trait recently. Although none of those changes have actually been published.

Current set_chaddr() function expects a [u8;16] slice due to requirement of copy_from_slice(). This cause the hlen been always set to 16.

This patch fix it by allowing variable length of input u8 slice and set hlen accordingly.

Tested in my person project with dnsmasq:

• Before: DHCPREQUEST(eth1.ep) 192.0.2.32 12:a9:63:e8:f0:b8:00:00:00:00:00:00:00:00:00:00

• After: DHCPREQUEST(eth1.ep) 192.0.2.32 12:a9:63:e8:f0:b8

closes #2

We could obviously use more docs, but this gets us started with some examples.

We were missing some public constructor methods for v6::Message, added those also/clarify which bytes get copied for xid.

Hey there! Amazing work with this crate :D

Since read_ipv4 will refuse any length different than four, I don't see why we couldn't use the const generics-backed read function

https://www.rfc-editor.org/rfc/rfc4361

allows a v6-style DUID to be encoded in the client id option (opt 61). We should probably change the ClientId option so that it is easier to pull out the DUID specified here: https://www.rfc-editor.org/rfc/rfc4361#section-6.1

maybe:

enum ClientId {
    Duid(Duid),
    Id(Id)
}
struct Duid {
   iaid: [u8; 4],
   duid: Vec<u8>
}
struct Id {
   id: Vec<u8>
}

With some convenience methods on ClientId to pull out either id or duid if a method is called.

I've been trying to actually use the Domain type for a few days and it has shortcomings. It's difficult to call some methods. I wonder if it isn't just better to re-export the trust_dns_proto Name type and the error type and let users call the methods directly.

trust-dns conditionally impls serde also so that is covered.

We need a more robust way to handle vendor specific information from https://www.rfc-editor.org/rfc/rfc2132#section-8.4

Right now dhcproto just stores the section as a big block of bytes, there is no way to pull out sub-options of vendor info, that's left as an exercise to the user.

I'm happy to do this, but I thought someone might want to take a crack at it or have thoughts. Here are the kea docs on it, the "notes" section at the bottom of this is helpful https://kea.readthedocs.io/en/kea-1.6.2/arm/dhcp4-srv.html#dhcpv4-private-options

I was thinking we could represent the vendor specific info as HashMap<VendorCode, UnknownOption>. where VendorCode I think would only have the variants Pad End & Unknown. Having the key literally just be u8 may also make sense.

We have a bit of an API issue with the UnknownOption

The fact that this doesn't work, is perhaps a little unexpected:

        let mut opts = DhcpOptions::new();
        opts.insert(DhcpOption::Unknown(UnknownOption {
            code: 1,
            data: vec![192, 168, 1, 1],
        }));
        let opt = opts.get(OptionCode::SubnetMask);
        dbg!(opt); // None

We can modify From<&DhcpOption> for OptionCode' so that the key in the map is a OptionCode::SubnetMask by changing the unknown branch from:

Unknown(n) => OptionCode::Unknown(n.code),

to

Unknown(n) => n.code.into(),

This way you don't need to retrieve the option value with OptionCode::Unknown(1) which feels pretty weird. But the value you will get back after making the above change for the original code will be:

[src/v4/options.rs:1950] opt = Some(
    Unknown(
        UnknownOption {
            code: 1,
            data: [
                192,
                168,
                1,
                1,
            ],
        },
    ),
)

Again, this feels a little strange. The Unknown option could have been inserted programmatically and unbeknownst to the programmer, if let Some(DhcpOption::SubnetMask(_)) = opts.get(OptionCode::SubnetMask) will not work.

We could decode UnknownOption on insert but that feels pretty icky and would likely require re-allocated to prefix the code u8 at the beginning of the data section.

If anyone has some better ideas, feel free to share

edit:

Another option, we could use the repr(u8) for OptionCode and store the keys as the u8 value. That would solve half of this problem. We may want to do this as part of more general option generating macros described in #43

Currently, new DHCP Options need to be added in 7 different places:

1. Added to the OptionCode enum
2. From<u8> for OptionCode
3. From<OptionCode> for u8
4. Added to the DhcpOption enum
5. decode_inner
6. and encode
7. From<&DhcpOption> for OptionCode

Aside from the extra typing, it increases the risk of human error and adds visual clutter. Would you consider a PR that replaces these 7 steps (or a subsection of them) with a proc-macro?

I have a small prototype that looks like

/// implements step 1 to 4 given sets of
/// {numeric_code, TypeName, "extra doc comment", (DataType)[optional] }
macros::declare_codes!(
    {0, Pad, "Padding"},
    {1,  SubnetMask, "Subnet Mask", (Ipv4Addr)},
    {2,  TimeOffset, "Time Offset", (i32)},
    {3,  Router, "Router", (Vec<Ipv4Addr>)},
    ///...
    {255, End, "end-of-list marker"}
);

That I'd be willing to PR after some small tweaks, if desired :)

Adds support for 3 TFTP related options (TFTServerName, TFTPServerAdress, and BootfileName) which can be used to boot devices from the network.

The capitalization of BootfileName follows the RFC and IANA spelling, it is different from the existing BootFileSize but that is consistent with the spec 😅