linux-kernel - Re: [PATCH v2] crypto: AF_ALG - add support for keys/asymmetric-type

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 13 Jan 2016 05:52:07 -0800
From:	Tadeusz Struk <tadeusz.struk@...el.com>
To:	David Woodhouse <dwmw2@...radead.org>,
	Tadeusz Struk <tstruk@...il.com>, herbert@...dor.apana.org.au
Cc:	smueller@...onox.de, linux-api@...r.kernel.org,
	marcel@...tmann.org, linux-kernel@...r.kernel.org,
	dhowells@...hat.com, keyrings@...r.kernel.org,
	linux-crypto@...r.kernel.org, zohar@...ux.vnet.ibm.com
Subject: Re: [PATCH v2] crypto: AF_ALG - add support for keys/asymmetric-type

Hello David,
On 01/13/2016 04:27 AM, David Woodhouse wrote:
>> +static int alg_setkey_id(void *private, const u8 *key, unsigned int keylen,
>> > +                        int (*setkey)(void *private, const u8 *key,
>> > +                                      unsigned int keylen))
>> > +{
>> > +       struct key *keyring;
>> > +       struct public_key *pkey;
>> > +       char key_name[12];
>> > +       u32 keyid = *((u32 *)key);
>> > +       int err;
>> > +
>> > +       sprintf(key_name, "id:%08x", keyid);
>> > +       keyring = request_key(&key_type_asymmetric, key_name, NULL);
>> > +
>> > +       err = -ENOKEY;
>> > +       if (IS_ERR(keyring))
>> > +               goto out;
>> > +
>> > +       pkey = keyring->payload.data[asym_crypto];
>> > +       if (!pkey) {
>> > +               key_put(keyring);
>> > +               goto out;
>> > +       }
>> > +
>> > +       err = setkey(private, pkey->key, pkey->keylen);
>> > +       key_put(keyring);
>> > +
>> > +out:
>> > +       return err;
>> > +}
> This seems entirely wrong to me. You cannot just assume that the
> private key data are available in software form to the kernel and can
> be extracted.

Thanks for your feedback.
Yes, I'm aware that there might be something missing, and it might not
work in all the cases, but as far as the user interface is concerned
do you think having the ALG_SET_KEY, ALG_SET_PUBKEY along with
ALG_SET_PUBKEY_ID and ALG_SET_KEY_ID should be enough for all cases.
That's the main point of this patch - to get the user interface right.
The worst what can happen now if the keys need to be explicitly
extracted from a TPM HW is that the user will get ENOKEY error.
>From the other hand shouldn't that happen under the hood in request_key()
It doesn't look right to me that the pf_alg module has to directly
interact with the TPM.

> 
> Please ensure you test this with a key in a TPM, or in a SGX software
> enclave or something like that.

I don't have any HW for this. I only tested it with the keys instantiated
using the keyctl tool.

Thanks,
-- 
TS