mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH] net: dns: Generate and verify transaction ID
@ 2022-05-05 10:08 Jules Maselbas
  2022-05-06 14:58 ` [PATCH] fixup! " Jules Maselbas
  2022-05-06 15:04 ` [PATCH] " Jules Maselbas
  0 siblings, 2 replies; 6+ messages in thread
From: Jules Maselbas @ 2022-05-05 10:08 UTC (permalink / raw)
  To: barebox; +Cc: Jules Maselbas

The transaction ID wasn't verified on received DNS responses, plus the
ID needs to be difficult to predict in order to avoid MitM (man in the
middle) being able to easily forge responses.

Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
---
 net/dns.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/net/dns.c b/net/dns.c
index 78588b96f..9ad316e33 100644
--- a/net/dns.c
+++ b/net/dns.c
@@ -58,6 +58,7 @@ struct header {
 
 static struct net_connection *dns_con;
 static uint64_t dns_timer_start;
+static uin32_t dns_req_id;
 static int dns_state;
 static IPaddr_t dns_ip;
 
@@ -70,9 +71,12 @@ static int dns_send(const char *name)
 	unsigned char *p, *s, *fullname, *dotptr;
 	const unsigned char *domain;
 
+	/* generate a random transaction id */
+	dns_req_id = dns_timer_start + random32();
+
 	/* Prepare DNS packet header */
 	header           = (struct header *)packet;
-	header->tid      = 1;
+	header->tid      = htons(dns_req_id);
 	header->flags    = htons(0x100);	/* standard query */
 	header->nqueries = htons(1);		/* Just one query */
 	header->nanswers = 0;
@@ -127,6 +131,10 @@ static void dns_recv(struct header *header, unsigned len)
 
 	pr_debug("%s\n", __func__);
 
+	/* Only accept responses with the expected request id */
+	if (ntohs(header->id) != dns_req_id)
+		return;
+
 	/* We sent 1 query. We want to see more that 1 answer. */
 	if (ntohs(header->nqueries) != 1)
 		return;
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] fixup! net: dns: Generate and verify transaction ID
  2022-05-05 10:08 [PATCH] net: dns: Generate and verify transaction ID Jules Maselbas
@ 2022-05-06 14:58 ` Jules Maselbas
  2022-05-06 14:59   ` Jules Maselbas
  2022-05-06 15:16   ` Jules Maselbas
  2022-05-06 15:04 ` [PATCH] " Jules Maselbas
  1 sibling, 2 replies; 6+ messages in thread
From: Jules Maselbas @ 2022-05-06 14:58 UTC (permalink / raw)
  To: barebox; +Cc: Jules Maselbas

Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
---
 net/dns.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/dns.c b/net/dns.c
index 9ad316e33..9803ef475 100644
--- a/net/dns.c
+++ b/net/dns.c
@@ -58,7 +58,7 @@ struct header {
 
 static struct net_connection *dns_con;
 static uint64_t dns_timer_start;
-static uin32_t dns_req_id;
+static uint32_t dns_req_id;
 static int dns_state;
 static IPaddr_t dns_ip;
 
@@ -132,7 +132,7 @@ static void dns_recv(struct header *header, unsigned len)
 	pr_debug("%s\n", __func__);
 
 	/* Only accept responses with the expected request id */
-	if (ntohs(header->id) != dns_req_id)
+	if (ntohs(header->tid) != dns_req_id)
 		return;
 
 	/* We sent 1 query. We want to see more that 1 answer. */
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] fixup! net: dns: Generate and verify transaction ID
  2022-05-06 14:58 ` [PATCH] fixup! " Jules Maselbas
@ 2022-05-06 14:59   ` Jules Maselbas
  2022-05-06 15:16   ` Jules Maselbas
  1 sibling, 0 replies; 6+ messages in thread
From: Jules Maselbas @ 2022-05-06 14:59 UTC (permalink / raw)
  To: barebox

I've typed too fast ^^'

On Fri, May 06, 2022 at 04:58:56PM +0200, Jules Maselbas wrote:
> Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
> ---
>  net/dns.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/dns.c b/net/dns.c
> index 9ad316e33..9803ef475 100644
> --- a/net/dns.c
> +++ b/net/dns.c
> @@ -58,7 +58,7 @@ struct header {
>  
>  static struct net_connection *dns_con;
>  static uint64_t dns_timer_start;
> -static uin32_t dns_req_id;
> +static uint32_t dns_req_id;
>  static int dns_state;
>  static IPaddr_t dns_ip;
>  
> @@ -132,7 +132,7 @@ static void dns_recv(struct header *header, unsigned len)
>  	pr_debug("%s\n", __func__);
>  
>  	/* Only accept responses with the expected request id */
> -	if (ntohs(header->id) != dns_req_id)
> +	if (ntohs(header->tid) != dns_req_id)
>  		return;
>  
>  	/* We sent 1 query. We want to see more that 1 answer. */
> -- 
> 2.17.1
> 





_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] net: dns: Generate and verify transaction ID
  2022-05-05 10:08 [PATCH] net: dns: Generate and verify transaction ID Jules Maselbas
  2022-05-06 14:58 ` [PATCH] fixup! " Jules Maselbas
@ 2022-05-06 15:04 ` Jules Maselbas
  2022-05-09  7:17   ` Sascha Hauer
  1 sibling, 1 reply; 6+ messages in thread
From: Jules Maselbas @ 2022-05-06 15:04 UTC (permalink / raw)
  To: barebox

Hi,

I would like some feedback on how to select a dns_req_id.
Although ths is likely not very critical to barebox, I think using both
dns_timer_start plus random32 is a bit overkill. Maybe simply using
random is sufficient.

On Thu, May 05, 2022 at 12:08:05PM +0200, Jules Maselbas wrote:
> The transaction ID wasn't verified on received DNS responses, plus the
> ID needs to be difficult to predict in order to avoid MitM (man in the
> middle) being able to easily forge responses.
> 
> Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
> ---
>  net/dns.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/net/dns.c b/net/dns.c
> index 78588b96f..9ad316e33 100644
> --- a/net/dns.c
> +++ b/net/dns.c
> @@ -58,6 +58,7 @@ struct header {
>  
>  static struct net_connection *dns_con;
>  static uint64_t dns_timer_start;
> +static uin32_t dns_req_id;
>  static int dns_state;
>  static IPaddr_t dns_ip;
>  
> @@ -70,9 +71,12 @@ static int dns_send(const char *name)
>  	unsigned char *p, *s, *fullname, *dotptr;
>  	const unsigned char *domain;
>  
> +	/* generate a random transaction id */
> +	dns_req_id = dns_timer_start + random32();
I am wondering if using only one of dns_timer_start or randome32 is
sufficient on its own. For the record musl uses clock_gettime without
random at all.

>  	/* Prepare DNS packet header */
>  	header           = (struct header *)packet;
> -	header->tid      = 1;
> +	header->tid      = htons(dns_req_id);
>  	header->flags    = htons(0x100);	/* standard query */
>  	header->nqueries = htons(1);		/* Just one query */
>  	header->nanswers = 0;
> @@ -127,6 +131,10 @@ static void dns_recv(struct header *header, unsigned len)
>  
>  	pr_debug("%s\n", __func__);
>  
> +	/* Only accept responses with the expected request id */
> +	if (ntohs(header->id) != dns_req_id)
> +		return;
> +
>  	/* We sent 1 query. We want to see more that 1 answer. */
>  	if (ntohs(header->nqueries) != 1)
>  		return;
> -- 
> 2.17.1
> 





_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] fixup! net: dns: Generate and verify transaction ID
  2022-05-06 14:58 ` [PATCH] fixup! " Jules Maselbas
  2022-05-06 14:59   ` Jules Maselbas
@ 2022-05-06 15:16   ` Jules Maselbas
  1 sibling, 0 replies; 6+ messages in thread
From: Jules Maselbas @ 2022-05-06 15:16 UTC (permalink / raw)
  To: barebox

On Fri, May 06, 2022 at 04:58:56PM +0200, Jules Maselbas wrote:
> Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
> ---
>  net/dns.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/dns.c b/net/dns.c
> index 9ad316e33..9803ef475 100644
> --- a/net/dns.c
> +++ b/net/dns.c
> @@ -58,7 +58,7 @@ struct header {
>  
>  static struct net_connection *dns_con;
>  static uint64_t dns_timer_start;
> -static uin32_t dns_req_id;
> +static uint32_t dns_req_id;
s/uint32_t/uint16_t/

>  static int dns_state;
>  static IPaddr_t dns_ip;
>  
> @@ -132,7 +132,7 @@ static void dns_recv(struct header *header, unsigned len)
>  	pr_debug("%s\n", __func__);
>  
>  	/* Only accept responses with the expected request id */
> -	if (ntohs(header->id) != dns_req_id)
> +	if (ntohs(header->tid) != dns_req_id)
>  		return;
>  
>  	/* We sent 1 query. We want to see more that 1 answer. */
> -- 
> 2.17.1
> 





_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] net: dns: Generate and verify transaction ID
  2022-05-06 15:04 ` [PATCH] " Jules Maselbas
@ 2022-05-09  7:17   ` Sascha Hauer
  0 siblings, 0 replies; 6+ messages in thread
From: Sascha Hauer @ 2022-05-09  7:17 UTC (permalink / raw)
  To: Jules Maselbas; +Cc: barebox

On Fri, May 06, 2022 at 05:04:05PM +0200, Jules Maselbas wrote:
> Hi,
> 
> I would like some feedback on how to select a dns_req_id.
> Although ths is likely not very critical to barebox, I think using both
> dns_timer_start plus random32 is a bit overkill. Maybe simply using
> random is sufficient.
> 
> On Thu, May 05, 2022 at 12:08:05PM +0200, Jules Maselbas wrote:
> > The transaction ID wasn't verified on received DNS responses, plus the
> > ID needs to be difficult to predict in order to avoid MitM (man in the
> > middle) being able to easily forge responses.
> > 
> > Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
> > ---
> >  net/dns.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/dns.c b/net/dns.c
> > index 78588b96f..9ad316e33 100644
> > --- a/net/dns.c
> > +++ b/net/dns.c
> > @@ -58,6 +58,7 @@ struct header {
> >  
> >  static struct net_connection *dns_con;
> >  static uint64_t dns_timer_start;
> > +static uin32_t dns_req_id;
> >  static int dns_state;
> >  static IPaddr_t dns_ip;
> >  
> > @@ -70,9 +71,12 @@ static int dns_send(const char *name)
> >  	unsigned char *p, *s, *fullname, *dotptr;
> >  	const unsigned char *domain;
> >  
> > +	/* generate a random transaction id */
> > +	dns_req_id = dns_timer_start + random32();
> I am wondering if using only one of dns_timer_start or randome32 is
> sufficient on its own. For the record musl uses clock_gettime without
> random at all.

random32() is a pseudo random generator, it will be initialized with the
same seed every reboot and thus doesn't add any value here. Using the
timer to generate an id should be better and sufficient.
The worst that can happen is that barebox sends DNS requests right after
startup, and I think the different times needed to get the link up
should introduce a certain jitter in the timer values used for the id.

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-05-09  7:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-05 10:08 [PATCH] net: dns: Generate and verify transaction ID Jules Maselbas
2022-05-06 14:58 ` [PATCH] fixup! " Jules Maselbas
2022-05-06 14:59   ` Jules Maselbas
2022-05-06 15:16   ` Jules Maselbas
2022-05-06 15:04 ` [PATCH] " Jules Maselbas
2022-05-09  7:17   ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox