Re: [PATCH] Export symbol ksize()

2009-02-13 Thread Kyle Moffett
On Fri, Feb 13, 2009 at 8:20 AM, Nick Piggin  wrote:
> On Friday 13 February 2009 10:37:01 Matt Mackall wrote:
>> On Fri, 2009-02-13 at 07:09 +0800, Herbert Xu wrote:
>> > On Fri, Feb 13, 2009 at 12:10:45AM +1100, Nick Piggin wrote:
>> > > I would be interested to know how that goes. You always have this
>> > > circular issue that if a little more space helps significantly, then
>> > > maybe it is a good idea to explicitly ask for those bytes. Of course
>> > > that larger allocation is also likely to have some slack bytes.
>> >
>> > Well, the thing is we don't know apriori whether we need the
>> > extra space.  The idea is to use the extra space if available
>> > to avoid reallocation when we hit things like IPsec.
>>
>> I'm not entirely convinced by this argument. If you're concerned about
>> space rather than performance, then you want an allocator that doesn't
>> waste space in the first place and you don't try to do "sub-allocations"
>> by hand. If you're concerned about performance, you instead optimize
>> your allocator to be as fast as possible and again avoid conditional
>> branches for sub-allocations.
>
> Well, my earlier reasoning is no longer so clear cut if eg. there
> are common cases where no extra space is required, but rare cases
> where extra space might be a big win if it eg avoids extra
> alloc, copy, free or something.
>
> Because even with performance oriented allocators, there is a non-zero
> cost to explicitly asking for more memory -- queues tend to get smaller
> at larger object sizes, and page allocation orders can increase. So if
> it is very uncommon to need extra space you don't want to burden the
> common case with it.

My concern would be that such extra-space reuse would be a very
non-obvious performance hit if allocation patterns changed slightly.
If being able to use the extra space really is a noticeable "big win"
for the rare case, then minor changes to the memory allocator could
dramatically impact performance in a totally nondeterministic way.  If
the change isn't performance-significant in the grand scheme of
things, then the use of ksize() would just be code obfuscation.  On
the other hand if it *is* performance-significant, it should be
redesigned to be able to guarantee that the space is available when it
is needed.

Cheers,
Kyle Moffett
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Initcall ordering problem (TTY vs modprobe vs MD5) and cryptomgr problem

2010-08-05 Thread Kyle Moffett
On Fri, Aug 6, 2010 at 00:20, Linus Torvalds
 wrote:
> On Thu, Aug 5, 2010 at 7:35 PM, Herbert Xu  
> wrote:
>> Because it can save data.  Each cryptographic algorithm (such as
>> AES) may have multiple impelmentations, some of which are hardware-
>> based.
>
> Umm. The _developer_ had better test the thing. That is absolutely
> _zero_ excuse for then forcing every boot for every poor user to re-do
> the test over and over again.
>
> Guys, this comes up every single time: you as a developer may think
> that your code is really important, but get over yourself already.
> It's not so important that everybody must be forced to do it.

Speaking as a user whose been bitten several times by bad crypto
implementations, I'd personally rather have this testing on by default
(if the crypto API it depends on is on).  It's pretty damn inexpensive
to do a few brief crypto operations during initialization as a quick
smoke test.  We already do something somewhat similar when loading the
RAID5/RAID6 driver, although admittedly that's a speed-test for
picking an optimized algorithm.

You should also realize that crypto drivers are very much *NOT* in the
same situation as most other drivers.  Without this test, adding a new
crypto hardware driver to the kernel is a completely unsafe operation,
because it could completely break users setups.  You have previously
said you're fine accepting new drivers even after the initial merge
window because they can't break anything, but in crypto that's not
true.

I've actually had it trigger in exactly the described situation.  I
had a box with an encrypted filesystem that I downloaded a new distro
kernel on with new drivers.  The new kernel included a bunch of new
"EXPERIMENTAL" drivers for hardware, none of which I thought I cared
about until I noticed in "dmesg" that one of them was getting enabled
and then failing tests.

So there are unique and compelling reasons for default-enabled basic
smoke tests of cryptographic support during boot.  To be honest, the
test and integration engineer in me would like it if there were more
intensive in-kernel POST tests that could be enabled by a kernel
parameter or something for high-reliability embedded devices.

Cheers,
Kyle Moffett
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 01/19] User-space API definition

2010-08-21 Thread Kyle Moffett
On Fri, Aug 20, 2010 at 04:45, Miloslav Trmač  wrote:
> This patch introduces the new user-space API, .
>
> Quick overview:
>
> * open("/dev/crypto") to get a FD, which acts as a namespace for key and
>  session identifiers.
>
> * ioctl(NCRIO_KEY_INIT) to allocate a key object; then generate the key
>  material inside the kernel, load a plaintext key, unwrap a key, or
>  derive a key.  Similarly the key material can be copied out of the
>  kernel or wrapped.
>
> [...snip...]

Ugh... We already have one very nice key/keyring API in the kernel
(see Documentation/keys.txt) that's being used for crypto keys for
NFSv4, AFS, etc.  Can't you just add a bunch of cryptoapi key types to
that API instead?

David Howells added to CC, since I believe he wrote most of that code initially.

Cheers,
Kyle Moffett
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 01/19] User-space API definition

2010-09-06 Thread Kyle Moffett
On Mon, Sep 6, 2010 at 11:50, Miloslav Trmac  wrote:
> - "Herbert Xu"  wrote:
>> On Mon, Aug 23, 2010 at 11:37:40AM -0400, Miloslav Trmac wrote:
>> > I have seriously considered the keyring API, and this is what I came
>> up with - but I'd love to be shown a better way.
>>
>> FWIW adding a second key management system to the kernel is
>> totally out of the question.
>>
>> If the existing system doesn't work for you, find a way to build
>> on it so that it does.  Adding a second system that pretty much
>> does the same thing is unacceptable.
> It does _not_ do the same thing, same as ramfs and file descriptors do not do 
> the same thing although they are both related to files.
>
> The kernel keyring service is basically a system-wide data storage service.  
> /dev/crypto needs a quick way to refer to short-lived, usually process-local, 
> kernel-space data structures from userspace.

The problem with the approach you're proposing is that we then have
two entirely separate classes of keys.  First we have the existing
keyring class, which can be securely and revokably passed between
different processes with limited rights, but cannot be handed up to
the kernel's cryptoapi.  Then we have your new class, which are
anonymous keys with a brand new security model (which doesn't even
have LSM hooks yet) and which cannot be referenced by name.

Another potential issue is that keys are never actually "unnamed", in
that sense.  If encryption keys truly were "anonymous" then you would
find it impossible to reliably decrypt the data on the other end.  For
example, every RSA private key should be indexed either by the X.509
DN or for bare SSH keys by the public modulus information.  Even
transient SSL session keys are always put into an SSL session cache by
apache or whatever to allow them to be reused across multiple TCP
streams!  So I would argue that an SSL implementation that uses this
should actually create or use a keyring specifically as an SSL session
cache (with keys indexed by SSL session ID).

It then becomes trivial to share an SSL session cache between 3
independent HTTPS server programs from different vendors, such that
the compromise of *any* of the processes would not in any way
compromise the security of the session keys.  This would be especially
true if the session keys are actually generated by a keyring+cryptoapi
operation in the kernel.

So my recommendation would be to create some new operations of the
existing keyring code:

(1) If you *really* care about anonymous transient keys that are not
identified by an SSL session ID or similar, then add a keyring
operation for "create an anonymous key in keyring X, where the kernel
creates a proper temporary name".  An SSL implementation would default
to using the process-local keyring, which means that everything would
automatically go away on process exit.

(2) Add cryptoapi hooks to automatically register keyring key types
based on the loaded cryptoapi modules.

(3) Add any necessary keyring operations for efficiently performing
zero-copy cryptoapi calls using those key types.

Cheers,
Kyle Moffett
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 01/19] User-space API definition

2010-09-06 Thread Kyle Moffett
On Mon, Sep 6, 2010 at 15:13, Nikos Mavrogiannopoulos
 wrote:
> On 09/06/2010 08:00 PM, Kyle Moffett wrote:
>>> The kernel keyring service is basically a system-wide data storage
>>> service.  /dev/crypto needs a quick way to refer to short-lived,
>>> usually process-local, kernel-space data structures from
>>> userspace.
>>
>> The problem with the approach you're proposing is that we then have
>> two entirely separate classes of keys.  First we have the existing
>> keyring class, which can be securely and revokably passed between
>> different processes with limited rights, but cannot be handed up to
>> the kernel's cryptoapi.
>
> I don't think this is the case. The NCR does not store any keys nor
> retrieves them. It does delegate the burden of that to userspace
> application. NCR exports a wrapped version of the key and the userspace
> application stores it. It could use the keyring to store the keys or
> could directly store them in the filesystem.

Hmm, I'm confused.  You say "The NCR does not store any keys nor
retrieves them", but ~75% of your API is specifically related to
storing keys into kernel memory or retrieving them out of kernel
memory.  Specifically, putting keys into and out of the kernel and
passing them around between processes is the *whole point* of the
keyring API.

So let me ask for some clarification:

You talk a lot in the patches about the API itself, but what is the
intended *use-case* for NCR?

Is it to provide a back-end for code such as enhanced-security OpenSSL
libraries?  For example, a privileged process would loads a key from
disk into the kernel, then fork the unprivileged SSL server process?

Is it just a canonical interface for userspace to encrypt or decrypt
data using the kernel's CryptoAPI?

Cheers,
Kyle Moffett
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 01/19] User-space API definition

2010-09-06 Thread Kyle Moffett
On Mon, Sep 6, 2010 at 17:11, Nikos Mavrogiannopoulos
 wrote:
> I suppose you mean the reference to the internal representation of the
> key. This might be valid for few seconds until the required operation is
> over.
>
> This is not really what I would call storage. The storage and retrieval
> of keys is being done using two ioctl() the STORAGE_WRAP and STORAGE_UNWRAP.
>
> An example of how NCR works:
> 1. A Process generates an RSA key pair
> 2. Stores the (encrypted) pair using the STORAGE_WRAP to a file.
>
> 3. Another process loads the file, unwraps it using STORAGE_UNWRAP and
> gets a reference to the key
> 4. Does an RSA decryption using the key
> 5. Discards the reference to the key
>
> Consider the reference as a file descriptor after you have opened a file
> (a wrapped key).
>
> How do you see keyring being involved in a setup like this?

Fundamentally the operations you described are *EXACTLY* the kind of
things that I believe the keyring API should support.  It does not
support them now, but that API is where the described functionality
really belongs.  Please consider just extending the keyring API to
support what you need.

For example, assuming that we automatically register 1 keyring key
type per algo it should be pretty straightforward.

You would want to write a "request_key()" handler for those key types
which can use whatever hardware support is available to automatically
generate a new random key or alternatively pass the operation off to
the /sbin/request-key process to generate and load one from userspace.
 The call might take a *long* time to complete, depending on the key
type and whether or not there is hardware support.

That would allow you to perform the "generate an RSA key" step in
either a trusted "request-key" process or directly in a piece of
hardware, helping to avoid accidental key material leaks from the
unprivileged process.

Then, you would want "KEYCTL_DECRYPT_KEY", and "KEYCTL_ENCRYPT_KEY"
which would use one key to encrypt another into a user buffer (or
decrypt a key from a user buffer).  These would probably need new LSM
hooks and maybe key DAC permission bits.

This would implement the "STORAGE_WRAP" and "STORAGE_UNWRAP"
functionality you want, but it would be extensible to much more than
just what NCR needs.  I could see this being very useful as an
extension to the existing Kerberos or NFS keyring usage.  Other
potential applications include very secure replacements for the SSH or
GPG agent programs.

For the final step of actually performing encryption/decryption of
user data you would then want generic keyctl() ops for
"KEYCTL_ENCRYPT" and "KEYCTL_DECRYPT", which would simply call
CryptoAPI with the user-provided input and output buffers.  Again,
you'd need new LSM hooks, etc.  These are very obviously extensible to
other applications.

It's getting a bit late here, so I apologize if anything I've written
above makes particularly little sense, but hopefully I've gotten the
gist across.

Cheers,
Kyle Moffett
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html