[3/8] batman-adv: randomize initial seqno to avoid collision

  Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
---
 bat_iv_ogm.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
  

On 2012-02-07 10:20, Marek Lindner wrote:
> Signed-off-by: Marek Lindner<lindner_marek@yahoo.de>
> ---
>   bat_iv_ogm.c |    5 +++++
>   1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
> index 3eff7f0..4ac2d1d 100644
> --- a/bat_iv_ogm.c
> +++ b/bat_iv_ogm.c
> @@ -33,6 +33,11 @@
>   static void bat_iv_ogm_iface_enable(struct hard_iface *hard_iface)
>   {
>   	struct batman_ogm_packet *batman_ogm_packet;
> +	unsigned long random_seqno;
> +
> +	/* randomize initial seqno to avoid collision */
> +	get_random_bytes(&random_seqno, sizeof(unsigned long));
> +	atomic_set(&hard_iface->seqno, (uint32_t)random_seqno);

Wouldn't it be better to cast "unsigned long" in the call to atomic_set()?
  

On Tuesday, February 07, 2012 17:33:14 Martin Hundebøll wrote:
> On 2012-02-07 10:20, Marek Lindner wrote:
> > Signed-off-by: Marek Lindner<lindner_marek@yahoo.de>
> > ---
> > 
> >   bat_iv_ogm.c |    5 +++++
> >   1 files changed, 5 insertions(+), 0 deletions(-)
> > 
> > diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
> > index 3eff7f0..4ac2d1d 100644
> > --- a/bat_iv_ogm.c
> > +++ b/bat_iv_ogm.c
> > @@ -33,6 +33,11 @@
> > 
> >   static void bat_iv_ogm_iface_enable(struct hard_iface *hard_iface)
> >   {
> >   
> >   	struct batman_ogm_packet *batman_ogm_packet;
> > 
> > +	unsigned long random_seqno;
> > +
> > +	/* randomize initial seqno to avoid collision */
> > +	get_random_bytes(&random_seqno, sizeof(unsigned long));
> > +	atomic_set(&hard_iface->seqno, (uint32_t)random_seqno);
> 
> Wouldn't it be better to cast "unsigned long" in the call to atomic_set()?

Why should it better ?

Regards,
Marek
  

On 2012-02-07 12:10, Marek Lindner wrote:
>>> +	unsigned long random_seqno;
>>> >  >  +
>>> >  >  +	/* randomize initial seqno to avoid collision */
>>> >  >  +	get_random_bytes(&random_seqno, sizeof(unsigned long));
>>> >  >  +	atomic_set(&hard_iface->seqno, (uint32_t)random_seqno);
>> >
>> >  Wouldn't it be better to cast "unsigned long" in the call to atomic_set()?
> Why should it better ?

Maybe not better, but at least it is consistent with the type of random_seqno, which is unsigned long. I know the two types are identical, but nevertheless, I like to use the same type of type :)
  

On Tuesday, February 07, 2012 19:13:27 Martin Hundebøll wrote:
> On 2012-02-07 12:10, Marek Lindner wrote:
> >>> +	unsigned long random_seqno;
> >>> 
> >>> >  >  +
> >>> >  >  +	/* randomize initial seqno to avoid collision */
> >>> >  >  +	get_random_bytes(&random_seqno, sizeof(unsigned long));
> >>> >  >  +	atomic_set(&hard_iface->seqno, (uint32_t)random_seqno);
> >> >  
> >> >  Wouldn't it be better to cast "unsigned long" in the call to
> >> >  atomic_set()?
> > 
> > Why should it better ?
> 
> Maybe not better, but at least it is consistent with the type of
> random_seqno, which is unsigned long. I know the two types are identical,
> but nevertheless, I like to use the same type of type :)

You lost me somewhere. Yes, random_seqno is unsigned long. If we wanted to 
store unsigned long we would not need a cast. 
However, in my kernel the second argument for atomic_set() is "int" and not 
"unsigned long" which why we have a cast there.

Regards,
Marek
  

On Tue, Feb 07, 2012 at 05:20:46PM +0800, Marek Lindner wrote:
> Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
> ---
>  bat_iv_ogm.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
> index 3eff7f0..4ac2d1d 100644
> --- a/bat_iv_ogm.c
> +++ b/bat_iv_ogm.c
> @@ -33,6 +33,11 @@
>  static void bat_iv_ogm_iface_enable(struct hard_iface *hard_iface)
>  {
>  	struct batman_ogm_packet *batman_ogm_packet;
> +	unsigned long random_seqno;
> +
> +	/* randomize initial seqno to avoid collision */
> +	get_random_bytes(&random_seqno, sizeof(unsigned long));
> +	atomic_set(&hard_iface->seqno, (uint32_t)random_seqno);

Hi Marek

Does this sequence number have any security relevance? Does it make
sense to use the TCP sequence number generation code?

      Andrew
  

On Tuesday, February 07, 2012 20:12:00 Andrew Lunn wrote:
> Does this sequence number have any security relevance? Does it make
> sense to use the TCP sequence number generation code?

There is no security relevance I know of. The idea was simply to start with 
random number. Random is a bit better than 1.  ;-)

Where can I find the TCP sequence number code you are referring to ?

Regards,
Marek
  

On Tue, Feb 07, 2012 at 08:21:55PM +0800, Marek Lindner wrote:
> On Tuesday, February 07, 2012 20:12:00 Andrew Lunn wrote:
> > Does this sequence number have any security relevance? Does it make
> > sense to use the TCP sequence number generation code?
> 
> There is no security relevance I know of. The idea was simply to start with 
> random number. Random is a bit better than 1.  ;-)
> 
> Where can I find the TCP sequence number code you are referring to ?

I had to go find it, since i've never looked at it before.

net/core/secure_seq.c:

__u32 secure_tcp_sequence_number(__be32 saddr, __be32 daddr,
                                 __be16 sport, __be16 dport)

but it does not look very re-usable, since it takes all these
addresses. What might be usable is:

__u32 secure_ip_id(__be32 daddr)
{
        u32 hash[MD5_DIGEST_WORDS];

        hash[0] = (__force __u32) daddr;
        hash[1] = net_secret[13];
        hash[2] = net_secret[14];
        hash[3] = net_secret[15];

        md5_transform(hash, net_secret);

        return hash[0];
}

passing it the last four bytes of the originator MAC address?

	Andrew
  

On Tuesday, February 07, 2012 17:20:46 Marek Lindner wrote:
> Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
> ---
>  bat_iv_ogm.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)

Applied in revision 852b9dc.

Regards,
Marek