--- linux-2.6.11.2/kernel/power/swsusp.c.ast 2005-04-10 14:08:55.000000000 +0200
+++ linux-2.6.11.2/kernel/power/swsusp.c 2005-04-11 00:50:45.000000000 +0200
@@ -31,6 +31,9 @@
* Alex Badea <[email protected]>:
* Fixed runaway init
*
+ * Andreas Steinmetz <[email protected]>:
+ * Added encrypted suspend option
+ *
* More state savers are welcome. Especially for the scsi layer...
*
* For TODOs,FIXMEs also look in Documentation/power/swsusp.txt
@@ -72,6 +75,16 @@
#include "power.h"
+#ifdef CONFIG_SWSUSP_ENCRYPT
+#include <linux/random.h>
+#include <linux/crypto.h>
+#include <asm/scatterlist.h>
+#endif
+
+#define CIPHER "aes"
+#define MAXKEY 32
+#define MAXIV 32
+
/* References to section boundaries */
extern const void __nosave_begin, __nosave_end;
@@ -103,15 +116,27 @@
#define SWSUSP_SIG "S1SUSPEND"
-static struct swsusp_header {
- char reserved[PAGE_SIZE - 20 - sizeof(swp_entry_t)];
+struct swsusp_header {
+ char reserved[PAGE_SIZE - 20 - MAXKEY - MAXIV - sizeof(swp_entry_t)];
+ u8 key[MAXKEY];
+ u8 iv[MAXIV];
swp_entry_t swsusp_info;
char orig_sig[10];
char sig[10];
-} __attribute__((packed, aligned(PAGE_SIZE))) swsusp_header;
+} __attribute__((packed, aligned(PAGE_SIZE)));
+
+static union {
+ struct swsusp_header __attribute__((packed, aligned(PAGE_SIZE))) swsusp_header;
+ u8 buffer[PAGE_SIZE] __attribute__((aligned(PAGE_SIZE)));
+} __attribute__((packed, aligned(PAGE_SIZE))) u;
static struct swsusp_info swsusp_info;
+#ifdef CONFIG_SWSUSP_ENCRYPT
+static u8 key[MAXKEY];
+static u8 iv[MAXIV];
+#endif
+
/*
* XXX: We try to keep some more pages free so that I/O operations succeed
* without paging. Might this be more?
@@ -130,22 +155,72 @@
static unsigned short swapfile_used[MAX_SWAPFILES];
static unsigned short root_swap;
+#ifdef CONFIG_SWSUSP_ENCRYPT
+static struct crypto_tfm *crypto_init(int mode)
+{
+ struct crypto_tfm *tfm;
+ int len;
+
+ tfm = crypto_alloc_tfm(CIPHER, CRYPTO_TFM_MODE_CBC);
+ if(!tfm) {
+ printk(KERN_ERR "swsusp: no tfm, suspend not possible\n");
+ return NULL;
+ }
+
+ if(sizeof(key) < crypto_tfm_alg_min_keysize(tfm)) {
+ printk("swsusp: key buffer too small, suspend not possible\n");
+ crypto_free_tfm(tfm);
+ return NULL;
+ }
+
+ if (sizeof(iv) < crypto_tfm_alg_ivsize(tfm)) {
+ printk("swsusp: iv buffer too small, suspend not possible\n");
+ crypto_free_tfm(tfm);
+ return NULL;
+ }
+
+ if (mode) {
+ get_random_bytes(key, MAXKEY);
+ get_random_bytes(iv, MAXIV);
+ }
+
+ len = crypto_tfm_alg_max_keysize(tfm);
+ if (len > sizeof(key))
+ len = sizeof(key);
+
+ if (crypto_cipher_setkey(tfm, key, len)) {
+ printk(KERN_ERR "swsusp: key setup failure, suspend not possible\n");
+ crypto_free_tfm(tfm);
+ return NULL;
+ }
+
+ len = crypto_tfm_alg_blocksize(tfm);
+ crypto_cipher_set_iv(tfm, iv, len);
+
+ return tfm;
+}
+#endif
+
static int mark_swapfiles(swp_entry_t prev)
{
int error;
rw_swap_page_sync(READ,
swp_entry(root_swap, 0),
- virt_to_page((unsigned long)&swsusp_header));
- if (!memcmp("SWAP-SPACE",swsusp_header.sig, 10) ||
- !memcmp("SWAPSPACE2",swsusp_header.sig, 10)) {
- memcpy(swsusp_header.orig_sig,swsusp_header.sig, 10);
- memcpy(swsusp_header.sig,SWSUSP_SIG, 10);
- swsusp_header.swsusp_info = prev;
+ virt_to_page((unsigned long)&u.swsusp_header));
+ if (!memcmp("SWAP-SPACE",u.swsusp_header.sig, 10) ||
+ !memcmp("SWAPSPACE2",u.swsusp_header.sig, 10)) {
+ memcpy(u.swsusp_header.orig_sig,u.swsusp_header.sig, 10);
+ memcpy(u.swsusp_header.sig,SWSUSP_SIG, 10);
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ memcpy(u.swsusp_header.key, key, MAXKEY);
+ memcpy(u.swsusp_header.iv, iv, MAXIV);
+#endif
+ u.swsusp_header.swsusp_info = prev;
error = rw_swap_page_sync(WRITE,
swp_entry(root_swap, 0),
virt_to_page((unsigned long)
- &swsusp_header));
+ &u.swsusp_header));
} else {
pr_debug("swsusp: Partition is not swap space.\n");
error = -ENODEV;
@@ -294,6 +369,19 @@
int error = 0;
int i;
unsigned int mod = nr_copy_pages / 100;
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ struct crypto_tfm *tfm;
+ struct scatterlist src, dst;
+
+ if (!(tfm = crypto_init(1)))
+ return -EINVAL;
+
+ src.offset = 0;
+ src.length = PAGE_SIZE;
+ dst.page = virt_to_page(u.buffer);
+ dst.offset = 0;
+ dst.length = PAGE_SIZE;
+#endif
if (!mod)
mod = 1;
@@ -302,10 +390,22 @@
for (i = 0; i < nr_copy_pages && !error; i++) {
if (!(i%mod))
printk( "\b\b\b\b%3d%%", i / mod );
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ src.page = virt_to_page((pagedir_nosave+i)->address);
+ error = crypto_cipher_encrypt_iv(tfm, &dst, &src,
+ PAGE_SIZE, iv);
+ if (!error)
+ error = write_page((unsigned long)u.buffer,
+ &((pagedir_nosave+i)->swap_address));
+#else
error = write_page((pagedir_nosave+i)->address,
&((pagedir_nosave+i)->swap_address));
+#endif
}
printk("\b\b\b\bdone\n");
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ crypto_free_tfm(tfm);
+#endif
return error;
}
@@ -404,6 +504,10 @@
if ((error = close_swap()))
goto FreePagedir;
Done:
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ memset(key, 0, MAXKEY);
+ memset(iv, 0, MAXIV);
+#endif
return error;
FreePagedir:
free_pagedir_entries();
@@ -1101,7 +1205,7 @@
const char * reason = NULL;
int error;
- if ((error = bio_read_page(swp_offset(swsusp_header.swsusp_info), &swsusp_info)))
+ if ((error = bio_read_page(swp_offset(u.swsusp_header.swsusp_info), &swsusp_info)))
return error;
/* Is this same machine? */
@@ -1118,16 +1222,21 @@
{
int error;
- memset(&swsusp_header, 0, sizeof(swsusp_header));
- if ((error = bio_read_page(0, &swsusp_header)))
+ memset(&u.swsusp_header, 0, sizeof(u.swsusp_header));
+ if ((error = bio_read_page(0, &u.swsusp_header)))
return error;
- if (!memcmp(SWSUSP_SIG, swsusp_header.sig, 10)) {
- memcpy(swsusp_header.sig, swsusp_header.orig_sig, 10);
-
+ if (!memcmp(SWSUSP_SIG, u.swsusp_header.sig, 10)) {
+ memcpy(u.swsusp_header.sig, u.swsusp_header.orig_sig, 10);
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ memcpy(key, u.swsusp_header.key, MAXKEY);
+ memcpy(iv, u.swsusp_header.iv, MAXIV);
+ memset(u.swsusp_header.key, 0, MAXKEY);
+ memset(u.swsusp_header.iv, 0, MAXIV);
+#endif
/*
* Reset swap signature now.
*/
- error = bio_write_page(0, &swsusp_header);
+ error = bio_write_page(0, &u.swsusp_header);
} else {
pr_debug(KERN_ERR "swsusp: Suspend partition has wrong signature?\n");
return -EINVAL;
@@ -1150,6 +1259,18 @@
int error;
int i;
int mod = nr_copy_pages / 100;
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ struct crypto_tfm *tfm;
+ struct scatterlist src, dst;
+
+ if (!(tfm = crypto_init(0)))
+ return -EINVAL;
+
+ src.offset = 0;
+ src.length = PAGE_SIZE;
+ dst.offset = 0;
+ dst.length = PAGE_SIZE;
+#endif
if (!mod)
mod = 1;
@@ -1163,8 +1284,18 @@
printk( "\b\b\b\b%3d%%", i / mod );
error = bio_read_page(swp_offset(p->swap_address),
(void *)p->address);
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ if (!error) {
+ src.page = dst.page = virt_to_page((void *)p->address);
+ error = crypto_cipher_decrypt_iv(tfm, &dst, &src,
+ PAGE_SIZE, iv);
+ }
+#endif
}
printk(" %d done.\n",i);
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ crypto_free_tfm(tfm);
+#endif
return error;
}
@@ -1233,6 +1364,11 @@
} else
error = PTR_ERR(resume_bdev);
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ memset(key, 0, MAXKEY);
+ memset(iv, 0, MAXIV);
+#endif
+
if (!error)
pr_debug("Reading resume file was successful\n");
else
Hi!
> The following patch adds the core functionality for the encrypted
> suspend image.
[Please inline patches, it makes it easier to comment on them.]
You seem to reuse same key/iv for all the blocks. I'm no crypto
expert, but I think that is seriously wrong... You probably should use
block number as a IV or something like that.
Does it slow down swsusp in measurable way, or is it just lost in the
noise?
Pavel
--
Boycott Kodak -- for their patent abuse against Java.
> > The following patch adds the core functionality for the encrypted
> > suspend image.
> [Please inline patches, it makes it easier to comment on them.]
> You seem to reuse same key/iv for all the blocks. I'm no crypto
> expert, but I think that is seriously wrong... You probably should use
> block number as a IV or something like that.
Or use a feedback loop: xor your data with the outcome of the previous
round. And for the initial block use 0x00...00 for 'previous block'-
value.
Folkert van Heusden
Auto te koop, zie: http://www.vanheusden.com/daihatsu.php
Op zoek naar een IT of Finance baan? Mail me voor de mogelijkheden.
--------------------------------------------------------------------
UNIX admin? Then give MultiTail (http://vanheusden.com/multitail/)
a try, it brings monitoring logfiles to a different level! See
http://vanheusden.com/multitail/features.html for a feature-list.
--------------------------------------------------------------------
Phone: +31-6-41278122, PGP-key: 1F28D8AE
Get your PGP/GPG key signed at http://www.biglumber.com!
Hi!
> > > The following patch adds the core functionality for the encrypted
> > > suspend image.
> > [Please inline patches, it makes it easier to comment on them.]
> > You seem to reuse same key/iv for all the blocks. I'm no crypto
> > expert, but I think that is seriously wrong... You probably should use
> > block number as a IV or something like that.
>
> Or use a feedback loop: xor your data with the outcome of the previous
> round. And for the initial block use 0x00...00 for 'previous block'-
> value.
I'd like to retain ability to read suspend image in any order (so that
code can be reused for swap encryption, etc).
Pavel
--
Boycott Kodak -- for their patent abuse against Java.
Hi!
> The following patch adds the core functionality for the encrypted
> suspend image.
+#ifdef CONFIG_SWSUSP_ENCRYPT
+static struct crypto_tfm *crypto_init(int mode)
+{
+ struct crypto_tfm *tfm;
+ int len;
+
+ tfm = crypto_alloc_tfm(CIPHER, CRYPTO_TFM_MODE_CBC);
+ if(!tfm) {
~ please put space between if and (
+ printk(KERN_ERR "swsusp: no tfm, suspend not
possible\n");
+ return NULL;
+ }
+
+ if(sizeof(key) < crypto_tfm_alg_min_keysize(tfm)) {
same here.
Was it really neccessary to include "union u"? I don't like its name,
and perhaps few casts are better than this. If not, it probably should
go in separate patch, and ASAP.
Splitting it to code/kconfig/doc probably does not make much sense, it
is small enough, already.
Pavel
--
Boycott Kodak -- for their patent abuse against Java.
> > > > The following patch adds the core functionality for the encrypted
> > > > suspend image.
> > > [Please inline patches, it makes it easier to comment on them.]
> > > You seem to reuse same key/iv for all the blocks. I'm no crypto
> > > expert, but I think that is seriously wrong... You probably should use
> > > block number as a IV or something like that.
> > Or use a feedback loop: xor your data with the outcome of the previous
> > round. And for the initial block use 0x00...00 for 'previous block'-
> > value.
> I'd like to retain ability to read suspend image in any order (so that
> code can be reused for swap encryption, etc).
In that case: encrypt the blocknumber with the key, and then use the
outcome as IV for the encryption of the data. Or calculate a hash over the
blocknumber and use the outcome of that as IV. Don't use the blocknumer
directly.
Folkert van Heusden
Auto te koop, zie: http://www.vanheusden.com/daihatsu.php
Op zoek naar een IT of Finance baan? Mail me voor de mogelijkheden.
--------------------------------------------------------------------
UNIX admin? Then give MultiTail (http://vanheusden.com/multitail/)
a try, it brings monitoring logfiles to a different level! See
http://vanheusden.com/multitail/features.html for a feature-list.
--------------------------------------------------------------------
Phone: +31-6-41278122, PGP-key: 1F28D8AE
Get your PGP/GPG key signed at http://www.biglumber.com!
[email protected] wrote:
>>>The following patch adds the core functionality for the encrypted
>>>suspend image.
>>
>>[Please inline patches, it makes it easier to comment on them.]
Aiyeeh - good ole Mozilla tends to reformat things when inlining...
>>You seem to reuse same key/iv for all the blocks. I'm no crypto
>>expert, but I think that is seriously wrong... You probably should use
>>block number as a IV or something like that.
>
>
> Or use a feedback loop: xor your data with the outcome of the previous
> round. And for the initial block use 0x00...00 for 'previous block'-
> value.
I'm already using cipher block chaining, look for CRYPTO_TFM_MODE_CBC in
swsusp.c. You may want to have a look at cbc_process in crypto/cipher.c.
Thus using the same key is ok. The only known drawback is a watermarking
"attack" but this can only used to look for the existence of specially
crafted files which are not stored on disk during software suspend.
I should, however, use crypto_cipher_en/decrypt instead of
crypto_cipher_en/decrypt_iv as I actually wanted to use the iv in the
tfm I did set up with crypto_cipher_set_iv instead of the local copy.
Going to fix that.
--
Andreas Steinmetz SPAMmers use [email protected]
Pavel Machek wrote:
> Hi!
>
>
>>The following patch adds the core functionality for the encrypted
>>suspend image.
>
>
> +#ifdef CONFIG_SWSUSP_ENCRYPT
> +static struct crypto_tfm *crypto_init(int mode)
> +{
> + struct crypto_tfm *tfm;
> + int len;
> +
> + tfm = crypto_alloc_tfm(CIPHER, CRYPTO_TFM_MODE_CBC);
> + if(!tfm) {
> ~ please put space between if and (
>
> + printk(KERN_ERR "swsusp: no tfm, suspend not
> possible\n");
> + return NULL;
> + }
> +
> + if(sizeof(key) < crypto_tfm_alg_min_keysize(tfm)) {
>
> same here.
>
> Was it really neccessary to include "union u"? I don't like its name,
> and perhaps few casts are better than this. If not, it probably should
> go in separate patch, and ASAP.
I'll revert this and use few casts.
--
Andreas Steinmetz SPAMmers use [email protected]
--- linux-2.6.11.2/kernel/power/swsusp.c.ast 2005-04-10 14:08:55.000000000 +0200
+++ linux-2.6.11.2/kernel/power/swsusp.c 2005-04-11 18:05:58.000000000 +0200
@@ -31,6 +31,9 @@
* Alex Badea <[email protected]>:
* Fixed runaway init
*
+ * Andreas Steinmetz <[email protected]>:
+ * Added encrypted suspend option
+ *
* More state savers are welcome. Especially for the scsi layer...
*
* For TODOs,FIXMEs also look in Documentation/power/swsusp.txt
@@ -72,6 +75,16 @@
#include "power.h"
+#ifdef CONFIG_SWSUSP_ENCRYPT
+#include <linux/random.h>
+#include <linux/crypto.h>
+#include <asm/scatterlist.h>
+#endif
+
+#define CIPHER "aes"
+#define MAXKEY 32
+#define MAXIV 32
+
/* References to section boundaries */
extern const void __nosave_begin, __nosave_end;
@@ -104,7 +117,9 @@
#define SWSUSP_SIG "S1SUSPEND"
static struct swsusp_header {
- char reserved[PAGE_SIZE - 20 - sizeof(swp_entry_t)];
+ char reserved[PAGE_SIZE - 20 - MAXKEY - MAXIV - sizeof(swp_entry_t)];
+ u8 key[MAXKEY];
+ u8 iv[MAXIV];
swp_entry_t swsusp_info;
char orig_sig[10];
char sig[10];
@@ -112,6 +127,11 @@
static struct swsusp_info swsusp_info;
+#ifdef CONFIG_SWSUSP_ENCRYPT
+static u8 key[MAXKEY];
+static u8 iv[MAXIV];
+#endif
+
/*
* XXX: We try to keep some more pages free so that I/O operations succeed
* without paging. Might this be more?
@@ -130,6 +150,52 @@
static unsigned short swapfile_used[MAX_SWAPFILES];
static unsigned short root_swap;
+#ifdef CONFIG_SWSUSP_ENCRYPT
+static struct crypto_tfm *crypto_init(int mode)
+{
+ struct crypto_tfm *tfm;
+ int len;
+
+ tfm = crypto_alloc_tfm(CIPHER, CRYPTO_TFM_MODE_CBC);
+ if(!tfm) {
+ printk(KERN_ERR "swsusp: no tfm, suspend not possible\n");
+ return NULL;
+ }
+
+ if(sizeof(key) < crypto_tfm_alg_min_keysize(tfm)) {
+ printk("swsusp: key buffer too small, suspend not possible\n");
+ crypto_free_tfm(tfm);
+ return NULL;
+ }
+
+ if (sizeof(iv) < crypto_tfm_alg_ivsize(tfm)) {
+ printk("swsusp: iv buffer too small, suspend not possible\n");
+ crypto_free_tfm(tfm);
+ return NULL;
+ }
+
+ if (mode) {
+ get_random_bytes(key, MAXKEY);
+ get_random_bytes(iv, MAXIV);
+ }
+
+ len = crypto_tfm_alg_max_keysize(tfm);
+ if (len > sizeof(key))
+ len = sizeof(key);
+
+ if (crypto_cipher_setkey(tfm, key, len)) {
+ printk(KERN_ERR "swsusp: key setup failure, suspend not possible\n");
+ crypto_free_tfm(tfm);
+ return NULL;
+ }
+
+ len = crypto_tfm_alg_blocksize(tfm);
+ crypto_cipher_set_iv(tfm, iv, len);
+
+ return tfm;
+}
+#endif
+
static int mark_swapfiles(swp_entry_t prev)
{
int error;
@@ -141,6 +207,10 @@
!memcmp("SWAPSPACE2",swsusp_header.sig, 10)) {
memcpy(swsusp_header.orig_sig,swsusp_header.sig, 10);
memcpy(swsusp_header.sig,SWSUSP_SIG, 10);
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ memcpy(swsusp_header.key, key, MAXKEY);
+ memcpy(swsusp_header.iv, iv, MAXIV);
+#endif
swsusp_header.swsusp_info = prev;
error = rw_swap_page_sync(WRITE,
swp_entry(root_swap, 0),
@@ -294,6 +364,19 @@
int error = 0;
int i;
unsigned int mod = nr_copy_pages / 100;
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ struct crypto_tfm *tfm;
+ struct scatterlist src, dst;
+
+ if (!(tfm = crypto_init(1)))
+ return -EINVAL;
+
+ src.offset = 0;
+ src.length = PAGE_SIZE;
+ dst.page = virt_to_page((void *)&swsusp_header);
+ dst.offset = 0;
+ dst.length = PAGE_SIZE;
+#endif
if (!mod)
mod = 1;
@@ -302,10 +385,21 @@
for (i = 0; i < nr_copy_pages && !error; i++) {
if (!(i%mod))
printk( "\b\b\b\b%3d%%", i / mod );
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ src.page = virt_to_page((pagedir_nosave+i)->address);
+ error = crypto_cipher_encrypt(tfm, &dst, &src, PAGE_SIZE);
+ if (!error)
+ error = write_page((unsigned long)&swsusp_header,
+ &((pagedir_nosave+i)->swap_address));
+#else
error = write_page((pagedir_nosave+i)->address,
&((pagedir_nosave+i)->swap_address));
+#endif
}
printk("\b\b\b\bdone\n");
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ crypto_free_tfm(tfm);
+#endif
return error;
}
@@ -404,6 +498,10 @@
if ((error = close_swap()))
goto FreePagedir;
Done:
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ memset(key, 0, MAXKEY);
+ memset(iv, 0, MAXIV);
+#endif
return error;
FreePagedir:
free_pagedir_entries();
@@ -1124,6 +1222,12 @@
if (!memcmp(SWSUSP_SIG, swsusp_header.sig, 10)) {
memcpy(swsusp_header.sig, swsusp_header.orig_sig, 10);
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ memcpy(key, swsusp_header.key, MAXKEY);
+ memcpy(iv, swsusp_header.iv, MAXIV);
+ memset(swsusp_header.key, 0, MAXKEY);
+ memset(swsusp_header.iv, 0, MAXIV);
+#endif
/*
* Reset swap signature now.
*/
@@ -1150,6 +1254,18 @@
int error;
int i;
int mod = nr_copy_pages / 100;
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ struct crypto_tfm *tfm;
+ struct scatterlist src, dst;
+
+ if (!(tfm = crypto_init(0)))
+ return -EINVAL;
+
+ src.offset = 0;
+ src.length = PAGE_SIZE;
+ dst.offset = 0;
+ dst.length = PAGE_SIZE;
+#endif
if (!mod)
mod = 1;
@@ -1163,8 +1279,18 @@
printk( "\b\b\b\b%3d%%", i / mod );
error = bio_read_page(swp_offset(p->swap_address),
(void *)p->address);
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ if (!error) {
+ src.page = dst.page = virt_to_page((void *)p->address);
+ error = crypto_cipher_decrypt(tfm, &dst, &src,
+ PAGE_SIZE);
+ }
+#endif
}
printk(" %d done.\n",i);
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ crypto_free_tfm(tfm);
+#endif
return error;
}
@@ -1233,6 +1359,11 @@
} else
error = PTR_ERR(resume_bdev);
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ memset(key, 0, MAXKEY);
+ memset(iv, 0, MAXIV);
+#endif
+
if (!error)
pr_debug("Reading resume file was successful\n");
else
Pavel Machek wrote:
> I'd like to retain ability to read suspend image in any order (so that
> code can be reused for swap encryption, etc).
> Pavel
This is not possible with cipher block chaining as used right now. One
would have to use a non-random iv set needs to set for every page. And
this leads to exactly the same problem why dm-crypt now offers the
'essiv' mode. I don't know if a random access feature is worth this
effort as sequential disk access (sequential write, sequential read) is
usally the fastest method anyway.
For regular swap encryption I do hope that the initrd feature of swsup2
will eventually find its way into the mainline kernel. This way you can
have an external key for dm-crypt to access the encrypted swap partition.
dm-crypt thus would guard the system during suspend/poweroff while the
encrypted suspend image guards against data gathering after
resume/reboot (the latter when mkswap is used).
--
Andreas Steinmetz SPAMmers use [email protected]
Hi!
> > I'd like to retain ability to read suspend image in any order (so that
> > code can be reused for swap encryption, etc).
>
> This is not possible with cipher block chaining as used right now. One
> would have to use a non-random iv set needs to set for every page. And
> this leads to exactly the same problem why dm-crypt now offers the
> 'essiv' mode. I don't know if a random access feature is worth this
> effort as sequential disk access (sequential write, sequential read) is
> usally the fastest method anyway.
I thought I'd just reuse your code for automagic swap encryption. Oh
well.
> For regular swap encryption I do hope that the initrd feature of swsup2
> will eventually find its way into the mainline kernel. This way you can
> have an external key for dm-crypt to access the encrypted swap
> partition.
Check -mm kernel, swsusp1 can now do resume from initrd.
Pavel
--
Boycott Kodak -- for their patent abuse against Java.
Hi,
On Monday, 11 of April 2005 18:11, Andreas Steinmetz wrote:
> Pavel Machek wrote:
> > Was it really neccessary to include "union u"? I don't like its name,
>
> Here comes the patch with this reverted. I'm now using casts when
> 'abusing' the space for encryption. Furthermore the iv set up in the tfm
> is used instead of the local copy.
I had no time to review your patch earlier, sorry. I'm inlining it so that I can
comment it:
> --- linux-2.6.11.2/kernel/power/swsusp.c.ast 2005-04-10 14:08:55.000000000 +0200
> +++ linux-2.6.11.2/kernel/power/swsusp.c 2005-04-11 18:05:58.000000000 +0200
> @@ -31,6 +31,9 @@
> * Alex Badea <[email protected]>:
> * Fixed runaway init
> *
> + * Andreas Steinmetz <[email protected]>:
> + * Added encrypted suspend option
> + *
> * More state savers are welcome. Especially for the scsi layer...
> *
> * For TODOs,FIXMEs also look in Documentation/power/swsusp.txt
> @@ -72,6 +75,16 @@
>
> #include "power.h"
>
> +#ifdef CONFIG_SWSUSP_ENCRYPT
> +#include <linux/random.h>
> +#include <linux/crypto.h>
> +#include <asm/scatterlist.h>
> +#endif
> +
> +#define CIPHER "aes"
> +#define MAXKEY 32
> +#define MAXIV 32
Why not to put these definitions under #ifdef?
> +
> /* References to section boundaries */
> extern const void __nosave_begin, __nosave_end;
>
> @@ -104,7 +117,9 @@
> #define SWSUSP_SIG "S1SUSPEND"
>
> static struct swsusp_header {
> - char reserved[PAGE_SIZE - 20 - sizeof(swp_entry_t)];
I would add #ifdef here as well.
> + char reserved[PAGE_SIZE - 20 - MAXKEY - MAXIV - sizeof(swp_entry_t)];
> + u8 key[MAXKEY];
> + u8 iv[MAXIV];
> swp_entry_t swsusp_info;
> char orig_sig[10];
> char sig[10];
> @@ -112,6 +127,11 @@
>
> static struct swsusp_info swsusp_info;
>
> +#ifdef CONFIG_SWSUSP_ENCRYPT
> +static u8 key[MAXKEY];
> +static u8 iv[MAXIV];
> +#endif
> +
> /*
> * XXX: We try to keep some more pages free so that I/O operations succeed
> * without paging. Might this be more?
> @@ -130,6 +150,52 @@
> static unsigned short swapfile_used[MAX_SWAPFILES];
> static unsigned short root_swap;
>
> +#ifdef CONFIG_SWSUSP_ENCRYPT
> +static struct crypto_tfm *crypto_init(int mode)
I think it's better if this function returns an int error code and the
messages are printed where it's called from. This way, the essential
part of the code would be easier to grasp (Pavel?).
> +{
> + struct crypto_tfm *tfm;
> + int len;
> +
> + tfm = crypto_alloc_tfm(CIPHER, CRYPTO_TFM_MODE_CBC);
> + if(!tfm) {
> + printk(KERN_ERR "swsusp: no tfm, suspend not possible\n");
> + return NULL;
> + }
> +
> + if(sizeof(key) < crypto_tfm_alg_min_keysize(tfm)) {
> + printk("swsusp: key buffer too small, suspend not possible\n");
> + crypto_free_tfm(tfm);
> + return NULL;
> + }
> +
> + if (sizeof(iv) < crypto_tfm_alg_ivsize(tfm)) {
> + printk("swsusp: iv buffer too small, suspend not possible\n");
> + crypto_free_tfm(tfm);
> + return NULL;
> + }
> +
> + if (mode) {
> + get_random_bytes(key, MAXKEY);
I hope you realize that this may give you a sequence of bits that you should
not use as a key ...
> + get_random_bytes(iv, MAXIV);
> + }
> +
> + len = crypto_tfm_alg_max_keysize(tfm);
You have used this value earlier. Why don't you initialize len at that time?
> + if (len > sizeof(key))
> + len = sizeof(key);
> +
> + if (crypto_cipher_setkey(tfm, key, len)) {
> + printk(KERN_ERR "swsusp: key setup failure, suspend not possible\n");
> + crypto_free_tfm(tfm);
On any error, you call crypto_free_tfm(tfm) and return. I would use a common
error handling code, like that:
if (error)
goto Error;
/* some code */
Error:
crypto_free_tfm(tfm);
return error;
> + return NULL;
> + }
> +
> + len = crypto_tfm_alg_blocksize(tfm);
> + crypto_cipher_set_iv(tfm, iv, len);
I would use crypto_tfm_alg_blocksize(tfm) here directly (shorter code).
> +
> + return tfm;
> +}
> +#endif
> +
> static int mark_swapfiles(swp_entry_t prev)
> {
> int error;
> @@ -141,6 +207,10 @@
If you used -p while making diffs, it would be easier to find out where the
changes actually started.
> !memcmp("SWAPSPACE2",swsusp_header.sig, 10)) {
> memcpy(swsusp_header.orig_sig,swsusp_header.sig, 10);
> memcpy(swsusp_header.sig,SWSUSP_SIG, 10);
> +#ifdef CONFIG_SWSUSP_ENCRYPT
> + memcpy(swsusp_header.key, key, MAXKEY);
> + memcpy(swsusp_header.iv, iv, MAXIV);
> +#endif
> swsusp_header.swsusp_info = prev;
> error = rw_swap_page_sync(WRITE,
> swp_entry(root_swap, 0),
> @@ -294,6 +364,19 @@
This change will not apply to the current swsusp.c (eg as in 2.6.12-rc2).
> int error = 0;
> int i;
> unsigned int mod = nr_copy_pages / 100;
> +#ifdef CONFIG_SWSUSP_ENCRYPT
> + struct crypto_tfm *tfm;
> + struct scatterlist src, dst;
> +
> + if (!(tfm = crypto_init(1)))
> + return -EINVAL;
It's not necessarily -EINVAL, I think. It's better to return different
error codes on different error conditions.
> +
> + src.offset = 0;
> + src.length = PAGE_SIZE;
> + dst.page = virt_to_page((void *)&swsusp_header);
> + dst.offset = 0;
> + dst.length = PAGE_SIZE;
> +#endif
>
> if (!mod)
> mod = 1;
> @@ -302,10 +385,21 @@
This change will not apply to the current swsusp.c (eg as in 2.6.12-rc2).
> for (i = 0; i < nr_copy_pages && !error; i++) {
> if (!(i%mod))
> printk( "\b\b\b\b%3d%%", i / mod );
> +#ifdef CONFIG_SWSUSP_ENCRYPT
> + src.page = virt_to_page((pagedir_nosave+i)->address);
> + error = crypto_cipher_encrypt(tfm, &dst, &src, PAGE_SIZE);
> + if (!error)
> + error = write_page((unsigned long)&swsusp_header,
> + &((pagedir_nosave+i)->swap_address));
I wouldn't use swsusp_header directly in this statement. It's a bit confusing.
> +#else
> error = write_page((pagedir_nosave+i)->address,
> &((pagedir_nosave+i)->swap_address));
> +#endif
> }
> printk("\b\b\b\bdone\n");
> +#ifdef CONFIG_SWSUSP_ENCRYPT
> + crypto_free_tfm(tfm);
> +#endif
> return error;
> }
>
> @@ -404,6 +498,10 @@
> if ((error = close_swap()))
> goto FreePagedir;
> Done:
> +#ifdef CONFIG_SWSUSP_ENCRYPT
> + memset(key, 0, MAXKEY);
> + memset(iv, 0, MAXIV);
> +#endif
> return error;
> FreePagedir:
> free_pagedir_entries();
> @@ -1124,6 +1222,12 @@
> if (!memcmp(SWSUSP_SIG, swsusp_header.sig, 10)) {
> memcpy(swsusp_header.sig, swsusp_header.orig_sig, 10);
>
> +#ifdef CONFIG_SWSUSP_ENCRYPT
> + memcpy(key, swsusp_header.key, MAXKEY);
> + memcpy(iv, swsusp_header.iv, MAXIV);
> + memset(swsusp_header.key, 0, MAXKEY);
> + memset(swsusp_header.iv, 0, MAXIV);
> +#endif
> /*
> * Reset swap signature now.
> */
> @@ -1150,6 +1254,18 @@
This change will not apply to the current swsusp.c (eg as in 2.6.12-rc2).
> int error;
> int i;
> int mod = nr_copy_pages / 100;
> +#ifdef CONFIG_SWSUSP_ENCRYPT
> + struct crypto_tfm *tfm;
> + struct scatterlist src, dst;
> +
> + if (!(tfm = crypto_init(0)))
> + return -EINVAL;
Same as in data_write() (ie not necessarily -EINVAL).
> +
> + src.offset = 0;
> + src.length = PAGE_SIZE;
> + dst.offset = 0;
> + dst.length = PAGE_SIZE;
> +#endif
>
> if (!mod)
> mod = 1;
> @@ -1163,8 +1279,18 @@
> printk( "\b\b\b\b%3d%%", i / mod );
> error = bio_read_page(swp_offset(p->swap_address),
> (void *)p->address);
> +#ifdef CONFIG_SWSUSP_ENCRYPT
> + if (!error) {
> + src.page = dst.page = virt_to_page((void *)p->address);
> + error = crypto_cipher_decrypt(tfm, &dst, &src,
> + PAGE_SIZE);
> + }
> +#endif
> }
> printk(" %d done.\n",i);
> +#ifdef CONFIG_SWSUSP_ENCRYPT
> + crypto_free_tfm(tfm);
> +#endif
> return error;
>
> }
> @@ -1233,6 +1359,11 @@
> } else
> error = PTR_ERR(resume_bdev);
>
> +#ifdef CONFIG_SWSUSP_ENCRYPT
> + memset(key, 0, MAXKEY);
> + memset(iv, 0, MAXIV);
> +#endif
> +
> if (!error)
> pr_debug("Reading resume file was successful\n");
> else
Greets,
Rafael
--
- Would you tell me, please, which way I ought to go from here?
- That depends a good deal on where you want to get to.
-- Lewis Carroll "Alice's Adventures in Wonderland"
Hi!
> I had no time to review your patch earlier, sorry. I'm inlining it so that I can
> comment it:
> > @@ -72,6 +75,16 @@
> >
> > #include "power.h"
> >
> > +#ifdef CONFIG_SWSUSP_ENCRYPT
> > +#include <linux/random.h>
> > +#include <linux/crypto.h>
> > +#include <asm/scatterlist.h>
> > +#endif
> > +
> > +#define CIPHER "aes"
> > +#define MAXKEY 32
> > +#define MAXIV 32
>
> Why not to put these definitions under #ifdef?
>
> > +
> > /* References to section boundaries */
> > extern const void __nosave_begin, __nosave_end;
> >
> > @@ -104,7 +117,9 @@
> > #define SWSUSP_SIG "S1SUSPEND"
> >
> > static struct swsusp_header {
> > - char reserved[PAGE_SIZE - 20 - sizeof(swp_entry_t)];
>
> I would add #ifdef here as well.
I think avoiding both ifdefs is actually right thing to do. Keep the
ifdef noise down.
> > + char reserved[PAGE_SIZE - 20 - MAXKEY - MAXIV - sizeof(swp_entry_t)];
> > + u8 key[MAXKEY];
> > + u8 iv[MAXIV];
> > swp_entry_t swsusp_info;
> > char orig_sig[10];
> > char sig[10];
> > @@ -112,6 +127,11 @@
> >
> > static struct swsusp_info swsusp_info;
> >
> > +#ifdef CONFIG_SWSUSP_ENCRYPT
> > +static u8 key[MAXKEY];
> > +static u8 iv[MAXIV];
> > +#endif
> > +
> > /*
> > * XXX: We try to keep some more pages free so that I/O operations succeed
> > * without paging. Might this be more?
> > @@ -130,6 +150,52 @@
> > static unsigned short swapfile_used[MAX_SWAPFILES];
> > static unsigned short root_swap;
> >
> > +#ifdef CONFIG_SWSUSP_ENCRYPT
> > +static struct crypto_tfm *crypto_init(int mode)
>
> I think it's better if this function returns an int error code and the
> messages are printed where it's called from. This way, the essential
> part of the code would be easier to grasp (Pavel?).
Agreed. Actually I do not care where messages are printed, but
returning different code for different errors seems right.
Pavel
--
Boycott Kodak -- for their patent abuse against Java.
Hi,
On Monday, 11 of April 2005 23:08, Pavel Machek wrote:
> Hi!
>
]--snip--[
> > > @@ -130,6 +150,52 @@
> > > static unsigned short swapfile_used[MAX_SWAPFILES];
> > > static unsigned short root_swap;
> > >
> > > +#ifdef CONFIG_SWSUSP_ENCRYPT
> > > +static struct crypto_tfm *crypto_init(int mode)
> >
> > I think it's better if this function returns an int error code and the
> > messages are printed where it's called from. This way, the essential
> > part of the code would be easier to grasp (Pavel?).
>
> Agreed. Actually I do not care where messages are printed, but
> returning different code for different errors seems right.
Hm. You probably don't want suspend-related messages to be printed during
resume (this function is called in two different places)? :-)
Rafael
--
- Would you tell me, please, which way I ought to go from here?
- That depends a good deal on where you want to get to.
-- Lewis Carroll "Alice's Adventures in Wonderland"
Rafael J. Wysocki wrote:
> Hi,
>
> On Monday, 11 of April 2005 23:08, Pavel Machek wrote:
>
>>Hi!
>>
>
> ]--snip--[
>
>>>>@@ -130,6 +150,52 @@
>>>> static unsigned short swapfile_used[MAX_SWAPFILES];
>>>> static unsigned short root_swap;
>>>>
>>>>+#ifdef CONFIG_SWSUSP_ENCRYPT
>>>>+static struct crypto_tfm *crypto_init(int mode)
>>>
>>>I think it's better if this function returns an int error code and the
>>>messages are printed where it's called from. This way, the essential
>>>part of the code would be easier to grasp (Pavel?).
>>
>>Agreed. Actually I do not care where messages are printed, but
>>returning different code for different errors seems right.
>
>
> Hm. You probably don't want suspend-related messages to be printed during
> resume (this function is called in two different places)? :-)
>
> Rafael
>
>
Will be changed.
--
Andreas Steinmetz SPAMmers use [email protected]
Rafael J. Wysocki wrote:
> Hi,
>
> On Monday, 11 of April 2005 18:11, Andreas Steinmetz wrote:
>
>>Pavel Machek wrote:
>>
>>>Was it really neccessary to include "union u"? I don't like its name,
>>
>>Here comes the patch with this reverted. I'm now using casts when
>>'abusing' the space for encryption. Furthermore the iv set up in the tfm
>>is used instead of the local copy.
>
>
> I had no time to review your patch earlier, sorry. I'm inlining it so that I can
> comment it:
>
>
>>--- linux-2.6.11.2/kernel/power/swsusp.c.ast 2005-04-10 14:08:55.000000000 +0200
>>+++ linux-2.6.11.2/kernel/power/swsusp.c 2005-04-11 18:05:58.000000000 +0200
>>@@ -31,6 +31,9 @@
>> * Alex Badea <[email protected]>:
>> * Fixed runaway init
>> *
>>+ * Andreas Steinmetz <[email protected]>:
>>+ * Added encrypted suspend option
>>+ *
>> * More state savers are welcome. Especially for the scsi layer...
>> *
>> * For TODOs,FIXMEs also look in Documentation/power/swsusp.txt
>>@@ -72,6 +75,16 @@
>>
>> #include "power.h"
>>
>>+#ifdef CONFIG_SWSUSP_ENCRYPT
>>+#include <linux/random.h>
>>+#include <linux/crypto.h>
>>+#include <asm/scatterlist.h>
>>+#endif
>>+
>>+#define CIPHER "aes"
>>+#define MAXKEY 32
>>+#define MAXIV 32
>
>
> Why not to put these definitions under #ifdef?
>
I keep it the way Pavel likes it here if you don't mind.
>
>>+
>> /* References to section boundaries */
>> extern const void __nosave_begin, __nosave_end;
>>
>>@@ -104,7 +117,9 @@
>> #define SWSUSP_SIG "S1SUSPEND"
>>
>> static struct swsusp_header {
>>- char reserved[PAGE_SIZE - 20 - sizeof(swp_entry_t)];
>
>
> I would add #ifdef here as well.
Same as above.
>
>
>>+ char reserved[PAGE_SIZE - 20 - MAXKEY - MAXIV - sizeof(swp_entry_t)];
>>+ u8 key[MAXKEY];
>>+ u8 iv[MAXIV];
>> swp_entry_t swsusp_info;
>> char orig_sig[10];
>> char sig[10];
>>@@ -112,6 +127,11 @@
>>
>> static struct swsusp_info swsusp_info;
>>
>>+#ifdef CONFIG_SWSUSP_ENCRYPT
>>+static u8 key[MAXKEY];
>>+static u8 iv[MAXIV];
>>+#endif
>>+
>> /*
>> * XXX: We try to keep some more pages free so that I/O operations succeed
>> * without paging. Might this be more?
>>@@ -130,6 +150,52 @@
>> static unsigned short swapfile_used[MAX_SWAPFILES];
>> static unsigned short root_swap;
>>
>>+#ifdef CONFIG_SWSUSP_ENCRYPT
>>+static struct crypto_tfm *crypto_init(int mode)
>
>
> I think it's better if this function returns an int error code and the
> messages are printed where it's called from. This way, the essential
> part of the code would be easier to grasp (Pavel?).
>
Pavel doesn't mind printing the errors here as far as I can see. The
messages will be adapted to suspend/resume stae.
>
>>+{
>>+ struct crypto_tfm *tfm;
>>+ int len;
>>+
>>+ tfm = crypto_alloc_tfm(CIPHER, CRYPTO_TFM_MODE_CBC);
>>+ if(!tfm) {
>>+ printk(KERN_ERR "swsusp: no tfm, suspend not possible\n");
>>+ return NULL;
>>+ }
>>+
>>+ if(sizeof(key) < crypto_tfm_alg_min_keysize(tfm)) {
>>+ printk("swsusp: key buffer too small, suspend not possible\n");
>>+ crypto_free_tfm(tfm);
>>+ return NULL;
>>+ }
>>+
>>+ if (sizeof(iv) < crypto_tfm_alg_ivsize(tfm)) {
>>+ printk("swsusp: iv buffer too small, suspend not possible\n");
>>+ crypto_free_tfm(tfm);
>>+ return NULL;
>>+ }
>>+
>>+ if (mode) {
>>+ get_random_bytes(key, MAXKEY);
>
>
> I hope you realize that this may give you a sequence of bits that you should
> not use as a key ...
I don't get what you mean here. As far as I know aes has no weak keys.
The only danger I can see here is that get_get_random_bytes returns a
consecutive sequence of zeroes (or some other constant value) on every
invocation. Otherwise a sequence of zeroes is just one of 2^256 possible
keys. I prefer to keep the key space as wide open as possible.
Please let me know if you did mean something different.
>
>
>>+ get_random_bytes(iv, MAXIV);
>>+ }
>>+
>>+ len = crypto_tfm_alg_max_keysize(tfm);
>
>
> You have used this value earlier. Why don't you initialize len at that time?
>
Not correct. Earlieron it is crypto_tfm_alg_min_keysize().
^^^
>
>>+ if (len > sizeof(key))
>>+ len = sizeof(key);
>>+
>>+ if (crypto_cipher_setkey(tfm, key, len)) {
>>+ printk(KERN_ERR "swsusp: key setup failure, suspend not possible\n");
>>+ crypto_free_tfm(tfm);
>
>
> On any error, you call crypto_free_tfm(tfm) and return. I would use a common
> error handling code, like that:
>
> if (error)
> goto Error;
> /* some code */
> Error:
> crypto_free_tfm(tfm);
> return error;
>
Will do.
>
>>+ return NULL;
>>+ }
>>+
>>+ len = crypto_tfm_alg_blocksize(tfm);
>>+ crypto_cipher_set_iv(tfm, iv, len);
>
>
> I would use crypto_tfm_alg_blocksize(tfm) here directly (shorter code).
>
I reworked this a bit. New patch will follow later.
>
>>+
>>+ return tfm;
>>+}
>>+#endif
>>+
>> static int mark_swapfiles(swp_entry_t prev)
>> {
>> int error;
>>@@ -141,6 +207,10 @@
>
>
> If you used -p while making diffs, it would be easier to find out where the
> changes actually started.
>
I'll try to remember.
>
>> !memcmp("SWAPSPACE2",swsusp_header.sig, 10)) {
>> memcpy(swsusp_header.orig_sig,swsusp_header.sig, 10);
>> memcpy(swsusp_header.sig,SWSUSP_SIG, 10);
>>+#ifdef CONFIG_SWSUSP_ENCRYPT
>>+ memcpy(swsusp_header.key, key, MAXKEY);
>>+ memcpy(swsusp_header.iv, iv, MAXIV);
>>+#endif
>> swsusp_header.swsusp_info = prev;
>> error = rw_swap_page_sync(WRITE,
>> swp_entry(root_swap, 0),
>>@@ -294,6 +364,19 @@
>
>
> This change will not apply to the current swsusp.c (eg as in 2.6.12-rc2).
>
Going to have a look at 2.6.12-rc2 later today.
>
>> int error = 0;
>> int i;
>> unsigned int mod = nr_copy_pages / 100;
>>+#ifdef CONFIG_SWSUSP_ENCRYPT
>>+ struct crypto_tfm *tfm;
>>+ struct scatterlist src, dst;
>>+
>>+ if (!(tfm = crypto_init(1)))
>>+ return -EINVAL;
>
>
> It's not necessarily -EINVAL, I think. It's better to return different
> error codes on different error conditions.
>
Different error codes are on their way.
>
>>+
>>+ src.offset = 0;
>>+ src.length = PAGE_SIZE;
>>+ dst.page = virt_to_page((void *)&swsusp_header);
>>+ dst.offset = 0;
>>+ dst.length = PAGE_SIZE;
>>+#endif
>>
>> if (!mod)
>> mod = 1;
>>@@ -302,10 +385,21 @@
>
>
> This change will not apply to the current swsusp.c (eg as in 2.6.12-rc2).
>
As above.
>
>> for (i = 0; i < nr_copy_pages && !error; i++) {
>> if (!(i%mod))
>> printk( "\b\b\b\b%3d%%", i / mod );
>>+#ifdef CONFIG_SWSUSP_ENCRYPT
>>+ src.page = virt_to_page((pagedir_nosave+i)->address);
>>+ error = crypto_cipher_encrypt(tfm, &dst, &src, PAGE_SIZE);
>>+ if (!error)
>>+ error = write_page((unsigned long)&swsusp_header,
>>+ &((pagedir_nosave+i)->swap_address));
>
>
> I wouldn't use swsusp_header directly in this statement. It's a bit confusing.
>
Hmm, I tried to solve this with a union which Pavel didn't like. An
extra buffer doesn't make sense here as it would be a lost page. Any
ideas? BTW, I need to use a buffer here as when swsusp fails later the
pages would have been encrypted in-place...
>
>>+#else
>> error = write_page((pagedir_nosave+i)->address,
>> &((pagedir_nosave+i)->swap_address));
>>+#endif
>> }
>> printk("\b\b\b\bdone\n");
>>+#ifdef CONFIG_SWSUSP_ENCRYPT
>>+ crypto_free_tfm(tfm);
>>+#endif
>> return error;
>> }
>>
>>@@ -404,6 +498,10 @@
>> if ((error = close_swap()))
>> goto FreePagedir;
>> Done:
>>+#ifdef CONFIG_SWSUSP_ENCRYPT
>>+ memset(key, 0, MAXKEY);
>>+ memset(iv, 0, MAXIV);
>>+#endif
>> return error;
>> FreePagedir:
>> free_pagedir_entries();
>>@@ -1124,6 +1222,12 @@
>> if (!memcmp(SWSUSP_SIG, swsusp_header.sig, 10)) {
>> memcpy(swsusp_header.sig, swsusp_header.orig_sig, 10);
>>
>>+#ifdef CONFIG_SWSUSP_ENCRYPT
>>+ memcpy(key, swsusp_header.key, MAXKEY);
>>+ memcpy(iv, swsusp_header.iv, MAXIV);
>>+ memset(swsusp_header.key, 0, MAXKEY);
>>+ memset(swsusp_header.iv, 0, MAXIV);
>>+#endif
>> /*
>> * Reset swap signature now.
>> */
>>@@ -1150,6 +1254,18 @@
>
>
> This change will not apply to the current swsusp.c (eg as in 2.6.12-rc2).
>
As above.
>
>> int error;
>> int i;
>> int mod = nr_copy_pages / 100;
>>+#ifdef CONFIG_SWSUSP_ENCRYPT
>>+ struct crypto_tfm *tfm;
>>+ struct scatterlist src, dst;
>>+
>>+ if (!(tfm = crypto_init(0)))
>>+ return -EINVAL;
>
>
> Same as in data_write() (ie not necessarily -EINVAL).
>
As above, consider this done.
>
>>+
>>+ src.offset = 0;
>>+ src.length = PAGE_SIZE;
>>+ dst.offset = 0;
>>+ dst.length = PAGE_SIZE;
>>+#endif
>>
>> if (!mod)
>> mod = 1;
>>@@ -1163,8 +1279,18 @@
>> printk( "\b\b\b\b%3d%%", i / mod );
>> error = bio_read_page(swp_offset(p->swap_address),
>> (void *)p->address);
>>+#ifdef CONFIG_SWSUSP_ENCRYPT
>>+ if (!error) {
>>+ src.page = dst.page = virt_to_page((void *)p->address);
>>+ error = crypto_cipher_decrypt(tfm, &dst, &src,
>>+ PAGE_SIZE);
>>+ }
>>+#endif
>> }
>> printk(" %d done.\n",i);
>>+#ifdef CONFIG_SWSUSP_ENCRYPT
>>+ crypto_free_tfm(tfm);
>>+#endif
>> return error;
>>
>> }
>>@@ -1233,6 +1359,11 @@
>> } else
>> error = PTR_ERR(resume_bdev);
>>
>>+#ifdef CONFIG_SWSUSP_ENCRYPT
>>+ memset(key, 0, MAXKEY);
>>+ memset(iv, 0, MAXIV);
>>+#endif
>>+
>> if (!error)
>> pr_debug("Reading resume file was successful\n");
>> else
>
>
> Greets,
> Rafael
>
>
--
Andreas Steinmetz SPAMmers use [email protected]
--- linux-2.6.12-rc2/kernel/power/swsusp.c.ast 2005-04-12 13:20:41.000000000 +0200
+++ linux-2.6.12-rc2/kernel/power/swsusp.c 2005-04-12 14:20:41.000000000 +0200
@@ -31,6 +31,9 @@
* Alex Badea <[email protected]>:
* Fixed runaway init
*
+ * Andreas Steinmetz <[email protected]>:
+ * Added encrypted suspend option
+ *
* More state savers are welcome. Especially for the scsi layer...
*
* For TODOs,FIXMEs also look in Documentation/power/swsusp.txt
@@ -72,6 +75,16 @@
#include "power.h"
+#ifdef CONFIG_SWSUSP_ENCRYPT
+#include <linux/random.h>
+#include <linux/crypto.h>
+#include <asm/scatterlist.h>
+#endif
+
+#define CIPHER "aes"
+#define MAXKEY 32
+#define MAXIV 32
+
/* References to section boundaries */
extern const void __nosave_begin, __nosave_end;
@@ -102,7 +115,9 @@ static suspend_pagedir_t *pagedir_save;
#define SWSUSP_SIG "S1SUSPEND"
static struct swsusp_header {
- char reserved[PAGE_SIZE - 20 - sizeof(swp_entry_t)];
+ char reserved[PAGE_SIZE - 20 - MAXKEY - MAXIV - sizeof(swp_entry_t)];
+ u8 key[MAXKEY];
+ u8 iv[MAXIV];
swp_entry_t swsusp_info;
char orig_sig[10];
char sig[10];
@@ -110,6 +125,11 @@ static struct swsusp_header {
static struct swsusp_info swsusp_info;
+#ifdef CONFIG_SWSUSP_ENCRYPT
+static u8 key[MAXKEY];
+static u8 iv[MAXIV];
+#endif
+
/*
* XXX: We try to keep some more pages free so that I/O operations succeed
* without paging. Might this be more?
@@ -128,6 +148,60 @@ static struct swsusp_info swsusp_info;
static unsigned short swapfile_used[MAX_SWAPFILES];
static unsigned short root_swap;
+#ifdef CONFIG_SWSUSP_ENCRYPT
+static int crypto_init(int mode, struct crypto_tfm **tfm)
+{
+ char *modemsg;
+ int len;
+ int error = 0;
+
+ modemsg = mode ? "suspend not possible" : "resume not possible";
+
+ *tfm = crypto_alloc_tfm(CIPHER, CRYPTO_TFM_MODE_CBC);
+ if(!*tfm) {
+ printk(KERN_ERR "swsusp: no tfm, %s\n", modemsg);
+ error = -EINVAL;
+ goto out;
+ }
+
+ if(sizeof(key) < crypto_tfm_alg_min_keysize(*tfm)) {
+ printk(KERN_ERR "swsusp: key buffer too small, %s\n", modemsg);
+ error = -ENOKEY;
+ goto fail;
+ }
+
+ if (mode) {
+ get_random_bytes(key, MAXKEY);
+ get_random_bytes(iv, MAXIV);
+ }
+
+ len = crypto_tfm_alg_max_keysize(*tfm);
+ if (len > sizeof(key))
+ len = sizeof(key);
+
+ if (crypto_cipher_setkey(*tfm, key, len)) {
+ printk(KERN_ERR "swsusp: key setup failure, %s\n", modemsg);
+ error = -EKEYREJECTED;
+ goto fail;
+ }
+
+ len = crypto_tfm_alg_ivsize(*tfm);
+
+ if (sizeof(iv) < len) {
+ printk(KERN_ERR "swsusp: iv buffer too small, %s\n", modemsg);
+ error = -EOVERFLOW;
+ goto fail;
+ }
+
+ crypto_cipher_set_iv(*tfm, iv, len);
+
+ goto out;
+
+fail: crypto_free_tfm(*tfm);
+out: return error;
+}
+#endif
+
static int mark_swapfiles(swp_entry_t prev)
{
int error;
@@ -139,6 +213,10 @@ static int mark_swapfiles(swp_entry_t pr
!memcmp("SWAPSPACE2",swsusp_header.sig, 10)) {
memcpy(swsusp_header.orig_sig,swsusp_header.sig, 10);
memcpy(swsusp_header.sig,SWSUSP_SIG, 10);
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ memcpy(swsusp_header.key, key, MAXKEY);
+ memcpy(swsusp_header.iv, iv, MAXIV);
+#endif
swsusp_header.swsusp_info = prev;
error = rw_swap_page_sync(WRITE,
swp_entry(root_swap, 0),
@@ -285,6 +363,19 @@ static int data_write(void)
int error = 0, i = 0;
unsigned int mod = nr_copy_pages / 100;
struct pbe *p;
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ struct crypto_tfm *tfm;
+ struct scatterlist src, dst;
+
+ if ((error = crypto_init(1, &tfm)))
+ return error;
+
+ src.offset = 0;
+ src.length = PAGE_SIZE;
+ dst.page = virt_to_page((void *)&swsusp_header);
+ dst.offset = 0;
+ dst.length = PAGE_SIZE;
+#endif
if (!mod)
mod = 1;
@@ -293,11 +384,22 @@ static int data_write(void)
for_each_pbe(p, pagedir_nosave) {
if (!(i%mod))
printk( "\b\b\b\b%3d%%", i / mod );
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ src.page = virt_to_page(p->address);
+ error = crypto_cipher_encrypt(tfm, &dst, &src, PAGE_SIZE);
+ if (!error)
+ error = write_page((unsigned long)&swsusp_header,
+ &(p->swap_address));
+#else
if ((error = write_page(p->address, &(p->swap_address))))
return error;
+#endif
i++;
}
printk("\b\b\b\bdone\n");
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ crypto_free_tfm(tfm);
+#endif
return error;
}
@@ -399,6 +501,10 @@ static int write_suspend_image(void)
if ((error = close_swap()))
goto FreePagedir;
Done:
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ memset(key, 0, MAXKEY);
+ memset(iv, 0, MAXIV);
+#endif
return error;
FreePagedir:
free_pagedir_entries();
@@ -1226,6 +1332,12 @@ static int check_sig(void)
if (!memcmp(SWSUSP_SIG, swsusp_header.sig, 10)) {
memcpy(swsusp_header.sig, swsusp_header.orig_sig, 10);
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ memcpy(key, swsusp_header.key, MAXKEY);
+ memcpy(iv, swsusp_header.iv, MAXIV);
+ memset(swsusp_header.key, 0, MAXKEY);
+ memset(swsusp_header.iv, 0, MAXIV);
+#endif
/*
* Reset swap signature now.
*/
@@ -1252,6 +1364,18 @@ static int data_read(struct pbe *pblist)
int error = 0;
int i = 0;
int mod = swsusp_info.image_pages / 100;
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ struct crypto_tfm *tfm;
+ struct scatterlist src, dst;
+
+ if ((error = crypto_init(0, &tfm)))
+ return error;
+
+ src.offset = 0;
+ src.length = PAGE_SIZE;
+ dst.offset = 0;
+ dst.length = PAGE_SIZE;
+#endif
if (!mod)
mod = 1;
@@ -1265,12 +1389,27 @@ static int data_read(struct pbe *pblist)
error = bio_read_page(swp_offset(p->swap_address),
(void *)p->address);
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ if (!error) {
+ src.page = dst.page = virt_to_page((void *)p->address);
+ error = crypto_cipher_decrypt(tfm, &dst, &src,
+ PAGE_SIZE);
+ }
+ if (error) {
+ crypto_free_tfm(tfm);
+ return error;
+ }
+#else
if (error)
return error;
+#endif
i++;
}
printk("\b\b\b\bdone\n");
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ crypto_free_tfm(tfm);
+#endif
return error;
}
@@ -1411,6 +1550,11 @@ int swsusp_read(void)
error = read_suspend_image();
blkdev_put(resume_bdev);
+#ifdef CONFIG_SWSUSP_ENCRYPT
+ memset(key, 0, MAXKEY);
+ memset(iv, 0, MAXIV);
+#endif
+
if (!error)
pr_debug("swsusp: Reading resume file was successful\n");
else
Andreas Steinmetz <[email protected]> wrote:
>
> Here comes the next incarnation, this time against 2.6.12rc2.
> Unfortunately only compile tested as 2.6.12rc2 happily oopses away
> (vanilla from kernel.org, oops already sent to lkml).
>
> Please let me know if you want any further changes.
What's wrong with using swap over dmcrypt + initramfs? People have
already used that to do encrypted swsusp.
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Herbert Xu wrote:
> What's wrong with using swap over dmcrypt + initramfs? People have
> already used that to do encrypted swsusp.
Nothing. The problem is the fact that after resume there is then
unencrypted(*) data on disk that should never have been there, e.g.
dm-crypt keys, ssh keys, ...
This may lead to nasty surprises that can occur after weeks or months.
(*) unencrypted from the point of view of the running system.
--
Andreas Steinmetz SPAMmers use [email protected]
Hi!
> Here comes the next incarnation, this time against 2.6.12rc2.
> Unfortunately only compile tested as 2.6.12rc2 happily oopses away
> (vanilla from kernel.org, oops already sent to lkml).
>
> Please let me know if you want any further changes.
Applied (it is *not* going to make it into 2.6.12, and not sure about
2.6.13, but it is in my local tree now. You had Kconfig and docs
changes, too, can you retransmit them?
Pavel
--
Boycott Kodak -- for their patent abuse against Java.
--- linux-2.6.11.2/Documentation/power/swsusp.txt.ast 2005-04-10 21:07:01.000000000 +0200
+++ linux-2.6.11.2/Documentation/power/swsusp.txt 2005-04-10 21:10:56.000000000 +0200
@@ -30,6 +30,13 @@
echo platform > /sys/power/disk; echo disk > /sys/power/state
+Encrypted suspend image:
+------------------------
+If you want to store your suspend image encrypted with a temporary
+key to prevent data gathering after resume you must compile
+crypto and the aes algorithm into the kernel - modules won't work
+as they cannot be loaded at resume time.
+
Article about goals and implementation of Software Suspend for Linux
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
On Wed, Apr 13, 2005 at 02:59:28PM +0200, Andreas Steinmetz wrote:
> Herbert Xu wrote:
> > What's wrong with using swap over dmcrypt + initramfs? People have
> > already used that to do encrypted swsusp.
>
> Nothing. The problem is the fact that after resume there is then
> unencrypted(*) data on disk that should never have been there, e.g.
> dm-crypt keys, ssh keys, ...
Why is that? In the case of swap over dmcrypt, swsusp never reads/writes
the disk directly. All operations are done through dmcrypt.
The user has to enter a password before the system can be resumed.
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Herbert Xu wrote:
> On Wed, Apr 13, 2005 at 02:59:28PM +0200, Andreas Steinmetz wrote:
>
>>Herbert Xu wrote:
>>
>>>What's wrong with using swap over dmcrypt + initramfs? People have
>>>already used that to do encrypted swsusp.
>>
>>Nothing. The problem is the fact that after resume there is then
>>unencrypted(*) data on disk that should never have been there, e.g.
>>dm-crypt keys, ssh keys, ...
>
>
> Why is that? In the case of swap over dmcrypt, swsusp never reads/writes
> the disk directly. All operations are done through dmcrypt.
>
> The user has to enter a password before the system can be resumed.
Think of it the following way: user suspend and later resumes. During
suspend some mlocked memory e.g. from ssh-agent gets dumped to swap.
Some days later the system gets broken in from a remote place.
Unfortunately the ssh keys are still on swap (assuming that ssh-agent is
not running then) and can be recovered by the intruder. The intruder can
then gain access to other sites with the data recovered from the earlier
suspend.
You see, it is not a matter of having encrypted swap, what matters here
is what suspend needs to write to swap and this can be stuff that does
not belong there so it needs to be erased after resume. And the easiest
way to do this is to encrypt the suspend image with a temporary key that
gets cleared on resume.
If you don't resume mkswap will also clear the key though i have to
admit that there's a small window then in which the key and data are
still accessible.
--
Andreas Steinmetz SPAMmers use [email protected]
On Thu, Apr 14, 2005 at 12:29:36AM +0200, Andreas Steinmetz wrote:
>
> > Why is that? In the case of swap over dmcrypt, swsusp never reads/writes
> > the disk directly. All operations are done through dmcrypt.
> >
> > The user has to enter a password before the system can be resumed.
>
> Think of it the following way: user suspend and later resumes. During
> suspend some mlocked memory e.g. from ssh-agent gets dumped to swap.
> Some days later the system gets broken in from a remote place.
> Unfortunately the ssh keys are still on swap (assuming that ssh-agent is
> not running then) and can be recovered by the intruder. The intruder can
The ssh keys are *encrypted* in the swap when dmcrypt is used.
When the swap runs over dmcrypt all writes including those from
swsusp are encrypted.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Čt 14-04-05 09:10:44, Herbert Xu wrote:
> On Thu, Apr 14, 2005 at 12:29:36AM +0200, Andreas Steinmetz wrote:
> >
> > > Why is that? In the case of swap over dmcrypt, swsusp never reads/writes
> > > the disk directly. All operations are done through dmcrypt.
> > >
> > > The user has to enter a password before the system can be resumed.
> >
> > Think of it the following way: user suspend and later resumes. During
> > suspend some mlocked memory e.g. from ssh-agent gets dumped to swap.
> > Some days later the system gets broken in from a remote place.
> > Unfortunately the ssh keys are still on swap (assuming that ssh-agent is
> > not running then) and can be recovered by the intruder. The intruder can
>
> The ssh keys are *encrypted* in the swap when dmcrypt is used.
> When the swap runs over dmcrypt all writes including those from
> swsusp are encrypted.
Andreas is right. They are encrypted in swap, but they should not be
there at all. And they are encrypted by key that is still available
after resume. Bad.
First version simply overwrote suspend image in swap with zeros. This
is more clever way to do same thing.
Pavel
--
Boycott Kodak -- for their patent abuse against Java.
On Thu, Apr 14, 2005 at 01:24:31AM +0200, Pavel Machek wrote:
>
> > The ssh keys are *encrypted* in the swap when dmcrypt is used.
> > When the swap runs over dmcrypt all writes including those from
> > swsusp are encrypted.
>
> Andreas is right. They are encrypted in swap, but they should not be
> there at all. And they are encrypted by key that is still available
> after resume. Bad.
The dmcrypt swap can only be unlocked by the user with a passphrase,
which is analogous to how you unlock your ssh private key stored
on the disk using a passphrase.
So I don't see the problem.
> First version simply overwrote suspend image in swap with zeros. This
> is more clever way to do same thing.
This version looks to me like a custom implementation of dmcrypt that
lives inside swsusp which ends up being either less secure than simply
using dmcrypt due to the lack of a passphrase, or it's going to involve
more hacks to get a passphrase from the user at resume time.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Čt 14-04-05 09:39:04, Herbert Xu wrote:
> On Thu, Apr 14, 2005 at 01:24:31AM +0200, Pavel Machek wrote:
> >
> > > The ssh keys are *encrypted* in the swap when dmcrypt is used.
> > > When the swap runs over dmcrypt all writes including those from
> > > swsusp are encrypted.
> >
> > Andreas is right. They are encrypted in swap, but they should not be
> > there at all. And they are encrypted by key that is still available
> > after resume. Bad.
>
> The dmcrypt swap can only be unlocked by the user with a passphrase,
> which is analogous to how you unlock your ssh private key stored
> on the disk using a passphrase.
Once more:
Andreas' implementation destroys the key during resume.
dm-crypt does not even know resume happened, so it can't destroy
key. (And it would also render system useless).
Pavel
--
Boycott Kodak -- for their patent abuse against Java.
On Thu, Apr 14, 2005 at 01:46:02AM +0200, Pavel Machek wrote:
> On ??t 14-04-05 09:39:04, Herbert Xu wrote:
> > On Thu, Apr 14, 2005 at 01:24:31AM +0200, Pavel Machek wrote:
> > >
> > > > The ssh keys are *encrypted* in the swap when dmcrypt is used.
> > > > When the swap runs over dmcrypt all writes including those from
> > > > swsusp are encrypted.
> > >
> > > Andreas is right. They are encrypted in swap, but they should not be
> > > there at all. And they are encrypted by key that is still available
> > > after resume. Bad.
> >
> > The dmcrypt swap can only be unlocked by the user with a passphrase,
> > which is analogous to how you unlock your ssh private key stored
> > on the disk using a passphrase.
>
> Once more:
>
> Andreas' implementation destroys the key during resume.
This solution is all wrong.
If you want security of the suspend image while "suspended", encrypt
with dm-crypt. If you want security of the swap image after resume,
zero out the portion of swap used. If you want both, do both.
You could even just zero out those regions which were mlocked at
suspend. If it wasn't mlocked, it might be on swap already anyway.
Re-implementing dm-crypt for this purpose is ridiculous.
--
Mathematics is the supreme nostalgia of our time.
In article <[email protected]> you wrote:
> The ssh keys are *encrypted* in the swap when dmcrypt is used.
> When the swap runs over dmcrypt all writes including those from
> swsusp are encrypted.
The problem is that after an resume the running system has access to the
swap, because the key is recoverd. And the left over hybernation data in the
swap can be read by the running kernel because the swap key is restored.
This is not an attack against a notebook in hybernaion, but an attack
against a running system with non-wiped bybernation leftovers - not critical
to me but an possible point of interest.
Gruss
Bernd
In article <[email protected]> you wrote:
> The dmcrypt swap can only be unlocked by the user with a passphrase,
> which is analogous to how you unlock your ssh private key stored
> on the disk using a passphrase.
We talk about the unlocked system getting hacked. However I am not why the
hacker would head for the swap if he can as well read the ram.
Gruss
Bernd
Hi!
> > > > > The ssh keys are *encrypted* in the swap when dmcrypt is used.
> > > > > When the swap runs over dmcrypt all writes including those from
> > > > > swsusp are encrypted.
> > > >
> > > > Andreas is right. They are encrypted in swap, but they should not be
> > > > there at all. And they are encrypted by key that is still available
> > > > after resume. Bad.
> > >
> > > The dmcrypt swap can only be unlocked by the user with a passphrase,
> > > which is analogous to how you unlock your ssh private key stored
> > > on the disk using a passphrase.
> >
> > Once more:
> >
> > Andreas' implementation destroys the key during resume.
>
> This solution is all wrong.
>
> If you want security of the suspend image while "suspended", encrypt
> with dm-crypt. If you want security of the swap image after resume,
> zero out the portion of swap used. If you want both, do both.
I want security of the swap image, and "just zeroing" is hard to do in
failed suspend case, see previous discussion.
Andreas, do you think you could write nice, long, FAQ entries so that
we don't have to go through this discussion over and over?
Pavel
--
Boycott Kodak -- for their patent abuse against Java.
On Thu, Apr 14, 2005 at 08:51:25AM +0200, Pavel Machek wrote:
>
> > This solution is all wrong.
> >
> > If you want security of the suspend image while "suspended", encrypt
> > with dm-crypt. If you want security of the swap image after resume,
> > zero out the portion of swap used. If you want both, do both.
Pavel, you're not answering our questions.
How is the proposed patch any more secure compared to swsusp over dmcrypt?
In fact if anything it is less secure. If I understand correctly the
proposal is to store the key used to encrypt the swsusp image in the
swap device. This means that anybody who gains access to the swap
device can trivially decrypt it.
Compare this to the properly setup dmcrypt case where the swap
device can only be decrypted with a passphrase obtained from the
user at resume time.
> I want security of the swap image, and "just zeroing" is hard to do in
> failed suspend case, see previous discussion.
As to the failed suspend case, the two approaches yield identical
results. In both cases we will be storing potentially sensitive
data encrypted on a physical storage device.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Čt 14-04-05 03:13:41, Bernd Eckenfels wrote:
> In article <[email protected]> you wrote:
> > The dmcrypt swap can only be unlocked by the user with a passphrase,
> > which is analogous to how you unlock your ssh private key stored
> > on the disk using a passphrase.
>
> We talk about the unlocked system getting hacked. However I am not why the
> hacker would head for the swap if he can as well read the ram.
Various openssl-s, ssh-s and others are pretty carefull to wipe their
RAM when it is no longer neccessary.
Pavel
--
Boycott Kodak -- for their patent abuse against Java.
On Thu, Apr 14, 2005 at 09:39:04AM +1000, Herbert Xu wrote:
> > Andreas is right. They are encrypted in swap, but they should not be
> > there at all. And they are encrypted by key that is still available
> > after resume. Bad.
>
> The dmcrypt swap can only be unlocked by the user with a passphrase,
> which is analogous to how you unlock your ssh private key stored
> on the disk using a passphrase.
Does dmcrypt have a simple operation mode like OpenBSD's encrypted swap?
I want to be able to 'sysctl -w kernel.swap_crypt=1' and get
* pages are encrypted as they're written to swap
* the key is automatically regenerated every 2 hours (or whatever); as
pages encrypted under the old key age out, it can be freed eventually
* I don't have to manage keys, choose algorithms, futz with /etc/fstab,
figure out complex configs for /dev/loopN, etc
In fact, I want swap_crypt=1 to be the default (you can turn it off for
embedded systems if you want), but giving me an easy knob is a good
start for 2.6.
Everything I've seen makes me think that configuring dmcrypt is hard.
Don't make users make useless choices. Sure, there are cases where the
complexity is warranted, but please make the 90% solution easy.
-andy
On Thu, Apr 14, 2005 at 01:31:19AM -0700, Andy Isaacson wrote:
>
> Does dmcrypt have a simple operation mode like OpenBSD's encrypted swap?
> I want to be able to 'sysctl -w kernel.swap_crypt=1' and get
It's not that easy however the distros can make it so that it's as
simple as running one command to enable/disable it.
> * pages are encrypted as they're written to swap
Yep.
> * the key is automatically regenerated every 2 hours (or whatever); as
> pages encrypted under the old key age out, it can be freed eventually
This'll require changes to dmcrypt but it's doable.
> * I don't have to manage keys, choose algorithms, futz with /etc/fstab,
> figure out complex configs for /dev/loopN, etc
You don't need to worry about keys unless you want to read from the swap
after a reboot, i.e., swsusp.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
>
> > * the key is automatically regenerated every 2 hours (or whatever); as
> > pages encrypted under the old key age out, it can be freed eventually
>
> This'll require changes to dmcrypt but it's doable.
but it's not useful since linux actually generally keeps the pages also
in swap even when they're back in memory (so that if there's memory
pressure again they can just be swapped out cheap again), this leads to
very very long lived swap pages
Hi,
On Thursday, 14 of April 2005 10:08, Herbert Xu wrote:
> On Thu, Apr 14, 2005 at 08:51:25AM +0200, Pavel Machek wrote:
> >
> > > This solution is all wrong.
> > >
> > > If you want security of the suspend image while "suspended", encrypt
> > > with dm-crypt. If you want security of the swap image after resume,
> > > zero out the portion of swap used. If you want both, do both.
>
> Pavel, you're not answering our questions.
>
> How is the proposed patch any more secure compared to swsusp over dmcrypt?
It is for different purpose. It is to prevent swsusp from leaving a readable
memory snapshot on the disk _after_ resume, even if the resume has _failed_
(ie if you encrypt the image during suspend and then destroy the key after
reading the image during resume, you don't need to zero out the swap partition,
which you can't do anyway if the resume has failed). IOW, please treat it as
a more sophisticated method of zeroing out the swap partition. :-)
Arguably, using dm-crypt is more secure, but it is also more complicated from
the Joe User POV. IMHO we should not force users to set up dm-crypt, remember
passwords etc., to get some basic security.
Greets,
Rafael
--
- Would you tell me, please, which way I ought to go from here?
- That depends a good deal on where you want to get to.
-- Lewis Carroll "Alice's Adventures in Wonderland"
On Čt 14-04-05 18:08:37, Herbert Xu wrote:
> On Thu, Apr 14, 2005 at 08:51:25AM +0200, Pavel Machek wrote:
> >
> > > This solution is all wrong.
> > >
> > > If you want security of the suspend image while "suspended", encrypt
> > > with dm-crypt. If you want security of the swap image after resume,
> > > zero out the portion of swap used. If you want both, do both.
>
> Pavel, you're not answering our questions.
>
> How is the proposed patch any more secure compared to swsusp over
> dmcrypt?
It is not "more secure". It solves completely different problem.
> In fact if anything it is less secure. If I understand correctly the
> proposal is to store the key used to encrypt the swsusp image in the
> swap device. This means that anybody who gains access to the swap
> device can trivially decrypt it.
Yes. It also means that key is gone after resume.
> Compare this to the properly setup dmcrypt case where the swap
> device can only be decrypted with a passphrase obtained from the
> user at resume time.
Solution above does not require passphrase (so users will actually use
it) and dmcrypt with passphrase does not destroy the key after resume,
so data can still be recovered.
They are orthogonal. You want both.
If something is still unclear, we can talk on irc somewhere, if you
agree to write FAQ entry afterwards ;-).
Pavel
--
Boycott Kodak -- for their patent abuse against Java.
Andy Isaacson <[email protected]> wrote:
> * the key is automatically regenerated every 2 hours (or whatever); as
> pages encrypted under the old key age out, it can be freed eventually
Changing the key would not help, since if you can get the swap pages on
a running system, you can also get the keys, and if you are using a limited
set of keys (you obviously want that), using more than one key will just add
a small linear factor to cracking the whole swap data of an offline system.
--
Field experience is something you don't get until just after you need it.
Fri?, Spammer: [email protected] [email protected] [email protected]
[email protected] [email protected] [email protected]
On Thu, Apr 14, 2005 at 11:04:39AM +0200, Rafael J. Wysocki wrote:
> Hi,
>
> On Thursday, 14 of April 2005 10:08, Herbert Xu wrote:
> > On Thu, Apr 14, 2005 at 08:51:25AM +0200, Pavel Machek wrote:
> > >
> > > > This solution is all wrong.
> > > >
> > > > If you want security of the suspend image while "suspended", encrypt
> > > > with dm-crypt. If you want security of the swap image after resume,
> > > > zero out the portion of swap used. If you want both, do both.
> >
> > Pavel, you're not answering our questions.
> >
> > How is the proposed patch any more secure compared to swsusp over dmcrypt?
>
> It is for different purpose. It is to prevent swsusp from leaving a readable
> memory snapshot on the disk _after_ resume, even if the resume has _failed_
> (ie if you encrypt the image during suspend and then destroy the key after
> reading the image during resume, you don't need to zero out the swap partition,
> which you can't do anyway if the resume has failed). IOW, please treat it as
> a more sophisticated method of zeroing out the swap partition. :-)
What is this resume failed case?
If it means the machine has crashed during resume, then so what? The
key is not on disk in the clear -ever- in the dm-crypt case. If the
attacker gets to poke around in the memory contents of the crashed
machine for the key (or the partially loaded suspend image).
If it means we fell back to a normal boot, normal boot can simply dd
over the swap at boot, generate a new ephemeral swap key, or whatever.
> Arguably, using dm-crypt is more secure, but it is also more
> complicated from the Joe User POV. IMHO we should not force users to
> set up dm-crypt, remember passwords etc., to get some basic
> security.
Any sensible solution here is going to require remembering passwords.
And arguably anywhere the user needs encrypted suspend, they'll want
encrypted swap as well.
--
Mathematics is the supreme nostalgia of our time.
Matt Mackall wrote:
> Any sensible solution here is going to require remembering passwords.
> And arguably anywhere the user needs encrypted suspend, they'll want
> encrypted swap as well.
But after entering the password and resuming, the encrypted swap is
accessible again and my ssh-key may be lying around in it, right?
So we would need to zero out the suspend image in swap to prevent the
retrieval of this data from the running machine (imagine a
remote-root-hole).
Zeroing out the suspend image means "write lots of megabytes to the
disk" which takes a lot of time.
The "encrypted suspend" case avoids this. It is absolutely useless for
the "machine is stolen while suspended" case, since the key for
decrypting the suspend image is stored in the suspend header (but
destroyed during resume).
We need both:
- encrypted swap for the "stolen while suspended" case
- encrypted suspend for "broken into after resume while still running"
case.
i hope this helps...
Stefan
--
seife
Never trust a computer you can't lift.
On Thu, Apr 14, 2005 at 09:27:22PM +0200, Stefan Seyfried wrote:
> Matt Mackall wrote:
>
> > Any sensible solution here is going to require remembering passwords.
> > And arguably anywhere the user needs encrypted suspend, they'll want
> > encrypted swap as well.
>
> But after entering the password and resuming, the encrypted swap is
> accessible again and my ssh-key may be lying around in it, right?
No. Because it's been zeroed in the resume process.
> So we would need to zero out the suspend image in swap to prevent the
> retrieval of this data from the running machine (imagine a
> remote-root-hole).
>
> Zeroing out the suspend image means "write lots of megabytes to the
> disk" which takes a lot of time.
Zero only the mlocked regions. This should take essentially no time at
all. Swsusp knows which these are because they have to be mlocked
after resume as well. If it's not mlocked, it's liable to be swapped
out anyway.
Again:
Use dm-crypt swap with password prompt to handle "stolen while
suspended"
Zero out mlocked regions on resume for "stolen while running".
Reinitialize swap or use a different swap session keys for "booting
without resume".
There are more refinements here, like generating session keys per boot
for swap. We want to do this anyway. We can encrypt the session key
with the user password for suspend purposes. Booting without resume
loses the old (encrypted) session key.
But this is all solvable without resorting to yet another encrypted
block device scheme. Such a scheme shouldn't even be considered until
all the other options are thoroughly explored. Any scheme that's
encrypting the suspend image and then storing the key in the clear is
either obviously broken or obviously doesn't actually need encryption.
--
Mathematics is the supreme nostalgia of our time.
Hi!
> > Arguably, using dm-crypt is more secure, but it is also more
> > complicated from the Joe User POV. IMHO we should not force users to
> > set up dm-crypt, remember passwords etc., to get some basic
> > security.
>
> Any sensible solution here is going to require remembering passwords.
Andreas has sensible good enough for preventing leaks from mlocked
data, and surprise, it does not need remembering anything.
Pavel
--
Boycott Kodak -- for their patent abuse against Java.
Hi!
> > So we would need to zero out the suspend image in swap to prevent the
> > retrieval of this data from the running machine (imagine a
> > remote-root-hole).
> >
> > Zeroing out the suspend image means "write lots of megabytes to the
> > disk" which takes a lot of time.
>
> Zero only the mlocked regions. This should take essentially no time at
> all. Swsusp knows which these are because they have to be mlocked
I believe this is tricky to implement. You are free to produce patch,
and if that patch is nicer/simpler than Anreas's code, I may consider
it.
> But this is all solvable without resorting to yet another encrypted
> block device scheme. Such a scheme shouldn't even be considered until
> all the other options are thoroughly explored. Any scheme that's
> encrypting the suspend image and then storing the key in the clear is
> either obviously broken or obviously doesn't actually need encryption.
Andreas solved real problem. You are waving hands. Obviously your next
mail should contain a patch.
Pavel
--
Boycott Kodak -- for their patent abuse against Java.
On Thu, Apr 14, 2005 at 12:53:52PM -0700, Matt Mackall wrote:
> On Thu, Apr 14, 2005 at 09:27:22PM +0200, Stefan Seyfried wrote:
> > Matt Mackall wrote:
> > > Any sensible solution here is going to require remembering passwords.
> > > And arguably anywhere the user needs encrypted suspend, they'll want
> > > encrypted swap as well.
> >
> > But after entering the password and resuming, the encrypted swap is
> > accessible again and my ssh-key may be lying around in it, right?
>
> No. Because it's been zeroed in the resume process.
Zeroing the entire swsusp region is a big job, especially if you want to
do it in a FIPS-conformant manner. Hard to test that you've done it
right and not missed any bits.
Much much easier to securely erase just the key storage.
And unless I'm missing something, you meant to say "it will have been
zeroed" (after the patch under discussion is merged), right? The
current state of the art (as of 2.6.12-rc2) is that mlocked regions are
written to the swsusp region and may linger there indefinitely after
resume.
(I haven't read the code, that's just my understanding of the
discussion.)
> > Zeroing out the suspend image means "write lots of megabytes to the
> > disk" which takes a lot of time.
>
> Zero only the mlocked regions. This should take essentially no time at
> all. Swsusp knows which these are because they have to be mlocked
> after resume as well. If it's not mlocked, it's liable to be swapped
> out anyway.
Ooof, that sounds complicated and error-prone, and likely to break
silently (a la input entropy gathering).
-andy
On Thu, Apr 14, 2005 at 10:18:12PM +0200, Pavel Machek wrote:
> Hi!
>
> > > So we would need to zero out the suspend image in swap to prevent the
> > > retrieval of this data from the running machine (imagine a
> > > remote-root-hole).
> > >
> > > Zeroing out the suspend image means "write lots of megabytes to the
> > > disk" which takes a lot of time.
> >
> > Zero only the mlocked regions. This should take essentially no time at
> > all. Swsusp knows which these are because they have to be mlocked
>
> I believe this is tricky to implement. You are free to produce patch,
> and if that patch is nicer/simpler than Anreas's code, I may consider
> it.
If I understand swsusp correctly, we can simply set a bit in the pbe
struct to indicate that it's a locked page.
This can be done by walking the vma list attached to the page's
address space with vma_prio_tree_foreach() and checking the
vma->vm_flags with VM_LOCKED. Analogous to what the swapout code does.
We can either do this in data_write() or preferably higher
(copy_data?) when we have the pfn handy. The lock bit can be stashed
in bit 0 of pbe->address, among other places. Then in data_read, we
check the bit and zero the source.
As I'm not about to actually use swsusp any time soon, someone else is
invited to implement the above. Should take about 10-20 lines.
--
Mathematics is the supreme nostalgia of our time.
On Thu, Apr 14, 2005 at 03:11:53PM -0700, Andy Isaacson wrote:
> On Thu, Apr 14, 2005 at 12:53:52PM -0700, Matt Mackall wrote:
> > On Thu, Apr 14, 2005 at 09:27:22PM +0200, Stefan Seyfried wrote:
> > > Matt Mackall wrote:
> > > > Any sensible solution here is going to require remembering passwords.
> > > > And arguably anywhere the user needs encrypted suspend, they'll want
> > > > encrypted swap as well.
> > >
> > > But after entering the password and resuming, the encrypted swap is
> > > accessible again and my ssh-key may be lying around in it, right?
> >
> > No. Because it's been zeroed in the resume process.
>
> Zeroing the entire swsusp region is a big job, especially if you want to
> do it in a FIPS-conformant manner. Hard to test that you've done it
> right and not missed any bits.
>
> Much much easier to securely erase just the key storage.
>
> And unless I'm missing something, you meant to say "it will have been
> zeroed" (after the patch under discussion is merged), right? The
> current state of the art (as of 2.6.12-rc2) is that mlocked regions are
> written to the swsusp region and may linger there indefinitely after
> resume.
I was referring not to the current implementation but to an ideal
solution. Which is:
- use dm-crypt for swap and suspend
- overwrite mlocked regions on resume
- use per-boot session keys for the swap partition
- password protect the session key on suspend
This doesn't have great FIPS-secure deletion properties if the
attacker can steal the box after resume but while it's still running
(though it's not too bad). As we currently have no solution for
protecting the in-memory ssh-agent, as opposed to the one in the
suspend image, this attack vector is not all that important.
A much more likely vector is stealing the laptop while it's suspended.
And the encrypted swsusp patch has -zero- security here: it writes the
key in the header in the clear. It's rather odd that everyone's hung
up on the "box rooted after resume" attack and completely ignoring the
much more common "stole my laptop" attack.
--
Mathematics is the supreme nostalgia of our time.
Matt Mackall wrote:
> A much more likely vector is stealing the laptop while it's suspended.
> And the encrypted swsusp patch has -zero- security here: it writes the
> key in the header in the clear. It's rather odd that everyone's hung
> up on the "box rooted after resume" attack and completely ignoring the
> much more common "stole my laptop" attack.
>
Thats because you have already have a solution for "stolen while
suspended" with dm-crypt and initrd/initramfs but you don't have a
solution for "rooted after resume".
--
Andreas Steinmetz SPAMmers use [email protected]
Matt Mackall wrote:
> Zero only the mlocked regions. This should take essentially no time at
> all. Swsusp knows which these are because they have to be mlocked
> after resume as well. If it's not mlocked, it's liable to be swapped
> out anyway.
Nitpicking:
What happens if the disk decides to relocate a close to failing sector
containing mlocked data during resume zeroing? This just means that
there will be sensitive data around on the disk that can't be zeroed
out anymore but which might be recovered by specialized
companies/institutions.
Encrypting these data in the first place reduces this problem to a
single potentially problematic sector.
If this risk is then still too high for you then there's always the
possiblity to use a sledgehammer :-)
--
Andreas Steinmetz SPAMmers use [email protected]
Hi!
> > Andreas, do you think you could write nice, long, FAQ entries so that
> > we don't have to go through this discussion over and over?
>
> I can do so over the weekend. Am I right that you mean the FAQ section
> of Documentation/power/swsusp.txt?
Yes.
> BTW: would it make sense to reset the suspend header via
> software_resume() if booted with noresume? Currently this code path does
> nothing.
I think distros are already doing it in userland, and I do not want to
add potential failure into "safe" boot path.
Pavel
--
Boycott Kodak -- for their patent abuse against Java.
Pavel Machek wrote:
> Andreas, do you think you could write nice, long, FAQ entries so that
> we don't have to go through this discussion over and over?
I can do so over the weekend. Am I right that you mean the FAQ section
of Documentation/power/swsusp.txt?
BTW: would it make sense to reset the suspend header via
software_resume() if booted with noresume? Currently this code path does
nothing.
--
Andreas Steinmetz SPAMmers use [email protected]
On Fri, Apr 15, 2005 at 11:44:06AM +0200, Andreas Steinmetz wrote:
> Matt Mackall wrote:
> > Zero only the mlocked regions. This should take essentially no time at
> > all. Swsusp knows which these are because they have to be mlocked
> > after resume as well. If it's not mlocked, it's liable to be swapped
> > out anyway.
>
> Nitpicking:
> What happens if the disk decides to relocate a close to failing sector
> containing mlocked data during resume zeroing? This just means that
> there will be sensitive data around on the disk that can't be zeroed
> out anymore but which might be recovered by specialized
> companies/institutions.
> Encrypting these data in the first place reduces this problem to a
> single potentially problematic sector.
Well that's what the dmcrypt phase is for.
--
Mathematics is the supreme nostalgia of our time.