linux-kernel - Re: [PATCH 1/2] crypto: KEYS: convert public key to the akcipher API

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]

Message-ID: <10795.1439475796@warthog.procyon.org.uk>
Date:	Thu, 13 Aug 2015 15:23:16 +0100
From:	David Howells <dhowells@...hat.com>
To:	Tadeusz Struk <tadeusz.struk@...el.com>
Cc:	dhowells@...hat.com, herbert@...dor.apana.org.au,
	keescook@...omium.org, jwboyer@...hat.com, smueller@...onox.de,
	richard@....at, steved@...hat.com, linux-kernel@...r.kernel.org,
	linux-crypto@...r.kernel.org, james.l.morris@...cle.com,
	jkosina@...e.cz, zohar@...ux.vnet.ibm.com, davem@...emloft.net,
	vgoyal@...hat.com
Subject: Re: [PATCH 1/2] crypto: KEYS: convert public key to the akcipher API

Tadeusz Struk <tadeusz.struk@...el.com> wrote:

>  const char *const pkey_algo_name[PKEY_ALGO__LAST] = {
> -	[PKEY_ALGO_DSA]		= "DSA",
> -	[PKEY_ALGO_RSA]		= "RSA",
> +	[PKEY_ALGO_DSA]		= "dsa",
> +	[PKEY_ALGO_RSA]		= "rsa",
>  };

Be aware that these are exposed to userspace through /proc.  The change
probably doesn't matter, but you might need to update the documentation.

> +int public_key_verify_signature(const struct public_key *pkey,
>  				const struct public_key_signature *sig)
>  {
> ...
> -	return algo->verify_signature(pk, sig);
> +	return rsa_pkcs1_v1_5_verify_signature(pkey, sig);
>  }

No.  You can't assume RSA here.  It's quite likely we'll have to support ECDSA
or similar soon.  This must be contingent on the algorithm selected.

>  {
>  	const struct public_key *pk = key->payload.data;
> +
>  	return public_key_verify_signature(pk, sig);
>  }

That's nothing to do with this patch.

> +static int rsa_signture_verify(const u8 *H, const u8 *EM, size_t k,

'signture' -> 'signature'.

> +/*
> + * Perform the RSA signature verification.
> + * @H: Value of hash of data and metadata
> + * @EM: The computed signature value
> + * @k: The size of EM (EM[0] is an invalid location but should hold 0x00)
> + * @hash_size: The size of H
> + * @asn1_template: The DigestInfo ASN.1 template
> + * @asn1_size: Size of asm1_template[]
> + */
> +static int rsa_signture_verify(const u8 *H, const u8 *EM, size_t k,
> +			       size_t hash_size, const u8 *asn1_template,
> +			       size_t asn1_size)
> +{

Why is this here and not in crypto/rsa.c?

> +	/* initlialzie out buf */

'initialise'.

> -	/* Decode the public key */
> -	ret = asn1_ber_decoder(&x509_rsakey_decoder, ctx,
> -			       ctx->key, ctx->key_size);
> -	if (ret < 0)
> +	cert->pub->key = kmemdup(ctx->key, ctx->key_size, GFP_KERNEL);
> +	if (!cert->pub->key)
>  		goto error_decode;

The generic public key code should *not* see the container wrappings (ASN.1
from an X.509 cert in this case).  The public key could be supplied by OpenPGP
instead, for example, or directly by a driver.

Further, at this point, we need to make sure that the data we were given has
the right bits and emit EBADMSG if it doesn't.

Okay, I can accept that the public_key struct might just have a list of void *
and size_t fields that get filled in, one for each integer that we extract
rather than MPIs, but we should not expose the generic code to the stuff we've
parsed away.

>  struct public_key {
> -	const struct public_key_algorithm *algo;
> -	u8	capabilities;
> -#define PKEY_CAN_ENCRYPT	0x01
> -#define PKEY_CAN_DECRYPT	0x02
> -#define PKEY_CAN_SIGN		0x04
> -#define PKEY_CAN_VERIFY		0x08

You still need the capabilities.  The X.509 certificate and the OpenPGP
message indicate restrictions on the key that we need to honour.

David
--
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/