linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* dm-crypt crypt_status reports key?
@ 2005-02-02 21:19 Matt Mackall
  2005-02-02 23:50 ` Alasdair G Kergon
  2005-02-03  1:33 ` Christophe Saout
  0 siblings, 2 replies; 16+ messages in thread
From: Matt Mackall @ 2005-02-02 21:19 UTC (permalink / raw)
  To: linux-kernel, Christophe Saout, Clemens Fruhwirth

>From looking at the dm_crypt code, it appears that it can be
interrogated to report the current key. Some quick testing shows:

# dmsetup table /dev/mapper/volume1
0 2000000 crypt aes-plain 0123456789abcdef0123456789abcdef 0 7:0 0

Obviously, root can in principle recover this password from the
running kernel but it seems silly to make it so easy.

Moreover, it seems this facility exists to support some form of
automated table storage (LVM?). As we don't want anyone/anything
accidentally storing our passwords on disk in the clear, we probably
shouldn't facilitate this. Perhaps we can stick something here like
"<secret>" that the dm_crypt constructor can reject.

-- 
Mathematics is the supreme nostalgia of our time.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: dm-crypt crypt_status reports key?
  2005-02-02 21:19 dm-crypt crypt_status reports key? Matt Mackall
@ 2005-02-02 23:50 ` Alasdair G Kergon
  2005-02-03  1:00   ` Matt Mackall
  2005-02-03 21:53   ` Pavel Machek
  2005-02-03  1:33 ` Christophe Saout
  1 sibling, 2 replies; 16+ messages in thread
From: Alasdair G Kergon @ 2005-02-02 23:50 UTC (permalink / raw)
  To: Matt Mackall; +Cc: linux-kernel, Christophe Saout, Clemens Fruhwirth, dm-crypt

On Wed, Feb 02, 2005 at 01:19:16PM -0800, Matt Mackall wrote:
> # dmsetup table /dev/mapper/volume1
> 0 2000000 crypt aes-plain 0123456789abcdef0123456789abcdef 0 7:0 0
 
> Obviously, root can in principle recover this password from the
> running kernel but it seems silly to make it so easy.
 
There seemed little point obfuscating it - someone will only
write a trivial utility that recovers it.

The current approach has the advantage of making it
obvious to you that if you have root access, you have
access to the password while the encrypted data volumes
are mounted.

Consider instead *why* you're worried about the password being
held in RAM and look for better solutions to *your*
perceived threats.


Threat: Someone could run "dmsetup" while I've gone for a coffee 
break leaving my laptop unattended logged on as root...


Threat: My laptop is stolen while it's got a screen saver running
(or suspended) and the thief could interrogate RAM and get the 
password, giving them access to my encrypted data volumes.

Possible fixes: Automatically unmount those volumes before
starting the screen saver.  Automatically unmount them
after a certain amount of inactivity.  Require the password
to be re-entered at regular intervals.  New risks: regular
entry of password means more chance of its being observed,
so consider one-time passwords to gain access to it, or
USB dongle periodically inserted etc. etc. 

Alasdair
-- 
agk@redhat.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: dm-crypt crypt_status reports key?
  2005-02-02 23:50 ` Alasdair G Kergon
@ 2005-02-03  1:00   ` Matt Mackall
  2005-02-03 21:53   ` Pavel Machek
  1 sibling, 0 replies; 16+ messages in thread
From: Matt Mackall @ 2005-02-03  1:00 UTC (permalink / raw)
  To: linux-kernel, Christophe Saout, Clemens Fruhwirth, dm-crypt

On Wed, Feb 02, 2005 at 11:50:02PM +0000, Alasdair G Kergon wrote:
> On Wed, Feb 02, 2005 at 01:19:16PM -0800, Matt Mackall wrote:
> > # dmsetup table /dev/mapper/volume1
> > 0 2000000 crypt aes-plain 0123456789abcdef0123456789abcdef 0 7:0 0
>  
> > Obviously, root can in principle recover this password from the
> > running kernel but it seems silly to make it so easy.
>  
> There seemed little point obfuscating it - someone will only
> write a trivial utility that recovers it.

So instead let's do the work for them? We could perhaps put it in the
root prompt. Pray tell, what is the value to the user of exposing the
whole password, ever?

> Consider instead *why* you're worried about the password being
> held in RAM and look for better solutions to *your*
> perceived threats.

My perceived threat, as I've already stated, is that automated suite
of utilities like LVM or EVMS or even initscripts will silently store
this information in the clear on disk in an effort to make life
easier, oblivious to the fact that it might contain security-sensitive
information.

What drives this perception is that the output of "dmsetup tables"
invites it: it appears tailor-made to be fed into a future "dmsetup
create". Thus someone clever (but unaware of dm_crypt) _will_
eventually try this if it's not already happening. Further, there is
nothing in the dmsetup manpage to suggest that its output should be
guarded, etc. See "principle of least surprise."

Given the potential risk of naive use of dmsetup tables, what's the
upside again?

-- 
Mathematics is the supreme nostalgia of our time.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: dm-crypt crypt_status reports key?
  2005-02-02 21:19 dm-crypt crypt_status reports key? Matt Mackall
  2005-02-02 23:50 ` Alasdair G Kergon
@ 2005-02-03  1:33 ` Christophe Saout
  2005-02-03  1:52   ` Matt Mackall
  1 sibling, 1 reply; 16+ messages in thread
From: Christophe Saout @ 2005-02-03  1:33 UTC (permalink / raw)
  To: Matt Mackall; +Cc: linux-kernel, Clemens Fruhwirth, dm-crypt, Alasdair G Kergon

[-- Attachment #1: Type: text/plain, Size: 2368 bytes --]

Am Mittwoch, den 02.02.2005, 13:19 -0800 schrieb Matt Mackall:

> From looking at the dm_crypt code, it appears that it can be
> interrogated to report the current key. Some quick testing shows:
> 
> # dmsetup table /dev/mapper/volume1
> 0 2000000 crypt aes-plain 0123456789abcdef0123456789abcdef 0 7:0 0
> 
> Obviously, root can in principle recover this password from the
> running kernel but it seems silly to make it so easy.

I already tried that. It took me about five minutes using a shell, dd
and hexdump to get the key out of the running kernel...

> Moreover, it seems this facility exists to support some form of
> automated table storage (LVM?). As we don't want anyone/anything
> accidentally storing our passwords on disk in the clear, we probably
> shouldn't facilitate this. Perhaps we can stick something here like
> "<secret>" that the dm_crypt constructor can reject.

Yes, the reason is that the device-mapper supports on-the-fly
modifications of the device. cryptsetup has a command to resize the
mapping for example. It can do that without asking for the password
again. Features like this are the reason I'm doing this. Userspace tools
should be able to assume that they can use the result of a table status
command to create a new table with this information.

An alternativ would be to use some form of handle to point to the key
after it has been given to the kernel. But that would require some more
infrastructure.

I could imagine something like this:

Some kernel infrastructure for key management. It can hold keys which
are referenced by some form of handle. The keys are refcounted and wiped
if the reference count drops to zero.

If you want to create a dm-crypt mapping (or something else that uses
some form of cryptographic key) you create a new handle and assign the
key. Then you give the handle to dm-crypt which increments reference
count. When cryptsetup exits its reference to the key is dropped but the
key still has a reference from the active dm-crypt mapping. Later on
another application could then safely do something with that handle. As
long as an application or in-kernel user references the key it won't be
dropped so it's safe for a userspace application to play around with it.

BTW: The setkey command also seems to return the keys in use for IPSEC
connections.


[-- Attachment #2: Dies ist ein digital signierter Nachrichtenteil --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: dm-crypt crypt_status reports key?
  2005-02-03  1:33 ` Christophe Saout
@ 2005-02-03  1:52   ` Matt Mackall
  2005-02-03  2:34     ` Christophe Saout
  0 siblings, 1 reply; 16+ messages in thread
From: Matt Mackall @ 2005-02-03  1:52 UTC (permalink / raw)
  To: Christophe Saout
  Cc: linux-kernel, Clemens Fruhwirth, dm-crypt, Alasdair G Kergon

On Thu, Feb 03, 2005 at 02:33:01AM +0100, Christophe Saout wrote:
> Am Mittwoch, den 02.02.2005, 13:19 -0800 schrieb Matt Mackall:
> 
> > From looking at the dm_crypt code, it appears that it can be
> > interrogated to report the current key. Some quick testing shows:
> > 
> > # dmsetup table /dev/mapper/volume1
> > 0 2000000 crypt aes-plain 0123456789abcdef0123456789abcdef 0 7:0 0
> > 
> > Obviously, root can in principle recover this password from the
> > running kernel but it seems silly to make it so easy.
> 
> I already tried that. It took me about five minutes using a shell, dd
> and hexdump to get the key out of the running kernel...

Indeed.

> Yes, the reason is that the device-mapper supports on-the-fly
> modifications of the device. cryptsetup has a command to resize the
> mapping for example. It can do that without asking for the password
> again. Features like this are the reason I'm doing this. Userspace tools
> should be able to assume that they can use the result of a table status
> command to create a new table with this information.

Hmm, interesting. A password per resize is not terribly onerous though.

> An alternativ would be to use some form of handle to point to the key
> after it has been given to the kernel. But that would require some more
> infrastructure.

There's been some talk about such infrastructure already. I believe
some pieces of it may already be in place.

> BTW: The setkey command also seems to return the keys in use for IPSEC
> connections.

While possibly suboptimal, this won't surprise anyone. But dmsetup has
no mention of security in its manpage and doesn't show keys in typical
LVM usage. So people might reasonably assume that data from dmsetup
tables is not secret.

-- 
Mathematics is the supreme nostalgia of our time.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: dm-crypt crypt_status reports key?
  2005-02-03  1:52   ` Matt Mackall
@ 2005-02-03  2:34     ` Christophe Saout
  2005-02-03  4:05       ` Matt Mackall
  0 siblings, 1 reply; 16+ messages in thread
From: Christophe Saout @ 2005-02-03  2:34 UTC (permalink / raw)
  To: Matt Mackall; +Cc: linux-kernel, Clemens Fruhwirth, dm-crypt, Alasdair G Kergon

[-- Attachment #1: Type: text/plain, Size: 1350 bytes --]

Am Mittwoch, den 02.02.2005, 17:52 -0800 schrieb Matt Mackall:

> > An alternativ would be to use some form of handle to point to the key
> > after it has been given to the kernel. But that would require some more
> > infrastructure.
> 
> There's been some talk about such infrastructure already. I believe
> some pieces of it may already be in place.

Yes, you are right. I didn't follow the discussion but it actually looks
very promising. The keys in the infrastructure are reference-counted.
That's good.

The keyrings can be attached to either thread, processes, sessions or
users. 

It seems that it's possible to have floating keys (not attached to any
keyring). So we would just need to figure out how to use these keyrings
to allow communication with userspace applications. Process keyrings
seem to have the advantage that the keyring is dropped when it exits so
that all keys that are not in use by the kernel are also dropped. A
keyring for the root user would have the problem that if the cryptsetup
application aborts in the middle you would end up with old keys lying
around forever.

The keyring API seems very flexible. You can define your own type of
keys and give them names. Well, the name is probably irrelevant here and
should be chosen randomly but it's less likely to collide with someone
else.


[-- Attachment #2: Dies ist ein digital signierter Nachrichtenteil --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: dm-crypt crypt_status reports key?
  2005-02-03  2:34     ` Christophe Saout
@ 2005-02-03  4:05       ` Matt Mackall
  2005-02-03 13:07         ` Christophe Saout
                           ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Matt Mackall @ 2005-02-03  4:05 UTC (permalink / raw)
  To: Christophe Saout
  Cc: linux-kernel, Clemens Fruhwirth, dm-crypt, Alasdair G Kergon

On Thu, Feb 03, 2005 at 03:34:29AM +0100, Christophe Saout wrote:
> The keyring API seems very flexible. You can define your own type of
> keys and give them names. Well, the name is probably irrelevant here and
> should be chosen randomly but it's less likely to collide with someone
> else.
 
Dunno here, seems that having one tool that gave the kernel a key named
"foo" and then telling dm-crypt to use key "foo" is probably not a bad
way to go. Then we don't have stuff like "echo <key> | dmsetup create"
and the like and the key-handling smarts can all be put in one
separate place.

Getting from here to there might be interesting though. Perhaps we can
teach dm-crypt to understand keys of the form "keyname:<foo>"? in
addition to raw keys to keep compatibility. Might even be possible to
push this down into crypt_decode_key() (or a smarter variant of same).

Meanwhile, I'd still like to hide the raw key in crypt_status().

-- 
Mathematics is the supreme nostalgia of our time.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: dm-crypt crypt_status reports key?
  2005-02-03 14:18         ` Fruhwirth Clemens
@ 2005-02-03 10:15           ` Christopher Warner
  2005-02-03 15:17             ` Fruhwirth Clemens
  2005-02-03 14:47           ` Andries Brouwer
  2005-02-04 13:27           ` [dm-crypt] " Fruhwirth Clemens
  2 siblings, 1 reply; 16+ messages in thread
From: Christopher Warner @ 2005-02-03 10:15 UTC (permalink / raw)
  To: Fruhwirth Clemens
  Cc: Matt Mackall, Christophe Saout, christopher, linux-kernel,
	dm-crypt, Alasdair G Kergon

On Thu, 2005-02-03 at 15:18 +0100, Fruhwirth Clemens wrote:
> On Wed, 2005-02-02 at 20:05 -0800, Matt Mackall wrote:
> 
> > Dunno here, seems that having one tool that gave the kernel a key named
> > "foo" and then telling dm-crypt to use key "foo" is probably not a bad
> > way to go. Then we don't have stuff like "echo <key> | dmsetup create"
> > and the like and the key-handling smarts can all be put in one
> > separate place.
> > 
> > Getting from here to there might be interesting though. Perhaps we can
> > teach dm-crypt to understand keys of the form "keyname:<foo>"? in
> > addition to raw keys to keep compatibility. Might even be possible to
> > push this down into crypt_decode_key() (or a smarter variant of same).
> 
> Way too complicated. This is a crypto project, why does nobody think of
> crypto to solve the problem :). Here's the idea:
> 
> Keys are handed to dm-crypt regularly the first time. But when dm-crypt
> hands keys back to user space, it uses some sort of blinding to make the
> keys meaningless for user space. 
> 
> That can easily be done by generating a kernel internal secret after
> boot, and then before handing out the keys to user space, XOR-ing the
> kernel secret into the key. When these keys are handed back from user
> space to the kernel, the process is reversed. 
> 
> That's an effective blinding mechanism. The kernel has to remember
> nothing but a single secret. No special out-of-band setup of "keyname:"
> tokens, no additional management for these tokens and blinded key
> becomes useless after reboot.
> 
> Of course, the blinded keys need to be distinguished from regular keys.
> I propose to prepend "!" to the keys handed back to the user space, so
> we have "!<hex..key>", and add a simple conditional post processing to
> crypt_decode_key.
> 
> Of course, one can use encryption instead of XOR-ing with the kernel
> secret as blinding mechanism, as the kernel secret can easily be
> recovered by setting up a all-zero key. But the main intend of this
> approach is to protect against incompetent roots and user space
> programs, so I think this XOR OTP is sufficient, and further, trivially
> to implement. (Actually it's a Multi Time Pad.)

I've been following this thread and i'm clearly at a loss as to how any
of this will prevent someone from writing a util to get the key?
Currently i'm trying to figure out exactly how one would prevent the key
from being retrieved. This after stumbling into dm-crypt almost 24 hours
ago and applying it into a system. Albeit, I haven't been thinking about
it long, none of the above will stop incompetent roots from leaving a
machine open with root. Subsequently, allowing one who's far less
incompetent from taking advantage of the machine. The only logical
solution seems to be perceived threats idea. Why put something into
place that isn't going to make it any more difficult? How about, don't
leave yourself logged in as root and if you're using scripts, etc make
sure they have the proper permissions set etc.

--
Christopher Warner


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: dm-crypt crypt_status reports key?
  2005-02-03  4:05       ` Matt Mackall
@ 2005-02-03 13:07         ` Christophe Saout
  2005-02-03 14:18         ` Fruhwirth Clemens
  2005-02-04 14:03         ` Christophe Saout
  2 siblings, 0 replies; 16+ messages in thread
From: Christophe Saout @ 2005-02-03 13:07 UTC (permalink / raw)
  To: Matt Mackall; +Cc: linux-kernel, Clemens Fruhwirth, dm-crypt, Alasdair G Kergon

[-- Attachment #1: Type: text/plain, Size: 1473 bytes --]

Am Mittwoch, den 02.02.2005, 20:05 -0800 schrieb Matt Mackall:

> On Thu, Feb 03, 2005 at 03:34:29AM +0100, Christophe Saout wrote:
> > The keyring API seems very flexible. You can define your own type of
> > keys and give them names. Well, the name is probably irrelevant here and
> > should be chosen randomly but it's less likely to collide with someone
> > else.
>  
> Dunno here, seems that having one tool that gave the kernel a key named
> "foo" and then telling dm-crypt to use key "foo" is probably not a bad
> way to go. Then we don't have stuff like "echo <key> | dmsetup create"
> and the like and the key-handling smarts can all be put in one
> separate place.

Yes. I could also change cryptsetup to not mlockall the whole
application just because the key is passed down to libdevmapper which
does not treat parameters with special care.

> Getting from here to there might be interesting though. Perhaps we can
> teach dm-crypt to understand keys of the form "keyname:<foo>"? in
> addition to raw keys to keep compatibility. Might even be possible to
> push this down into crypt_decode_key() (or a smarter variant of same).
> 
> Meanwhile, I'd still like to hide the raw key in crypt_status().

Well, I don't. I don't know any tools that actually use the
DM_DEVICE_TABLE command except cryptsetup. I don't like to make the
interface inconsistent just because there might be an incompetent root
sitting in front of the machine.


[-- Attachment #2: Dies ist ein digital signierter Nachrichtenteil --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: dm-crypt crypt_status reports key?
  2005-02-03  4:05       ` Matt Mackall
  2005-02-03 13:07         ` Christophe Saout
@ 2005-02-03 14:18         ` Fruhwirth Clemens
  2005-02-03 10:15           ` Christopher Warner
                             ` (2 more replies)
  2005-02-04 14:03         ` Christophe Saout
  2 siblings, 3 replies; 16+ messages in thread
From: Fruhwirth Clemens @ 2005-02-03 14:18 UTC (permalink / raw)
  To: Matt Mackall; +Cc: Christophe Saout, linux-kernel, dm-crypt, Alasdair G Kergon

[-- Attachment #1: Type: text/plain, Size: 2184 bytes --]

On Wed, 2005-02-02 at 20:05 -0800, Matt Mackall wrote:

> Dunno here, seems that having one tool that gave the kernel a key named
> "foo" and then telling dm-crypt to use key "foo" is probably not a bad
> way to go. Then we don't have stuff like "echo <key> | dmsetup create"
> and the like and the key-handling smarts can all be put in one
> separate place.
> 
> Getting from here to there might be interesting though. Perhaps we can
> teach dm-crypt to understand keys of the form "keyname:<foo>"? in
> addition to raw keys to keep compatibility. Might even be possible to
> push this down into crypt_decode_key() (or a smarter variant of same).

Way too complicated. This is a crypto project, why does nobody think of
crypto to solve the problem :). Here's the idea:

Keys are handed to dm-crypt regularly the first time. But when dm-crypt
hands keys back to user space, it uses some sort of blinding to make the
keys meaningless for user space. 

That can easily be done by generating a kernel internal secret after
boot, and then before handing out the keys to user space, XOR-ing the
kernel secret into the key. When these keys are handed back from user
space to the kernel, the process is reversed. 

That's an effective blinding mechanism. The kernel has to remember
nothing but a single secret. No special out-of-band setup of "keyname:"
tokens, no additional management for these tokens and blinded key
becomes useless after reboot.

Of course, the blinded keys need to be distinguished from regular keys.
I propose to prepend "!" to the keys handed back to the user space, so
we have "!<hex..key>", and add a simple conditional post processing to
crypt_decode_key.

Of course, one can use encryption instead of XOR-ing with the kernel
secret as blinding mechanism, as the kernel secret can easily be
recovered by setting up a all-zero key. But the main intend of this
approach is to protect against incompetent roots and user space
programs, so I think this XOR OTP is sufficient, and further, trivially
to implement. (Actually it's a Multi Time Pad.)

-- 
Fruhwirth Clemens <clemens@endorphin.org>  http://clemens.endorphin.org

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: dm-crypt crypt_status reports key?
  2005-02-03 14:18         ` Fruhwirth Clemens
  2005-02-03 10:15           ` Christopher Warner
@ 2005-02-03 14:47           ` Andries Brouwer
  2005-02-03 15:00             ` Fruhwirth Clemens
  2005-02-04 13:27           ` [dm-crypt] " Fruhwirth Clemens
  2 siblings, 1 reply; 16+ messages in thread
From: Andries Brouwer @ 2005-02-03 14:47 UTC (permalink / raw)
  To: Fruhwirth Clemens
  Cc: Matt Mackall, Christophe Saout, linux-kernel, dm-crypt,
	Alasdair G Kergon

On Thu, Feb 03, 2005 at 03:18:20PM +0100, Fruhwirth Clemens wrote:

> (Actually it's a Multi Time Pad.)

And you call this "crypto"?


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: dm-crypt crypt_status reports key?
  2005-02-03 14:47           ` Andries Brouwer
@ 2005-02-03 15:00             ` Fruhwirth Clemens
  0 siblings, 0 replies; 16+ messages in thread
From: Fruhwirth Clemens @ 2005-02-03 15:00 UTC (permalink / raw)
  To: Andries Brouwer
  Cc: Matt Mackall, Christophe Saout, linux-kernel, dm-crypt,
	Alasdair G Kergon

[-- Attachment #1: Type: text/plain, Size: 328 bytes --]

On Thu, 2005-02-03 at 15:47 +0100, Andries Brouwer wrote:
> On Thu, Feb 03, 2005 at 03:18:20PM +0100, Fruhwirth Clemens wrote:
> 
> > (Actually it's a Multi Time Pad.)
> 
> And you call this "crypto"?

Is the quoted part all you have read?

-- 
Fruhwirth Clemens <clemens@endorphin.org>  http://clemens.endorphin.org

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: dm-crypt crypt_status reports key?
  2005-02-03 10:15           ` Christopher Warner
@ 2005-02-03 15:17             ` Fruhwirth Clemens
  0 siblings, 0 replies; 16+ messages in thread
From: Fruhwirth Clemens @ 2005-02-03 15:17 UTC (permalink / raw)
  To: Christopher Warner
  Cc: Matt Mackall, Christophe Saout, christopher, linux-kernel,
	dm-crypt, Alasdair G Kergon

[-- Attachment #1: Type: text/plain, Size: 773 bytes --]

On Thu, 2005-02-03 at 05:15 -0500, Christopher Warner wrote:
> On Thu, 2005-02-03 at 15:18 +0100, Fruhwirth Clemens wrote:
> > 
> > Keys are handed to dm-crypt regularly the first time. But when dm-crypt
> > hands keys back to user space, it uses some sort of blinding to make the
> > keys meaningless for user space. 

> I've been following this thread and i'm clearly at a loss as to how any
> of this will prevent someone from writing a util to get the key?

This is not about trying to hide something which cannot be hidden.

See http://lkml.org/lkml/2005/2/2/256 . It's about a design that can
cope with unintentional program/user errors. Think of it as a trigger
safety. 

-- 
Fruhwirth Clemens <clemens@endorphin.org>  http://clemens.endorphin.org

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: dm-crypt crypt_status reports key?
  2005-02-02 23:50 ` Alasdair G Kergon
  2005-02-03  1:00   ` Matt Mackall
@ 2005-02-03 21:53   ` Pavel Machek
  1 sibling, 0 replies; 16+ messages in thread
From: Pavel Machek @ 2005-02-03 21:53 UTC (permalink / raw)
  To: Matt Mackall, linux-kernel, Christophe Saout, Clemens Fruhwirth,
	dm-crypt

Hi!

> > # dmsetup table /dev/mapper/volume1
> > 0 2000000 crypt aes-plain 0123456789abcdef0123456789abcdef 0 7:0 0
>  
> > Obviously, root can in principle recover this password from the
> > running kernel but it seems silly to make it so easy.
>  
> There seemed little point obfuscating it - someone will only
> write a trivial utility that recovers it.
> 
> The current approach has the advantage of making it
> obvious to you that if you have root access, you have
> access to the password while the encrypted data volumes
> are mounted.
> 
> Consider instead *why* you're worried about the password being
> held in RAM and look for better solutions to *your*
> perceived threats.

Actually, this *is* bad. I bet someone is going to post their secret
key to lkml when debugging...

Or I can see conversation like this:

admin: "My devices work too slowly, is there something wrong with
device mapper?"

Pavel walks to his console, says: "Okay, show me your
/dev/mapper/volume1"

admin does that.

For this to be usefull Pavel'd have to remember the key before admin
realizes what he has done, but..... Or imagine pavel shoulder-surfing
admin trying to debug device mapper.

								Pavel 
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dm-crypt] Re: dm-crypt crypt_status reports key?
  2005-02-03 14:18         ` Fruhwirth Clemens
  2005-02-03 10:15           ` Christopher Warner
  2005-02-03 14:47           ` Andries Brouwer
@ 2005-02-04 13:27           ` Fruhwirth Clemens
  2 siblings, 0 replies; 16+ messages in thread
From: Fruhwirth Clemens @ 2005-02-04 13:27 UTC (permalink / raw)
  To: dm-crypt
  Cc: Christophe Saout, linux-kernel, dm-crypt, Alasdair G Kergon,
	Matt Mackall

On Thu, Feb 03, 2005 at 03:18:20PM +0100, Fruhwirth Clemens wrote:

> Way too complicated. This is a crypto project, why does nobody think of
> crypto to solve the problem :). Here's the idea:

> [see original post, http://lkml.org/lkml/2005/2/3/109 , for idea]

Very simple patch. With that, it's really hard for root to reveal his real
keys accidentally.

Of course the intended 

dmsetup table foo > foo-table
dmsetup remove foo
dmsetup create foo foo-table 

works. If anyone is interested in that feature, this fellow has to clean the
patch and push it.

--- linux-2.6.11-rc1-mm1-therp/drivers/md/dm-crypt.c.orig	2005-02-04 12:53:57.000000000 +0100
+++ linux-2.6.11-rc1-mm1-therp/drivers/md/dm-crypt.c	2005-02-04 14:14:34.927560784 +0100
@@ -18,6 +18,7 @@
 #include <asm/scatterlist.h>
 #include <asm/page.h>
 
+#include <linux/random.h>
 #include "dm.h"
 
 #define PFX	"crypt: "
@@ -488,6 +489,33 @@
 	queue_work(_kcryptd_workqueue, &io->work);
 }
 
+/* Trigger safety */
+
+static char *asure_dc_secret(int want_size) {
+	static char *secret = NULL;
+	static int secret_size = 0;
+
+	// FIXME: obtain some lock
+	if(secret_size < want_size) {
+		char *new_secret = kmalloc(want_size,GFP_KERNEL);
+		// FIXME malloc fail check
+		if(secret) {
+			memcpy(new_secret, secret, secret_size);
+			kfree(secret);
+		}
+
+		get_random_bytes(new_secret+secret_size, want_size - secret_size);
+		secret = new_secret; secret_size = want_size;
+	}
+	return secret;
+}
+
+static void xor_with_secret(char *p, int size) {
+	char *secret = asure_dc_secret(size);
+	while(size--)
+		*p++ ^= *secret++;
+}
+
 /*
  * Decode key from its hex representation
  */
@@ -496,9 +524,14 @@
 	char buffer[3];
 	char *endp;
 	unsigned int i;
+	int post_process = 0;
 
 	buffer[2] = '\0';
 
+	if(*hex == '!') {
+		post_process = 1;
+		hex++;
+	}
 	for(i = 0; i < size; i++) {
 		buffer[0] = *hex++;
 		buffer[1] = *hex++;
@@ -512,6 +545,9 @@
 	if (*hex != '\0')
 		return -EINVAL;
 
+	if (post_process)
+		xor_with_secret(key,size);
+
 	return 0;
 }
 
@@ -522,6 +558,7 @@
 {
 	unsigned int i;
 
+	*hex++ = '!';
 	for(i = 0; i < size; i++) {
 		sprintf(hex, "%02x", *key);
 		hex += 2;
@@ -689,6 +726,8 @@
 	} else
 		cc->iv_mode = NULL;
 
+	xor_with_secret(cc->key, cc->key_size);
+
 	ti->private = cc;
 	return 0;
 
@@ -899,11 +938,11 @@
 			DMEMIT("%s-%s ", cipher, chainmode);
 
 		if (cc->key_size > 0) {
-			if ((maxlen - sz) < ((cc->key_size << 1) + 1))
+			if ((maxlen - sz) < ((cc->key_size << 1) + 2))
 				return -ENOMEM;
 
 			crypt_encode_key(result + sz, cc->key, cc->key_size);
-			sz += cc->key_size << 1;
+			sz += (cc->key_size << 1) + 1;
 		} else {
 			if (sz >= maxlen)
 				return -ENOMEM;

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dm-crypt] Re: dm-crypt crypt_status reports key?
  2005-02-03  4:05       ` Matt Mackall
  2005-02-03 13:07         ` Christophe Saout
  2005-02-03 14:18         ` Fruhwirth Clemens
@ 2005-02-04 14:03         ` Christophe Saout
  2 siblings, 0 replies; 16+ messages in thread
From: Christophe Saout @ 2005-02-04 14:03 UTC (permalink / raw)
  To: dm-crypt; +Cc: linux-kernel, Clemens Fruhwirth, Alasdair G Kergon


[-- Attachment #1.1: Type: text/plain, Size: 1918 bytes --]

Am Mittwoch, den 02.02.2005, 20:05 -0800 schrieb Matt Mackall:

> Dunno here, seems that having one tool that gave the kernel a key named
> "foo" and then telling dm-crypt to use key "foo" is probably not a bad
> way to go. Then we don't have stuff like "echo <key> | dmsetup create"
> and the like and the key-handling smarts can all be put in one
> separate place.
> 
> Getting from here to there might be interesting though. Perhaps we can
> teach dm-crypt to understand keys of the form "keyname:<foo>"? in
> addition to raw keys to keep compatibility. Might even be possible to
> push this down into crypt_decode_key() (or a smarter variant of same).

I've hacked together a working implementation. It's just a prototype
though. The patch requires that the key retention service is activated
unconditionally. It would probably be better to introduce #ifdefs and
the possibility turn of the cleartext key way in the kernel
configuration.

I've separated out the crypto configuration from the device-mapper
configuration. Several device mappings can now share/reference the same
key. This might be useful if you want to span your mapping over several
devices (without stacking device mappings on top of each other). So
basically the crypto and the device-mapper part of the configuration are
in its own "classes" now. It makes even more sense now to rename IV
operations to crypto/key operations (which only work on the new
structure) like it's done in the LRW patch.

This crypto configuration that I separated out can be referenced by the
device-mapper target directly (old way) or indirectly by the new key
retention service. It holds the cipher, chaining mode and IV mechanisms
as well as the key itself.

A dm table configuration line then looks like "0 1000000 crypt key
my_key_name 0 /dev/bla 0".

And dm-crypt.c is slowly getting large. Thirty functions already. Hmm.


[-- Attachment #1.2: Type: text/x-patch, Size: 20655 bytes --]

--- linux-2.6.11-rc3.orig/drivers/md/dm-crypt.c	2005-02-04 01:00:51.983910752 +0100
+++ linux-2.6.11-rc3/drivers/md/dm-crypt.c	2005-02-04 14:30:47.629117856 +0100
@@ -13,6 +13,7 @@
 #include <linux/mempool.h>
 #include <linux/slab.h>
 #include <linux/crypto.h>
+#include <linux/key.h>
 #include <linux/workqueue.h>
 #include <asm/atomic.h>
 #include <asm/scatterlist.h>
@@ -48,14 +49,25 @@
 	int write;
 };
 
-struct crypt_config;
+/*
+ * key and associated crypto information
+ */
+struct crypt_key {
+	struct crypt_iv_operations *iv_gen_ops;
+	char *iv_mode;
+	void *iv_gen_private;
+	unsigned int iv_size;
+
+	struct crypto_tfm *tfm;
+	unsigned int key_size;
+	u8 key[0];
+};
 
 struct crypt_iv_operations {
-	int (*ctr)(struct crypt_config *cc, struct dm_target *ti,
-	           const char *opts);
-	void (*dtr)(struct crypt_config *cc);
-	const char *(*status)(struct crypt_config *cc);
-	int (*generator)(struct crypt_config *cc, u8 *iv, sector_t sector);
+	int (*ctr)(struct crypt_key *ck, const char *opts, char **error_msg);
+	void (*dtr)(struct crypt_key *ck);
+	const char *(*status)(struct crypt_key *ck);
+	int (*generator)(struct crypt_key *ck, u8 *iv, sector_t sector);
 };
 
 /*
@@ -73,18 +85,9 @@
 	mempool_t *io_pool;
 	mempool_t *page_pool;
 
-	/*
-	 * crypto related data
-	 */
-	struct crypt_iv_operations *iv_gen_ops;
-	char *iv_mode;
-	void *iv_gen_private;
 	sector_t iv_offset;
-	unsigned int iv_size;
-
-	struct crypto_tfm *tfm;
-	unsigned int key_size;
-	u8 key[0];
+	struct crypt_key *crypto;
+	struct key *key;
 };
 
 #define MIN_IOS        256
@@ -121,16 +124,16 @@
  * http://article.gmane.org/gmane.linux.kernel.device-mapper.dm-crypt/454
  */
 
-static int crypt_iv_plain_gen(struct crypt_config *cc, u8 *iv, sector_t sector)
+static int crypt_iv_plain_gen(struct crypt_key *ck, u8 *iv, sector_t sector)
 {
-	memset(iv, 0, cc->iv_size);
+	memset(iv, 0, ck->iv_size);
 	*(u32 *)iv = cpu_to_le32(sector & 0xffffffff);
 
 	return 0;
 }
 
-static int crypt_iv_essiv_ctr(struct crypt_config *cc, struct dm_target *ti,
-	                      const char *opts)
+static int crypt_iv_essiv_ctr(struct crypt_key *ck, const char *opts,
+                              char **error_msg)
 {
 	struct crypto_tfm *essiv_tfm;
 	struct crypto_tfm *hash_tfm;
@@ -139,19 +142,19 @@
 	u8 *salt;
 
 	if (opts == NULL) {
-		ti->error = PFX "Digest algorithm missing for ESSIV mode";
+		*error_msg = PFX "Digest algorithm missing for ESSIV mode";
 		return -EINVAL;
 	}
 
 	/* Hash the cipher key with the given hash algorithm */
 	hash_tfm = crypto_alloc_tfm(opts, 0);
 	if (hash_tfm == NULL) {
-		ti->error = PFX "Error initializing ESSIV hash";
+		*error_msg = PFX "Error initializing ESSIV hash";
 		return -EINVAL;
 	}
 
 	if (crypto_tfm_alg_type(hash_tfm) != CRYPTO_ALG_TYPE_DIGEST) {
-		ti->error = PFX "Expected digest algorithm for ESSIV hash";
+		*error_msg = PFX "Expected digest algorithm for ESSIV hash";
 		crypto_free_tfm(hash_tfm);
 		return -EINVAL;
 	}
@@ -159,63 +162,63 @@
 	saltsize = crypto_tfm_alg_digestsize(hash_tfm);
 	salt = kmalloc(saltsize, GFP_KERNEL);
 	if (salt == NULL) {
-		ti->error = PFX "Error kmallocing salt storage in ESSIV";
+		*error_msg = PFX "Error kmallocing salt storage in ESSIV";
 		crypto_free_tfm(hash_tfm);
 		return -ENOMEM;
 	}
 
-	sg.page = virt_to_page(cc->key);
-	sg.offset = offset_in_page(cc->key);
-	sg.length = cc->key_size;
+	sg.page = virt_to_page(ck->key);
+	sg.offset = offset_in_page(ck->key);
+	sg.length = ck->key_size;
 	crypto_digest_digest(hash_tfm, &sg, 1, salt);
 	crypto_free_tfm(hash_tfm);
 
 	/* Setup the essiv_tfm with the given salt */
-	essiv_tfm = crypto_alloc_tfm(crypto_tfm_alg_name(cc->tfm),
+	essiv_tfm = crypto_alloc_tfm(crypto_tfm_alg_name(ck->tfm),
 	                             CRYPTO_TFM_MODE_ECB);
 	if (essiv_tfm == NULL) {
-		ti->error = PFX "Error allocating crypto tfm for ESSIV";
+		*error_msg = PFX "Error allocating crypto tfm for ESSIV";
 		kfree(salt);
 		return -EINVAL;
 	}
 	if (crypto_tfm_alg_blocksize(essiv_tfm)
-	    != crypto_tfm_alg_ivsize(cc->tfm)) {
-		ti->error = PFX "Block size of ESSIV cipher does "
+	    != crypto_tfm_alg_ivsize(ck->tfm)) {
+		*error_msg = PFX "Block size of ESSIV cipher does "
 			        "not match IV size of block cipher";
 		crypto_free_tfm(essiv_tfm);
 		kfree(salt);
 		return -EINVAL;
 	}
 	if (crypto_cipher_setkey(essiv_tfm, salt, saltsize) < 0) {
-		ti->error = PFX "Failed to set key for ESSIV cipher";
+		*error_msg = PFX "Failed to set key for ESSIV cipher";
 		crypto_free_tfm(essiv_tfm);
 		kfree(salt);
 		return -EINVAL;
 	}
 	kfree(salt);
 
-	cc->iv_gen_private = (void *)essiv_tfm;
+	ck->iv_gen_private = (void *)essiv_tfm;
 	return 0;
 }
 
-static void crypt_iv_essiv_dtr(struct crypt_config *cc)
+static void crypt_iv_essiv_dtr(struct crypt_key *ck)
 {
-	crypto_free_tfm((struct crypto_tfm *)cc->iv_gen_private);
-	cc->iv_gen_private = NULL;
+	crypto_free_tfm((struct crypto_tfm *)ck->iv_gen_private);
+	ck->iv_gen_private = NULL;
 }
 
-static int crypt_iv_essiv_gen(struct crypt_config *cc, u8 *iv, sector_t sector)
+static int crypt_iv_essiv_gen(struct crypt_key *ck, u8 *iv, sector_t sector)
 {
 	struct scatterlist sg = { NULL, };
 
-	memset(iv, 0, cc->iv_size);
+	memset(iv, 0, ck->iv_size);
 	*(u64 *)iv = cpu_to_le64(sector);
 
 	sg.page = virt_to_page(iv);
 	sg.offset = offset_in_page(iv);
-	sg.length = cc->iv_size;
-	crypto_cipher_encrypt((struct crypto_tfm *)cc->iv_gen_private,
-	                      &sg, &sg, cc->iv_size);
+	sg.length = ck->iv_size;
+	crypto_cipher_encrypt((struct crypto_tfm *)ck->iv_gen_private,
+	                      &sg, &sg, ck->iv_size);
 
 	return 0;
 }
@@ -236,23 +239,24 @@
                           struct scatterlist *in, unsigned int length,
                           int write, sector_t sector)
 {
-	u8 iv[cc->iv_size];
+	struct crypt_key *ck = cc->crypto;
+	u8 iv[ck->iv_size];
 	int r;
 
-	if (cc->iv_gen_ops) {
-		r = cc->iv_gen_ops->generator(cc, iv, sector);
+	if (ck->iv_gen_ops) {
+		r = ck->iv_gen_ops->generator(ck, iv, sector);
 		if (r < 0)
 			return r;
 
 		if (write)
-			r = crypto_cipher_encrypt_iv(cc->tfm, out, in, length, iv);
+			r = crypto_cipher_encrypt_iv(ck->tfm, out, in, length, iv);
 		else
-			r = crypto_cipher_decrypt_iv(cc->tfm, out, in, length, iv);
+			r = crypto_cipher_decrypt_iv(ck->tfm, out, in, length, iv);
 	} else {
 		if (write)
-			r = crypto_cipher_encrypt(cc->tfm, out, in, length);
+			r = crypto_cipher_encrypt(ck->tfm, out, in, length);
 		else
-			r = crypto_cipher_decrypt(cc->tfm, out, in, length);
+			r = crypto_cipher_decrypt(ck->tfm, out, in, length);
 	}
 
 	return r;
@@ -529,52 +533,34 @@
 	}
 }
 
-/*
- * Construct an encryption mapping:
- * <cipher> <key> <iv_offset> <dev_path> <start>
- */
-static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
+static int crypt_key_ctr(struct crypt_key **retval, char *crypto,
+                         const u8 *key, unsigned int key_size,
+                         char **error_msg)
 {
-	struct crypt_config *cc;
+	unsigned int crypto_flags;
 	struct crypto_tfm *tfm;
-	char *tmp;
+	struct crypt_key *ck;
 	char *cipher;
 	char *chainmode;
 	char *ivmode;
 	char *ivopts;
-	unsigned int crypto_flags;
-	unsigned int key_size;
 
-	if (argc != 5) {
-		ti->error = PFX "Not enough arguments";
-		return -EINVAL;
+	ck = kmalloc(sizeof(*ck) + key_size * sizeof(u8), GFP_KERNEL);
+	if (ck == NULL) {
+		*error_msg = PFX "Out of memory allocating key data";
+		return -ENOMEM;
 	}
 
-	tmp = argv[0];
-	cipher = strsep(&tmp, "-");
-	chainmode = strsep(&tmp, "-");
-	ivopts = strsep(&tmp, "-");
+	memcpy(ck->key, key, key_size * sizeof(u8));
+
+	cipher = strsep(&crypto, "-");
+	chainmode = strsep(&crypto, "-");
+	ivopts = strsep(&crypto, "-");
 	ivmode = strsep(&ivopts, ":");
 
-	if (tmp)
+	if (crypto)
 		DMWARN(PFX "Unexpected additional cipher options");
 
-	key_size = strlen(argv[1]) >> 1;
-
-	cc = kmalloc(sizeof(*cc) + key_size * sizeof(u8), GFP_KERNEL);
-	if (cc == NULL) {
-		ti->error =
-			PFX "Cannot allocate transparent encryption context";
-		return -ENOMEM;
-	}
-
-	cc->key_size = key_size;
-	if ((!key_size && strcmp(argv[1], "-") != 0) ||
-	    (key_size && crypt_decode_key(cc->key, argv[1], key_size) < 0)) {
-		ti->error = PFX "Error decoding key";
-		goto bad1;
-	}
-
 	/* Compatiblity mode for old dm-crypt cipher strings */
 	if (!chainmode || (strcmp(chainmode, "plain") == 0 && !ivmode)) {
 		chainmode = "cbc";
@@ -587,123 +573,217 @@
 	else if (strcmp(chainmode, "ecb") == 0)
 		crypto_flags = CRYPTO_TFM_MODE_ECB;
 	else {
-		ti->error = PFX "Unknown chaining mode";
+		*error_msg = PFX "Unknown chaining mode";
 		goto bad1;
 	}
 
 	if (crypto_flags != CRYPTO_TFM_MODE_ECB && !ivmode) {
-		ti->error = PFX "This chaining mode requires an IV mechanism";
+		*error_msg = PFX "This chaining mode requires an IV mechanism";
 		goto bad1;
 	}
 
 	tfm = crypto_alloc_tfm(cipher, crypto_flags);
 	if (!tfm) {
-		ti->error = PFX "Error allocating crypto tfm";
+		*error_msg = PFX "Error allocating crypto tfm";
 		goto bad1;
 	}
 	if (crypto_tfm_alg_type(tfm) != CRYPTO_ALG_TYPE_CIPHER) {
-		ti->error = PFX "Expected cipher algorithm";
+		*error_msg= PFX "Expected cipher algorithm";
 		goto bad2;
 	}
 
-	cc->tfm = tfm;
+	ck->tfm = tfm;
 
 	/*
 	 * Choose ivmode. Valid modes: "plain", "essiv:<esshash>".
 	 * See comments at iv code
 	 */
-
 	if (ivmode == NULL)
-		cc->iv_gen_ops = NULL;
+		ck->iv_gen_ops = NULL;
 	else if (strcmp(ivmode, "plain") == 0)
-		cc->iv_gen_ops = &crypt_iv_plain_ops;
+		ck->iv_gen_ops = &crypt_iv_plain_ops;
 	else if (strcmp(ivmode, "essiv") == 0)
-		cc->iv_gen_ops = &crypt_iv_essiv_ops;
+		ck->iv_gen_ops = &crypt_iv_essiv_ops;
 	else {
-		ti->error = PFX "Invalid IV mode";
+		*error_msg = PFX "Invalid IV mode";
 		goto bad2;
 	}
 
-	if (cc->iv_gen_ops && cc->iv_gen_ops->ctr &&
-	    cc->iv_gen_ops->ctr(cc, ti, ivopts) < 0)
+	if (ck->iv_gen_ops && ck->iv_gen_ops->ctr &&
+	    ck->iv_gen_ops->ctr(ck, ivopts, error_msg) < 0)
 		goto bad2;
 
 	if (tfm->crt_cipher.cit_decrypt_iv && tfm->crt_cipher.cit_encrypt_iv)
 		/* at least a 64 bit sector number should fit in our buffer */
-		cc->iv_size = max(crypto_tfm_alg_ivsize(tfm),
+		ck->iv_size = max(crypto_tfm_alg_ivsize(tfm),
 		                  (unsigned int)(sizeof(u64) / sizeof(u8)));
 	else {
-		cc->iv_size = 0;
-		if (cc->iv_gen_ops) {
+		ck->iv_size = 0;
+		if (ck->iv_gen_ops) {
 			DMWARN(PFX "Selected cipher does not support IVs");
-			if (cc->iv_gen_ops->dtr)
-				cc->iv_gen_ops->dtr(cc);
-			cc->iv_gen_ops = NULL;
+			if (ck->iv_gen_ops->dtr)
+				ck->iv_gen_ops->dtr(ck);
+			ck->iv_gen_ops = NULL;
 		}
 	}
 
+	ck->key_size = key_size;
+	if (tfm->crt_cipher.cit_setkey(tfm, ck->key, key_size) < 0) {
+		*error_msg = PFX "Error setting key";
+		goto bad3;
+	}
+
+	if (ivmode && ck->iv_gen_ops) {
+		if (ivopts)
+			*(ivopts - 1) = ':';
+		ck->iv_mode = kmalloc(strlen(ivmode) + 1, GFP_KERNEL);
+		if (ck->iv_mode == NULL) {
+			*error_msg = PFX "Error kmallocing iv_mode string";
+			goto bad3;
+		}
+		strcpy(ck->iv_mode, ivmode);
+	} else
+		ck->iv_mode = NULL;
+
+	*retval = ck;
+	return 0;
+
+bad3:
+	if (ck->iv_gen_ops && ck->iv_gen_ops->dtr)
+		ck->iv_gen_ops->dtr(ck);
+bad2:
+	crypto_free_tfm(tfm);
+bad1:
+	kfree(ck);
+
+	return -EINVAL;
+}
+
+static void crypt_key_dtr(struct crypt_key *ck)
+{
+	if (ck->iv_mode)
+		kfree(ck->iv_mode);
+	if (ck->iv_gen_ops && ck->iv_gen_ops->dtr)
+		ck->iv_gen_ops->dtr(ck);
+	crypto_free_tfm(ck->tfm);
+	kfree(ck);
+}
+
+static struct key_type crypt_key_type;
+
+/*
+ * Construct an encryption mapping:
+ * <cipher> <key> <iv_offset> <dev_path> <start>
+ */
+static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
+{
+	struct crypt_config *cc;
+	struct crypt_key *ck;
+	struct key *key = NULL;
+	int retval;
+
+	if (argc != 5) {
+		ti->error = PFX "Wrong argument count";
+		return -EINVAL;
+	}
+
+	if (strcmp(argv[0], "key") == 0) {
+		key = search_process_keyrings(&crypt_key_type, argv[1]);
+		if (IS_ERR(key)) {
+			ti->error = PFX "Key not found in process keyrings";
+			return PTR_ERR(key);
+		}
+
+		ck = key->payload.data;
+		BUG_ON(!ck);
+	} else {
+		unsigned int key_size;
+		int err;
+		u8 *key_data;
+
+		key_size = strlen(argv[1]) >> 1;
+		key_data = kmalloc(key_size, GFP_KERNEL);
+		if (key_data == NULL) {
+			ti->error = PFX "Out of memory allocating key buffer";
+			return -ENOMEM;
+		}
+
+		if ((!key_size && strcmp(argv[1], "-") != 0) ||
+		    (key_size &&
+		     crypt_decode_key(key_data, argv[1], key_size) < 0)) {
+			ti->error = PFX "Error decoding key";
+			kfree(key_data);
+			return -EINVAL;
+		}
+
+		err = crypt_key_ctr(&ck, argv[0], key_data, key_size,
+		                    &ti->error);
+
+		kfree(key);
+
+		if (err < 0)
+			return err;
+	}
+
+	retval = -ENOMEM;
+
+	cc = kmalloc(sizeof(*cc), GFP_KERNEL);
+	if (cc == NULL) {
+		ti->error =
+			PFX "Cannot allocate transparent encryption context";
+		goto bad1;
+	}
+
+	cc->crypto = ck;
+	cc->key = key;
+
 	cc->io_pool = mempool_create(MIN_IOS, mempool_alloc_slab,
 				     mempool_free_slab, _crypt_io_pool);
 	if (!cc->io_pool) {
 		ti->error = PFX "Cannot allocate crypt io mempool";
-		goto bad3;
+		goto bad2;
 	}
 
 	cc->page_pool = mempool_create(MIN_POOL_PAGES, mempool_alloc_page,
 				       mempool_free_page, NULL);
 	if (!cc->page_pool) {
 		ti->error = PFX "Cannot allocate page mempool";
-		goto bad4;
+		goto bad3;
 	}
 
-	if (tfm->crt_cipher.cit_setkey(tfm, cc->key, key_size) < 0) {
-		ti->error = PFX "Error setting key";
-		goto bad5;
-	}
+	retval = -EINVAL;
 
 	if (sscanf(argv[2], SECTOR_FORMAT, &cc->iv_offset) != 1) {
 		ti->error = PFX "Invalid iv_offset sector";
-		goto bad5;
+		goto bad4;
 	}
 
 	if (sscanf(argv[4], SECTOR_FORMAT, &cc->start) != 1) {
 		ti->error = PFX "Invalid device sector";
-		goto bad5;
+		goto bad4;
 	}
 
 	if (dm_get_device(ti, argv[3], cc->start, ti->len,
 	                  dm_table_get_mode(ti->table), &cc->dev)) {
 		ti->error = PFX "Device lookup failed";
-		goto bad5;
+		goto bad4;
 	}
 
-	if (ivmode && cc->iv_gen_ops) {
-		if (ivopts)
-			*(ivopts - 1) = ':';
-		cc->iv_mode = kmalloc(strlen(ivmode) + 1, GFP_KERNEL);
-		if (!cc->iv_mode) {
-			ti->error = PFX "Error kmallocing iv_mode string";
-			goto bad5;
-		}
-		strcpy(cc->iv_mode, ivmode);
-	} else
-		cc->iv_mode = NULL;
-
 	ti->private = cc;
 	return 0;
 
-bad5:
-	mempool_destroy(cc->page_pool);
 bad4:
-	mempool_destroy(cc->io_pool);
+	mempool_destroy(cc->page_pool);
 bad3:
-	if (cc->iv_gen_ops && cc->iv_gen_ops->dtr)
-		cc->iv_gen_ops->dtr(cc);
+	mempool_destroy(cc->io_pool);
 bad2:
-	crypto_free_tfm(tfm);
-bad1:
 	kfree(cc);
-	return -EINVAL;
+bad1:
+	if (key)
+		key_put(key);
+	else
+		crypt_key_dtr(ck);
+	return retval;
 }
 
 static void crypt_dtr(struct dm_target *ti)
@@ -713,11 +793,11 @@
 	mempool_destroy(cc->page_pool);
 	mempool_destroy(cc->io_pool);
 
-	if (cc->iv_mode)
-		kfree(cc->iv_mode);
-	if (cc->iv_gen_ops && cc->iv_gen_ops->dtr)
-		cc->iv_gen_ops->dtr(cc);
-	crypto_free_tfm(cc->tfm);
+	if (cc->key)
+		key_put(cc->key);
+	else
+		crypt_key_dtr(cc->crypto);
+
 	dm_put_device(ti, cc->dev);
 	kfree(cc);
 }
@@ -865,13 +945,51 @@
 	return -ENOMEM;
 }
 
+static unsigned int crypt_key_status(struct crypt_key *ck, char *result,
+                                     unsigned int maxlen)
+{
+	const char *cipher;
+	const char *chainmode = NULL;
+	unsigned int sz = 0;
+
+	cipher = crypto_tfm_alg_name(ck->tfm);
+
+	switch(ck->tfm->crt_cipher.cit_mode) {
+	case CRYPTO_TFM_MODE_CBC:
+		chainmode = "cbc";
+		break;
+	case CRYPTO_TFM_MODE_ECB:
+		chainmode = "ecb";
+		break;
+	default:
+		BUG();
+	}
+
+	if (ck->iv_mode)
+		DMEMIT("%s-%s-%s ", cipher, chainmode, ck->iv_mode);
+	else
+		DMEMIT("%s-%s ", cipher, chainmode);
+
+	if (ck->key_size > 0) {
+		if ((maxlen - sz) < ((ck->key_size << 1) + 1))
+			return -ENOMEM;
+
+		crypt_encode_key(result + sz, ck->key, ck->key_size);
+		sz += ck->key_size << 1;
+	} else {
+		if (sz >= maxlen)
+			return -ENOMEM;
+		result[sz++] = '-';
+	}
+
+	return sz;
+}
+
 static int crypt_status(struct dm_target *ti, status_type_t type,
 			char *result, unsigned int maxlen)
 {
 	struct crypt_config *cc = (struct crypt_config *) ti->private;
 	char buffer[32];
-	const char *cipher;
-	const char *chainmode = NULL;
 	unsigned int sz = 0;
 
 	switch (type) {
@@ -880,35 +998,12 @@
 		break;
 
 	case STATUSTYPE_TABLE:
-		cipher = crypto_tfm_alg_name(cc->tfm);
-
-		switch(cc->tfm->crt_cipher.cit_mode) {
-		case CRYPTO_TFM_MODE_CBC:
-			chainmode = "cbc";
-			break;
-		case CRYPTO_TFM_MODE_ECB:
-			chainmode = "ecb";
-			break;
-		default:
-			BUG();
-		}
-
-		if (cc->iv_mode)
-			DMEMIT("%s-%s-%s ", cipher, chainmode, cc->iv_mode);
-		else
-			DMEMIT("%s-%s ", cipher, chainmode);
-
-		if (cc->key_size > 0) {
-			if ((maxlen - sz) < ((cc->key_size << 1) + 1))
-				return -ENOMEM;
-
-			crypt_encode_key(result + sz, cc->key, cc->key_size);
-			sz += cc->key_size << 1;
-		} else {
-			if (sz >= maxlen)
-				return -ENOMEM;
-			result[sz++] = '-';
-		}
+		if (cc->key) {
+			read_lock(&cc->key->lock);
+			DMEMIT("key %s", cc->key->description);
+			read_unlock(&cc->key->lock);
+		} else
+			sz = crypt_key_status(cc->crypto, result, maxlen);
 
 		format_dev_t(buffer, cc->dev->bdev->bd_dev);
 		DMEMIT(" " SECTOR_FORMAT " %s " SECTOR_FORMAT,
@@ -918,6 +1013,85 @@
 	return 0;
 }
 
+static int crypt_key_instantiate(struct key *key, const void *data,
+                                 size_t datalen)
+{
+	struct crypt_key *ck;
+	unsigned int len;
+	char *error_msg = NULL;
+	char *crypto;
+	int retval = -EINVAL;
+
+	len = strnlen(data, datalen);
+	if (len >= datalen)
+		return -EINVAL;
+
+	crypto = kmalloc(len + 1, GFP_KERNEL);
+	if (crypto < 0)
+		return -ENOMEM;
+	strcpy(crypto, (const char *)data);
+
+	data += len + 1;
+	datalen -= len + 1;
+
+	if (len < sizeof(u16))
+		goto bad1;
+
+	/* FIXME: byte ordering? Host byte ordering should be ok */
+	len = *(u16 *)data;
+	data += sizeof(u16);
+	datalen -= sizeof(u16);
+
+	if (datalen != len * sizeof(u8))
+		goto bad1;
+
+	retval = crypt_key_ctr(&ck, crypto, (u8 *)data, len, &error_msg);
+	if (retval < 0) {
+		DMERR("%s", error_msg);
+		goto bad1;
+	}
+
+	/* FIXME: An approximation of the payload size */
+	len = sizeof(*ck) + ck->key_size + sizeof(*ck->tfm);
+	retval = key_payload_reserve(key, len);
+	if (retval < 0)
+		goto bad2;
+
+	kfree(crypto);
+
+	key->payload.data = ck;
+
+	return 0;
+
+bad2:
+	crypt_key_dtr(ck);
+bad1:
+	kfree(crypto);
+	return retval;
+}
+
+static int crypt_key_match(const struct key *key, const void *description)
+{
+	return strcmp(key->description, description) == 0;
+}
+
+static void crypt_key_destroy(struct key *key)
+{
+	struct crypt_key *ck = (struct crypt_key *)key->payload.data;
+
+	if (ck)
+		crypt_key_dtr(ck);
+}
+
+static struct key_type crypt_key_type = {
+	.name		= "dm-crypt",
+	.def_datalen	= sizeof(struct crypt_key) +
+			  sizeof(struct crypto_tfm),
+	.instantiate	= crypt_key_instantiate,
+	.match		= crypt_key_match,
+	.destroy	= crypt_key_destroy,
+};
+
 static struct target_type crypt_target = {
 	.name   = "crypt",
 	.version= {1, 1, 0},
@@ -947,12 +1121,20 @@
 
 	r = dm_register_target(&crypt_target);
 	if (r < 0) {
-		DMERR(PFX "register failed %d", r);
+		DMERR(PFX "dm register failed %d", r);
 		goto bad2;
 	}
 
+	r = register_key_type(&crypt_key_type);
+	if (r < 0) {
+		DMERR(PFX "key type register failed %d", r);
+		goto bad3;
+	}
+
 	return 0;
 
+bad3:
+	dm_unregister_target(&crypt_target);
 bad2:
 	destroy_workqueue(_kcryptd_workqueue);
 bad1:
@@ -962,8 +1144,11 @@
 
 static void __exit dm_crypt_exit(void)
 {
-	int r = dm_unregister_target(&crypt_target);
+	int r;
+
+	unregister_key_type(&crypt_key_type);
 
+	r = dm_unregister_target(&crypt_target);
 	if (r < 0)
 		DMERR(PFX "unregister failed %d", r);
 

[-- Attachment #2: Dies ist ein digital signierter Nachrichtenteil --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2005-02-04 14:09 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-02-02 21:19 dm-crypt crypt_status reports key? Matt Mackall
2005-02-02 23:50 ` Alasdair G Kergon
2005-02-03  1:00   ` Matt Mackall
2005-02-03 21:53   ` Pavel Machek
2005-02-03  1:33 ` Christophe Saout
2005-02-03  1:52   ` Matt Mackall
2005-02-03  2:34     ` Christophe Saout
2005-02-03  4:05       ` Matt Mackall
2005-02-03 13:07         ` Christophe Saout
2005-02-03 14:18         ` Fruhwirth Clemens
2005-02-03 10:15           ` Christopher Warner
2005-02-03 15:17             ` Fruhwirth Clemens
2005-02-03 14:47           ` Andries Brouwer
2005-02-03 15:00             ` Fruhwirth Clemens
2005-02-04 13:27           ` [dm-crypt] " Fruhwirth Clemens
2005-02-04 14:03         ` Christophe Saout

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).