From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1755308AbbDIN2u (ORCPT <rfc822;w@1wt.eu>);
	Thu, 9 Apr 2015 09:28:50 -0400
Received: from mail-wg0-f51.google.com ([74.125.82.51]:33102 "EHLO
	mail-wg0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753235AbbDIN2k (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Thu, 9 Apr 2015 09:28:40 -0400
Date: Thu, 9 Apr 2015 15:28:36 +0200
From: Pali =?utf-8?B?Um9ow6Fy?= <pali.rohar@gmail.com>
To: Mike Snitzer <snitzer@redhat.com>
Cc: Alasdair Kergon <agk@redhat.com>, Neil Brown <neilb@suse.de>,
        "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
        dm-devel@redhat.com, linux-raid@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org
Subject: Re: [PATCH 0/3] dm-crypt: Adds support for wiping key when doing
 suspend/hibernation
Message-ID: <20150409132836.GD12339@atrey.karlin.mff.cuni.cz>
References: <1428254419-7334-1-git-send-email-pali.rohar@gmail.com>
 <20150406130045.GA18583@redhat.com>
 <201504061529.57299@pali>
 <20150409131208.GA9504@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20150409131208.GA9504@redhat.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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

So proper way is to wipe luks crypto keys *after* userspace processes
are freezed.

-- 
Pali Rohár
pali.rohar@gmail.com