[V2] resolv: use arc4random_uniform in the generation of random ids
Checks
Commit Message
In the context of DNS it is important to have highest possible
quality ids.
v2: fix off by one calculation of upper limit, suggested by Florian.
Signed-off-by: Cristian Rodríguez <crrodriguez@opensuse.org>
---
resolv/res_mkquery.c | 3 +--
resolv/res_randomid.c | 4 ++--
2 files changed, 3 insertions(+), 4 deletions(-)
Comments
On Fri, Jul 22, 2022 at 5:35 PM Cristian Rodríguez via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> In the context of DNS it is important to have highest possible
> quality ids.
>
> v2: fix off by one calculation of upper limit, suggested by Florian.
>
> Signed-off-by: Cristian Rodríguez <crrodriguez@opensuse.org>
This fails 32-bit pre-commit CI:
https://patchwork.sourceware.org/project/glibc/patch/20220722213501.32506-1-crrodriguez@opensuse.org/
Please review the CI results.
The link namespace results look like a real problem, we need an internal alias.
The mtrace leaks need review too.
> ---
> resolv/res_mkquery.c | 3 +--
> resolv/res_randomid.c | 4 ++--
> 2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/resolv/res_mkquery.c b/resolv/res_mkquery.c
> index 5bc5b41531..066ee2c97e 100644
> --- a/resolv/res_mkquery.c
> +++ b/resolv/res_mkquery.c
> @@ -93,7 +93,6 @@
> #include <string.h>
> #include <sys/time.h>
> #include <shlib-compat.h>
> -#include <random-bits.h>
>
> int
> __res_context_mkquery (struct resolv_context *ctx, int op, const char *dname,
> @@ -116,7 +115,7 @@ __res_context_mkquery (struct resolv_context *ctx, int op, const char *dname,
> /* We randomize the IDs every time. The old code just incremented
> by one after the initial randomization which still predictable if
> the application does multiple requests. */
> - hp->id = random_bits ();
> + hp->id = res_randomid ();
> hp->opcode = op;
> if (ctx->resp->options & RES_TRUSTAD)
> hp->ad = 1;
> diff --git a/resolv/res_randomid.c b/resolv/res_randomid.c
> index fb1fa17539..b6211686c0 100644
> --- a/resolv/res_randomid.c
> +++ b/resolv/res_randomid.c
> @@ -84,10 +84,10 @@
>
> #include <resolv.h>
> #include <unistd.h>
> -#include <random-bits.h>
> +#include <stdlib.h>
>
> unsigned int
> res_randomid (void) {
> - return 0xffff & random_bits ();
> + return arc4random_uniform (UINT16_MAX + 1);
> }
> libc_hidden_def (__res_randomid)
> --
> 2.37.1
>
On Sun, Jul 24, 2022 at 10:12 AM Carlos O'Donell
<carlos@systemhalted.org> wrote:
>
> On Fri, Jul 22, 2022 at 5:35 PM Cristian Rodríguez via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
> >
> > In the context of DNS it is important to have highest possible
> > quality ids.
> >
> > v2: fix off by one calculation of upper limit, suggested by Florian.
> >
> > Signed-off-by: Cristian Rodríguez <crrodriguez@opensuse.org>
>
> This fails 32-bit pre-commit CI:
> https://patchwork.sourceware.org/project/glibc/patch/20220722213501.32506-1-crrodriguez@opensuse.org/
>
> Please review the CI results.
>
> The link namespace results look like a real problem, we need an internal alias.
>
> The mtrace leaks need review too.
Until the dust settles on the arc4random patches I won't be pushing
for these changes again
Cheers..
@@ -93,7 +93,6 @@
#include <string.h>
#include <sys/time.h>
#include <shlib-compat.h>
-#include <random-bits.h>
int
__res_context_mkquery (struct resolv_context *ctx, int op, const char *dname,
@@ -116,7 +115,7 @@ __res_context_mkquery (struct resolv_context *ctx, int op, const char *dname,
/* We randomize the IDs every time. The old code just incremented
by one after the initial randomization which still predictable if
the application does multiple requests. */
- hp->id = random_bits ();
+ hp->id = res_randomid ();
hp->opcode = op;
if (ctx->resp->options & RES_TRUSTAD)
hp->ad = 1;
@@ -84,10 +84,10 @@
#include <resolv.h>
#include <unistd.h>
-#include <random-bits.h>
+#include <stdlib.h>
unsigned int
res_randomid (void) {
- return 0xffff & random_bits ();
+ return arc4random_uniform (UINT16_MAX + 1);
}
libc_hidden_def (__res_randomid)