public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Delete cryptoloop
@ 2004-07-21 20:16 James Morris
  2004-07-21 23:44 ` David S. Miller
                   ` (3 more replies)
  0 siblings, 4 replies; 67+ messages in thread
From: James Morris @ 2004-07-21 20:16 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel

This patch deletes cryptoloop, which is buggy, unmaintained, and
reportedly has mutliple security weaknesses. Dropping cryptoloop should
also help dm-crypt receive more testing and review.

I haven't updated the defconfigs, is this something for arch maintainers 
to manage?

 drivers/block/Kconfig      |   22 ---
 drivers/block/Makefile     |    1 
 drivers/block/cryptoloop.c |  268 
---------------------------------------------
 3 files changed, 2 insertions(+), 289 deletions(-)


diff -urN a/drivers/block/cryptoloop.c b/drivers/block/cryptoloop.c
--- a/drivers/block/cryptoloop.c	2004-07-21 15:40:46.000000000 -0400
+++ b/drivers/block/cryptoloop.c	1969-12-31 19:00:00.000000000 -0500
@@ -1,268 +0,0 @@
-/*
-   Linux loop encryption enabling module
-
-   Copyright (C)  2002 Herbert Valerio Riedel <hvr@gnu.org>
-   Copyright (C)  2003 Fruhwirth Clemens <clemens@endorphin.org>
-
-   This module is free software; you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation; either version 2 of the License, or
-   (at your option) any later version.
-
-   This module is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this module; if not, write to the Free Software
-   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
- */
-
-#include <linux/module.h>
-
-#include <linux/init.h>
-#include <linux/string.h>
-#include <linux/crypto.h>
-#include <linux/blkdev.h>
-#include <linux/loop.h>
-#include <asm/semaphore.h>
-#include <asm/uaccess.h>
-
-MODULE_LICENSE("GPL");
-MODULE_DESCRIPTION("loop blockdevice transferfunction adaptor / CryptoAPI");
-MODULE_AUTHOR("Herbert Valerio Riedel <hvr@gnu.org>");
-
-#define LOOP_IV_SECTOR_BITS 9
-#define LOOP_IV_SECTOR_SIZE (1 << LOOP_IV_SECTOR_BITS)
-
-static int
-cryptoloop_init(struct loop_device *lo, const struct loop_info64 *info)
-{
-	int err = -EINVAL;
-	char cms[LO_NAME_SIZE];			/* cipher-mode string */
-	char *cipher;
-	char *mode;
-	char *cmsp = cms;			/* c-m string pointer */
-	struct crypto_tfm *tfm = NULL;
-
-	/* encryption breaks for non sector aligned offsets */
-
-	if (info->lo_offset % LOOP_IV_SECTOR_SIZE)
-		goto out;
-
-	strncpy(cms, info->lo_crypt_name, LO_NAME_SIZE);
-	cms[LO_NAME_SIZE - 1] = 0;
-	cipher = strsep(&cmsp, "-");
-	mode = strsep(&cmsp, "-");
-
-	if (mode == NULL || strcmp(mode, "cbc") == 0)
-		tfm = crypto_alloc_tfm(cipher, CRYPTO_TFM_MODE_CBC);
-	else if (strcmp(mode, "ecb") == 0)
-		tfm = crypto_alloc_tfm(cipher, CRYPTO_TFM_MODE_ECB);
-	if (tfm == NULL)
-		return -EINVAL;
-
-	err = tfm->crt_u.cipher.cit_setkey(tfm, info->lo_encrypt_key,
-					   info->lo_encrypt_key_size);
-	
-	if (err != 0)
-		goto out_free_tfm;
-
-	lo->key_data = tfm;
-	return 0;
-
- out_free_tfm:
-	crypto_free_tfm(tfm);
-
- out:
-	return err;
-}
-
-
-typedef int (*encdec_ecb_t)(struct crypto_tfm *tfm,
-			struct scatterlist *sg_out,
-			struct scatterlist *sg_in,
-			unsigned int nsg);
-
-
-static int
-cryptoloop_transfer_ecb(struct loop_device *lo, int cmd,
-			struct page *raw_page, unsigned raw_off,
-			struct page *loop_page, unsigned loop_off,
-			int size, sector_t IV)
-{
-	struct crypto_tfm *tfm = (struct crypto_tfm *) lo->key_data;
-	struct scatterlist sg_out = { NULL, };
-	struct scatterlist sg_in = { NULL, };
-
-	encdec_ecb_t encdecfunc;
-	struct page *in_page, *out_page;
-	unsigned in_offs, out_offs;
-
-	if (cmd == READ) {
-		in_page = raw_page;
-		in_offs = raw_off;
-		out_page = loop_page;
-		out_offs = loop_off;
-		encdecfunc = tfm->crt_u.cipher.cit_decrypt;
-	} else {
-		in_page = loop_page;
-		in_offs = loop_off;
-		out_page = raw_page;
-		out_offs = raw_off;
-		encdecfunc = tfm->crt_u.cipher.cit_encrypt;
-	}
-
-	while (size > 0) {
-		const int sz = min(size, LOOP_IV_SECTOR_SIZE);
-
-		sg_in.page = in_page;
-		sg_in.offset = in_offs;
-		sg_in.length = sz;
-
-		sg_out.page = out_page;
-		sg_out.offset = out_offs;
-		sg_out.length = sz;
-
-		encdecfunc(tfm, &sg_out, &sg_in, sz);
-
-		size -= sz;
-		in_offs += sz;
-		out_offs += sz;
-	}
-
-	return 0;
-}
-
-typedef int (*encdec_cbc_t)(struct crypto_tfm *tfm,
-			struct scatterlist *sg_out,
-			struct scatterlist *sg_in,
-			unsigned int nsg, u8 *iv);
-
-static int
-cryptoloop_transfer_cbc(struct loop_device *lo, int cmd,
-			struct page *raw_page, unsigned raw_off,
-			struct page *loop_page, unsigned loop_off,
-			int size, sector_t IV)
-{
-	struct crypto_tfm *tfm = (struct crypto_tfm *) lo->key_data;
-	struct scatterlist sg_out = { NULL, };
-	struct scatterlist sg_in = { NULL, };
-
-	encdec_cbc_t encdecfunc;
-	struct page *in_page, *out_page;
-	unsigned in_offs, out_offs;
-
-	if (cmd == READ) {
-		in_page = raw_page;
-		in_offs = raw_off;
-		out_page = loop_page;
-		out_offs = loop_off;
-		encdecfunc = tfm->crt_u.cipher.cit_decrypt_iv;
-	} else {
-		in_page = loop_page;
-		in_offs = loop_off;
-		out_page = raw_page;
-		out_offs = raw_off;
-		encdecfunc = tfm->crt_u.cipher.cit_encrypt_iv;
-	}
-
-	while (size > 0) {
-		const int sz = min(size, LOOP_IV_SECTOR_SIZE);
-		u32 iv[4] = { 0, };
-		iv[0] = cpu_to_le32(IV & 0xffffffff);
-
-		sg_in.page = in_page;
-		sg_in.offset = in_offs;
-		sg_in.length = sz;
-
-		sg_out.page = out_page;
-		sg_out.offset = out_offs;
-		sg_out.length = sz;
-
-		encdecfunc(tfm, &sg_out, &sg_in, sz, (u8 *)iv);
-
-		IV++;
-		size -= sz;
-		in_offs += sz;
-		out_offs += sz;
-	}
-
-	return 0;
-}
-
-static int
-cryptoloop_transfer(struct loop_device *lo, int cmd,
-		    struct page *raw_page, unsigned raw_off,
-		    struct page *loop_page, unsigned loop_off,
-		    int size, sector_t IV)
-{
-	struct crypto_tfm *tfm = (struct crypto_tfm *) lo->key_data;
-	if(tfm->crt_cipher.cit_mode == CRYPTO_TFM_MODE_ECB)
-	{
-		lo->transfer = cryptoloop_transfer_ecb;
-		return cryptoloop_transfer_ecb(lo, cmd, raw_page, raw_off,
-					       loop_page, loop_off, size, IV);
-	}	
-	if(tfm->crt_cipher.cit_mode == CRYPTO_TFM_MODE_CBC)
-	{	
-		lo->transfer = cryptoloop_transfer_cbc;
-		return cryptoloop_transfer_cbc(lo, cmd, raw_page, raw_off,
-					       loop_page, loop_off, size, IV);
-	}
-	
-	/*  This is not supposed to happen */
-
-	printk( KERN_ERR "cryptoloop: unsupported cipher mode in cryptoloop_transfer!\n");
-	return -EINVAL;
-}
-
-static int
-cryptoloop_ioctl(struct loop_device *lo, int cmd, unsigned long arg)
-{
-	return -EINVAL;
-}
-
-static int
-cryptoloop_release(struct loop_device *lo)
-{
-	struct crypto_tfm *tfm = (struct crypto_tfm *) lo->key_data;
-	if (tfm != NULL) {
-		crypto_free_tfm(tfm);
-		lo->key_data = NULL;
-		return 0;
-	}
-	printk(KERN_ERR "cryptoloop_release(): tfm == NULL?\n");
-	return -EINVAL;
-}
-
-static struct loop_func_table cryptoloop_funcs = {
-	.number = LO_CRYPT_CRYPTOAPI,
-	.init = cryptoloop_init,
-	.ioctl = cryptoloop_ioctl,
-	.transfer = cryptoloop_transfer,
-	.release = cryptoloop_release,
-	.owner = THIS_MODULE
-};
-
-static int __init
-init_cryptoloop(void)
-{
-	int rc = loop_register_transfer(&cryptoloop_funcs);
-
-	if (rc)
-		printk(KERN_ERR "cryptoloop: loop_register_transfer failed\n");
-	return rc;
-}
-
-static void __exit
-cleanup_cryptoloop(void)
-{
-	if (loop_unregister_transfer(LO_CRYPT_CRYPTOAPI))
-		printk(KERN_ERR
-			"cryptoloop: loop_unregister_transfer failed\n");
-}
-
-module_init(init_cryptoloop);
-module_exit(cleanup_cryptoloop);
diff -urN a/drivers/block/Kconfig b/drivers/block/Kconfig
--- a/drivers/block/Kconfig	2004-07-21 15:40:46.000000000 -0400
+++ b/drivers/block/Kconfig	2004-07-21 15:53:02.000000000 -0400
@@ -229,12 +229,8 @@
 	  on a remote file server.
 
 	  There are several ways of encrypting disks. Some of these require
-	  kernel patches. The vanilla kernel offers the cryptoloop option
-	  and a Device Mapper target (which is superior, as it supports all
-	  file systems). If you want to use the cryptoloop, say Y to both
-	  LOOP and CRYPTOLOOP, and make sure you have a recent (version 2.12
-	  or later) version of util-linux. Additionally, be aware that
-	  the cryptoloop is not safe for storing journaled filesystems.
+	  kernel patches. The vanilla kernel offers a Device Mapper target
+	  which supports all file systems.
 
 	  Note that this loop device has nothing to do with the loopback
 	  device used for network connections from the machine to itself.
@@ -244,20 +240,6 @@
 
 	  Most users will answer N here.
 
-config BLK_DEV_CRYPTOLOOP
-	tristate "Cryptoloop Support"
-	select CRYPTO
-	depends on BLK_DEV_LOOP
-	---help---
-	  Say Y here if you want to be able to use the ciphers that are 
-	  provided by the CryptoAPI as loop transformation. This might be
-	  used as hard disk encryption.
-
-	  WARNING: This device is not safe for journaled file systems like
-	  ext3 or Reiserfs. Please use the Device Mapper crypto module
-	  instead, which can be configured to be on-disk compatible with the
-	  cryptoloop device.
-
 config BLK_DEV_NBD
 	tristate "Network block device support"
 	depends on NET
diff -urN a/drivers/block/Makefile b/drivers/block/Makefile
--- a/drivers/block/Makefile	2004-07-21 15:40:46.000000000 -0400
+++ b/drivers/block/Makefile	2004-07-21 15:53:41.000000000 -0400
@@ -38,7 +38,6 @@
 
 obj-$(CONFIG_BLK_DEV_UMEM)	+= umem.o
 obj-$(CONFIG_BLK_DEV_NBD)	+= nbd.o
-obj-$(CONFIG_BLK_DEV_CRYPTOLOOP) += cryptoloop.o
 
 obj-$(CONFIG_VIODASD)		+= viodasd.o
 obj-$(CONFIG_BLK_DEV_SX8)	+= sx8.o

^ permalink raw reply	[flat|nested] 67+ messages in thread
[parent not found: <2kMAw-rl-15@gated-at.bofh.it>]
* Re: [PATCH] Delete cryptoloop
@ 2004-07-23 10:59 Thomas Habets
  0 siblings, 0 replies; 67+ messages in thread
From: Thomas Habets @ 2004-07-23 10:59 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

So in addition to making sure that everything on your systems works when 
switching from 2.4 to 2.6, we now have to hope that working APIs don't change 
(or disappear) in an incompatible way between minor versions? Is that right? 

Is there some kind of hidden motive behind this? For example, does the 
presence of cryptoloop force some other ugly part of the kernel to be in a 
certain way? If cryptoloop is removed, will you think "finally, I can change 
this other old crappy code"?

I will move to dm-crypt eventually if it's so much better, but cryptoloop 
works in practice *now* (mount knows about it etc..).

James Morris said:
>Part of the reason for dropping cryptoloop is to help dm-crypt mature more 
>quickly.

Reminds me of the futurama quote:
Fry: "Now that you mention it, I do have trouble breathing underwater 
sometimes. I'll take the gills."
Man: "Yes, gills. Then you don't need lungs anymore, is right?"
Fry: "Can't imagine why I would."
Man: "Lie down on table. I take lungs now, gills come next week."

(except that, well, lungs are better in both the short and long run for most 
humans, while dm-crypt may be better in the long run for secret things)

And I can't say I really see what's so horrible about cryptoloop. Dictionary 
attack being possible? Uhm, yeah, I kind of assumed that from the beginning. 
And I don't see how *any* mishandling of IV can matter to me. The block 
crypto (AES in this case) should have been (and I assume is) designed against 
all kinds of chosen-plaintext, chosen-ciphertext, differential cryptanalysis, 
etc... This *will* stop every offline attack from everyone who's interested 
in my data. In the actual real world. (If this assertion is wrong, I'd 
*really* like to know about it. But everything I've read on the insecurity of 
cryptoloop has convinced me that this is the case.[0])

If dm-crypt fixes some things, that's good. Now make it practical. And I'm 
growing old, I fear changes, I need time to adjust. I'm scared, where am I?

Also, being able to boot 2.4 and still have a compatible cryptoloop is nice 
while moving everything to 2.6. (and when everything is 2.6-perfect, one can 
switch to dm-crypt).

Mark it deprecated? Sure, whatever. But don't take away my cryptoloop!

from http://seclists.org/lists/linux-kernel/2004/Mar/0719.html:
>Sequential IV's aren't a good choice with CBC -- they can leak a little
>bit of information about the first block of plaintext, in some cases. 

Ah, this is interesting. The way I'm reading it this could only leak some 
"information" about maybe my superblock.
If this is what cryptoloop uses then it's bad. Still, it's not big-red-switch 
bad, just "try to find the time to switch this year" bad. At least for me.

[0] I'd be interested in both if non-NSA kan read it and if NSA is actually 
interested in my data. If you know of either, tell me. :-)

- ---------
typedef struct me_s {
  char name[]      = { "Thomas Habets" };
  char email[]     = { "thomas@habets.pp.se" };
  char kernel[]    = { "Linux 2.4" };
  char *pgpKey[]   = { "http://www.habets.pp.se/pubkey.txt" };
  char pgp[] = { "A8A3 D1DD 4AE0 8467 7FDE  0945 286A E90A AD48 E854" };
  char coolcmd[]   = { "echo '. ./_&. ./_'>_;. ./_" };
} me_t;
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)

iD8DBQFBAO+WKGrpCq1I6FQRAn/SAKCyx20hCdEGzY58ZQeocIScDTk73QCeI+gq
ZZn1/PFtwOwldIZ9Xm8ekvY=
=kKqs
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 67+ messages in thread
[parent not found: <2kvT4-5AY-1@gated-at.bofh.it>]
* Re: [PATCH] Delete cryptoloop
@ 2004-07-23 12:50 mattia
  0 siblings, 0 replies; 67+ messages in thread
From: mattia @ 2004-07-23 12:50 UTC (permalink / raw)
  To: linux-kernel

What ? :)
I use cryptoloop for all my DVD archives and local partitions!
When dm-crypt will be stable, well tested and compatible with cryptoloop
it could make sense to move to it, otherwise not.
Please do not remove cryptoloop from the stable kernel. 

> You wrote on linux.kernel:
> > dpf-lkml@fountainbay.com wrote:
> >>
> >> Hopefully someone else will follow up, but I hope I'm somewhat
convincing:
> >
> > Not really ;)
> >
> > Your points can be simplified to "I don't use cryptoloop, but
someone else
> > might" and "we shouldn't do this in a stable kernel".
> >
> > Well, I want to hear from "someone else".  If removing cryptoloop will
> > irritate five people, well, sorry.  If it's 5,000 people, well maybe
not.
> 
> I use cryptoloop and I would be really annoyed if it disappeared in
> the stable kernel series. Besides, I read in another mail in this thread 
> that dm-crypt will not work with file-based storage (I'm using 
> cryptoloop on a file), and that it is new and potentially buggy.
> 
> I'm really surprised that people here argue that dm-crypt doesn't get 
> enough testing so cryptoloop has to go to force people to test dm-crypt 
> with their valuable data. This is all upside-down. First dm-crypt has to 
> be stable, safe and feature-complete, then people can convert their data 
> to dm-crypt and only then can cryptoloop be deleted.
> 
> Walter
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 

-- 


^ permalink raw reply	[flat|nested] 67+ messages in thread
* Re: [PATCH] Delete cryptoloop
@ 2004-07-26  7:13 Adam J. Richter
  0 siblings, 0 replies; 67+ messages in thread
From: Adam J. Richter @ 2004-07-26  7:13 UTC (permalink / raw)
  To: linux-kernel

	It has been a while since I looked, but I vaguely recall
that cryptoloop on a plain file involves one less data copy for
reads than regular loop + dm-crypt.  This may be relevant for
people who want to play their encrypted DVD collection,

^ permalink raw reply	[flat|nested] 67+ messages in thread
* Re: [PATCH] Delete cryptoloop
@ 2004-07-30  8:43 Markku-Juhani O. Saarinen
  0 siblings, 0 replies; 67+ messages in thread
From: Markku-Juhani O. Saarinen @ 2004-07-30  8:43 UTC (permalink / raw)
  To: linux-kernel

Hi,

  My paper, "Encrypted Watermarks and Linux Laptop Security" which
describes the watermark attack recently discussed in "Delete cryptoloop"
thread was earlier this week accepted to WISA 2004 (Korea, Aug 23-25.).
I previously restricted its distribution due to the anonymous
review process. A current draft version is now available from:

  http://www.tcs.hut.fi/~mjos/doc/wisa2004.pdf

  All comments are welcome (on list or private).

Cheers,
- mjos

Markku-Juhani O. Saarinen <mjos@tcs.hut.fi> Helsinki University of Technology

^ permalink raw reply	[flat|nested] 67+ messages in thread
end of thread, other threads:[~2004-08-07 16:32 UTC | newest]

Thread overview: 67+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-07-21 20:16 [PATCH] Delete cryptoloop James Morris
2004-07-21 23:44 ` David S. Miller
2004-07-22  6:00 ` Andrew Morton
2004-07-22  3:30   ` James Morris
2004-07-22  7:43     ` Matthias Urlichs
2004-07-22 14:14       ` H. Peter Anvin
2004-07-22 14:58         ` Jack Lloyd
2004-07-28 20:24     ` David Wagner
2004-07-29  0:27       ` James Morris
2004-07-29 15:50         ` Christophe Saout
2004-07-29 21:15           ` David Wagner
2004-07-30 13:13             ` Christophe Saout
2004-07-31  0:44               ` David Wagner
2004-07-31  2:05                 ` Matt Mackall
2004-07-31 17:29                   ` Marc Ballarin
2004-08-02 22:54                   ` David Wagner
2004-08-02 23:16                     ` James Morris
2004-08-07 16:27                       ` Jean-Luc Cooke
2004-07-22  4:26   ` dpf-lkml
2004-07-22  5:22     ` James Morris
2004-07-22 11:58       ` Paul Rolland
2004-07-22 20:40         ` Martin Schlemmer
2004-07-22  8:46     ` Andrew Morton
2004-07-22  6:13       ` Dale Fountain
2004-07-22  6:47         ` Tim Connors
2004-07-22 15:02           ` Petr Baudis
2004-07-22 11:36         ` Aiko Barz
2004-07-24 15:11           ` Andreas Jellinghaus
2004-07-24 15:53       ` gadgeteer
2004-07-29 16:12       ` Andries Brouwer
2004-07-29 17:23         ` James Morris
2004-07-29 19:48           ` Andries Brouwer
2004-07-22 22:13 ` Bill Davidsen
2004-07-24 12:41 ` Fruhwirth Clemens
2004-07-24 16:52   ` Andrew Morton
2004-07-24 14:08     ` Andreas Henriksson
2004-07-24 19:54       ` Paul Jackson
2004-07-27 20:02     ` Bill Davidsen
2004-07-25 11:42   ` Jari Ruusu
2004-07-25 13:24     ` Fruhwirth Clemens
2004-07-25 15:24       ` Marc Ballarin
2004-07-25 16:57       ` Andreas Jellinghaus
2004-07-25 17:25       ` Jari Ruusu
2004-07-25 18:02         ` Fruhwirth Clemens
2004-07-25 19:09           ` Lee Revell
2004-07-25 19:15             ` Fruhwirth Clemens
2004-07-25 19:44           ` Marc Ballarin
2004-07-25 20:58             ` Fruhwirth Clemens
2004-07-26 10:54           ` Jari Ruusu
2004-07-26 12:45             ` Fruhwirth Clemens
2004-07-26 18:11               ` Jari Ruusu
2004-07-26 22:59                 ` Fruhwirth Clemens
2004-07-26 20:01               ` Matt Mackall
     [not found]                 ` <fa.edslbgp.q763qd@ifi.uio.no>
2004-07-27  8:40                   ` Junio C Hamano
2004-07-27  8:53                     ` Matt Mackall
2004-07-27 10:10                     ` Marc Ballarin
2004-07-26 22:04               ` Marc Ballarin
2004-07-27 19:56   ` Bill Davidsen
     [not found] <2kMAw-rl-15@gated-at.bofh.it>
2004-07-22 19:44 ` Pascal Brisset
  -- strict thread matches above, loose matches on Subject: below --
2004-07-23 10:59 Thomas Habets
     [not found] <2kvT4-5AY-1@gated-at.bofh.it>
     [not found] ` <2kC85-1AH-11@gated-at.bofh.it>
     [not found]   ` <2kDxa-2sB-1@gated-at.bofh.it>
     [not found]     ` <2kECW-3a0-7@gated-at.bofh.it>
2004-07-23 12:34       ` Walter Hofmann
2004-07-23 14:01         ` Kevin Corry
2004-07-23 18:20           ` Christophe Saout
2004-07-27 19:47         ` Bill Davidsen
2004-07-23 12:50 mattia
2004-07-26  7:13 Adam J. Richter
2004-07-30  8:43 Markku-Juhani O. Saarinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox