On 08/13/2015 07:23 AM, David Howells wrote:

Tadeusz Struk [off-list ref] wrote:

quoted

 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.

quoted

+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.

quoted

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

That's nothing to do with this patch.

quoted

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

'signture' -> 'signature'.

quoted

+/*
+ * 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?

quoted

+	/* initlialzie out buf */

'initialise'.

quoted

-	/* 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.

quoted

 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.

Thanks David for all your feedback. I'll rework it according to your comments.
Regards,
T