Bug 23196 - RFE: Do not offer hibernate with encrypted swap with random key
Summary: RFE: Do not offer hibernate with encrypted swap with random key
Status: NEW
Alias: None
Product: systemd
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: systemd-bugs
QA Contact: systemd-bugs
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-08-07 06:20 UTC by Dustin Kirkland
Modified: 2016-06-10 12:50 UTC (History)
7 users (show)

See Also:
i915 platform:
i915 features:


Attachments
what's in git master (6.54 KB, patch)
2009-10-16 03:32 UTC, Richard Hughes
Details | Splinter Review
Patch to allow swapping to "decrypt_derived" swap devices. (2.11 KB, patch)
2010-03-29 12:31 UTC, Erich Schubert
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Dustin Kirkland 2009-08-07 06:20:22 UTC
Hello,

I think devicekit-power could be improved by using the pm-is-supported script provided by pm-utils.

It has a bit of logic that can determine if --suspend and --hibernate methods are supported.  It goes a bit beyond the basic kernel check that devicekit-power does:

        /* does the kernel advertise this */
        daemon->priv->can_suspend = (g_strstr_len (contents, -1, "mem") != NULL);
        daemon->priv->can_hibernate = (g_strstr_len (contents, -1, "disk") != NULL);

I'm working on adding some logic to pm-is-supported --hibernate which will also check if swap space exists, for instance (beyond the basic kernel support for hibernation).  It would be nice if devicekit could leverage this too.

Cheers,
:-Dustin
Comment 1 Martin Pitt 2009-08-07 06:54:58 UTC
Since dk-p is using pm-suspend and pm-hibernate, this makes sense to me.
Comment 2 Martin Pitt 2009-08-07 06:55:28 UTC
I'll be on vac the next two weeks, I'm happy to look at this after that, unless someone else beats me to it.
Comment 3 Richard Hughes 2009-08-07 07:10:20 UTC
(In reply to comment #0)
> I'm working on adding some logic to pm-is-supported --hibernate which will also
> check if swap space exists, for instance (beyond the basic kernel support for
> hibernation).

Doesn't DeviceKit-power already do these sort of checks with git master?

Richard.
Comment 4 Martin Pitt 2009-08-07 07:15:27 UTC
I believe Dustin works on checks which test if swap is encrypted. (He's one of the ecryptfs upstreams, and works on integration of disk encryption). If your swap is encrypted with a random passphrase, there is no way to resume from this.
Comment 5 Richard Hughes 2009-08-07 07:29:58 UTC
Sure, if this is the case we need to tell the user what the problem is, rather than just "it's not going to work"... And in this sense, pm-is-supported fails. I would gladly take a patch for DeviceKit-power if this is okay with Dustin.
Comment 6 Dustin Kirkland 2009-08-07 09:14:59 UTC
Richard-

Okay, I was planning on patching pm-is-supported.

What exactly are you looking for from me in terms of a DeviceKit patch?  Something independent from the pm-is-supported patch I'm using?  Detect the situation, and "why" a failure exists directly within DeviceKit, and print a nicer message?

:-Dustin
Comment 7 Richard Hughes 2009-08-08 05:29:47 UTC
(In reply to comment #6)
> What exactly are you looking for from me in terms of a DeviceKit patch? 
> Something independent from the pm-is-supported patch I'm using?  Detect the
> situation, and "why" a failure exists directly within DeviceKit, and print a
> nicer message?

Sure, to be honest, if you give me a few lines of C on how to test for encrypted swap, I'll just add the same logic to DeviceKit-power for you.
Comment 8 Martin Pitt 2009-10-01 10:47:57 UTC
Dustin, ping?
Comment 9 Dustin Kirkland 2009-10-15 14:44:12 UTC
Sorry for the delay.  I haven't had much time to devote to eCryptfs lately.

So this is shell, rather than C code, but here's the script that we use to setup the encrypted swap.

http://bazaar.launchpad.net/~ecryptfs/ecryptfs/ecryptfs-utils/annotate/head%3A/src/utils/ecryptfs-setup-swap

Toward the bottom, you can see a series of "warn" calls, that check if the device is already setup for encryption.

Basically, on my system with encrypted swap, I have:

kirkland@x200:~$ cat /proc/swaps 
Filename                                Type            Size    Used    Priority
/dev/mapper/cryptswap1                  partition       4803392 35872   -1
kirkland@x200:~$ cat /etc/crypttab 
# <target name> <source device>         <key file>      <options>
cryptswap1 /dev/sda5 /dev/urandom swap,cipher=aes-cbc-essiv:sha256
kirkland@x200:~$ grep swap /etc/fstab 
# swap was on /dev/sda5 during installation
#UUID=0f683971-6543-46cf-ab65-ff332df913b9 none            swap    sw              0       0
/dev/mapper/cryptswap1 none swap sw 0 0

This will be relatively standard for Ubuntu encrypted-swap setups, and a pretty straight-forward, frequently-used way of doing this.  However, I doubt that this is the be-all, end-all of ways to encrypt swap.

I think you should be able to loop over the swap partitions in /proc/swaps, looking for matches in /etc/crypttab should do it.  It would *certainly* be better than what we have now, which is nothing.

:-Dustin

:-Dustin
Comment 10 Richard Hughes 2009-10-16 03:32:56 UTC
Created attachment 30477 [details] [review]
what's in git master
Comment 11 Dustin Kirkland 2009-10-16 11:27:09 UTC
Cheers, thanks, Richard.
Comment 12 Michael Biebl 2009-10-20 13:15:44 UTC
(In reply to comment #10)
> Created an attachment (id=30477) [details]
> what's in git master

Unfortunately this broke hiberate alltogether. Now whenever I try to hibernate, I get in .xsession-errors
(gnome-power-manager:4373): devkit-power-gobject-WARNING **: Couldn't hibernate: Swap space is encrypted

I don't have a encrypted swap partion or neither /etc/crypttab.

It also fails to propagate the error message to gnome-power-manager. At least g-p-m is completely silent about the hibernate failure and doesn't show an error message.
Comment 13 Jan de Groot 2009-10-21 01:00:03 UTC
+	/* encrypted swap? */
+	if (!daemon->priv->hibernate_has_encrypted_swap) {
+		error = g_error_new (DKP_DAEMON_ERROR,
+				     DKP_DAEMON_ERROR_GENERAL,
+				     "Swap space is encrypted");
+		g_error_free (error_local);
+		dbus_g_method_return_error (context, error);
+		goto out;
+	}
+

The check is wrong. hibernate_has_encrypted_swap is TRUE when swap is encrypted, FALSE otherwise. That's what is causing the error from comment 12.
Comment 14 Richard Hughes 2009-10-21 01:28:12 UTC
(In reply to comment #13)
> The check is wrong. hibernate_has_encrypted_swap is TRUE when swap is
> encrypted, FALSE otherwise. That's what is causing the error from comment 12.

I've fixed this in git, thanks for spotting. I can only offer my apologies.
Comment 15 Michael Biebl 2009-10-21 06:09:55 UTC
(In reply to comment #14)
> (In reply to comment #13)
> > The check is wrong. hibernate_has_encrypted_swap is TRUE when swap is
> > encrypted, FALSE otherwise. That's what is causing the error from comment 12.
> 
> I've fixed this in git, thanks for spotting. I can only offer my apologies.
> 

Thanks for fixing this so quickly.
About the issue, that this incident was not reported by g-p-m, it seems I had set notify/sleep_failed to false. After setting it back to its default, true, a notification dialog is shown.
The dialog is pretty generic though. It doesn't show the failure message.
Wasn't this the whole point to move this check into g-p-m to be able to show a meaningful error message?
Comment 16 Yves-Alexis 2009-11-11 07:33:19 UTC
Hi,

it seems that the current behavior will break hibernation on my laptop.

I first reported the bug on debian at http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=555712

Basically, devkit-power doesn't think it could hibernate my laptop, while pm-hibernate works pretty fine (and echo disk > /sys/power/state too).

It seems it could be related to me using full disk encryption with luks.

Some info:

/etc/fstab:
----
# /etc/fstab: static file system information.
#
# <file system> <mount point>   <type>  <options>       <dump>  <pass>
proc            /proc           proc    defaults        0       0
/dev/mapper/hidalgo-root /      ext3    errors=remount-ro,relatime,commit=30 0 1
/dev/sda1       /boot           ext3    defaults,relatime        0       2
/dev/mapper/hidalgo-home /home  ext3    defaults,relatime,commit=30 0 2
/dev/mapper/hidalgo-swap_1 none swap    sw              0       0
/dev/cdrom1        /media/cdrom0 udf,iso9660 user,noauto     0       0
----

/etc/crypttab:
----
sda5_crypt /dev/sda5 none luks
----

/proc/swaps:
----
Filename				Type		Size	Used	Priority
/dev/mapper/hidalgo-swap_1              partition	1048568	117996	-1
----

Encrypted swap is especially useful when using hibernation (because there's no point in having disk encryption at all if you don't encrypt the swap when hibernating). Maybe in the case of swap recreated at each boot it would make sense, but in my case, the setup is:

/dev/sda5 is encrypted to /dev/mapper/sda5_crypt (using dm-crypt and luks)
/dev/mapper/sda5_crypt has a LVM volume group hidalgo
hidalgo VG has multiple LV: root, home and swap_1, swap_1 being used as swap.

In my case, encryption key is provided at boot/resume time and everything works fine.

I'm not 100% sure my problem comes from encryption, but if it does, I think it'd make sense to allow it.

Cheers,
--
Yves-Alexis
Comment 17 Richard Hughes 2009-11-17 04:00:27 UTC
(In reply to comment #16)
> In my case, encryption key is provided at boot/resume time and everything works
> fine.

How do you do that?
Comment 18 Martin Pitt 2009-11-17 04:32:34 UTC
(In reply to comment #17)
> How do you do that? 

The standard setup for this looks like this:

sda1: /boot with initramfs (including cryptsetup and scripts)
sda2: crypto_LUKS partition which contains an LVM PV, like /dev/mapper/crypto-sda2
/dev/mapper/crypto-sda2: from that, build an VG "debian"
VG "debian" -> usually has three LVs: "home", "system", "swap"

The trick is to wrap swap, and all other partitions into a VG which is put on an encrypted PV wholesale. So during boot, the initramfs asks for a password for decrypting this PV, which also works for resuming from hibernation.
Comment 19 Richard Hughes 2009-11-17 05:02:31 UTC
(In reply to comment #18)
> The trick is to wrap swap, and all other partitions into a VG which is put on
> an encrypted PV wholesale. So during boot, the initramfs asks for a password
> for decrypting this PV, which also works for resuming from hibernation.

So how can we detect that case? Any ideas?
Comment 20 Sam Morris 2009-11-17 06:08:51 UTC
(In reply to comment #17)
> (In reply to comment #16)
> > In my case, encryption key is provided at boot/resume time and everything works
> > fine.
> 
> How do you do that?
> 

My setup is different from Martin's.

# parted /dev/sda print
Model: ATA FUJITSU MHY2250B (scsi)
Disk /dev/sda: 250GB
Sector size (logical/physical): 512B/512B
Partition Table: msdos

Number  Start   End    Size   Type     File system  Flags
 1      32.3kB  250GB  250GB  primary               boot, lvm

# pvs
  PV         VG       Fmt  Attr PSize   PFree 
  /dev/sda1  durandal lvm2 a-   232.88g 88.88g

# vgs
  VG       #PV #LV #SN Attr   VSize   VFree 
  durandal   1   3   0 wz--n- 232.88g 88.88g

# lvs
  LV   VG       Attr   LSize   Origin Snap%  Move Log Copy%  Convert
  home durandal -wi-ao 127.00g                                      
  root durandal -wi-ao  15.00g                                      
  swap durandal -wi-ao   2.00g                                      

the 'root' LV contains the filesystem for /; the 'home' and 'swap' LVs are LUKS-encrypted volumes. I have /dev/mapper/durandal-home_crypt mounted at /home and /dev/mapper/durandal-swap_crypt as a swap device. Just another possible configuration to be aware of.
Comment 21 Richard Hughes 2009-11-17 06:11:37 UTC
What about "is swap is in a LV" as a test condition?
Comment 22 Pasi Sjöholm 2009-11-25 13:55:09 UTC
Actually hibernation works ok with encrypted swap with static encryption keys like luks-password or password-file given in boot time.

But it does not work when the encryption key is initialized from /dev/random or /dev/urandom at the boot time.

Here is the difference in the /etc/crypttab:

Works:
decSWAP    /dev/mapper/VGRAID-encSWAP

Does not work:
decSWAP    /dev/mapper/VGRAID-encSWAP    /dev/random    swap,cipher=aes-cbc-essiv:sha256

It does not matter where the swap is located if it is readable and accesible at the boot time after the system has the correct encryption key.

So please check if the encrypted swap is initialized from the /dev/random or /dev/urandom. Currently this patch breaks "hibernate" from the X as I can just click the "hibernate"-button because it does not exist in gnome therefore I have to type in "pm-hibernate" in the console.

Comment 23 Sam Morris 2009-11-30 02:15:18 UTC
What about just using pm-is-supported --hibernate?
Comment 24 Bas Mevissen 2009-11-30 08:41:42 UTC
Bug report on Fedora 12: https://bugzilla.redhat.com/show_bug.cgi?id=540067
Comment 25 Sam Morris 2010-01-25 02:27:19 UTC
Is there some problem with just using pm-is-supported --hibernate? It correctly tells me that my laptop is capable of hibernation... but I have been unable to actulaly suspend it for three months since the chance in devicekit...
Comment 26 Yves-Alexis 2010-02-02 14:33:32 UTC
(In reply to comment #25)
> Is there some problem with just using pm-is-supported --hibernate? It correctly
> tells me that my laptop is capable of hibernation... but I have been unable to
> actulaly suspend it for three months since the chance in devicekit...

Agreed, it seems to work perfectly fine here.
Comment 27 Erich Schubert 2010-03-29 12:14:27 UTC
Can we add a "can_hibernate" option to /etc/crypttab, and have upower accept swap devices marked as "can_hibernate"? A setup tool such as the Debian installier should add this option when using decrypt_derived; changing it is up to the user.
This allows users to use their own scripts for decrypt_derived.

IMHO this is cleaner than checking for /dev/random and /dev/urandom; although it requires current users of encrypted swap to modify their crypttab.
Comment 28 Erich Schubert 2010-03-29 12:31:39 UTC
Created attachment 34532 [details] [review]
Patch to allow swapping to "decrypt_derived" swap devices.

This is a hack to allow current users of the (known good) decrypt_derived script to swap. It's probably not the cleanest way of doing this, but you'll get the idea.
For example, instead of doing the split, one could just jump to the third tab character, and scan from there for "can_hibernate", jumping to the next "," until EOL. This would avoid having all the cleanup code, and doesn't add mallocs.
Comment 29 Erich Schubert 2010-03-30 00:36:34 UTC
Thinking of it, it might also be okay to support both:
1. allow swapping for users that use the provided decrypt_derived script (with full pathname - note that this could vary from distribution to distribution?)
2. allow swapping when the crypttab entry is flagged can_hibernate

This way, current users of the safe "decrypt_derived" method won't need to change anything.
Is anyone in contact with cryptsetup developers to discuss the can_hibernate flag in crypttab?
Comment 30 Erich Schubert 2010-04-07 15:31:36 UTC
FYI: I contacted cryptsetup upstream, but basically they replied that they're not affected, since /etc/crypttab is essentially handled by shell scripts by the distributions, and the actual cryptsetup tool isn't reading the file.

Juding from /lib/cryptsetup/cryptdisks.functions (parse_opts function) on my Debian system it should be safe to add a "can_hibernate" option to the crypttab.
Other distributions might of course vary. But it seems that crypttab is rather common?
Comment 31 Richard Hughes 2010-04-14 02:09:57 UTC
In git master there is now a /etc/UPower/UPower.conf for people wanting to modify this setting either way. At the moment it defaults to letting people with encrypted swap hibernate, unless it's modified the other way.
Comment 32 Bastien Nocera 2013-10-12 12:43:37 UTC
And the setting has now migrated to systemd. Is this still a problem?
Comment 33 Zbigniew Jedrzejewski-Szmek 2013-10-12 14:39:19 UTC
(In reply to comment #32)
> And the setting has now migrated to systemd. Is this still a problem?
I think the issue is still valid, but before we add code to systemd, we need to have a full picture of how this is supposed to work, supporting all cases:

1. no encryption -> hibernate OK
2. encryption with random key -> hibernate not OK
3. encryption with static key -> hibernate OK

I'd prefer not to add explicit configuration unless we really cannot make it work otherwise. For encrypted devices udev has:

E: UDISKS_DM_TARGETS_COUNT=1
E: UDISKS_DM_TARGETS_LENGTH=624637274
E: UDISKS_DM_TARGETS_START=0
E: UDISKS_DM_TARGETS_TYPE=crypt

It would be necessary to "drill down", from the swap device, all the way to the bottom. If any of the devices is encrypted with a random key, hibernation is not possible. We could then check if the device has a random key by checking in /etc/crypttab. Supporting crypt devices configured through other means would be not supported (they would be ignored for the purposes of those checks).

BTW, I think that the checks as currently implemented in dk-p are overly restrictive, because the kernel will always use the first partition in /proc/swaps for hibernation, so there's no need to loop over devices.

As a temporary fix, with systemd, if you want to disable hibernation, it should be enough to add 'HibernateMode=disabled' to /etc/systemd/sleep.conf.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.