Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 8 Jun 2017 02:34:40 +0200
From: "Jason A. Donenfeld" <Jason@...c4.com>
To: "Theodore Ts'o" <tytso@....edu>, "Jason A. Donenfeld" <Jason@...c4.com>, 
	Linux Crypto Mailing List <linux-crypto@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>, 
	kernel-hardening@...ts.openwall.com, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, David Miller <davem@...emloft.net>, 
	Eric Biggers <ebiggers3@...il.com>, Steve French <sfrench@...ba.org>
Subject: Re: [PATCH v4 08/13] cifs: use get_random_u32 for
 32-bit lock random

On Thu, Jun 8, 2017 at 2:25 AM, Theodore Ts'o <tytso@....edu> wrote:
> There's a bigger problem here, which is that cifs_lock_secret is a
> 32-bit value which is being used to obscure flock->fl_owner before it
> is sent across the wire.  But flock->fl_owner is a pointer to the
> struct file *, so 64-bit architecture, the high 64-bits of a kernel
> pointer is being exposed to anyone using tcpdump.  (Oops, I'm showing
> my age; I guess all the cool kids are using Wireshark these days.)
>
> Worse, the obscuring is being done using XOR.  How an active attacker
> might be able to trivially reverse engineer the 32-bit "secret" is
> left as an exercise to the reader.  The bottom line is if the goal is
> to hide the memory location of a struct file from an attacker,
> cifs_lock_secret is about as useful as a TSA agent doing security
> theatre at an airport.  Which is to say, it makes the civilians feel
> good.  :-)

High five for taking the deep dive and actually reading how this all
works. Nice bug!

> Not waiting
> for the CRNG to be fully initialized is the *least* of its problems.

The kernel is vast and filled with tons of bugs of many sorts. On this
reasoning, maybe I should spend my time auditing web apps instead,
which are usually the "front door" of bugs? I like the puzzles of
random.c. I also had a real world need for wait_for_random_bytes() in
a module I'm writing.

But anyway, your general point is a really good one. Tons of callers
of the random functions are doing it wrong in one way or another.
Spending time looking at those is probably a good idea...

> Anyway, I'll include this commit in the dev branch of the random tree,
> since it's not going to make things worse.

Great, thanks.

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.