lkml.org 
[lkml]   [1998]   [Jul]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: loop.c: DES bugfixes
Hi All

Please read this mail if you are using the filesystem encryption option
in Linux. It appears that there is a serious flaw in the DES code...

A while ago (7th May) Solar Designer (solar@false.com) sent some mail
to linux-kernel. I didn't see any followup mail, but I have decided
to work through his message and confirm that there are problems. It's
important that these be sorted out before Linux-2.2 is released, otherwise
people plugging stuff into the kernel are going to be completely
stuffed. Solar Designer's patch is at Message-Id:
<199805070756.LAA01136@false.com>

I have updated the patches that he referred to (from berkeley.edu)
to work with the latest 2.0.* kernel. They are available from
ftp://ftp.is.co.za/linux/local/kernel/crypto/

I am going to write a kernel-crypto doc file for either the Configure
help or for Documentation/fs - hopefully this weekend.

> doesn't fix loop.c, everything appears to work, but the encryption is no
> stronger than a XOR -- even worse.

It certainly appears that way!

> 1. A wrong variable is passed to des_set_key(), so the key is not used. %-)

Correct!

For those that don't know the code:

A user-level program calls lo_ioctl() to associate one of the loop devices
with a file. lo_ioctl works though the available ioctl options for loop
devices, and since it's a 'add loop device request', lo_ioctl() calls
loop_set_status() with the client's data in the value 'arg' and the
destination loop device structure in 'struct loop_device *lo'
This 'arg' value is copied into the 'struct loop_info info' variable with
copy_from_user.

des.c: void des_set_key(des_cblock *key,des_key_schedule schedule)

If the new loop device is to use encryption, this snippet of code is
called. This is in loop_set_status in loop.c:

---------
case LO_CRYPT_NONE:
break;
case LO_CRYPT_XOR:
if (info.lo_encrypt_key_size < 0)
return -EINVAL;
break;
#ifdef DES_AVAILABLE
case LO_CRYPT_DES:
if (info.lo_encrypt_key_size != 8)
return -EINVAL;
des_set_key((des_cblock *) lo->lo_encrypt_key,
lo->lo_des_key);
memcpy(lo->lo_des_init,info.lo_init,8);
---------

Look at 'LO_CRYPT_XOR' - it clearly checks info.lo_encrypt_key_size, as
does the DES section. DES, however, then uses the password in
lo->lo_encrypt_key (not info->lo_encrypt_key!).

We then clobber stuff like so:

----------
lo->lo_encrypt_key_size = info.lo_encrypt_key_size;
if (info.lo_encrypt_key_size)
memcpy(lo->lo_encrypt_key, info.lo_encrypt_key,
info.lo_encrypt_key_size);
----------
cool, hey? The NSA must have been packing up laughing :)

If I am just going crazy, someone please tell me... I can't really believe
that it's been broken this long... doesn't this mean that your password
would ALWAYS work?

> 2. The attempt to implement PCBC mode has failed because of a bug, so in
> reality we get a kind of ECB.

Well, I can't find any reference to 'PCBC' mode in applied cryptography,
so I can't agree or disagree :)

Ok - here my crypto-knowledge (or, rather, the lack thereof) is going to
show:

Surely CBC mode can't work across the whole loopback device anyway? It
only applies across the whole device it would mean we couldn't random-seek,
right?

I can imagine some kind of replay attack on your disk (think log files,
each entry of a specific size. You are someone that wants to trash
a couple entries in the logs...), so we need to stop these attacks if
possible, but I am not sure that it is. If I had to guess I would say that
we are CBC encoding blocks handled per write/read. If this is the case then
is it even worth the hassle?

> 3. Standard distribution of mount(8) (version 2.7l) simply passes the key
> into kernel as entered by the user, and the kernel, in turn, is trying to
...
> are significant, not lower bits as many might expect. This bug effectively
> reduces the keyspace down to less than 48 bits.

This is why the Berkeley patches SHA hash the pass phrase. I think that
user-space is the best place to do this...

> There're problems with IDEA, too (ECB mode), but I'm not even going to go

Comments above apply, then?

> supported by standard versions of user-level utils, while DES is, so it's

> more important to make DES work as documented.
You mean work at all?

> Finally, there're bugs in the user-level utils, mount(8) and losetup(8):
> 1. mount(8) sets an alarm while checking the lock, and forgets to reset it
> afterwards, so that it may expire while entering the password.

Ahh - I have patched this here, I'll submit the patches to the
linux-utils maintainers shortly. The people that maintain the losetup code
seem to have ignored my previous update patch to the losetup stuff
(no mail from them at least).

> 2. It looks like there was no real need to ask for an IV, better just set
> it to zero. Asking for the IV at each mount can lead to problems, since if
> one enters an incorrect IV, then CBC mode decryption will only differ from
> the correct one by its first word, so the filesystem will mount, but with
> a bunch of ext2fs errors. Not good.

The Berkeley patches use part of the extra hash output for the IV.

> 3. (Not really a bug, just something that can be improved, and there's even
> a patch available to do so.) It should be possible to enter long passphrases
> that are hashed (by user mode code) into DES keys. At this time, it may also
> be possible to add variable iteration count to the hash to slow attacks down.

Well you definitely read the berkeley patches, but nobody else did :)

From the berkelet patch:
- des_set_key((des_cblock *) lo->lo_encrypt_key,
- lo->lo_des_key);
- memcpy(lo->lo_des_init,info.lo_init,8);
+ des_set_key((des_cblock *) info.lo_encrypt_key,
+ lo->lo_crypt.des.key);
+ des_set_key((des_cblock *) info.lo_iv_key,
+ lo->lo_crypt.des.iv_keysched);

> ftp://ftp.csua.berkeley.edu/pub/cypherpunks/filesystems/linux/

> Also, it seems to me that the entire loop encryption code is broken, and
> needs re-coding to make it easy to add new ciphers, possibly via modules
> or similar.

I believe that this would mean that people can't export the kernel then,
since I believe that encryption hooks are still illegal.

> At the end of this post you'll find a patch for loop.c that I just did (for
> 2.0.34pre10, but should work for 2.1 too), that does the following:

> 1. Broken PCBC is replaced with working CBC. I don't see a reason to use
> PCBC here, and I do see a reason to use plain CBC: reliability.
> 2. DES key is now properly passed to des_set_key().
> 3. The key is converted before passing to des_set_key() -- I'm doing this
> the way bdes(1) does.

I have a hunch that Linus is going to want to put the key manipulation
stuff into the user-level code. It seems to be a habit of his :)

(2) is definitely the most important.

> (In general, it seems like a good idea to have some bdes(1) compatibility.

Or at least include a little util that will do the kernel stuff with
losetup and friends.

> If we made the CBC mode work for the filesystem as a whole, not just the
> blocks, then with my current patch it would be possible to encrypt/decrypt

Ok - so you have confirmed that it works on a block basis.

Doesn't this mean that we can't transfer an FS made on one platform to a
different architecture? (eg i386 to Alpha - I believe that Alpha have
4k blocks - if my memory serves me)

> Anyway, this third fix could obviously be done in user space. I put it into
> the kernel now for the following reasons:
> 1. If someone forgot to upgrade the mount binary, they would be vulnerable,
> if we only fixed that in mount(8), not the kernel.

Agreed.

> 2. Having this code in the kernel doesn't affect our ability to introduce
> hashed passphrases in the future anyway: the order in which we use the hash
> bits doesn't affect security at all.

It's also a very minimal amount of code.

Oskar

Oskar
---
"Haven't slept at all. I don't see why people insist on sleeping. You feel
so much better if you don't. And how can anyone want to lose a minute -
a single minute of being alive?" -- Think Twice

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.altern.org/andrebalsa/doc/lkml-faq.html

\
 
 \ /
  Last update: 2005-03-22 13:43    [W:0.122 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site