On 08/13/2015 07:23 AM, David Howells wrote: > Tadeusz Struk <tadeusz.struk@xxxxxxxxx> 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. Thanks David for all your feedback. I'll rework it according to your comments. Regards, T -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html