Messages in this thread | | | Subject | Re: [PATCH v10 5/9] x509: Detect sm2 keys by their parameters OID | From | Stefan Berger <> | Date | Fri, 5 Mar 2021 13:02:09 -0500 |
| |
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.
| |