| Message ID | 20210127123350.817593-1-stefanb@linux.vnet.ibm.com |
|---|---|
| Headers | show |
| Series | Add support for x509 certs with NIST p256 and p192 keys | expand |
Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
> k=$(keyctrl newring test @u)
keyctl - but I can fix that.
David
On Wed, Jan 27, 2021 at 02:22:08PM +0000, David Howells wrote: > > I've pulled this into my keys-next branch. David, please drop them because there are issues with the Crypto API bits. Thanks,
On 1/27/21 11:12 AM, Nym Seddon wrote: > Hi Stefan, > > In the recommendations from SafeCurves (https://safecurves.cr.yp.to/twist.html) there are a number of attacks against ECC twists. Two of those attacks are relevant against NIST P192: invalid-curve attacks and invalid-curve attacks against ladders. > > Both attacks can be mitigated by checking the supplied public key is on the correct curve, before performing curve operations. > > Not sure if the right place for those checks are in the signature verification code provided in these patches, or when reading public keys from the certificates. Does the kernel provide functions for checking curve points satisfy their respective curve equations? > > There are also tables describing the cost of combined attacks on various curves, where NIST P224 already falls below the safe threshold. Because of that, I would recommend not implementing support for NIST P192 (since it would fair even worse). > > What are your thoughts? I am calling into a function performing such a test at the end of the function parsing the public key: return ecc_is_pubkey_valid_full(ctx->curve, ctx->pub_key) https://elixir.bootlin.com/linux/latest/source/crypto/ecc.c#L1458 Is that good 'enough' ? Stefan
On Wed, 2021-01-27 at 07:33 -0500, Stefan Berger wrote: > From: Stefan Berger <stefanb@linux.ibm.com> > > Detect whether a key is an sm2 type of key by its OID in the parameters > array rather than assuming that everything under OID_id_ecPublicKey > is sm2, which is not the case. > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > --- > crypto/asymmetric_keys/x509_cert_parser.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c > index 52c9b455fc7d..4643fe5ed69a 100644 > --- a/crypto/asymmetric_keys/x509_cert_parser.c > +++ b/crypto/asymmetric_keys/x509_cert_parser.c > @@ -459,6 +459,7 @@ int x509_extract_key_data(void *context, size_t hdrlen, > const void *value, size_t vlen) > { > struct x509_parse_context *ctx = context; > + enum OID oid; > > ctx->key_algo = ctx->last_oid; > switch (ctx->last_oid) { > @@ -470,7 +471,17 @@ int x509_extract_key_data(void *context, size_t hdrlen, > ctx->cert->pub->pkey_algo = "ecrdsa"; > break; > case OID_id_ecPublicKey: > - ctx->cert->pub->pkey_algo = "sm2"; > + if (ctx->params_size < 2) Either a named constant, or at least a comment instead of just '2'. > + return -ENOPKG; > + > + oid = look_up_OID(ctx->params + 2, ctx->params_size - 2); > + switch (oid) { > + case OID_sm2: > + ctx->cert->pub->pkey_algo = "sm2"; > + break; > + default: > + return -ENOPKG; > + } > break; > default: > return -ENOPKG; /Jarkko
On 1/30/21 4:26 PM, Jarkko Sakkinen wrote: > On Wed, 2021-01-27 at 07:33 -0500, Stefan Berger wrote: >> From: Stefan Berger <stefanb@linux.ibm.com> >> >> Detect whether a key is an sm2 type of key by its OID in the parameters >> array rather than assuming that everything under OID_id_ecPublicKey >> is sm2, which is not the case. >> >> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >> --- >> crypto/asymmetric_keys/x509_cert_parser.c | 13 ++++++++++++- >> 1 file changed, 12 insertions(+), 1 deletion(-) >> >> diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c >> index 52c9b455fc7d..4643fe5ed69a 100644 >> --- a/crypto/asymmetric_keys/x509_cert_parser.c >> +++ b/crypto/asymmetric_keys/x509_cert_parser.c >> @@ -459,6 +459,7 @@ int x509_extract_key_data(void *context, size_t hdrlen, >> const void *value, size_t vlen) >> { >> struct x509_parse_context *ctx = context; >> + enum OID oid; >> >> ctx->key_algo = ctx->last_oid; >> switch (ctx->last_oid) { >> @@ -470,7 +471,17 @@ int x509_extract_key_data(void *context, size_t hdrlen, >> ctx->cert->pub->pkey_algo = "ecrdsa"; >> break; >> case OID_id_ecPublicKey: >> - ctx->cert->pub->pkey_algo = "sm2"; >> + if (ctx->params_size < 2) > Either a named constant, or at least a comment instead of just '2'. I will look at the 2 entries whether they contain the expected values: ASN1_OID and length Thanks! Stefan
On Sat, Jan 30, 2021 at 09:57:40PM -0500, Stefan Berger wrote: > On 1/30/21 4:26 PM, Jarkko Sakkinen wrote: > > On Wed, 2021-01-27 at 07:33 -0500, Stefan Berger wrote: > > > From: Stefan Berger <stefanb@linux.ibm.com> > > > > > > Detect whether a key is an sm2 type of key by its OID in the parameters > > > array rather than assuming that everything under OID_id_ecPublicKey > > > is sm2, which is not the case. > > > > > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > > > --- > > > crypto/asymmetric_keys/x509_cert_parser.c | 13 ++++++++++++- > > > 1 file changed, 12 insertions(+), 1 deletion(-) > > > > > > diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c > > > index 52c9b455fc7d..4643fe5ed69a 100644 > > > --- a/crypto/asymmetric_keys/x509_cert_parser.c > > > +++ b/crypto/asymmetric_keys/x509_cert_parser.c > > > @@ -459,6 +459,7 @@ int x509_extract_key_data(void *context, size_t hdrlen, > > > const void *value, size_t vlen) > > > { > > > struct x509_parse_context *ctx = context; > > > + enum OID oid; > > > ctx->key_algo = ctx->last_oid; > > > switch (ctx->last_oid) { > > > @@ -470,7 +471,17 @@ int x509_extract_key_data(void *context, size_t hdrlen, > > > ctx->cert->pub->pkey_algo = "ecrdsa"; > > > break; > > > case OID_id_ecPublicKey: > > > - ctx->cert->pub->pkey_algo = "sm2"; > > > + if (ctx->params_size < 2) > > Either a named constant, or at least a comment instead of just '2'. > > > I will look at the 2 entries whether they contain the expected values: > ASN1_OID and length > > Thanks! > > Stefan Just add inline comment that explains that. /Jarkko
From: Stefan Berger <stefanb@linux.ibm.com> This series of patches adds support for x509 certificates signed by a CA that uses NIST p256 or p192 keys for signing. It also adds support for certificates where the public key is a NIST p256 or p192 key. The math for ECDSA signature verification is also added. Since self-signed certificates are verified upon loading, the following script can be used for testing: k=$(keyctrl newring test @u) while :; do for hash in sha1 sha224 sha256 sha384 sha512; do openssl req \ -x509 \ -${hash} \ -newkey ec \ -pkeyopt ec_paramgen_curve:prime256v1 \ -keyout key.pem \ -days 365 \ -subj '/CN=test' \ -nodes \ -outform der \ -out cert.der keyctl padd asymmetric testkey $k < cert.der if [ $? -ne 0 ]; then echo "ERROR" exit 1 fi done done It also works with restricted keyrings where an RSA key is used to sign a NIST P256/P192 key. Scripts for testing are here: https://github.com/stefanberger/eckey-testing The ECDSA signature verification will be used by IMA Appraisal where ECDSA file signatures stored in RPM packages will use substantially less space than if RSA signatures were to be used. Stefan v2->v3: - patch 2 now includes linux/scatterlist.h v1->v2: - using faster vli_sub rather than newly added vli_mod_fast to 'reduce' result - rearranged switch statements to follow after RSA - 3rd patch from 1st posting is now 1st patch Stefan Berger (3): x509: Detect sm2 keys by their parameters OID x509: Add support for parsing x509 certs with NIST p256 keys x509: Add support for NIST p192 keys in certificates and akcipher crypto/Makefile | 9 +- crypto/asymmetric_keys/public_key.c | 19 ++ crypto/asymmetric_keys/x509_cert_parser.c | 45 ++- crypto/ecc.c | 318 ++++++++++++++++++++++ crypto/ecc.h | 2 + crypto/ecc_curve_defs.h | 4 + crypto/eccsignature.asn1 | 4 + include/linux/oid_registry.h | 6 + 8 files changed, 404 insertions(+), 3 deletions(-) create mode 100644 crypto/eccsignature.asn1