KUMAAN
2011-Jul-18 16:15 UTC
[klibc] ipconfig:About the length of 'options' field of DHCP packet
Hi, I had the problem that 'ipconfig' as DHCP client in Debian-squeeze could not get DHCP Offer reply from the built-in DHCP server of the cheap route that I have. The name of the router is 'MegaBitGear TE4571E' which I got at contract of ADSL Internet access service in 2009. The router is not too old and can reply to other DHCP clients like ISC-dhclient, WindowsXP and Vista. This is why I tried to change the 'ipconfig' code a bit paying attention to compatibility with now code, other clients and RFC. Concretely, I added the function to 'ipconfig' that append padding to the 'options' field of DHCP Discover and Request packet like ISC-dhclient. This function is enabled if 'ipconfig' fail to get DHCP Offer reply, retry to send DHCP Discover and the length of the 'options' field is shorter than 64 octets. I inspected the octet length of 'options' field of DHCP Discover and Request packets which other DHCP clients in default settings send by using Wireshark. 'p' means that padding is included. Dis. Req. Client 35 47 ipconfig(master of git) 64p 64p isc-dhclient(4.1.1-P1 in Debian-squeeze) 64p 73 WindowsXP 32bit(my x86 PC) 64p 76 WindowsVista 64bit(my amd64 PC) I referred to RFC2131, RFC1541 and RFC951 for the length of 'options' (called 'vend' in BOOTP) field of DHCP Discover and Request packet. Basing on RFC, if the length is ... 64 octets: BOOP server which may be able to interpret DHCP too and DHCP server which can interpret 'options' of variable length will reply. 312 octets: DHCP server which can not interpret 'options' of variable length and DHCP server which can interpret 'options' of variable length will reply. the others: DHCP server which can interpret 'options' of variable length will reply. The above made me think that appending padding might increase the probability of getting DHCP Offer reply. Of course appending padding made my cheap router send DHCP Offer reply. If you will consider whether this change like the following patch is worth or not, I'm happy. P.S. Thank you for reading this mail in broken English. -- KUMAAN 9maaan at gmail.com -- $ git format-patch HEAD^ -->From cb6c9c38207ea24df43680282cb1036b78b4c16f Mon Sep 17 00:00:00 2001From: KUMAAN <9maaan at gmail.com> Date: Tue, 19 Jul 2011 00:12:55 +0900 Subject: [PATCH] ipconfig:append padding to DHCP options when fail to get reply and retry. This patch add the function to ipconfig that append padding to the 'options' field of DHCP Discover and Request packet like ISC-dhclient. This function is enabled if ipconfig fail to get DHCP Offer reply, retry to send DHCP Discover and the length of the 'options' field is shorter than 64 octets. --- usr/kinit/ipconfig/bootp_proto.c | 6 ++++++ usr/kinit/ipconfig/dhcp_proto.c | 24 +++++++++++++++++++++++- usr/kinit/ipconfig/main.c | 6 ++++++ usr/kinit/ipconfig/netdev.h | 1 + 4 files changed, 36 insertions(+), 1 deletions(-) diff --git a/usr/kinit/ipconfig/bootp_proto.c b/usr/kinit/ipconfig/bootp_proto.c index f2cc90c..19c61ef 100644 --- a/usr/kinit/ipconfig/bootp_proto.c +++ b/usr/kinit/ipconfig/bootp_proto.c @@ -212,5 +212,11 @@ int bootp_init_if(struct netdev *dev) dev->bootp.xid = (uint32_t) lrand48(); dev->open_time = time(NULL); + /* + * 'process_timeout_event(main.c)' will increment 'fail_count' + * only once independent of a failure. + */ + dev->bootp.fail_count = ~0; + return 0; } diff --git a/usr/kinit/ipconfig/dhcp_proto.c b/usr/kinit/ipconfig/dhcp_proto.c index afd2eca..91dd695 100644 --- a/usr/kinit/ipconfig/dhcp_proto.c +++ b/usr/kinit/ipconfig/dhcp_proto.c @@ -49,7 +49,7 @@ static uint8_t dhcp_end[] = { /* Both iovecs below have to have the same structure, since dhcp_send() pokes at the internals */ -#define DHCP_IOV_LEN 7 +#define DHCP_IOV_LEN 8 static struct iovec dhcp_discover_iov[DHCP_IOV_LEN] = { /* [0] = ip + udp header */ @@ -59,6 +59,7 @@ static struct iovec dhcp_discover_iov[DHCP_IOV_LEN] = { /* [4] = optional vendor class */ /* [5] = optional hostname */ /* [6] = {dhcp_end, sizeof(dhcp_end)} */ + /* [7] = optional padding */ }; static struct iovec dhcp_request_iov[DHCP_IOV_LEN] = { @@ -69,6 +70,7 @@ static struct iovec dhcp_request_iov[DHCP_IOV_LEN] = { /* [4] = optional vendor class */ /* [5] = optional hostname */ /* [6] = {dhcp_end, sizeof(dhcp_end)} */ + /* [7] = optional padding */ }; /* @@ -167,6 +169,7 @@ static int dhcp_send(struct netdev *dev, struct iovec *vec) { struct bootp_hdr bootp; char dhcp_hostname[SYS_NMLN+2]; + uint8_t options_padding[64]; int i = 4; memset(&bootp, 0, sizeof(struct bootp_hdr)); @@ -212,6 +215,25 @@ static int dhcp_send(struct netdev *dev, struct iovec *vec) vec[i].iov_base = dhcp_end; vec[i].iov_len = sizeof(dhcp_end); + + if(dev->bootp.fail_count & 1){ + /* + * Append padding if the length of options field is + * shorter than 64 octets. + */ + int options_padding_len = 64; + int j; + for (j=2; j<=i; j++){ + options_padding_len -= vec[j].iov_len; + } + if(options_padding_len > 0){ + memset(&options_padding, 0, options_padding_len); + i++; + vec[i].iov_base = options_padding; + vec[i].iov_len = options_padding_len; + } + } + return packet_send(dev, vec, i + 1); } diff --git a/usr/kinit/ipconfig/main.c b/usr/kinit/ipconfig/main.c index 8782ae7..b65dd50 100644 --- a/usr/kinit/ipconfig/main.c +++ b/usr/kinit/ipconfig/main.c @@ -297,6 +297,12 @@ static void process_timeout_event(struct state *s, time_t now) s->state = s->restart_state; /* + * Except the first arrival, an arrival of this line means + * that a failure like a timeout or an error occurred. + */ + s->dev->bootp.fail_count++; + + /* * Now send a packet depending on our state. */ switch (s->state) { diff --git a/usr/kinit/ipconfig/netdev.h b/usr/kinit/ipconfig/netdev.h index 26d076a..d3b144e 100644 --- a/usr/kinit/ipconfig/netdev.h +++ b/usr/kinit/ipconfig/netdev.h @@ -22,6 +22,7 @@ struct netdev { int fd; uint32_t xid; uint32_t gateway; /* BOOTP/DHCP gateway */ + uint8_t fail_count; /* of DHCP Discover or BOOTP/DHCP Request */ } bootp; struct { /* RARP information */ -- 1.7.2.5
maximilian attems
2011-Jul-31 09:07 UTC
[klibc] ipconfig:About the length of 'options' field of DHCP packet
hello, On Tue, 19 Jul 2011, KUMAAN wrote:> > I had the problem that 'ipconfig' as DHCP client in Debian-squeeze could not get > DHCP Offer reply from the built-in DHCP server of the cheap route that I have. > The name of the router is 'MegaBitGear TE4571E' which > I got at contract of ADSL Internet access service in 2009. > The router is not too old and can reply to other > DHCP clients like ISC-dhclient, WindowsXP and Vista. > > This is why I tried to change the 'ipconfig' code a bit > paying attention to compatibility with now code, other clients and RFC. > Concretely, I added the function to 'ipconfig' that append padding to > the 'options' field of DHCP Discover and Request packet like ISC-dhclient. > This function is enabled if 'ipconfig' fail to get DHCP Offer reply, > retry to send DHCP Discover and the length of the 'options' field is > shorter than 64 octets. > > I inspected the octet length of 'options' field of DHCP Discover and Request packets > which other DHCP clients in default settings send by using Wireshark. > 'p' means that padding is included. > > Dis. Req. Client > 35 47 ipconfig(master of git) > 64p 64p isc-dhclient(4.1.1-P1 in Debian-squeeze) > 64p 73 WindowsXP 32bit(my x86 PC) > 64p 76 WindowsVista 64bit(my amd64 PC)thank you for the analysis.> I referred to RFC2131, RFC1541 and RFC951 for the length of 'options' > (called 'vend' in BOOTP) field of DHCP Discover and Request packet. > Basing on RFC, if the length is ... > 64 octets: > BOOP server which may be able to interpret DHCP too and > DHCP server which can interpret 'options' of variable length will reply. > 312 octets: > DHCP server which can not interpret 'options' of variable length and > DHCP server which can interpret 'options' of variable length will reply. > the others: > DHCP server which can interpret 'options' of variable length will reply. > > The above made me think that appending padding > might increase the probability of getting DHCP Offer reply. > Of course appending padding made my cheap router send DHCP Offer reply. > > > If you will consider whether this change like the following patch > is worth or not, I'm happy.Hmm first I think we should implement missing options in ipconfig: domain-search and lease time are the ones that are asked for. http://bugs.debian.org/627166 If then the padding is still to small, it should be fixed. In the ISC dhclient I see a BOOTP_MIN_LEN defined to 300. Without checking with current implementation I'd guess we'd fall below that?> P.S. Thank you for reading this mail in broken English.In fact your posting was very clear, sorry for coming back only now, but had been away for a ten day remote vacations and mailinglist had a bunch of patches to review and/or merge.> -- $ git format-patch HEAD^ -- > >From cb6c9c38207ea24df43680282cb1036b78b4c16f Mon Sep 17 00:00:00 2001 > From: KUMAAN <9maaan at gmail.com> > Date: Tue, 19 Jul 2011 00:12:55 +0900 > Subject: [PATCH] ipconfig:append padding to DHCP options when fail to get reply and retry. > > This patch add the function to ipconfig that append padding to > the 'options' field of DHCP Discover and Request packet like ISC-dhclient. > This function is enabled if ipconfig fail to get DHCP Offer reply, > retry to send DHCP Discover and the length of the 'options' field is > shorter than 64 octets.the patch by itself has minor Codingstyle errors, which can be seen when checked against the scripts/checkpatch.pl in linux source. (trailing spaces, missing spaces around operators like `=' or ifs) My main grieve is that we should do it right on the first try and not only later on.> --- > usr/kinit/ipconfig/bootp_proto.c | 6 ++++++ > usr/kinit/ipconfig/dhcp_proto.c | 24 +++++++++++++++++++++++- > usr/kinit/ipconfig/main.c | 6 ++++++ > usr/kinit/ipconfig/netdev.h | 1 + > 4 files changed, 36 insertions(+), 1 deletions(-) > > diff --git a/usr/kinit/ipconfig/bootp_proto.c b/usr/kinit/ipconfig/bootp_proto.c > index f2cc90c..19c61ef 100644 > --- a/usr/kinit/ipconfig/bootp_proto.c > +++ b/usr/kinit/ipconfig/bootp_proto.c > @@ -212,5 +212,11 @@ int bootp_init_if(struct netdev *dev) > dev->bootp.xid = (uint32_t) lrand48(); > dev->open_time = time(NULL); > > + /* > + * 'process_timeout_event(main.c)' will increment 'fail_count' > + * only once independent of a failure. > + */ > + dev->bootp.fail_count = ~0; > + > return 0; > } > diff --git a/usr/kinit/ipconfig/dhcp_proto.c b/usr/kinit/ipconfig/dhcp_proto.c > index afd2eca..91dd695 100644 > --- a/usr/kinit/ipconfig/dhcp_proto.c > +++ b/usr/kinit/ipconfig/dhcp_proto.c > @@ -49,7 +49,7 @@ static uint8_t dhcp_end[] = { > > /* Both iovecs below have to have the same structure, since dhcp_send() > pokes at the internals */ > -#define DHCP_IOV_LEN 7 > +#define DHCP_IOV_LEN 8 > > static struct iovec dhcp_discover_iov[DHCP_IOV_LEN] = { > /* [0] = ip + udp header */ > @@ -59,6 +59,7 @@ static struct iovec dhcp_discover_iov[DHCP_IOV_LEN] = { > /* [4] = optional vendor class */ > /* [5] = optional hostname */ > /* [6] = {dhcp_end, sizeof(dhcp_end)} */ > + /* [7] = optional padding */ > }; > > static struct iovec dhcp_request_iov[DHCP_IOV_LEN] = { > @@ -69,6 +70,7 @@ static struct iovec dhcp_request_iov[DHCP_IOV_LEN] = { > /* [4] = optional vendor class */ > /* [5] = optional hostname */ > /* [6] = {dhcp_end, sizeof(dhcp_end)} */ > + /* [7] = optional padding */ > }; > > /* > @@ -167,6 +169,7 @@ static int dhcp_send(struct netdev *dev, struct iovec *vec) > { > struct bootp_hdr bootp; > char dhcp_hostname[SYS_NMLN+2]; > + uint8_t options_padding[64]; > int i = 4; > > memset(&bootp, 0, sizeof(struct bootp_hdr)); > @@ -212,6 +215,25 @@ static int dhcp_send(struct netdev *dev, struct iovec *vec) > vec[i].iov_base = dhcp_end; > vec[i].iov_len = sizeof(dhcp_end); > > + > + if(dev->bootp.fail_count & 1){ > + /* > + * Append padding if the length of options field is > + * shorter than 64 octets. > + */ > + int options_padding_len = 64; > + int j; > + for (j=2; j<=i; j++){ > + options_padding_len -= vec[j].iov_len; > + } > + if(options_padding_len > 0){ > + memset(&options_padding, 0, options_padding_len); > + i++; > + vec[i].iov_base = options_padding; > + vec[i].iov_len = options_padding_len; > + } > + } > + > return packet_send(dev, vec, i + 1); > } > > diff --git a/usr/kinit/ipconfig/main.c b/usr/kinit/ipconfig/main.c > index 8782ae7..b65dd50 100644 > --- a/usr/kinit/ipconfig/main.c > +++ b/usr/kinit/ipconfig/main.c > @@ -297,6 +297,12 @@ static void process_timeout_event(struct state *s, time_t now) > s->state = s->restart_state; > > /* > + * Except the first arrival, an arrival of this line means > + * that a failure like a timeout or an error occurred. > + */ > + s->dev->bootp.fail_count++; > + > + /* > * Now send a packet depending on our state. > */ > switch (s->state) { > diff --git a/usr/kinit/ipconfig/netdev.h b/usr/kinit/ipconfig/netdev.h > index 26d076a..d3b144e 100644 > --- a/usr/kinit/ipconfig/netdev.h > +++ b/usr/kinit/ipconfig/netdev.h > @@ -22,6 +22,7 @@ struct netdev { > int fd; > uint32_t xid; > uint32_t gateway; /* BOOTP/DHCP gateway */ > + uint8_t fail_count; /* of DHCP Discover or BOOTP/DHCP Request */ > } bootp; > > struct { /* RARP information */ > -- > 1.7.2.5would you mind giving the asked options a try? and if the length then is too small correct the bootp packet length? thank you. -- maks
maximilian attems
2012-May-19 17:47 UTC
[klibc] [PATCH 6/6] ipconfig: A bit more robust bootp/dhcp option parsing
On Tue, 23 Aug 2011, KUMAAN wrote:> Be a bit more strict about our BOOTP/DHCP option parsing to avoid > segmentation faults. > > Signed-off-by: KUMAAN <9maaan at gmail.com> > --- > usr/kinit/ipconfig/bootp_proto.c | 4 ++++ > usr/kinit/ipconfig/dhcp_proto.c | 33 ++++++++++++++++++++++++--------- > 2 files changed, 28 insertions(+), 9 deletions(-) >thank you for your patience! applied to latest git.