|
Message-ID: <CAHmME9prHVM+faDB61hQUSa9crSQqmLysWmTakf=9WcUyWwq4A@mail.gmail.com> Date: Thu, 8 Jun 2017 02:31:23 +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.