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 <stef...@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 <dhowe...@redhat.com>
Cc: keyri...@vger.kernel.org
Signed-off-by: Stefan Berger <stef...@linux.ibm.com>
Reviewed-by: Tianjia Zhang <tianjia.zh...@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.


Reply via email to