Messages in this thread | | | Date | Thu, 16 Jul 1998 11:54:51 +0200 | From | Oskar Pearson <> | Subject | Re: 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
| |