| Message ID | 20210309091044.2298-2-jlee@suse.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Check codeSigning extended key usage extension | expand |
Hi, On 3/9/21 10:10 AM, Lee, Chun-Yi wrote: > This patch adds the logic for parsing the CodeSign extended key usage > extension in X.509. The parsing result will be set to the eku flag > which is carried by public key. It can be used in the PKCS#7 > verification. > > Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com> > --- > crypto/asymmetric_keys/x509_cert_parser.c | 24 ++++++++++++++++++++++++ > include/crypto/public_key.h | 1 + > include/linux/oid_registry.h | 5 +++++ > 3 files changed, 30 insertions(+) > > diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c > index 52c9b455fc7d..65721313b265 100644 > --- a/crypto/asymmetric_keys/x509_cert_parser.c > +++ b/crypto/asymmetric_keys/x509_cert_parser.c > @@ -497,6 +497,8 @@ int x509_process_extension(void *context, size_t hdrlen, > struct x509_parse_context *ctx = context; > struct asymmetric_key_id *kid; > const unsigned char *v = value; > + int i = 0; > + enum OID oid; > > pr_debug("Extension: %u\n", ctx->last_oid); > > @@ -526,6 +528,28 @@ int x509_process_extension(void *context, size_t hdrlen, > return 0; > } > > + if (ctx->last_oid == OID_extKeyUsage) { > + if (v[0] != ((ASN1_UNIV << 6) | ASN1_CONS_BIT | ASN1_SEQ) || > + v[1] != vlen - 2) A bad cert might get here with vlen < 2, which would cause indexing into v to break. Please add a check for vlen >= 2 before this. > + return -EBADMSG; > + i += 2; > + > + while (i < vlen) { > + /* A 10 bytes EKU OID Octet blob = > + * ASN1_OID + size byte + 8 bytes OID */ > + if (v[i] != ASN1_OID || v[i + 1] != 8 || (i + 10) > vlen) Same here, for i == (vlen - 1), v[i + 1] would fetch outside of v. Or, does the ASN.1 layout protect against this? > + return -EBADMSG; > + > + oid = look_up_OID(v + i + 2, v[i + 1]); > + if (oid == OID_codeSigning) { > + ctx->cert->pub->eku |= EKU_codeSigning; > + } > + i += 10; > + } > + pr_debug("extKeyUsage: %d\n", ctx->cert->pub->eku); > + return 0; > + } > + > return 0; > } > > diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h > index 47accec68cb0..1ccaebe2a28b 100644 > --- a/include/crypto/public_key.h > +++ b/include/crypto/public_key.h > @@ -28,6 +28,7 @@ struct public_key { > bool key_is_private; > const char *id_type; > const char *pkey_algo; > + unsigned int eku : 9; /* Extended Key Usage (9-bit) */ > }; > > extern void public_key_free(struct public_key *key); > diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h > index 4462ed2c18cd..e20e8eb53b21 100644 > --- a/include/linux/oid_registry.h > +++ b/include/linux/oid_registry.h > @@ -113,9 +113,14 @@ enum OID { > OID_SM2_with_SM3, /* 1.2.156.10197.1.501 */ > OID_sm3WithRSAEncryption, /* 1.2.156.10197.1.504 */ > > + /* Extended key purpose OIDs [RFC 5280] */ > + OID_codeSigning, /* 1.3.6.1.5.5.7.3.3 */ > + > OID__NR > }; > > +#define EKU_codeSigning (1 << 2) > + > extern enum OID look_up_OID(const void *data, size_t datasize); > extern int sprint_oid(const void *, size_t, char *, size_t); > extern int sprint_OID(enum OID, char *, size_t); > -- SUSE Software Solutions Germany GmbH Maxfeldstr. 5 90409 Nürnberg Germany HRB 36809, AG Nürnberg Geschäftsführer: Felix Imendörffer
Hi Varad, Thanks for your review! On Tue, Apr 13, 2021 at 04:28:11PM +0200, Varad Gautam wrote: > Hi, > > On 3/9/21 10:10 AM, Lee, Chun-Yi wrote: > > This patch adds the logic for parsing the CodeSign extended key usage > > extension in X.509. The parsing result will be set to the eku flag > > which is carried by public key. It can be used in the PKCS#7 > > verification. > > > > Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com> > > --- > > crypto/asymmetric_keys/x509_cert_parser.c | 24 ++++++++++++++++++++++++ > > include/crypto/public_key.h | 1 + > > include/linux/oid_registry.h | 5 +++++ > > 3 files changed, 30 insertions(+) > > > > diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c > > index 52c9b455fc7d..65721313b265 100644 > > --- a/crypto/asymmetric_keys/x509_cert_parser.c > > +++ b/crypto/asymmetric_keys/x509_cert_parser.c > > @@ -497,6 +497,8 @@ int x509_process_extension(void *context, size_t hdrlen, > > struct x509_parse_context *ctx = context; > > struct asymmetric_key_id *kid; > > const unsigned char *v = value; > > + int i = 0; > > + enum OID oid; > > > > pr_debug("Extension: %u\n", ctx->last_oid); > > > > @@ -526,6 +528,28 @@ int x509_process_extension(void *context, size_t hdrlen, > > return 0; > > } > > > > + if (ctx->last_oid == OID_extKeyUsage) { > > + if (v[0] != ((ASN1_UNIV << 6) | ASN1_CONS_BIT | ASN1_SEQ) || > > + v[1] != vlen - 2) > > A bad cert might get here with vlen < 2, which would cause indexing into v to break. > Please add a check for vlen >= 2 before this. > I will add the check, thanks for your suggestion! > > + return -EBADMSG; > > + i += 2; > > + > > + while (i < vlen) { > > + /* A 10 bytes EKU OID Octet blob = > > + * ASN1_OID + size byte + 8 bytes OID */ > > + if (v[i] != ASN1_OID || v[i + 1] != 8 || (i + 10) > vlen) > > Same here, for i == (vlen - 1), v[i + 1] would fetch outside of v. Or, does the > ASN.1 layout protect against this? > I will move the "(i + 10) > vlen" to the front of "v[i + 1] != 8". It can avoid that the last octet blob is less than 10 bytes. Thanks! Joey Lee > > + return -EBADMSG; > > + > > + oid = look_up_OID(v + i + 2, v[i + 1]); > > + if (oid == OID_codeSigning) { > > + ctx->cert->pub->eku |= EKU_codeSigning; > > + } > > + i += 10; > > + } > > + pr_debug("extKeyUsage: %d\n", ctx->cert->pub->eku); > > + return 0; > > + } > > + > > return 0; > > } > > > > diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h > > index 47accec68cb0..1ccaebe2a28b 100644 > > --- a/include/crypto/public_key.h > > +++ b/include/crypto/public_key.h > > @@ -28,6 +28,7 @@ struct public_key { > > bool key_is_private; > > const char *id_type; > > const char *pkey_algo; > > + unsigned int eku : 9; /* Extended Key Usage (9-bit) */ > > }; > > > > extern void public_key_free(struct public_key *key); > > diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h > > index 4462ed2c18cd..e20e8eb53b21 100644 > > --- a/include/linux/oid_registry.h > > +++ b/include/linux/oid_registry.h > > @@ -113,9 +113,14 @@ enum OID { > > OID_SM2_with_SM3, /* 1.2.156.10197.1.501 */ > > OID_sm3WithRSAEncryption, /* 1.2.156.10197.1.504 */ > > > > + /* Extended key purpose OIDs [RFC 5280] */ > > + OID_codeSigning, /* 1.3.6.1.5.5.7.3.3 */ > > + > > OID__NR > > }; > > > > +#define EKU_codeSigning (1 << 2) > > + > > extern enum OID look_up_OID(const void *data, size_t datasize); > > extern int sprint_oid(const void *, size_t, char *, size_t); > > extern int sprint_OID(enum OID, char *, size_t); > > > > -- > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5 > 90409 Nürnberg > Germany > > HRB 36809, AG Nürnberg > Geschäftsführer: Felix Imendörffer
diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c index 52c9b455fc7d..65721313b265 100644 --- a/crypto/asymmetric_keys/x509_cert_parser.c +++ b/crypto/asymmetric_keys/x509_cert_parser.c @@ -497,6 +497,8 @@ int x509_process_extension(void *context, size_t hdrlen, struct x509_parse_context *ctx = context; struct asymmetric_key_id *kid; const unsigned char *v = value; + int i = 0; + enum OID oid; pr_debug("Extension: %u\n", ctx->last_oid); @@ -526,6 +528,28 @@ int x509_process_extension(void *context, size_t hdrlen, return 0; } + if (ctx->last_oid == OID_extKeyUsage) { + if (v[0] != ((ASN1_UNIV << 6) | ASN1_CONS_BIT | ASN1_SEQ) || + v[1] != vlen - 2) + return -EBADMSG; + i += 2; + + while (i < vlen) { + /* A 10 bytes EKU OID Octet blob = + * ASN1_OID + size byte + 8 bytes OID */ + if (v[i] != ASN1_OID || v[i + 1] != 8 || (i + 10) > vlen) + return -EBADMSG; + + oid = look_up_OID(v + i + 2, v[i + 1]); + if (oid == OID_codeSigning) { + ctx->cert->pub->eku |= EKU_codeSigning; + } + i += 10; + } + pr_debug("extKeyUsage: %d\n", ctx->cert->pub->eku); + return 0; + } + return 0; } diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h index 47accec68cb0..1ccaebe2a28b 100644 --- a/include/crypto/public_key.h +++ b/include/crypto/public_key.h @@ -28,6 +28,7 @@ struct public_key { bool key_is_private; const char *id_type; const char *pkey_algo; + unsigned int eku : 9; /* Extended Key Usage (9-bit) */ }; extern void public_key_free(struct public_key *key); diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h index 4462ed2c18cd..e20e8eb53b21 100644 --- a/include/linux/oid_registry.h +++ b/include/linux/oid_registry.h @@ -113,9 +113,14 @@ enum OID { OID_SM2_with_SM3, /* 1.2.156.10197.1.501 */ OID_sm3WithRSAEncryption, /* 1.2.156.10197.1.504 */ + /* Extended key purpose OIDs [RFC 5280] */ + OID_codeSigning, /* 1.3.6.1.5.5.7.3.3 */ + OID__NR }; +#define EKU_codeSigning (1 << 2) + extern enum OID look_up_OID(const void *data, size_t datasize); extern int sprint_oid(const void *, size_t, char *, size_t); extern int sprint_OID(enum OID, char *, size_t);
This patch adds the logic for parsing the CodeSign extended key usage extension in X.509. The parsing result will be set to the eku flag which is carried by public key. It can be used in the PKCS#7 verification. Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com> --- crypto/asymmetric_keys/x509_cert_parser.c | 24 ++++++++++++++++++++++++ include/crypto/public_key.h | 1 + include/linux/oid_registry.h | 5 +++++ 3 files changed, 30 insertions(+)