| Message ID | 20210305005203.3547587-6-stefanb@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for x509 certs with NIST P384/256/192 keys | expand |
Hi, On 3/5/21 8:51 AM, 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. > > Cc: David Howells <dhowells@redhat.com> > Cc: keyrings@vger.kernel.org > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > Reviewed-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com> > --- > crypto/asymmetric_keys/x509_cert_parser.c | 12 +++++++++++- > include/linux/oid_registry.h | 1 + > lib/oid_registry.c | 13 +++++++++++++ > 3 files changed, 25 insertions(+), 1 deletion(-) > > diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c > index 52c9b455fc7d..1621ceaf5c95 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,16 @@ 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 (parse_OID(ctx->params, ctx->params_size, &oid) != 0) > + return -EBADMSG; > + > + switch (oid) { > + case OID_sm2: > + ctx->cert->pub->pkey_algo = "sm2"; > + break; > + default: > + return -ENOPKG; > + } > break; > default: > return -ENOPKG; > diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h > index b504e2f36b25..f32d91895e4d 100644 > --- a/include/linux/oid_registry.h > +++ b/include/linux/oid_registry.h > @@ -121,6 +121,7 @@ enum OID { > }; > > extern enum OID look_up_OID(const void *data, size_t datasize); > +extern int parse_OID(const void *data, size_t datasize, enum OID *oid); > extern int sprint_oid(const void *, size_t, char *, size_t); > extern int sprint_OID(enum OID, char *, size_t); > > diff --git a/lib/oid_registry.c b/lib/oid_registry.c > index f7ad43f28579..508e0b34b5f0 100644 > --- a/lib/oid_registry.c > +++ b/lib/oid_registry.c > @@ -11,6 +11,7 @@ > #include <linux/kernel.h> > #include <linux/errno.h> > #include <linux/bug.h> > +#include <linux/asn1.h> > #include "oid_registry_data.c" > > MODULE_DESCRIPTION("OID Registry"); > @@ -92,6 +93,18 @@ enum OID look_up_OID(const void *data, size_t datasize) > } > EXPORT_SYMBOL_GPL(look_up_OID); > > +int parse_OID(const void *data, size_t datasize, enum OID *oid) > +{ > + const unsigned char *v = data; > + > + if (datasize < 2 || v[0] != ASN1_OID || v[1] != datasize - 2) > + return -EBADMSG; > + > + *oid = look_up_OID(data + 2, datasize - 2); > + return 0; > +} > +EXPORT_SYMBOL_GPL(parse_OID); > + > /* > * sprint_OID - Print an Object Identifier into a buffer > * @data: The encoded OID to print > Tested-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com> Best regards, Tianjia
On Thu, Mar 04, 2021 at 07:51:59PM -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. > > Cc: David Howells <dhowells@redhat.com> > Cc: keyrings@vger.kernel.org > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > Reviewed-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com> > --- > crypto/asymmetric_keys/x509_cert_parser.c | 12 +++++++++++- > include/linux/oid_registry.h | 1 + > lib/oid_registry.c | 13 +++++++++++++ > 3 files changed, 25 insertions(+), 1 deletion(-) > > diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c > index 52c9b455fc7d..1621ceaf5c95 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; enum is not a real type, and it is hard to recall how much it allocates from stack. I would replace this with plain int. > > ctx->key_algo = ctx->last_oid; > switch (ctx->last_oid) { > @@ -470,7 +471,16 @@ 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 (parse_OID(ctx->params, ctx->params_size, &oid) != 0) > + return -EBADMSG; > + > + switch (oid) { > + case OID_sm2: > + ctx->cert->pub->pkey_algo = "sm2"; > + break; > + default: > + return -ENOPKG; > + } > break; > default: > return -ENOPKG; > diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h > index b504e2f36b25..f32d91895e4d 100644 > --- a/include/linux/oid_registry.h > +++ b/include/linux/oid_registry.h > @@ -121,6 +121,7 @@ enum OID { > }; > > extern enum OID look_up_OID(const void *data, size_t datasize); > +extern int parse_OID(const void *data, size_t datasize, enum OID *oid); > extern int sprint_oid(const void *, size_t, char *, size_t); > extern int sprint_OID(enum OID, char *, size_t); > > diff --git a/lib/oid_registry.c b/lib/oid_registry.c > index f7ad43f28579..508e0b34b5f0 100644 > --- a/lib/oid_registry.c > +++ b/lib/oid_registry.c > @@ -11,6 +11,7 @@ > #include <linux/kernel.h> > #include <linux/errno.h> > #include <linux/bug.h> > +#include <linux/asn1.h> > #include "oid_registry_data.c" > > MODULE_DESCRIPTION("OID Registry"); > @@ -92,6 +93,18 @@ enum OID look_up_OID(const void *data, size_t datasize) > } > EXPORT_SYMBOL_GPL(look_up_OID); > > +int parse_OID(const void *data, size_t datasize, enum OID *oid) An exported function without kdoc. > +{ > + const unsigned char *v = data; > + > + if (datasize < 2 || v[0] != ASN1_OID || v[1] != datasize - 2) > + return -EBADMSG; '1' and '2' are magic numbers unless you either have constants defining them, or at least an inline comment with explanation. > + > + *oid = look_up_OID(data + 2, datasize - 2); > + return 0; > +} > +EXPORT_SYMBOL_GPL(parse_OID); > + > /* > * sprint_OID - Print an Object Identifier into a buffer > * @data: The encoded OID to print > -- > 2.29.2 > > /Jarkko
On 3/5/21 12:16 PM, Jarkko Sakkinen wrote: > On Thu, Mar 04, 2021 at 07:51:59PM -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. >> >> Cc: David Howells <dhowells@redhat.com> >> Cc: keyrings@vger.kernel.org >> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >> Reviewed-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com> >> --- >> crypto/asymmetric_keys/x509_cert_parser.c | 12 +++++++++++- >> include/linux/oid_registry.h | 1 + >> lib/oid_registry.c | 13 +++++++++++++ >> 3 files changed, 25 insertions(+), 1 deletion(-) >> >> diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c >> index 52c9b455fc7d..1621ceaf5c95 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; > enum is not a real type, and it is hard to recall how much it allocates > from stack. I would replace this with plain int. That would be worse considering existing code: https://elixir.bootlin.com/linux/latest/source/lib/oid_registry.c#L25 > >> >> ctx->key_algo = ctx->last_oid; >> switch (ctx->last_oid) { >> @@ -470,7 +471,16 @@ 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 (parse_OID(ctx->params, ctx->params_size, &oid) != 0) >> + return -EBADMSG; >> + >> + switch (oid) { >> + case OID_sm2: >> + ctx->cert->pub->pkey_algo = "sm2"; >> + break; >> + default: >> + return -ENOPKG; >> + } >> break; >> default: >> return -ENOPKG; >> diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h >> index b504e2f36b25..f32d91895e4d 100644 >> --- a/include/linux/oid_registry.h >> +++ b/include/linux/oid_registry.h >> @@ -121,6 +121,7 @@ enum OID { >> }; >> >> extern enum OID look_up_OID(const void *data, size_t datasize); >> +extern int parse_OID(const void *data, size_t datasize, enum OID *oid); >> extern int sprint_oid(const void *, size_t, char *, size_t); >> extern int sprint_OID(enum OID, char *, size_t); >> >> diff --git a/lib/oid_registry.c b/lib/oid_registry.c >> index f7ad43f28579..508e0b34b5f0 100644 >> --- a/lib/oid_registry.c >> +++ b/lib/oid_registry.c >> @@ -11,6 +11,7 @@ >> #include <linux/kernel.h> >> #include <linux/errno.h> >> #include <linux/bug.h> >> +#include <linux/asn1.h> >> #include "oid_registry_data.c" >> >> MODULE_DESCRIPTION("OID Registry"); >> @@ -92,6 +93,18 @@ enum OID look_up_OID(const void *data, size_t datasize) >> } >> EXPORT_SYMBOL_GPL(look_up_OID); >> >> +int parse_OID(const void *data, size_t datasize, enum OID *oid) > An exported function without kdoc. > >> +{ >> + const unsigned char *v = data; >> + >> + if (datasize < 2 || v[0] != ASN1_OID || v[1] != datasize - 2) >> + return -EBADMSG; > '1' and '2' are magic numbers unless you either have constants defining > them, or at least an inline comment with explanation. I can add those.
diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c index 52c9b455fc7d..1621ceaf5c95 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,16 @@ 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 (parse_OID(ctx->params, ctx->params_size, &oid) != 0) + return -EBADMSG; + + switch (oid) { + case OID_sm2: + ctx->cert->pub->pkey_algo = "sm2"; + break; + default: + return -ENOPKG; + } break; default: return -ENOPKG; diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h index b504e2f36b25..f32d91895e4d 100644 --- a/include/linux/oid_registry.h +++ b/include/linux/oid_registry.h @@ -121,6 +121,7 @@ enum OID { }; extern enum OID look_up_OID(const void *data, size_t datasize); +extern int parse_OID(const void *data, size_t datasize, enum OID *oid); extern int sprint_oid(const void *, size_t, char *, size_t); extern int sprint_OID(enum OID, char *, size_t); diff --git a/lib/oid_registry.c b/lib/oid_registry.c index f7ad43f28579..508e0b34b5f0 100644 --- a/lib/oid_registry.c +++ b/lib/oid_registry.c @@ -11,6 +11,7 @@ #include <linux/kernel.h> #include <linux/errno.h> #include <linux/bug.h> +#include <linux/asn1.h> #include "oid_registry_data.c" MODULE_DESCRIPTION("OID Registry"); @@ -92,6 +93,18 @@ enum OID look_up_OID(const void *data, size_t datasize) } EXPORT_SYMBOL_GPL(look_up_OID); +int parse_OID(const void *data, size_t datasize, enum OID *oid) +{ + const unsigned char *v = data; + + if (datasize < 2 || v[0] != ASN1_OID || v[1] != datasize - 2) + return -EBADMSG; + + *oid = look_up_OID(data + 2, datasize - 2); + return 0; +} +EXPORT_SYMBOL_GPL(parse_OID); + /* * sprint_OID - Print an Object Identifier into a buffer * @data: The encoded OID to print