Message-ID: <5530C9DE.2040302@redhat.com>
Date:	Fri, 17 Apr 2015 10:52:46 +0200
From:	Ondrej Kozina <okozina@...hat.com>
To:	device-mapper development <dm-devel@...hat.com>,
	Alex Elsayed <eternaleye@...il.com>,
	Mike Snitzer <msnitzer@...hat.com>
CC:	linux-raid@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-pm@...r.kernel.org
Subject: Re: [dm-devel] [PATCH 0/3] dm-crypt: Adds support for wiping key
 when doing suspend/hibernation

On 04/17/2015 09:52 AM, Mike Snitzer wrote:
> On Thu, Apr 16 2015 at  5:23am -0400,
> Alex Elsayed <eternaleye@...il.com> wrote:
>
>> Mike Snitzer wrote:
>>
>>> On Thu, Apr 09 2015 at  9:28am -0400,
>>> Pali Rohár <pali.rohar@...il.com> wrote:
>>>
>>>> On Thursday 09 April 2015 09:12:08 Mike Snitzer wrote:
>>>>> On Mon, Apr 06 2015 at  9:29am -0400,
>>>>> Pali Rohár <pali.rohar@...il.com> wrote:
>>>>>
>>>>>> On Monday 06 April 2015 15:00:46 Mike Snitzer wrote:
>>>>>>> On Sun, Apr 05 2015 at  1:20pm -0400,
>>>>>>>
>>>>>>> Pali Rohár <pali.rohar@...il.com> wrote:
>>>>>>>> This patch series increase security of suspend and hibernate
>>>>>>>> actions. It allows user to safely wipe crypto keys before
>>>>>>>> suspend and hibernate actions starts without race
>>>>>>>> conditions on userspace process with heavy I/O.
>>>>>>>>
>>>>>>>> To automatically wipe cryto key for <device> before
>>>>>>>> hibernate action call: $ dmsetup message <device> 0 key
>>>>>>>> wipe_on_hibernation 1
>>>>>>>>
>>>>>>>> To automatically wipe cryto key for <device> before suspend
>>>>>>>> action call: $ dmsetup message <device> 0 key
>>>>>>>> wipe_on_suspend 1
>>>>>>>>
>>>>>>>> (Value 0 after wipe_* string reverts original behaviour - to
>>>>>>>> not wipe key)
>>>>>>>
>>>>>>> Can you elaborate on the attack vector your changes are meant
>>>>>>> to protect against?  The user already authorized access, why
>>>>>>> is it inherently dangerous to _not_ wipe the associated key
>>>>>>> across these events?
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> yes, I will try to explain current problems with cryptsetup
>>>>>> luksSuspend command and hibernation.
>>>>>>
>>>>>> First, sometimes it is needed to put machine into other hands.
>>>>>> You can still watch other person what is doing with machine, but
>>>>>> once if you let machine unlocked (e.g opened luks disk), she/he
>>>>>> can access encrypted data.
>>>>>>
>>>>>> If you turn off machine, it could be safe, because luks disk
>>>>>> devices are locked. But if you enter machine into suspend or
>>>>>> hibernate state luks devices are still open. And my patches try
>>>>>> to achieve similar security as when machine is off (= no crypto
>>>>>> keys in RAM or on swap).
>>>>>>
>>>>>> When doing hibernate on unencrypted swap it is to prevent leaking
>>>>>> crypto keys to hibernate image (which is stored in swap).
>>>>>>
>>>>>> When doing suspend action it is again to prevent leaking crypto
>>>>>> keys. E.g when you suspend laptop and put it off (somebody can
>>>>>> remove RAMs and do some cold boot attack).
>>>>>>
>>>>>> The most common situation is:
>>>>>> You have mounted partition from dm-crypt device (e.g. /home/),
>>>>>> some userspace processes access it (e.g opened firefox which
>>>>>> still reads/writes to cache ~/.firefox/) and you want to drop
>>>>>> crypto keys from kernel for some time.
>>>>>>
>>>>>> For that operation there is command cryptsetup luksSuspend, which
>>>>>> suspend dm device and then tell kernel to wipe crypto keys. All
>>>>>> I/O operations are then stopped and userspace processes which
>>>>>> want to do some those I/O operations are stopped too (until you
>>>>>> call cryptsetup luksResume and enter correct key).
>>>>>>
>>>>>> Now if you want to suspend/hiberate your machine (when some of dm
>>>>>> devices are suspeneded and some processes are stopped due to
>>>>>> pending I/O) it is not possible. Kernel freeze_processes function
>>>>>> will fail because userspace processes are still stopped inside
>>>>>> some I/O syscall (read/write, etc,...).
>>>>>>
>>>>>> My patches fixes this problem and do those operations (suspend dm
>>>>>> device, wipe crypto keys, enter suspend/hiberate) in correct
>>>>>> order and without race condition.
>>>>>>
>>>>>> dm device is suspended *after* userspace processes are freezed
>>>>>> and after that are crypto keys wiped. And then computer/laptop
>>>>>> enters into suspend/hibernate state.
>>>>>
>>>>> Wouldn't it be better to fix freeze_processes() to be tolerant of
>>>>> processes that are hung as a side-effect of their backing storage being
>>>>> suspended?  A hibernate shouldn't fail simply because a user chose to
>>>>> suspend a DM device.
>>>>>
>>>>> Then this entire problem goes away and the key can be wiped from
>>>>> userspace (like you said above).
>>>>
>>>> Still there will be race condition. Before hibernation (and device
>>>> poweroff) we should have synced disks and filesystems to prevent data
>>>> lose (or other damage) as more as we can. And if there will be some
>>>> application which using lot of I/O (e.g normal firefox) then there
>>>> always will be race condtion.
>>>
>>> The DM suspend will take care of flushing any pending I/O.  So I don't
>>> see where the supposed race is...
>>>
>>> Anything else that is trapped in userspace memory will be there when the
>>> machine resumes.
>>>
>>>> So proper way is to wipe luks crypto keys *after* userspace processes
>>>> are freezed.
>>>
>>> I know you believe that I'm just not accepting that at face value.
>>
>> Um, pardon me if I'm being naive, but what about the case of hibernation
>> where the swapdev and the root device are both LVs on the same dm_crypt
>> device?
>>
>> The kernel is writing to swap _after_ userspace processes are all frozen;
>> that seems to me like an ordering dependency entirely incompatible with
>> userspace dropping the key...
>
> Good point, definitely not compatible with the Pali's approach.

Ouch! I'm afraid this effectively killed one of my experiments with 
dm-crypt suspend. Good to get reminded sooner than later!

> (but is swap really configured ontop of the same dm-crypt device like
> this in practice?  I've not heard of that being a common pattern but I
> could just be sheltered)

yes. It's one among many perfectly valid setups.

(For some the goal here would be to have whole disk encrypted including 
boot partition unlocked during boot)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists