Re: [PATCH] crypto: Fix ASN.1 key handling for RSA akcipher
From: Stephan Mueller <hidden>
Date: 2015-08-28 09:20:50
Am Freitag, 28. August 2015, 07:28:58 schrieb Marcel Holtmann: Hi Marcel,
quoted hunk ↗ jump to hunk
The RSA algorithm provides two ASN.1 key types. One for RSA Private Key and another for RSA Public Key. Use these two already defined ASN.1 definitions instead of inventing a new one. Signed-off-by: Marcel Holtmann <marcel@holtmann.org> --- crypto/Makefile | 9 ++++++--- crypto/rsa_helper.c | 13 +++++++++---- crypto/rsakey.asn1 | 5 ----- crypto/rsaprivatekey.asn1 | 13 +++++++++++++ crypto/rsapublickey.asn1 | 4 ++++ 5 files changed, 32 insertions(+), 12 deletions(-) delete mode 100644 crypto/rsakey.asn1 create mode 100644 crypto/rsaprivatekey.asn1 create mode 100644 crypto/rsapublickey.asn1diff --git a/crypto/Makefile b/crypto/Makefile index 3cc91c3301c7..0b056c411aa7 100644 --- a/crypto/Makefile +++ b/crypto/Makefile@@ -31,10 +31,13 @@ obj-$(CONFIG_CRYPTO_HASH2) += crypto_hash.oobj-$(CONFIG_CRYPTO_PCOMP2) += pcompress.o obj-$(CONFIG_CRYPTO_AKCIPHER2) += akcipher.o -$(obj)/rsakey-asn1.o: $(obj)/rsakey-asn1.c $(obj)/rsakey-asn1.h -clean-files += rsakey-asn1.c rsakey-asn1.h +$(obj)/rsapublickey-asn1.o: $(obj)/rsapublickey-asn1.c $(obj)/rsapublickey-asn1.h +clean-files += rsapublickey-asn1.c rsapublickey-asn1.h -rsa_generic-y := rsakey-asn1.o +$(obj)/rsaprivatekey-asn1.o: $(obj)/rsaprivatekey-asn1.c $(obj)/rsaprivatekey-asn1.h +clean-files += rsaprivatekey-asn1.c rsaprivatekey-asn1.h + +rsa_generic-y := rsapublickey-asn1.o rsaprivatekey-asn1.o rsa_generic-y += rsa.o rsa_generic-y += rsa_helper.o obj-$(CONFIG_CRYPTO_RSA) += rsa_generic.odiff --git a/crypto/rsa_helper.c b/crypto/rsa_helper.c index 8d96ce969b44..26617e3132fb 100644 --- a/crypto/rsa_helper.c +++ b/crypto/rsa_helper.c@@ -15,7 +15,8 @@#include <linux/err.h> #include <linux/fips.h> #include <crypto/internal/rsa.h> -#include "rsakey-asn1.h" +#include "rsapublickey-asn1.h" +#include "rsaprivatekey-asn1.h" int rsa_get_n(void *context, size_t hdrlen, unsigned char tag, const void *value, size_t vlen)@@ -109,9 +110,13 @@ int rsa_parse_key(struct rsa_key *rsa_key, const void*key, int ret; free_mpis(rsa_key); - ret = asn1_ber_decoder(&rsakey_decoder, rsa_key, key, key_len); - if (ret < 0) - goto error; + ret = asn1_ber_decoder(&rsapublickey_decoder, rsa_key, key, key_len); + if (ret < 0) { + ret = asn1_ber_decoder(&rsaprivatekey_decoder, rsa_key, + key, key_len);
Wouldn't it be better to have 2 parse_key functions here? We (will) have 2 setkey functions which are the callers of parse_key. The reason is that the added if requires CPU cycles that can be easily avoided. Hence I propose: rsa_parse_pubkey() rsapublickey_decoder rsa_parse_privkey() rsaprivatekey_decoder to avoid parsing a key as pub key even though we already know it is a priv key.
quoted hunk ↗ jump to hunk
+ if (ret < 0) + goto error; + } return 0; error:diff --git a/crypto/rsakey.asn1 b/crypto/rsakey.asn1 deleted file mode 100644 index 3c7b5df7b428..000000000000 --- a/crypto/rsakey.asn1 +++ /dev/null@@ -1,5 +0,0 @@ -RsaKey ::= SEQUENCE { - n INTEGER ({ rsa_get_n }), - e INTEGER ({ rsa_get_e }), - d INTEGER ({ rsa_get_d }) -}diff --git a/crypto/rsaprivatekey.asn1 b/crypto/rsaprivatekey.asn1 new file mode 100644 index 000000000000..58dddc7c1536 --- /dev/null +++ b/crypto/rsaprivatekey.asn1@@ -0,0 +1,13 @@ +RSAPrivateKey ::= SEQUENCE { + version Version, + modulus INTEGER ({ rsa_get_n }), -- n + publicExponent INTEGER ({ rsa_get_e }), -- e + privateExponent INTEGER ({ rsa_get_d }), -- d + prime1 INTEGER, -- p + prime2 INTEGER, -- q + exponent1 INTEGER, -- d mod (p-1) + exponent2 INTEGER, -- d mod (q-1) + coefficient INTEGER -- (inverse of q) mod
p
quoted hunk ↗ jump to hunk
+} + +Version ::= INTEGERdiff --git a/crypto/rsapublickey.asn1 b/crypto/rsapublickey.asn1 new file mode 100644 index 000000000000..8f7f8760f2a9 --- /dev/null +++ b/crypto/rsapublickey.asn1@@ -0,0 +1,4 @@ +RSAPublicKey ::= SEQUENCE { + modulus INTEGER ({ rsa_get_n }), -- n + publicExponent INTEGER ({ rsa_get_e }) -- e +}
Ciao Stephan