[uclibc-ng-devel] Re: (fwd) Fwd: [ICS-VU-638779, CERT/CC VU#473698] Predictable DNS Transaction IDs in uClibc, uClibc-ng [labs-advisory@nozominetworks.com]

Hi All,

As mentioned in my previous message, attached is a patch which uses /dev/urandom to generate a random id for dns query id field. However if it is not able to access urandom file, then it falls back to the old logic of simple incrementing counter. Have done some minimal test on my linux system and it seems to be working fine.

Hi,

Do note that the local_id passed to dnsrand_next requires pre increment for simple counter based fallback to work. 

ie ++local_id rather than local_id++

Hi,

On Wed, May 04, 2022 at 05:31:55PM -0000, hanishkvc@gmail.com wrote:
> Hi All,
> 
> As mentioned in my previous message, attached is a patch which uses /dev/urandom
> to generate a random id for dns query id field. However if it is not able to access
> urandom file, then it falls back to the old logic of simple incrementing counter.
> Have done some minimal test on my linux system and it seems to be working fine.

I've took a look at how to fix this yesterday and maybe this can be
fixed the same way musl does, from src/network/res_mkquery.c:

    /* Make a reasonably unpredictable id */
    clock_gettime(CLOCK_REALTIME, &ts);
    id = ts.tv_nsec + ts.tv_nsec/65536UL & 0xffff;

My 2cent

> diff --git a/libc/inet/resolv.c b/libc/inet/resolv.c
> index 92a65d0dc..c37f6fab7 100644
> --- a/libc/inet/resolv.c
> +++ b/libc/inet/resolv.c
> @@ -255,6 +255,7 @@ Domain name in a message can be represented as either:
>  #include <sys/stat.h>
>  #include <sys/param.h>
>  #include <bits/uClibc_mutex.h>
> +#include <fcntl.h>
>  #include "internal/parse_config.h"
>  
>  /* poll() is not supported in kernel <= 2.0, therefore if __NR_poll is
> @@ -1045,6 +1046,20 @@ static int __decode_answer(const unsigned char *message, /* packet */
>  	return i + RRFIXEDSZ + a->rdlength;
>  }
>  
> +
> +uint16_t dnsrand_next(int urand_fd, int def_value) {
> +	if (urand_fd == -1) return def_value;
> +	uint16_t val;
> +	if(read(urand_fd, &val, 2) != 2) return def_value;
> +	return val;
> +}
> +
> +int dnsrand_setup(int *urand_fd, int def_value) {
> +	*urand_fd = open("/dev/urandom", O_RDONLY);
> +	if (*urand_fd == -1) return def_value;
> +	return dnsrand_next(*urand_fd, def_value);
> +}
> +
>  /* On entry:
>   *  a.buf(len) = auxiliary buffer for IP addresses after first one
>   *  a.add_count = how many additional addresses are there already
> @@ -1070,6 +1085,7 @@ int __dns_lookup(const char *name,
>  	/* Protected by __resolv_lock: */
>  	static int last_ns_num = 0;
>  	static uint16_t last_id = 1;
> +	static int urand_fd = -1;
>  
>  	int i, j, fd, rc;
>  	int packet_len;
> @@ -1149,7 +1165,7 @@ int __dns_lookup(const char *name,
>  		}
>  		/* first time? pick starting server etc */
>  		if (local_ns_num < 0) {
> -			local_id = last_id;
> +			local_id = dnsrand_setup(&urand_fd, last_id);
>  /*TODO: implement /etc/resolv.conf's "options rotate"
>   (a.k.a. RES_ROTATE bit in _res.options)
>  			local_ns_num = 0;
> @@ -1159,8 +1175,9 @@ int __dns_lookup(const char *name,
>  		}
>  		if (local_ns_num >= __nameservers)
>  			local_ns_num = 0;
> -		local_id++;
> +		local_id = dnsrand_next(urand_fd, local_id++);
>  		local_id &= 0xffff;
> +		DPRINTF("uClibc:DBUG:local_id:0x%hx\n", local_id);
>  		/* write new values back while still under lock */
>  		last_id = local_id;
>  		last_ns_num = local_ns_num;

Hi,

I agree with you partly, but given that on embedded targets realtime clock is a optional feature enabled/available or otherwise, one needs to provide multiple options, do have a look at the newer patch I had released earlier,

It provides options for either 

a) retaining the simple counter logic as existing logic OR

b) use urandom if available (one may have many embedded systems with linux or bsds or so with urandom but not necessarily realtime clock).

c) else try realtime clock if available,

d) if nothing else just use a ever running prng.

In the case of urandom or clock it will be used to seed the prng periodically.

On Thu, May 05, 2022 at 04:46:06PM -0000, hanishkvc@gmail.com wrote:
> Hi,
> 
> I agree with you partly, but given that on embedded targets realtime clock is a optional feature enabled/available or otherwise, one needs to provide multiple options, do have a look at the newer patch I had released earlier,
Yes, I agree, I've expected clock not being availble but I didn't knew it was optional.
(I didn't looked at your patch yet, I've just subscribed)

> 
> It provides options for either 
> 
> a) retaining the simple counter logic as existing logic OR
> 
> b) use urandom if available (one may have many embedded systems with linux or bsds or so with urandom but not necessarily realtime clock).
> 
> c) else try realtime clock if available,
> 
> d) if nothing else just use a ever running prng.
> 
> In the case of urandom or clock it will be used to seed the prng periodically.
>  
Sounds resonable!

Cheers