Re: Entropy Patch with Linebreaks

[Samuel Thibault]

Hi,

Michael Casadevall, le Sat 11 Aug 2007 23:03:37 -0400, a écrit :
> --- device/blkio.c    25 Feb 1997 21:28:13 -0000      1.1.1.1
> +++ device/blkio.c    12 Aug 2007 06:35:59 -0000
> @@ -149,6 +151,10 @@
>       do {
>           prev = next;
>           next = prev->io_next;
> +#ifdef MACH_ENTROPY
> +         /* Let's grab the cylinder numbers for entropy.  */
> +         entropy_putdata (prev, sizeof(io_req_t), ENTROPY_HIGH_QUALITY);
> +#endif
>       } while (next != 0 && prev->io_cylinder == next->io_cylinder);
>  
>       if (next == 0) {

Mmm, haven't you noticed that this function is actually never used? :)
Anyway, it may still be useful since disk drivers are supposed to use
it. However, I don't see why you're putting the whole list into the
entropy pool, should it be just the request that is to added? (i.e. just
ior).

> Index: device/cons.c
> ===================================================================
> RCS file: /sources/hurd/gnumach/device/Attic/cons.c,v
> retrieving revision 1.2.4.6
> diff -u -r1.2.4.6 cons.c
> --- device/cons.c     13 Nov 2006 21:30:36 -0000      1.2.4.6
> +++ device/cons.c     12 Aug 2007 06:35:59 -0000
> @@ -230,9 +234,19 @@
>  cngetc()
>  {
>       if (cn_tab)
> +#if defined(MACH_KERNEL) && defined(MACH_ENTROPY)
> +         entropy_putchar (cn_tab->cn_dev, ENTROPY_MEDIUM_QUALITY);
> +#endif /* MACH_ENTROPY and MACH_ENTROPY */

Technically speaking, that should be entropy_putshort, since cn_dev is a
short.

> +         return ((*cn_tab->cn_getc)(cn_tab->cn_dev, 1));

The characher which was returned really deserves adding to the entropy
pool.

> +     if (romgetc) 
> +       {
> +#if defined (MACH_KERNEL) && defined(MACH_ENTROPY)
> +         entropy_putchar(*romgetc, ENTROPY_MEDIUM_QUALITY);
> +#endif /* MACH_KERNEL && MACH_ENTROPY */ 

That should be an entropy_putptr, shouldn't it? Or you can define a
generic entropy_put macro with typeof() & sizeof() which is able to
infer the size.

> Index: i386/i386at/kd.c
> ===================================================================
> RCS file: /sources/hurd/gnumach/i386/i386at/Attic/kd.c,v
> retrieving revision 1.5.2.13
> diff -u -r1.5.2.13 kd.c
> --- i386/i386at/kd.c  7 May 2007 22:04:53 -0000       1.5.2.13
> +++ i386/i386at/kd.c  12 Aug 2007 06:36:00 -0000
> @@ -811,6 +816,16 @@
>               up = TRUE;
>               scancode &= ~K_UP;
>       }
> +
> +#ifdef MACH_KERNEL
> +#ifdef MACH_ENTROPY                                                          
>              
> +      /* Sune Kirkeby's entropy patch (which was a port of the
> +         linux entropy drivers for GNU mach) placed the keyboard
> +         entropy source here. I looked at that for an idea of where
> +         how to do write this driver.  */
> +     entropy_putchar(scancode | (up ? 0200 : 0), ENTROPY_LOW_QUALITY);

Why low quality? I know people always use the same commands (ls, mv...)
but the order in which they type them is to my mind a very good source
of entropy. Also, the _time_ when characters are typed should be the
best source of entropy, you should really take that into account.

> diff -u -r1.3.2.8 kd_mouse.c
> --- i386/i386at/kd_mouse.c    13 Nov 2006 21:30:36 -0000      1.3.2.8
> +++ i386/i386at/kd_mouse.c    12 Aug 2007 06:36:00 -0000
> @@ -677,6 +680,11 @@
>       moved.mm_deltaX = (char)mousebuf[1] + (char)mousebuf[3];
>       moved.mm_deltaY = (char)mousebuf[2] + (char)mousebuf[4];
>  
> +#ifdef MACH_ENTROPY
> +     /* Kick some mouse data to the entropy driver.  */
> +     entropy_putchar((buttonchanges + moved.mm_deltaX
> +                      + moved.mm_deltaY), ENTROPY_HIGH_QUALITY);
> +#endif

Ok for the high quality, same story for the time.

> Index: linux/dev/glue/misc.c
> ===================================================================
> RCS file: /sources/hurd/gnumach/linux/dev/glue/Attic/misc.c,v
> retrieving revision 1.2
> diff -u -r1.2 misc.c
> --- linux/dev/glue/misc.c     18 Sep 2001 21:14:19 -0000      1.2
> +++ linux/dev/glue/misc.c     12 Aug 2007 06:36:00 -0000
> @@ -224,6 +228,15 @@
>  void
>  add_blkdev_randomness (int major)
>  {
> +#ifdef MACH_ENTROPY
> +  /* This is useless for good quality, so we'll only use if it nothing
> +     else is available - The problem is that mach only has 1 block
> +     device, floppy (major 3 corresponds to Ctrl C) so this is useless
> +     for entropic sources. If we ever get more block devices the
> +     quality should be upped for additional entropy. */
> +  
> +  entropy_putchar(major, ENTROPY_POOR_QUALITY);
> +#endif
>  }

People themselves usually have very few devices actually. But the point
of add_blkdev_randomness is not to push the number of the device, but
the _time_ when it gets called, usually the i/o completion time, which
is a somewhat good source of entropy.

> diff -u -r1.1.4.7 net.c
> --- linux/dev/glue/net.c      27 Mar 2007 22:47:11 -0000      1.1.4.7
> +++ linux/dev/glue/net.c      12 Aug 2007 06:36:00 -0000
> @@ -299,6 +304,11 @@
>    ph->length = (skb->len - sizeof (struct ether_header)
>               + sizeof (struct packet_header));
>  
> +#ifdef MACH_ENTROPY
> +  /* Grab the packet for entropy purposes.  */
> +  entropy_putdata(ph + 1, skb->len - sizeof(struct ether_header), 
> ENTROPY_HIGH_QUALITY);
> +#endif
> +
>    dev_kfree_skb (skb, FREE_READ);
>  
>    net_kmsg(kmsg)->sent = FALSE; /* Mark packet as received.  */

Same here: data is somewhat good, but what is a lot better is the time
when it arrives.

BTW, I think you should add a notion of security: letting network
packets alter the entropy is fine for general desktop purpose, but not
for security purpose: the random numbers could somehow get fooled by
sending the appropriate network packets...

Also, shouldn't you add sent packets to the entropy pool ?

> diff -u -r1.2 blk.h
> --- linux/dev/include/linux/blk.h     5 Apr 2001 06:39:21 -0000       1.2
> +++ linux/dev/include/linux/blk.h     12 Aug 2007 06:36:00 -0000
> @@ -90,7 +90,7 @@
>  #endif /* CONFIG_BLK_DEV_MD */
>  
>  extern void set_device_ro(kdev_t dev,int flag);
> -void add_blkdev_randomness(int major);
> +extern void add_blkdev_randomness(int major);
>  
>  extern int floppy_init(void);
>  extern void rd_load(void);

Well, it's just the same for the compiler :)

Samuel