diff mbox series

[v6,2/4] x509: Detect sm2 keys by their parameters OID

Message ID 20210131233301.1301787-3-stefanb@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series Add support for x509 certs with NIST p256 and p192 keys | expand

Commit Message

Stefan Berger Jan. 31, 2021, 11:32 p.m. UTC
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>
Cc: David Howells <dhowells@redhat.com>
Cc: keyrings@vger.kernel.org
---
 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(-)

Comments

tianjia.zhang Feb. 1, 2021, 10:39 a.m. UTC | #1
On 2/1/21 7:32 AM, Stefan Berger wrote:
> 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>
> Cc: David Howells <dhowells@redhat.com>
> Cc: keyrings@vger.kernel.org
> ---
>   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 4462ed2c18cd..d4982e42c0d2 100644
> --- a/include/linux/oid_registry.h
> +++ b/include/linux/oid_registry.h
> @@ -117,6 +117,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
> 

Great job, I'm just curious why we need to add a new function, this 
seems unnecessary, if possible, please add

Reviewed-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>

Best regards,
Tianjia
Stefan Berger Feb. 1, 2021, 1:02 p.m. UTC | #2
On 2/1/21 5:39 AM, Tianjia Zhang wrote:
>
>> 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
>>
>
> Great job, I'm just curious why we need to add a new function, this 
> seems unnecessary, if possible, please add


Thanks. I call this function in two places now. I thought it was 'worth it'.


>
> Reviewed-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
>
> Best regards,
> Tianjia
diff mbox series

Patch

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 4462ed2c18cd..d4982e42c0d2 100644
--- a/include/linux/oid_registry.h
+++ b/include/linux/oid_registry.h
@@ -117,6 +117,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