mbox series

[v4,0/4] Check codeSigning extended key usage extension

Message ID 20210120090517.23851-1-jlee@suse.com
Headers show
Series Check codeSigning extended key usage extension | expand

Message

Lee, Chun-Yi Jan. 20, 2021, 9:05 a.m. UTC
NIAP PP_OS certification requests that the OS shall validate the
CodeSigning extended key usage extension field for integrity
verifiction of exectable code:

    https://www.niap-ccevs.org/MMO/PP/-442-/
        FIA_X509_EXT.1.1

This patchset adds the logic for parsing the codeSigning EKU extension
field in X.509. And checking the CodeSigning EKU when verifying
signature of kernel module or kexec PE binary in PKCS#7.

v4:
Fixed the wording in patch description.

v3:
- Add codeSigning EKU to x509.genkey key generation config.
- Add openssl command option example for generating CodeSign EKU to
  module-signing.rst document. 

v2:
Changed the help wording in the Kconfig.

Lee, Chun-Yi (4):
  X.509: Add CodeSigning extended key usage parsing
  PKCS#7: Check codeSigning EKU for kernel module and kexec pe
    verification
  modsign: Add codeSigning EKU when generating X.509 key generation
    config
  Documentation/admin-guide/module-signing.rst: add openssl command
    option example for CodeSign EKU

 Documentation/admin-guide/module-signing.rst |  6 +++++
 certs/Makefile                               |  1 +
 certs/system_keyring.c                       |  2 +-
 crypto/asymmetric_keys/Kconfig               |  9 +++++++
 crypto/asymmetric_keys/pkcs7_trust.c         | 37 +++++++++++++++++++++++++---
 crypto/asymmetric_keys/x509_cert_parser.c    | 24 ++++++++++++++++++
 include/crypto/pkcs7.h                       |  3 ++-
 include/crypto/public_key.h                  |  1 +
 include/linux/oid_registry.h                 |  5 ++++
 9 files changed, 83 insertions(+), 5 deletions(-)

Comments

Jarkko Sakkinen Jan. 20, 2021, 11:40 p.m. UTC | #1
On Wed, Jan 20, 2021 at 05:05:14PM +0800, 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)
> +			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);

With eBPF around, does this make any sense?

/Jarkko

> +		return 0;
> +	}
> +
>  	return 0;
>  }
>  
> diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> index 948c5203ca9c..07a1b28460a2 100644
> --- a/include/crypto/public_key.h
> +++ b/include/crypto/public_key.h
> @@ -29,6 +29,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);
> -- 
> 2.16.4
> 
>
Lee, Chun-Yi Jan. 21, 2021, 4:23 a.m. UTC | #2
Hi Jarkko,

On Thu, Jan 21, 2021 at 01:40:48AM +0200, Jarkko Sakkinen wrote:
> On Wed, Jan 20, 2021 at 05:05:14PM +0800, 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)

> > +			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);

> 

> With eBPF around, does this make any sense?

>


I think that the dynamic debug log is still easier for checking the EKU
setting.

Thanks
Joey Lee
Jarkko Sakkinen Jan. 21, 2021, 2:32 p.m. UTC | #3
On Thu, Jan 21, 2021 at 12:23:53PM +0800, joeyli wrote:
> Hi Jarkko,

> 

> On Thu, Jan 21, 2021 at 01:40:48AM +0200, Jarkko Sakkinen wrote:

> > On Wed, Jan 20, 2021 at 05:05:14PM +0800, 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)

> > > +			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);

> > 

> > With eBPF around, does this make any sense?

> >

> 

> I think that the dynamic debug log is still easier for checking the EKU

> setting.


Why?

> Thanks

> Joey Lee


/Jarkko
Lee, Chun-Yi Jan. 21, 2021, 3:23 p.m. UTC | #4
On Thu, Jan 21, 2021 at 04:32:26PM +0200, Jarkko Sakkinen wrote:
> On Thu, Jan 21, 2021 at 12:23:53PM +0800, joeyli wrote:

> > Hi Jarkko,

> > 

> > On Thu, Jan 21, 2021 at 01:40:48AM +0200, Jarkko Sakkinen wrote:

> > > On Wed, Jan 20, 2021 at 05:05:14PM +0800, 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)

> > > > +			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);

> > > 

> > > With eBPF around, does this make any sense?

> > >

> > 

> > I think that the dynamic debug log is still easier for checking the EKU

> > setting.

> 

> Why?


There have some certificates may loaded when system boot. In booting stage,
using dynamic debug log for checking EKU is easier than eBPF. 

I am not good on eBPF. Correct me if I missed anything, please!

Thanks a lot!
Joey Lee
Jarkko Sakkinen Jan. 22, 2021, 5:28 p.m. UTC | #5
On Thu, Jan 21, 2021 at 11:23:06PM +0800, joeyli wrote:
> On Thu, Jan 21, 2021 at 04:32:26PM +0200, Jarkko Sakkinen wrote:

> > On Thu, Jan 21, 2021 at 12:23:53PM +0800, joeyli wrote:

> > > Hi Jarkko,

> > > 

> > > On Thu, Jan 21, 2021 at 01:40:48AM +0200, Jarkko Sakkinen wrote:

> > > > On Wed, Jan 20, 2021 at 05:05:14PM +0800, 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)

> > > > > +			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);

> > > > 

> > > > With eBPF around, does this make any sense?

> > > >

> > > 

> > > I think that the dynamic debug log is still easier for checking the EKU

> > > setting.

> > 

> > Why?

> 

> There have some certificates may loaded when system boot. In booting stage,

> using dynamic debug log for checking EKU is easier than eBPF. 

> 

> I am not good on eBPF. Correct me if I missed anything, please!


I think it's fine have it, thanks.

/Jarkko
David Howells Jan. 27, 2021, 9:40 a.m. UTC | #6
Jarkko Sakkinen <jarkko@kernel.org> wrote:

> With eBPF around, does this make any sense?


bpf/ebpf may be partially disabled if you boot in secure boot mode - not sure
whether that affects this.

David