Do not offer hibernate with encrypted swap

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

Since dk-p is using pm-suspend and pm-hibernate, this makes sense to me.

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.

(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.

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.

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.

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

(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.

Dustin, ping?

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

Created attachment 30477
what's in git master

Cheers, thanks, Richard.

(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.

+ /* 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.

(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.

(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?

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

(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?

(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.

(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?

(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.

What about "is swap is in a LV" as a test condition?

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.

What about just using pm-is-supported --hibernate?

Bug report on Fedora 12: https://bugzilla.redhat.com/show_bug.cgi?id=540067

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...

(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.

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.

Created attachment 34532
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.

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?

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?

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.

And the setting has now migrated to systemd. Is this still a problem?

(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.