Re: [Dnsmasq-discuss] [PATCH] DHCPv6 IAID should be of unsigned type

[Simon Kelley]

On 22/10/2019 20:41, Roy Marples wrote:
> On 22/10/2019 17:17, Normen Kowalewski wrote:
>> FAIW - i was curious to see if RFC 8415 of November 2018, the update
>> of the now officially obsoleted RFC 3315, uses some other wording, but
>> it also just speaks about 4 octets that jointly are an unsigned integer
>> https://tools.ietf.org/html/rfc8415#section-21.21
> 
> https://tools.ietf.org/html/rfc8415#section-8
> 
>    All values in the message header and in options are in network byte
>    order.
> 
> Pretty clear in my view.
> 
>> The RFC obviously expects an implementor to know which data type in
>> his specific environment and implementation would match this
>> requirement - obviously for example it would be good if the on the
>> wire format would not change by big endian/little endian diversions
>> and so on...
> 
> No, the RFC (unless otherwise noted) expects all unsigned integers to be
> used in ntohl(3) and htonl(3) (or equivalents thereof depending on
> platform and integer size) to ensure endian does not affect on wire format.
> 

In the dnsmasq case opt6_uint()  and put_opt6_long do the byte-order
swaps as the data in copied from/to packet data, so everywhere in the
code it's in host order. Not that is matters much as it's pretty much
treated as an opaque type, except where it's stored in the leasefile as
ascii number, where it's printed and read as unsigned.

I  think it gets logged somewhere too.


Simon.

> Roy
> 
> ___
> Dnsmasq-discuss mailing list
> Dnsmasq-discuss@lists.thekelleys.org.uk
> http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
> 


___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss

Re: [Dnsmasq-discuss] [PATCH] DHCPv6 IAID should be of unsigned type

[Roy Marples]

On 22/10/2019 17:17, Normen Kowalewski wrote:
FAIW - i was curious to see if RFC 8415 of November 2018, the update of 
the now officially obsoleted RFC 3315, uses some other wording, but it 
also just speaks about 4 octets that jointly are an unsigned integer

https://tools.ietf.org/html/rfc8415#section-21.21


https://tools.ietf.org/html/rfc8415#section-8

   All values in the message header and in options are in network byte
   order.

Pretty clear in my view.

The RFC obviously expects an implementor to know which data type in his 
specific environment and implementation would match this requirement - 
obviously for example it would be good if the on the wire format would 
not change by big endian/little endian diversions and so on...


No, the RFC (unless otherwise noted) expects all unsigned integers to be 
used in ntohl(3) and htonl(3) (or equivalents thereof depending on 
platform and integer size) to ensure endian does not affect on wire format.


Roy

___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss

Re: [Dnsmasq-discuss] [PATCH] DHCPv6 IAID should be of unsigned type

[Normen Kowalewski]

Hi Geert, Dominik,

FAIW - i was curious to see if RFC 8415 of November 2018, the update of the now 
officially obsoleted RFC 3315, uses some other wording, but it also just speaks 
about 4 octets that jointly are an unsigned integer
https://tools.ietf.org/html/rfc8415#section-21.21 


The RFC obviously expects an implementor to know which data type in his 
specific environment and implementation would match this requirement - 
obviously for example it would be good if the on the wire format would not 
change by big endian/little endian diversions and so on...

BR, 

Normen

> On 20. Oct 2019, at 20:24, Geert Stappers  wrote:
> 
> On Sun, Oct 20, 2019 at 07:19:13PM +0200, Dominik DL6ER wrote:
>> Dear mailing list,
>> 
>> The proposed patch ensures that the DHCPv6 IAID is of unsigned type.
>> This is entirely uncritical, however, as the variable is already now
>> interpreted and handled as being of unsigned type in
>> * lease.c:read_leases(),
>> * helper.c:create_helper(),
>> * dbus.c:dbus_add_lease(), and
>> * outpacket.c:put_opt6_long(),
>> its definition should reflect this to avoid inconsistencies.
>> 
>> RFC3315 (section 22.4) confirms that the IAID is a 4 bytes long
>> unsigned integer.
>> 
>> Best,
>> Dominik
> 
>> From 93490e98789bf91d86d46e96c643feea4a08e387 Mon Sep 17 00:00:00 2001
>> From: Dominik DL6ER mailto:dl...@dl6er.de>>
>> Date: Sun, 20 Oct 2019 18:51:52 +0200
>> Subject: [PATCH] DHCPv6 IAID should be of unsigned type.
>> It is derived from strtoul() in lease.c:read_leases() and already
>> now interpreted as unsigned in helper.c:276 and outpacket.c:put_opt6_long().
>> RFC3315 (section 22.4) shows that the IAID is 4
>> bytes long so we do not need to go up to unsigned long.
>> To: dnsmasq-discuss > >
>> 
>> Signed-off-by: Dominik DL6ER mailto:dl...@dl6er.de>>
>> ---
> 
> But how long is a int?  And that is for which compiler?
> 
> 
> So where the patch is now someting like `s/int iaid/unsigned int iaid/`,
> would I like to see something like `s/int iaid/uint32 iaid/`.
> Because uint32 is always 4 bytes long.
> 
> 
> Cheers
> Geert Stappers
> 
> ___
> Dnsmasq-discuss mailing list
> Dnsmasq-discuss@lists.thekelleys.org.uk 
> 
> http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss 
> 
___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss

Re: [Dnsmasq-discuss] [PATCH] DHCPv6 IAID should be of unsigned type

[Geert Stappers]

On Sun, Oct 20, 2019 at 07:19:13PM +0200, Dominik DL6ER wrote:
> Dear mailing list,
> 
> The proposed patch ensures that the DHCPv6 IAID is of unsigned type.
> This is entirely uncritical, however, as the variable is already now
> interpreted and handled as being of unsigned type in
> * lease.c:read_leases(),
> * helper.c:create_helper(),
> * dbus.c:dbus_add_lease(), and
> * outpacket.c:put_opt6_long(),
> its definition should reflect this to avoid inconsistencies.
> 
> RFC3315 (section 22.4) confirms that the IAID is a 4 bytes long
> unsigned integer.
> 
> Best,
> Dominik

> From 93490e98789bf91d86d46e96c643feea4a08e387 Mon Sep 17 00:00:00 2001
> From: Dominik DL6ER 
> Date: Sun, 20 Oct 2019 18:51:52 +0200
> Subject: [PATCH] DHCPv6 IAID should be of unsigned type.
>  It is derived from strtoul() in lease.c:read_leases() and already
>  now interpreted as unsigned in helper.c:276 and outpacket.c:put_opt6_long().
>  RFC3315 (section 22.4) shows that the IAID is 4
>  bytes long so we do not need to go up to unsigned long.
> To: dnsmasq-discuss 
> 
> Signed-off-by: Dominik DL6ER 
> ---

But how long is a int?  And that is for which compiler?


So where the patch is now someting like `s/int iaid/unsigned int iaid/`,
would I like to see something like `s/int iaid/uint32 iaid/`.
Because uint32 is always 4 bytes long.


Cheers
Geert Stappers

___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss

[Dnsmasq-discuss] [PATCH] DHCPv6 IAID should be of unsigned type

[Dominik DL6ER]

Dear mailing list,

The proposed patch ensures that the DHCPv6 IAID is of unsigned type.
This is entirely uncritical, however, as the variable is already now
interpreted and handled as being of unsigned type in
* lease.c:read_leases(),
* helper.c:create_helper(),
* dbus.c:dbus_add_lease(), and
* outpacket.c:put_opt6_long(),
its definition should reflect this to avoid inconsistencies.

RFC3315 (section 22.4) confirms that the IAID is a 4 bytes long
unsigned integer.

Best,
Dominik
From 93490e98789bf91d86d46e96c643feea4a08e387 Mon Sep 17 00:00:00 2001
From: Dominik DL6ER 
Date: Sun, 20 Oct 2019 18:51:52 +0200
Subject: [PATCH] DHCPv6 IAID should be of unsigned type.
 It is derived from strtoul() in lease.c:read_leases() and already
 now interpreted as unsigned in helper.c:276 and outpacket.c:put_opt6_long().
 RFC3315 (section 22.4) shows that the IAID is 4
 bytes long so we do not need to go up to unsigned long.
To: dnsmasq-discuss 

Signed-off-by: Dominik DL6ER 
---
 src/dhcp6.c   |  2 +-
 src/dnsmasq.h | 11 ++-
 src/helper.c  |  3 ++-
 src/lease.c   |  9 ++---
 src/rfc3315.c |  4 ++--
 5 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/src/dhcp6.c b/src/dhcp6.c
index 5badc46..ce682db 100644
--- a/src/dhcp6.c
+++ b/src/dhcp6.c
@@ -423,7 +423,7 @@ struct dhcp_config *config_find_by_address6(struct dhcp_config *configs, struct
 }
 
 struct dhcp_context *address6_allocate(struct dhcp_context *context,  unsigned char *clid, int clid_len, int temp_addr,
-   int iaid, int serial, struct dhcp_netid *netids, int plain_range, struct in6_addr *ans)   
+   unsigned int iaid, int serial, struct dhcp_netid *netids, int plain_range, struct in6_addr *ans)
 {
   /* Find a free address: exclude anything in use and anything allocated to
  a particular hwaddr/clientid/hostname in our configuration.
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index 149739d..9262381 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -721,7 +721,7 @@ struct dhcp_lease {
   int new_prefixlen; /* and its prefix length */
 #ifdef HAVE_DHCP6
   struct in6_addr addr6;
-  int iaid;
+  unsigned int iaid;
   struct slaac_address {
 struct in6_addr addr;
 time_t ping_time;
@@ -1393,14 +1393,15 @@ struct dhcp_lease *lease4_allocate(struct in_addr addr);
 #ifdef HAVE_DHCP6
 struct dhcp_lease *lease6_allocate(struct in6_addr *addrp, int lease_type);
 struct dhcp_lease *lease6_find(unsigned char *clid, int clid_len, 
-			   int lease_type, int iaid, struct in6_addr *addr);
+			   int lease_type, unsigned int iaid, struct in6_addr *addr);
 void lease6_reset(void);
-struct dhcp_lease *lease6_find_by_client(struct dhcp_lease *first, int lease_type, unsigned char *clid, int clid_len, int iaid);
+struct dhcp_lease *lease6_find_by_client(struct dhcp_lease *first, int lease_type,
+	 unsigned char *clid, int clid_len, unsigned int iaid);
 struct dhcp_lease *lease6_find_by_addr(struct in6_addr *net, int prefix, u64 addr);
 u64 lease_find_max_addr6(struct dhcp_context *context);
 void lease_ping_reply(struct in6_addr *sender, unsigned char *packet, char *interface);
 void lease_update_slaac(time_t now);
-void lease_set_iaid(struct dhcp_lease *lease, int iaid);
+void lease_set_iaid(struct dhcp_lease *lease, unsigned int iaid);
 void lease_make_duid(time_t now);
 #endif
 void lease_set_hwaddr(struct dhcp_lease *lease, const unsigned char *hwaddr,
@@ -1518,7 +1519,7 @@ int get_incoming_mark(union mysockaddr *peer_addr, union all_addr *local_addr,
 void dhcp6_init(void);
 void dhcp6_packet(time_t now);
 struct dhcp_context *address6_allocate(struct dhcp_context *context,  unsigned char *clid, int clid_len, int temp_addr,
-   int iaid, int serial, struct dhcp_netid *netids, int plain_range, struct in6_addr *ans);
+   unsigned int iaid, int serial, struct dhcp_netid *netids, int plain_range, struct in6_addr *ans);
 int config_valid(struct dhcp_config *config, struct dhcp_context *context, struct in6_addr *addr);
 struct dhcp_context *address6_available(struct dhcp_context *context, 
 	struct in6_addr *taddr,
diff --git a/src/helper.c b/src/helper.c
index c392eec..62ac9cf 100644
--- a/src/helper.c
+++ b/src/helper.c
@@ -66,7 +66,8 @@ struct script_data
 #endif
   struct in6_addr addr6;
 #ifdef HAVE_DHCP6
-  int iaid, vendorclass_count;
+  int vendorclass_count;
+  unsigned int iaid;
 #endif
   unsigned char hwaddr[DHCP_CHADDR_MAX];
   char interface[IF_NAMESIZE];
diff --git a/src/lease.c b/src/lease.c
index f14d128..081d90e 100644
--- a/src/lease.c
+++ b/src/lease.c
@@ -635,7 +635,8 @@ struct dhcp_lease *lease_find_by_addr(struct in_addr addr)
 #ifdef HAVE_DHCP6
 /* find address for {CLID, IAID, address} */
 struct dhcp_lease *lease6_find(unsigned char *clid, int clid_len, 
-			   int lease_type, int iaid, struct in6_addr *addr)
+			   int lease_type, unsigned int iaid,
+			   struct in6_addr *addr)
 {
   struct dhcp_lease *lease;
   
@@ -667,7 +668,9 @@ void