[PATCH v5 0/6] Add CA enforcement keyring restrictions

[PATCH v5 0/6] Add CA enforcement keyring restrictions

[Eric Snowberg]

Prior to the introduction of the machine keyring, most distros simply 
allowed all keys contained within the platform keyring to be used
for both kernel and module verification.  This was done by an out of
tree patch.  Some distros took it even further and loaded all these keys
into the secondary trusted keyring.  This also allowed the system owner 
to add their own key for IMA usage.

Each distro contains similar documentation on how to sign kernel modules
and enroll the key into the MOK.  The process is fairly straightforward.
With the introduction of the machine keyring, the process remains
basically the same, without the need for any out of tree patches.

The machine keyring allowed distros to eliminate the out of tree patches
for kernel module signing.  However, it falls short in allowing the end 
user to add their own keys for IMA. Currently, the machine keyring can not 
be used as another trust anchor for adding keys to the ima keyring, since 
CA enforcement does not currently exist.  This would expand the current 
integrity gap. The IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY 
Kconfig states that keys may be added to the ima keyrings if the key is 
validly signed by a CA cert in the system built-in or secondary trusted 
keyring.  Currently, there is not code that enforces the contents of a
CA cert.

This series introduces a way to do CA enforcement with the machine
keyring.  It introduces three different ways to configure the machine
keyring.  New Kconfig options are added to control the types of keys
that may be added to it.  The default option allows all MOK keys into the
machine keyring.  When CONFIG_INTEGRITY_CA_MACHINE_KEYRING is selected,
the X.509 CA bit must be true and the key usage must contain keyCertSign; 
any other usage field may also be set.  When
CONFIG_INTEGRITY_CA_MACHINE_KEYRING_MAX is also selected, the X.509 CA
bit must be true and the key usage must contain keyCertSign. With this
option digitialSignature usage may not be set.  If a key doesn't pass 
the CA restriction check, instead of going into the machine keyring, it 
is added to the platform keyring.  With the ability to configure the
machine keyring with CA restrictions, code that prevented the machine
keyring from being enabled with
IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY has been removed.

Changelog:
v5:
- Removed the Kconfig _MIN Kconfig option and split it into different
  entries.
- Added requested commit message changes

v4:
- Removed all code that validated the certificate chain back to the root
  CA. Now the only restriction is what is initially placed in the
  machine keyring.
- Check and store if the X.509 usage contains digitalSignature
- New Kconfig menu item with none, min and max CA restriction on the 
  machine keyring

v3:
- Allow Intermediate CA certs to be enrolled through the MOK. The
  Intermediate CA cert must contain keyCertSign key usage and have the 
  CA bit set to true. This was done by removing the self signed
  requirement.

Eric Snowberg (6):
  KEYS: Create static version of public_key_verify_signature
  KEYS: Add missing function documentation
  KEYS: X.509: Parse Basic Constraints for CA
  KEYS: X.509: Parse Key Usage
  KEYS: CA link restriction
  integrity: machine keyring CA configuration

 certs/system_keyring.c                    | 14 +++++--
 crypto/asymmetric_keys/restrict.c         | 40 ++++++++++++++++++
 crypto/asymmetric_keys/x509_cert_parser.c | 50 +++++++++++++++++++++++
 include/crypto/public_key.h               | 28 +++++++++++++
 security/integrity/Kconfig                | 23 ++++++++++-
 security/integrity/digsig.c               |  8 +++-
 6 files changed, 157 insertions(+), 6 deletions(-)


base-commit: c9c3395d5e3dcc6daee66c6908354d47bf98cb0c
-- 
2.27.0

[PATCH v5 1/6] KEYS: Create static version of public_key_verify_signature

[Eric Snowberg]

The kernel test robot reports undefined reference to
public_key_verify_signature when CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE is
not defined. Create a static version in this case and return -EINVAL.

Fixes: db6c43bd2132 ("crypto: KEYS: convert public key and digsig asym to the akcipher api")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
---
 include/crypto/public_key.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index 68f7aa2a7e55..6d61695e1cde 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -80,7 +80,16 @@ extern int create_signature(struct kernel_pkey_params *, const void *, void *);
 extern int verify_signature(const struct key *,
 			    const struct public_key_signature *);
 
+#if IS_REACHABLE(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE)
 int public_key_verify_signature(const struct public_key *pkey,
 				const struct public_key_signature *sig);
+#else
+static inline
+int public_key_verify_signature(const struct public_key *pkey,
+				const struct public_key_signature *sig)
+{
+	return -EINVAL;
+}
+#endif
 
 #endif /* _LINUX_PUBLIC_KEY_H */
-- 
2.27.0

Re: [PATCH v5 1/6] KEYS: Create static version of public_key_verify_signature

[Jarkko Sakkinen]

On Thu, 2023-03-02 at 11:46 -0500, Eric Snowberg wrote:
> The kernel test robot reports undefined reference to
> public_key_verify_signature when CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE
> is
> not defined. Create a static version in this case and return -EINVAL.
> 
> Fixes: db6c43bd2132 ("crypto: KEYS: convert public key and digsig
> asym to the akcipher api")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> ---
>  include/crypto/public_key.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/include/crypto/public_key.h
> b/include/crypto/public_key.h
> index 68f7aa2a7e55..6d61695e1cde 100644
> --- a/include/crypto/public_key.h
> +++ b/include/crypto/public_key.h
> @@ -80,7 +80,16 @@ extern int create_signature(struct
> kernel_pkey_params *, const void *, void *);
>  extern int verify_signature(const struct key *,
>                             const struct public_key_signature *);
>  
> +#if IS_REACHABLE(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE)
>  int public_key_verify_signature(const struct public_key *pkey,
>                                 const struct public_key_signature
> *sig);
> +#else
> +static inline
> +int public_key_verify_signature(const struct public_key *pkey,
> +                               const struct public_key_signature
> *sig)
> +{
> +       return -EINVAL;
> +}
> +#endif
>  
>  #endif /* _LINUX_PUBLIC_KEY_H */

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

[PATCH v5 2/6] KEYS: Add missing function documentation

[Eric Snowberg]

Compiling with 'W=1' results in warnings that 'Function parameter or member
not described'

Add the missing parameters for
restrict_link_by_builtin_and_secondary_trusted and
restrict_link_to_builtin_trusted.

Use /* instead of /** for get_builtin_and_secondary_restriction, since
it is a static function.

Fix wrong function name restrict_link_to_builtin_trusted.

Fixes: d3bfe84129f6 ("certs: Add a secondary system keyring that can be added to dynamically")
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 certs/system_keyring.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 5042cc54fa5e..a7a49b17ceb1 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -33,7 +33,11 @@ extern __initconst const unsigned long system_certificate_list_size;
 extern __initconst const unsigned long module_cert_size;
 
 /**
- * restrict_link_to_builtin_trusted - Restrict keyring addition by built in CA
+ * restrict_link_by_builtin_trusted - Restrict keyring addition by built-in CA
+ * @dest_keyring: Keyring being linked to.
+ * @type: The type of key being added.
+ * @payload: The payload of the new key.
+ * @restriction_key: A ring of keys that can be used to vouch for the new cert.
  *
  * Restrict the addition of keys into a keyring based on the key-to-be-added
  * being vouched for by a key in the built in system keyring.
@@ -50,7 +54,11 @@ int restrict_link_by_builtin_trusted(struct key *dest_keyring,
 #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
 /**
  * restrict_link_by_builtin_and_secondary_trusted - Restrict keyring
- *   addition by both builtin and secondary keyrings
+ *   addition by both built-in and secondary keyrings.
+ * @dest_keyring: Keyring being linked to.
+ * @type: The type of key being added.
+ * @payload: The payload of the new key.
+ * @restrict_key: A ring of keys that can be used to vouch for the new cert.
  *
  * Restrict the addition of keys into a keyring based on the key-to-be-added
  * being vouched for by a key in either the built-in or the secondary system
@@ -75,7 +83,7 @@ int restrict_link_by_builtin_and_secondary_trusted(
 					  secondary_trusted_keys);
 }
 
-/**
+/*
  * Allocate a struct key_restriction for the "builtin and secondary trust"
  * keyring. Only for use in system_trusted_keyring_init().
  */
-- 
2.27.0

Re: [PATCH v5 2/6] KEYS: Add missing function documentation

[Jarkko Sakkinen]

On Thu, Mar 02, 2023 at 11:46:48AM -0500, Eric Snowberg wrote:
> Compiling with 'W=1' results in warnings that 'Function parameter or member
> not described'
> 
> Add the missing parameters for
> restrict_link_by_builtin_and_secondary_trusted and
> restrict_link_to_builtin_trusted.
> 
> Use /* instead of /** for get_builtin_and_secondary_restriction, since
> it is a static function.
> 
> Fix wrong function name restrict_link_to_builtin_trusted.
> 
> Fixes: d3bfe84129f6 ("certs: Add a secondary system keyring that can be added to dynamically")
> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
>  certs/system_keyring.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> index 5042cc54fa5e..a7a49b17ceb1 100644
> --- a/certs/system_keyring.c
> +++ b/certs/system_keyring.c
> @@ -33,7 +33,11 @@ extern __initconst const unsigned long system_certificate_list_size;
>  extern __initconst const unsigned long module_cert_size;
>  
>  /**
> - * restrict_link_to_builtin_trusted - Restrict keyring addition by built in CA
> + * restrict_link_by_builtin_trusted - Restrict keyring addition by built-in CA
> + * @dest_keyring: Keyring being linked to.
> + * @type: The type of key being added.
> + * @payload: The payload of the new key.
> + * @restriction_key: A ring of keys that can be used to vouch for the new cert.
>   *
>   * Restrict the addition of keys into a keyring based on the key-to-be-added
>   * being vouched for by a key in the built in system keyring.
> @@ -50,7 +54,11 @@ int restrict_link_by_builtin_trusted(struct key *dest_keyring,
>  #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
>  /**
>   * restrict_link_by_builtin_and_secondary_trusted - Restrict keyring
> - *   addition by both builtin and secondary keyrings
> + *   addition by both built-in and secondary keyrings.
> + * @dest_keyring: Keyring being linked to.
> + * @type: The type of key being added.
> + * @payload: The payload of the new key.
> + * @restrict_key: A ring of keys that can be used to vouch for the new cert.
>   *
>   * Restrict the addition of keys into a keyring based on the key-to-be-added
>   * being vouched for by a key in either the built-in or the secondary system
> @@ -75,7 +83,7 @@ int restrict_link_by_builtin_and_secondary_trusted(
>  					  secondary_trusted_keys);
>  }
>  
> -/**
> +/*
>   * Allocate a struct key_restriction for the "builtin and secondary trust"
>   * keyring. Only for use in system_trusted_keyring_init().
>   */
> -- 
> 2.27.0
> 

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@iki.fi>

BR, Jarkko

[PATCH v5 3/6] KEYS: X.509: Parse Basic Constraints for CA

[Eric Snowberg]

Parse the X.509 Basic Constraints.  The basic constraints extension
identifies whether the subject of the certificate is a CA.

BasicConstraints ::= SEQUENCE {
        cA                      BOOLEAN DEFAULT FALSE,
        pathLenConstraint       INTEGER (0..MAX) OPTIONAL }

If the CA is true, store it in the public_key.  This will be used
in a follow on patch that requires knowing if the public key is a CA.

Link: https://www.rfc-editor.org/rfc/rfc5280#section-4.2.1.9
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 crypto/asymmetric_keys/x509_cert_parser.c | 22 ++++++++++++++++++++++
 include/crypto/public_key.h               |  2 ++
 2 files changed, 24 insertions(+)

diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index 7a9b084e2043..77547d4bd94d 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -586,6 +586,28 @@ int x509_process_extension(void *context, size_t hdrlen,
 		return 0;
 	}
 
+	if (ctx->last_oid == OID_basicConstraints) {
+		/*
+		 * Get hold of the basicConstraints
+		 * v[1] is the encoding size
+		 *	(Expect 0x2 or greater, making it 1 or more bytes)
+		 * v[2] is the encoding type
+		 *	(Expect an ASN1_BOOL for the CA)
+		 * v[3] is the contents of the ASN1_BOOL
+		 *      (Expect 1 if the CA is TRUE)
+		 * vlen should match the entire extension size
+		 */
+		if (v[0] != (ASN1_CONS_BIT | ASN1_SEQ))
+			return -EBADMSG;
+		if (vlen < 2)
+			return -EBADMSG;
+		if (v[1] != vlen - 2)
+			return -EBADMSG;
+		if (vlen >= 4 && v[1] != 0 && v[2] == ASN1_BOOL && v[3] == 1)
+			ctx->cert->pub->key_eflags |= 1 << KEY_EFLAG_CA;
+		return 0;
+	}
+
 	return 0;
 }
 
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index 6d61695e1cde..c401762850f2 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -28,6 +28,8 @@ struct public_key {
 	bool key_is_private;
 	const char *id_type;
 	const char *pkey_algo;
+	unsigned long key_eflags;	/* key extension flags */
+#define KEY_EFLAG_CA		0	/* set if the CA basic constraints is set */
 };
 
 extern void public_key_free(struct public_key *key);
-- 
2.27.0

[PATCH v5 4/6] KEYS: X.509: Parse Key Usage

[Eric Snowberg]

Parse the X.509 Key Usage.  The key usage extension defines the purpose of
the key contained in the certificate.

   id-ce-keyUsage OBJECT IDENTIFIER ::=  { id-ce 15 }

      KeyUsage ::= BIT STRING {
           digitalSignature        (0),
           contentCommitment       (1),
           keyEncipherment         (2),
           dataEncipherment        (3),
           keyAgreement            (4),
           keyCertSign             (5),
           cRLSign                 (6),
           encipherOnly            (7),
           decipherOnly            (8) }

If the keyCertSign or digitalSignature is set, store it in the
public_key structure. Having the purpose of the key being stored
during parsing, allows enforcement on the usage field in the future.
This will be used in a follow on patch that requires knowing the
certificate key usage type.

Link: https://www.rfc-editor.org/rfc/rfc5280#section-4.2.1.3
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
 crypto/asymmetric_keys/x509_cert_parser.c | 28 +++++++++++++++++++++++
 include/crypto/public_key.h               |  2 ++
 2 files changed, 30 insertions(+)

diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index 77547d4bd94d..0a7049b470c1 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -579,6 +579,34 @@ int x509_process_extension(void *context, size_t hdrlen,
 		return 0;
 	}
 
+	if (ctx->last_oid == OID_keyUsage) {
+		/*
+		 * Get hold of the keyUsage bit string
+		 * v[1] is the encoding size
+		 *       (Expect either 0x02 or 0x03, making it 1 or 2 bytes)
+		 * v[2] is the number of unused bits in the bit string
+		 *       (If >= 3 keyCertSign is missing when v[1] = 0x02)
+		 * v[3] and possibly v[4] contain the bit string
+		 *
+		 * From RFC 5280 4.2.1.3:
+		 *   0x04 is where keyCertSign lands in this bit string
+		 *   0x80 is where digitalSignature lands in this bit string
+		 */
+		if (v[0] != ASN1_BTS)
+			return -EBADMSG;
+		if (vlen < 4)
+			return -EBADMSG;
+		if (v[2] >= 8)
+			return -EBADMSG;
+		if (v[3] & 0x80)
+			ctx->cert->pub->key_eflags |= 1 << KEY_EFLAG_DIGITALSIG;
+		if (v[1] == 0x02 && v[2] <= 2 && (v[3] & 0x04))
+			ctx->cert->pub->key_eflags |= 1 << KEY_EFLAG_KEYCERTSIGN;
+		else if (vlen > 4 && v[1] == 0x03 && (v[3] & 0x04))
+			ctx->cert->pub->key_eflags |= 1 << KEY_EFLAG_KEYCERTSIGN;
+		return 0;
+	}
+
 	if (ctx->last_oid == OID_authorityKeyIdentifier) {
 		/* Get hold of the CA key fingerprint */
 		ctx->raw_akid = v;
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index c401762850f2..03c3fb990d59 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -30,6 +30,8 @@ struct public_key {
 	const char *pkey_algo;
 	unsigned long key_eflags;	/* key extension flags */
 #define KEY_EFLAG_CA		0	/* set if the CA basic constraints is set */
+#define KEY_EFLAG_DIGITALSIG	1	/* set if the digitalSignature usage is set */
+#define KEY_EFLAG_KEYCERTSIGN	2	/* set if the keyCertSign usage is set */
 };
 
 extern void public_key_free(struct public_key *key);
-- 
2.27.0

Re: [PATCH v5 4/6] KEYS: X.509: Parse Key Usage

[Jarkko Sakkinen]

On Thu, Mar 02, 2023 at 11:46:50AM -0500, Eric Snowberg wrote:
> Parse the X.509 Key Usage.  The key usage extension defines the purpose of
> the key contained in the certificate.
> 
>    id-ce-keyUsage OBJECT IDENTIFIER ::=  { id-ce 15 }
> 
>       KeyUsage ::= BIT STRING {
>            digitalSignature        (0),
>            contentCommitment       (1),
>            keyEncipherment         (2),
>            dataEncipherment        (3),
>            keyAgreement            (4),
>            keyCertSign             (5),
>            cRLSign                 (6),
>            encipherOnly            (7),
>            decipherOnly            (8) }
> 
> If the keyCertSign or digitalSignature is set, store it in the
> public_key structure. Having the purpose of the key being stored
> during parsing, allows enforcement on the usage field in the future.
> This will be used in a follow on patch that requires knowing the
> certificate key usage type.
> 
> Link: https://www.rfc-editor.org/rfc/rfc5280#section-4.2.1.3
> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>  crypto/asymmetric_keys/x509_cert_parser.c | 28 +++++++++++++++++++++++
>  include/crypto/public_key.h               |  2 ++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
> index 77547d4bd94d..0a7049b470c1 100644
> --- a/crypto/asymmetric_keys/x509_cert_parser.c
> +++ b/crypto/asymmetric_keys/x509_cert_parser.c
> @@ -579,6 +579,34 @@ int x509_process_extension(void *context, size_t hdrlen,
>  		return 0;
>  	}
>  
> +	if (ctx->last_oid == OID_keyUsage) {
> +		/*
> +		 * Get hold of the keyUsage bit string
> +		 * v[1] is the encoding size
> +		 *       (Expect either 0x02 or 0x03, making it 1 or 2 bytes)
> +		 * v[2] is the number of unused bits in the bit string
> +		 *       (If >= 3 keyCertSign is missing when v[1] = 0x02)
> +		 * v[3] and possibly v[4] contain the bit string
> +		 *
> +		 * From RFC 5280 4.2.1.3:
> +		 *   0x04 is where keyCertSign lands in this bit string
> +		 *   0x80 is where digitalSignature lands in this bit string
> +		 */
> +		if (v[0] != ASN1_BTS)
> +			return -EBADMSG;
> +		if (vlen < 4)
> +			return -EBADMSG;
> +		if (v[2] >= 8)
> +			return -EBADMSG;
> +		if (v[3] & 0x80)
> +			ctx->cert->pub->key_eflags |= 1 << KEY_EFLAG_DIGITALSIG;
> +		if (v[1] == 0x02 && v[2] <= 2 && (v[3] & 0x04))
> +			ctx->cert->pub->key_eflags |= 1 << KEY_EFLAG_KEYCERTSIGN;
> +		else if (vlen > 4 && v[1] == 0x03 && (v[3] & 0x04))
> +			ctx->cert->pub->key_eflags |= 1 << KEY_EFLAG_KEYCERTSIGN;
> +		return 0;
> +	}
> +
>  	if (ctx->last_oid == OID_authorityKeyIdentifier) {
>  		/* Get hold of the CA key fingerprint */
>  		ctx->raw_akid = v;
> diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> index c401762850f2..03c3fb990d59 100644
> --- a/include/crypto/public_key.h
> +++ b/include/crypto/public_key.h
> @@ -30,6 +30,8 @@ struct public_key {
>  	const char *pkey_algo;
>  	unsigned long key_eflags;	/* key extension flags */
>  #define KEY_EFLAG_CA		0	/* set if the CA basic constraints is set */
> +#define KEY_EFLAG_DIGITALSIG	1	/* set if the digitalSignature usage is set */
> +#define KEY_EFLAG_KEYCERTSIGN	2	/* set if the keyCertSign usage is set */
>  };
>  
>  extern void public_key_free(struct public_key *key);
> -- 
> 2.27.0
> 

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

[PATCH v5 5/6] KEYS: CA link restriction

[Eric Snowberg]

Add a new link restriction.  Restrict the addition of keys in a keyring
based on the key to be added being a CA.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
 crypto/asymmetric_keys/restrict.c | 38 +++++++++++++++++++++++++++++++
 include/crypto/public_key.h       | 15 ++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c
index 6b1ac5f5896a..48457c6f33f9 100644
--- a/crypto/asymmetric_keys/restrict.c
+++ b/crypto/asymmetric_keys/restrict.c
@@ -108,6 +108,44 @@ int restrict_link_by_signature(struct key *dest_keyring,
 	return ret;
 }
 
+/**
+ * restrict_link_by_ca - Restrict additions to a ring of CA keys
+ * @dest_keyring: Keyring being linked to.
+ * @type: The type of key being added.
+ * @payload: The payload of the new key.
+ * @trust_keyring: Unused.
+ *
+ * Check if the new certificate is a CA. If it is a CA, then mark the new
+ * certificate as being ok to link.
+ *
+ * Returns 0 if the new certificate was accepted, -ENOKEY if the
+ * certificate is not a CA. -ENOPKG if the signature uses unsupported
+ * crypto, or some other error if there is a matching certificate but
+ * the signature check cannot be performed.
+ */
+int restrict_link_by_ca(struct key *dest_keyring,
+			const struct key_type *type,
+			const union key_payload *payload,
+			struct key *trust_keyring)
+{
+	const struct public_key *pkey;
+
+	if (type != &key_type_asymmetric)
+		return -EOPNOTSUPP;
+
+	pkey = payload->data[asym_crypto];
+	if (!pkey)
+		return -ENOPKG;
+	if (!test_bit(KEY_EFLAG_CA, &pkey->key_eflags))
+		return -ENOKEY;
+	if (!test_bit(KEY_EFLAG_KEYCERTSIGN, &pkey->key_eflags))
+		return -ENOKEY;
+	if (test_bit(KEY_EFLAG_DIGITALSIG, &pkey->key_eflags))
+		return -ENOKEY;
+
+	return 0;
+}
+
 static bool match_either_id(const struct asymmetric_key_id **pair,
 			    const struct asymmetric_key_id *single)
 {
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index 03c3fb990d59..653992a6e941 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -75,6 +75,21 @@ extern int restrict_link_by_key_or_keyring_chain(struct key *trust_keyring,
 						 const union key_payload *payload,
 						 struct key *trusted);
 
+#if IS_REACHABLE(CONFIG_ASYMMETRIC_KEY_TYPE)
+extern int restrict_link_by_ca(struct key *dest_keyring,
+			       const struct key_type *type,
+			       const union key_payload *payload,
+			       struct key *trust_keyring);
+#else
+static inline int restrict_link_by_ca(struct key *dest_keyring,
+				      const struct key_type *type,
+				      const union key_payload *payload,
+				      struct key *trust_keyring)
+{
+	return 0;
+}
+#endif
+
 extern int query_asymmetric_key(const struct kernel_pkey_params *,
 				struct kernel_pkey_query *);
 
-- 
2.27.0

Re: [PATCH v5 5/6] KEYS: CA link restriction

[Jarkko Sakkinen]

On Thu, Mar 02, 2023 at 11:46:51AM -0500, Eric Snowberg wrote:
> Add a new link restriction.  Restrict the addition of keys in a keyring
> based on the key to be added being a CA.
> 
> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>  crypto/asymmetric_keys/restrict.c | 38 +++++++++++++++++++++++++++++++
>  include/crypto/public_key.h       | 15 ++++++++++++
>  2 files changed, 53 insertions(+)
> 
> diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c
> index 6b1ac5f5896a..48457c6f33f9 100644
> --- a/crypto/asymmetric_keys/restrict.c
> +++ b/crypto/asymmetric_keys/restrict.c
> @@ -108,6 +108,44 @@ int restrict_link_by_signature(struct key *dest_keyring,
>  	return ret;
>  }
>  
> +/**
> + * restrict_link_by_ca - Restrict additions to a ring of CA keys
> + * @dest_keyring: Keyring being linked to.
> + * @type: The type of key being added.
> + * @payload: The payload of the new key.
> + * @trust_keyring: Unused.
> + *
> + * Check if the new certificate is a CA. If it is a CA, then mark the new
> + * certificate as being ok to link.
> + *
> + * Returns 0 if the new certificate was accepted, -ENOKEY if the
> + * certificate is not a CA. -ENOPKG if the signature uses unsupported
> + * crypto, or some other error if there is a matching certificate but
> + * the signature check cannot be performed.
> + */
> +int restrict_link_by_ca(struct key *dest_keyring,
> +			const struct key_type *type,
> +			const union key_payload *payload,
> +			struct key *trust_keyring)
> +{
> +	const struct public_key *pkey;
> +
> +	if (type != &key_type_asymmetric)
> +		return -EOPNOTSUPP;
> +
> +	pkey = payload->data[asym_crypto];
> +	if (!pkey)
> +		return -ENOPKG;
> +	if (!test_bit(KEY_EFLAG_CA, &pkey->key_eflags))
> +		return -ENOKEY;
> +	if (!test_bit(KEY_EFLAG_KEYCERTSIGN, &pkey->key_eflags))
> +		return -ENOKEY;
> +	if (test_bit(KEY_EFLAG_DIGITALSIG, &pkey->key_eflags))
> +		return -ENOKEY;

nit: would be more readable, if conditions were separated by
empty lines.

> +
> +	return 0;
> +}
> +
>  static bool match_either_id(const struct asymmetric_key_id **pair,
>  			    const struct asymmetric_key_id *single)
>  {
> diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> index 03c3fb990d59..653992a6e941 100644
> --- a/include/crypto/public_key.h
> +++ b/include/crypto/public_key.h
> @@ -75,6 +75,21 @@ extern int restrict_link_by_key_or_keyring_chain(struct key *trust_keyring,
>  						 const union key_payload *payload,
>  						 struct key *trusted);
>  
> +#if IS_REACHABLE(CONFIG_ASYMMETRIC_KEY_TYPE)
> +extern int restrict_link_by_ca(struct key *dest_keyring,
> +			       const struct key_type *type,
> +			       const union key_payload *payload,
> +			       struct key *trust_keyring);
> +#else
> +static inline int restrict_link_by_ca(struct key *dest_keyring,
> +				      const struct key_type *type,
> +				      const union key_payload *payload,
> +				      struct key *trust_keyring)
> +{
> +	return 0;
> +}
> +#endif
> +
>  extern int query_asymmetric_key(const struct kernel_pkey_params *,
>  				struct kernel_pkey_query *);
>  
> -- 
> 2.27.0
> 

BR, Jarkko

Re: [PATCH v5 5/6] KEYS: CA link restriction

[Eric Snowberg]

> On Mar 11, 2023, at 3:10 PM, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> 
> On Thu, Mar 02, 2023 at 11:46:51AM -0500, Eric Snowberg wrote:
>> Add a new link restriction.  Restrict the addition of keys in a keyring
>> based on the key to be added being a CA.
>> 
>> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
>> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
>> ---
>> crypto/asymmetric_keys/restrict.c | 38 +++++++++++++++++++++++++++++++
>> include/crypto/public_key.h       | 15 ++++++++++++
>> 2 files changed, 53 insertions(+)
>> 
>> diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c
>> index 6b1ac5f5896a..48457c6f33f9 100644
>> --- a/crypto/asymmetric_keys/restrict.c
>> +++ b/crypto/asymmetric_keys/restrict.c
>> @@ -108,6 +108,44 @@ int restrict_link_by_signature(struct key *dest_keyring,
>> 	return ret;
>> }
>> 
>> +/**
>> + * restrict_link_by_ca - Restrict additions to a ring of CA keys
>> + * @dest_keyring: Keyring being linked to.
>> + * @type: The type of key being added.
>> + * @payload: The payload of the new key.
>> + * @trust_keyring: Unused.
>> + *
>> + * Check if the new certificate is a CA. If it is a CA, then mark the new
>> + * certificate as being ok to link.
>> + *
>> + * Returns 0 if the new certificate was accepted, -ENOKEY if the
>> + * certificate is not a CA. -ENOPKG if the signature uses unsupported
>> + * crypto, or some other error if there is a matching certificate but
>> + * the signature check cannot be performed.
>> + */
>> +int restrict_link_by_ca(struct key *dest_keyring,
>> +			const struct key_type *type,
>> +			const union key_payload *payload,
>> +			struct key *trust_keyring)
>> +{
>> +	const struct public_key *pkey;
>> +
>> +	if (type != &key_type_asymmetric)
>> +		return -EOPNOTSUPP;
>> +
>> +	pkey = payload->data[asym_crypto];
>> +	if (!pkey)
>> +		return -ENOPKG;
>> +	if (!test_bit(KEY_EFLAG_CA, &pkey->key_eflags))
>> +		return -ENOKEY;
>> +	if (!test_bit(KEY_EFLAG_KEYCERTSIGN, &pkey->key_eflags))
>> +		return -ENOKEY;
>> +	if (test_bit(KEY_EFLAG_DIGITALSIG, &pkey->key_eflags))
>> +		return -ENOKEY;
> 
> nit: would be more readable, if conditions were separated by
> empty lines.

Ok, I will make this change in the next round.  Thanks.

Re: [PATCH v5 5/6] KEYS: CA link restriction

[Jarkko Sakkinen]

On Mon, Mar 20, 2023 at 05:35:05PM +0000, Eric Snowberg wrote:
> 
> 
> > On Mar 11, 2023, at 3:10 PM, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > 
> > On Thu, Mar 02, 2023 at 11:46:51AM -0500, Eric Snowberg wrote:
> >> Add a new link restriction.  Restrict the addition of keys in a keyring
> >> based on the key to be added being a CA.
> >> 
> >> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
> >> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> >> ---
> >> crypto/asymmetric_keys/restrict.c | 38 +++++++++++++++++++++++++++++++
> >> include/crypto/public_key.h       | 15 ++++++++++++
> >> 2 files changed, 53 insertions(+)
> >> 
> >> diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c
> >> index 6b1ac5f5896a..48457c6f33f9 100644
> >> --- a/crypto/asymmetric_keys/restrict.c
> >> +++ b/crypto/asymmetric_keys/restrict.c
> >> @@ -108,6 +108,44 @@ int restrict_link_by_signature(struct key *dest_keyring,
> >> 	return ret;
> >> }
> >> 
> >> +/**
> >> + * restrict_link_by_ca - Restrict additions to a ring of CA keys
> >> + * @dest_keyring: Keyring being linked to.
> >> + * @type: The type of key being added.
> >> + * @payload: The payload of the new key.
> >> + * @trust_keyring: Unused.
> >> + *
> >> + * Check if the new certificate is a CA. If it is a CA, then mark the new
> >> + * certificate as being ok to link.
> >> + *
> >> + * Returns 0 if the new certificate was accepted, -ENOKEY if the
> >> + * certificate is not a CA. -ENOPKG if the signature uses unsupported
> >> + * crypto, or some other error if there is a matching certificate but
> >> + * the signature check cannot be performed.
> >> + */
> >> +int restrict_link_by_ca(struct key *dest_keyring,
> >> +			const struct key_type *type,
> >> +			const union key_payload *payload,
> >> +			struct key *trust_keyring)
> >> +{
> >> +	const struct public_key *pkey;
> >> +
> >> +	if (type != &key_type_asymmetric)
> >> +		return -EOPNOTSUPP;
> >> +
> >> +	pkey = payload->data[asym_crypto];
> >> +	if (!pkey)
> >> +		return -ENOPKG;
> >> +	if (!test_bit(KEY_EFLAG_CA, &pkey->key_eflags))
> >> +		return -ENOKEY;
> >> +	if (!test_bit(KEY_EFLAG_KEYCERTSIGN, &pkey->key_eflags))
> >> +		return -ENOKEY;
> >> +	if (test_bit(KEY_EFLAG_DIGITALSIG, &pkey->key_eflags))
> >> +		return -ENOKEY;
> > 
> > nit: would be more readable, if conditions were separated by
> > empty lines.
> 
> Ok, I will make this change in the next round.  Thanks.

Cool! Mimi have you tested these patches with IMA applied?

BR, Jarkko

Re: [PATCH v5 5/6] KEYS: CA link restriction

[Mimi Zohar]

On Mon, 2023-03-20 at 20:28 +0200, Jarkko Sakkinen wrote:
> On Mon, Mar 20, 2023 at 05:35:05PM +0000, Eric Snowberg wrote:
> > 
> > 
> > > On Mar 11, 2023, at 3:10 PM, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > > 
> > > On Thu, Mar 02, 2023 at 11:46:51AM -0500, Eric Snowberg wrote:
> > >> Add a new link restriction.  Restrict the addition of keys in a keyring
> > >> based on the key to be added being a CA.
> > >> 
> > >> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
> > >> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> > >> ---
> > >> crypto/asymmetric_keys/restrict.c | 38 +++++++++++++++++++++++++++++++
> > >> include/crypto/public_key.h       | 15 ++++++++++++
> > >> 2 files changed, 53 insertions(+)
> > >> 
> > >> diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c
> > >> index 6b1ac5f5896a..48457c6f33f9 100644
> > >> --- a/crypto/asymmetric_keys/restrict.c
> > >> +++ b/crypto/asymmetric_keys/restrict.c
> > >> @@ -108,6 +108,44 @@ int restrict_link_by_signature(struct key *dest_keyring,
> > >> 	return ret;
> > >> }
> > >> 
> > >> +/**
> > >> + * restrict_link_by_ca - Restrict additions to a ring of CA keys
> > >> + * @dest_keyring: Keyring being linked to.
> > >> + * @type: The type of key being added.
> > >> + * @payload: The payload of the new key.
> > >> + * @trust_keyring: Unused.
> > >> + *
> > >> + * Check if the new certificate is a CA. If it is a CA, then mark the new
> > >> + * certificate as being ok to link.
> > >> + *
> > >> + * Returns 0 if the new certificate was accepted, -ENOKEY if the
> > >> + * certificate is not a CA. -ENOPKG if the signature uses unsupported
> > >> + * crypto, or some other error if there is a matching certificate but
> > >> + * the signature check cannot be performed.
> > >> + */
> > >> +int restrict_link_by_ca(struct key *dest_keyring,
> > >> +			const struct key_type *type,
> > >> +			const union key_payload *payload,
> > >> +			struct key *trust_keyring)
> > >> +{
> > >> +	const struct public_key *pkey;
> > >> +
> > >> +	if (type != &key_type_asymmetric)
> > >> +		return -EOPNOTSUPP;
> > >> +
> > >> +	pkey = payload->data[asym_crypto];
> > >> +	if (!pkey)
> > >> +		return -ENOPKG;
> > >> +	if (!test_bit(KEY_EFLAG_CA, &pkey->key_eflags))
> > >> +		return -ENOKEY;
> > >> +	if (!test_bit(KEY_EFLAG_KEYCERTSIGN, &pkey->key_eflags))
> > >> +		return -ENOKEY;
> > >> +	if (test_bit(KEY_EFLAG_DIGITALSIG, &pkey->key_eflags))
> > >> +		return -ENOKEY;
> > > 
> > > nit: would be more readable, if conditions were separated by
> > > empty lines.
> > 
> > Ok, I will make this change in the next round.  Thanks.
> 
> Cool! Mimi have you tested these patches with IMA applied?

Yes, it's working as expected.

-- 
Mimi

Re: [PATCH v5 5/6] KEYS: CA link restriction

[Jarkko Sakkinen]

On Mon, Mar 20, 2023 at 04:35:33PM -0400, Mimi Zohar wrote:
> On Mon, 2023-03-20 at 20:28 +0200, Jarkko Sakkinen wrote:
> > On Mon, Mar 20, 2023 at 05:35:05PM +0000, Eric Snowberg wrote:
> > > 
> > > 
> > > > On Mar 11, 2023, at 3:10 PM, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > > > 
> > > > On Thu, Mar 02, 2023 at 11:46:51AM -0500, Eric Snowberg wrote:
> > > >> Add a new link restriction.  Restrict the addition of keys in a keyring
> > > >> based on the key to be added being a CA.
> > > >> 
> > > >> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
> > > >> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> > > >> ---
> > > >> crypto/asymmetric_keys/restrict.c | 38 +++++++++++++++++++++++++++++++
> > > >> include/crypto/public_key.h       | 15 ++++++++++++
> > > >> 2 files changed, 53 insertions(+)
> > > >> 
> > > >> diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c
> > > >> index 6b1ac5f5896a..48457c6f33f9 100644
> > > >> --- a/crypto/asymmetric_keys/restrict.c
> > > >> +++ b/crypto/asymmetric_keys/restrict.c
> > > >> @@ -108,6 +108,44 @@ int restrict_link_by_signature(struct key *dest_keyring,
> > > >> 	return ret;
> > > >> }
> > > >> 
> > > >> +/**
> > > >> + * restrict_link_by_ca - Restrict additions to a ring of CA keys
> > > >> + * @dest_keyring: Keyring being linked to.
> > > >> + * @type: The type of key being added.
> > > >> + * @payload: The payload of the new key.
> > > >> + * @trust_keyring: Unused.
> > > >> + *
> > > >> + * Check if the new certificate is a CA. If it is a CA, then mark the new
> > > >> + * certificate as being ok to link.
> > > >> + *
> > > >> + * Returns 0 if the new certificate was accepted, -ENOKEY if the
> > > >> + * certificate is not a CA. -ENOPKG if the signature uses unsupported
> > > >> + * crypto, or some other error if there is a matching certificate but
> > > >> + * the signature check cannot be performed.
> > > >> + */
> > > >> +int restrict_link_by_ca(struct key *dest_keyring,
> > > >> +			const struct key_type *type,
> > > >> +			const union key_payload *payload,
> > > >> +			struct key *trust_keyring)
> > > >> +{
> > > >> +	const struct public_key *pkey;
> > > >> +
> > > >> +	if (type != &key_type_asymmetric)
> > > >> +		return -EOPNOTSUPP;
> > > >> +
> > > >> +	pkey = payload->data[asym_crypto];
> > > >> +	if (!pkey)
> > > >> +		return -ENOPKG;
> > > >> +	if (!test_bit(KEY_EFLAG_CA, &pkey->key_eflags))
> > > >> +		return -ENOKEY;
> > > >> +	if (!test_bit(KEY_EFLAG_KEYCERTSIGN, &pkey->key_eflags))
> > > >> +		return -ENOKEY;
> > > >> +	if (test_bit(KEY_EFLAG_DIGITALSIG, &pkey->key_eflags))
> > > >> +		return -ENOKEY;
> > > > 
> > > > nit: would be more readable, if conditions were separated by
> > > > empty lines.
> > > 
> > > Ok, I will make this change in the next round.  Thanks.
> > 
> > Cool! Mimi have you tested these patches with IMA applied?
> 
> Yes, it's working as expected.

OK, I will pick these.

BR, Jarkko

Re: [PATCH v5 5/6] KEYS: CA link restriction

[Jarkko Sakkinen]

On Mon, Mar 20, 2023 at 04:35:33PM -0400, Mimi Zohar wrote:
> On Mon, 2023-03-20 at 20:28 +0200, Jarkko Sakkinen wrote:
> > On Mon, Mar 20, 2023 at 05:35:05PM +0000, Eric Snowberg wrote:
> > > 
> > > 
> > > > On Mar 11, 2023, at 3:10 PM, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > > > 
> > > > On Thu, Mar 02, 2023 at 11:46:51AM -0500, Eric Snowberg wrote:
> > > >> Add a new link restriction.  Restrict the addition of keys in a keyring
> > > >> based on the key to be added being a CA.
> > > >> 
> > > >> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
> > > >> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> > > >> ---
> > > >> crypto/asymmetric_keys/restrict.c | 38 +++++++++++++++++++++++++++++++
> > > >> include/crypto/public_key.h       | 15 ++++++++++++
> > > >> 2 files changed, 53 insertions(+)
> > > >> 
> > > >> diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c
> > > >> index 6b1ac5f5896a..48457c6f33f9 100644
> > > >> --- a/crypto/asymmetric_keys/restrict.c
> > > >> +++ b/crypto/asymmetric_keys/restrict.c
> > > >> @@ -108,6 +108,44 @@ int restrict_link_by_signature(struct key *dest_keyring,
> > > >> 	return ret;
> > > >> }
> > > >> 
> > > >> +/**
> > > >> + * restrict_link_by_ca - Restrict additions to a ring of CA keys
> > > >> + * @dest_keyring: Keyring being linked to.
> > > >> + * @type: The type of key being added.
> > > >> + * @payload: The payload of the new key.
> > > >> + * @trust_keyring: Unused.
> > > >> + *
> > > >> + * Check if the new certificate is a CA. If it is a CA, then mark the new
> > > >> + * certificate as being ok to link.
> > > >> + *
> > > >> + * Returns 0 if the new certificate was accepted, -ENOKEY if the
> > > >> + * certificate is not a CA. -ENOPKG if the signature uses unsupported
> > > >> + * crypto, or some other error if there is a matching certificate but
> > > >> + * the signature check cannot be performed.
> > > >> + */
> > > >> +int restrict_link_by_ca(struct key *dest_keyring,
> > > >> +			const struct key_type *type,
> > > >> +			const union key_payload *payload,
> > > >> +			struct key *trust_keyring)
> > > >> +{
> > > >> +	const struct public_key *pkey;
> > > >> +
> > > >> +	if (type != &key_type_asymmetric)
> > > >> +		return -EOPNOTSUPP;
> > > >> +
> > > >> +	pkey = payload->data[asym_crypto];
> > > >> +	if (!pkey)
> > > >> +		return -ENOPKG;
> > > >> +	if (!test_bit(KEY_EFLAG_CA, &pkey->key_eflags))
> > > >> +		return -ENOKEY;
> > > >> +	if (!test_bit(KEY_EFLAG_KEYCERTSIGN, &pkey->key_eflags))
> > > >> +		return -ENOKEY;
> > > >> +	if (test_bit(KEY_EFLAG_DIGITALSIG, &pkey->key_eflags))
> > > >> +		return -ENOKEY;
> > > > 
> > > > nit: would be more readable, if conditions were separated by
> > > > empty lines.
> > > 
> > > Ok, I will make this change in the next round.  Thanks.
> > 
> > Cool! Mimi have you tested these patches with IMA applied?
> 
> Yes, it's working as expected.

Thank you. Please check that I filled additional tags correctly:

https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/log/

I will then put these also to my 'next' branch and they will get mirrored
to linux-next.

BR, Jarkko

Re: [PATCH v5 5/6] KEYS: CA link restriction

[Mimi Zohar]

On Thu, 2023-03-30 at 02:27 +0300, Jarkko Sakkinen wrote:
> On Mon, Mar 20, 2023 at 04:35:33PM -0400, Mimi Zohar wrote:
> > On Mon, 2023-03-20 at 20:28 +0200, Jarkko Sakkinen wrote:
> > > On Mon, Mar 20, 2023 at 05:35:05PM +0000, Eric Snowberg wrote:
> > > > 
> > > > 
> > > > > On Mar 11, 2023, at 3:10 PM, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > > > > 
> > > > > On Thu, Mar 02, 2023 at 11:46:51AM -0500, Eric Snowberg wrote:
> > > > >> Add a new link restriction.  Restrict the addition of keys in a keyring
> > > > >> based on the key to be added being a CA.
> > > > >> 
> > > > >> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
> > > > >> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> > > > >> ---
> > > > >> crypto/asymmetric_keys/restrict.c | 38 +++++++++++++++++++++++++++++++
> > > > >> include/crypto/public_key.h       | 15 ++++++++++++
> > > > >> 2 files changed, 53 insertions(+)
> > > > >> 
> > > > >> diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c
> > > > >> index 6b1ac5f5896a..48457c6f33f9 100644
> > > > >> --- a/crypto/asymmetric_keys/restrict.c
> > > > >> +++ b/crypto/asymmetric_keys/restrict.c
> > > > >> @@ -108,6 +108,44 @@ int restrict_link_by_signature(struct key *dest_keyring,
> > > > >> 	return ret;
> > > > >> }
> > > > >> 
> > > > >> +/**
> > > > >> + * restrict_link_by_ca - Restrict additions to a ring of CA keys
> > > > >> + * @dest_keyring: Keyring being linked to.
> > > > >> + * @type: The type of key being added.
> > > > >> + * @payload: The payload of the new key.
> > > > >> + * @trust_keyring: Unused.
> > > > >> + *
> > > > >> + * Check if the new certificate is a CA. If it is a CA, then mark the new
> > > > >> + * certificate as being ok to link.
> > > > >> + *
> > > > >> + * Returns 0 if the new certificate was accepted, -ENOKEY if the
> > > > >> + * certificate is not a CA. -ENOPKG if the signature uses unsupported
> > > > >> + * crypto, or some other error if there is a matching certificate but
> > > > >> + * the signature check cannot be performed.
> > > > >> + */
> > > > >> +int restrict_link_by_ca(struct key *dest_keyring,
> > > > >> +			const struct key_type *type,
> > > > >> +			const union key_payload *payload,
> > > > >> +			struct key *trust_keyring)
> > > > >> +{
> > > > >> +	const struct public_key *pkey;
> > > > >> +
> > > > >> +	if (type != &key_type_asymmetric)
> > > > >> +		return -EOPNOTSUPP;
> > > > >> +
> > > > >> +	pkey = payload->data[asym_crypto];
> > > > >> +	if (!pkey)
> > > > >> +		return -ENOPKG;
> > > > >> +	if (!test_bit(KEY_EFLAG_CA, &pkey->key_eflags))
> > > > >> +		return -ENOKEY;
> > > > >> +	if (!test_bit(KEY_EFLAG_KEYCERTSIGN, &pkey->key_eflags))
> > > > >> +		return -ENOKEY;
> > > > >> +	if (test_bit(KEY_EFLAG_DIGITALSIG, &pkey->key_eflags))
> > > > >> +		return -ENOKEY;
> > > > > 
> > > > > nit: would be more readable, if conditions were separated by
> > > > > empty lines.
> > > > 
> > > > Ok, I will make this change in the next round.  Thanks.
> > > 
> > > Cool! Mimi have you tested these patches with IMA applied?
> > 
> > Yes, it's working as expected.
> 
> Thank you. Please check that I filled additional tags correctly:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/log/
> 
> I will then put these also to my 'next' branch and they will get mirrored
> to linux-next.

Thanks, Jarkko.  The tags look good.

-- 
thanks,

Mimi

Re: [PATCH v5 5/6] KEYS: CA link restriction

[Jarkko Sakkinen]

On Thu, Mar 30, 2023 at 02:01:52AM -0400, Mimi Zohar wrote:
> On Thu, 2023-03-30 at 02:27 +0300, Jarkko Sakkinen wrote:
> > On Mon, Mar 20, 2023 at 04:35:33PM -0400, Mimi Zohar wrote:
> > > On Mon, 2023-03-20 at 20:28 +0200, Jarkko Sakkinen wrote:
> > > > On Mon, Mar 20, 2023 at 05:35:05PM +0000, Eric Snowberg wrote:
> > > > > 
> > > > > 
> > > > > > On Mar 11, 2023, at 3:10 PM, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > > > > > 
> > > > > > On Thu, Mar 02, 2023 at 11:46:51AM -0500, Eric Snowberg wrote:
> > > > > >> Add a new link restriction.  Restrict the addition of keys in a keyring
> > > > > >> based on the key to be added being a CA.
> > > > > >> 
> > > > > >> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
> > > > > >> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> > > > > >> ---
> > > > > >> crypto/asymmetric_keys/restrict.c | 38 +++++++++++++++++++++++++++++++
> > > > > >> include/crypto/public_key.h       | 15 ++++++++++++
> > > > > >> 2 files changed, 53 insertions(+)
> > > > > >> 
> > > > > >> diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c
> > > > > >> index 6b1ac5f5896a..48457c6f33f9 100644
> > > > > >> --- a/crypto/asymmetric_keys/restrict.c
> > > > > >> +++ b/crypto/asymmetric_keys/restrict.c
> > > > > >> @@ -108,6 +108,44 @@ int restrict_link_by_signature(struct key *dest_keyring,
> > > > > >> 	return ret;
> > > > > >> }
> > > > > >> 
> > > > > >> +/**
> > > > > >> + * restrict_link_by_ca - Restrict additions to a ring of CA keys
> > > > > >> + * @dest_keyring: Keyring being linked to.
> > > > > >> + * @type: The type of key being added.
> > > > > >> + * @payload: The payload of the new key.
> > > > > >> + * @trust_keyring: Unused.
> > > > > >> + *
> > > > > >> + * Check if the new certificate is a CA. If it is a CA, then mark the new
> > > > > >> + * certificate as being ok to link.
> > > > > >> + *
> > > > > >> + * Returns 0 if the new certificate was accepted, -ENOKEY if the
> > > > > >> + * certificate is not a CA. -ENOPKG if the signature uses unsupported
> > > > > >> + * crypto, or some other error if there is a matching certificate but
> > > > > >> + * the signature check cannot be performed.
> > > > > >> + */
> > > > > >> +int restrict_link_by_ca(struct key *dest_keyring,
> > > > > >> +			const struct key_type *type,
> > > > > >> +			const union key_payload *payload,
> > > > > >> +			struct key *trust_keyring)
> > > > > >> +{
> > > > > >> +	const struct public_key *pkey;
> > > > > >> +
> > > > > >> +	if (type != &key_type_asymmetric)
> > > > > >> +		return -EOPNOTSUPP;
> > > > > >> +
> > > > > >> +	pkey = payload->data[asym_crypto];
> > > > > >> +	if (!pkey)
> > > > > >> +		return -ENOPKG;
> > > > > >> +	if (!test_bit(KEY_EFLAG_CA, &pkey->key_eflags))
> > > > > >> +		return -ENOKEY;
> > > > > >> +	if (!test_bit(KEY_EFLAG_KEYCERTSIGN, &pkey->key_eflags))
> > > > > >> +		return -ENOKEY;
> > > > > >> +	if (test_bit(KEY_EFLAG_DIGITALSIG, &pkey->key_eflags))
> > > > > >> +		return -ENOKEY;
> > > > > > 
> > > > > > nit: would be more readable, if conditions were separated by
> > > > > > empty lines.
> > > > > 
> > > > > Ok, I will make this change in the next round.  Thanks.
> > > > 
> > > > Cool! Mimi have you tested these patches with IMA applied?
> > > 
> > > Yes, it's working as expected.
> > 
> > Thank you. Please check that I filled additional tags correctly:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/log/
> > 
> > I will then put these also to my 'next' branch and they will get mirrored
> > to linux-next.
> 
> Thanks, Jarkko.  The tags look good.

Hi, sorry for radio silence. I've been transitioning to a new job.

Commits are in my next branch, and I will include them to my PR.

BR, Jarkko

[PATCH v5 6/6] integrity: machine keyring CA configuration

[Eric Snowberg]

Add machine keyring CA restriction options to control the type of
keys that may be added to it. The motivation is separation of
certificate signing from code signing keys. Subsquent work will
limit certificates being loaded into the IMA keyring to code
signing keys used for signature verification.

When no restrictions are selected, all Machine Owner Keys (MOK) are added
to the machine keyring.  When CONFIG_INTEGRITY_CA_MACHINE_KEYRING is
selected, the CA bit must be true.  Also the key usage must contain
keyCertSign, any other usage field may be set as well.

When CONFIG_INTEGRITY_CA_MACHINE_KEYRING_MAX is selected, the CA bit must
be true. Also the key usage must contain keyCertSign and the
digitialSignature usage may not be set.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
 crypto/asymmetric_keys/restrict.c |  2 ++
 security/integrity/Kconfig        | 23 ++++++++++++++++++++++-
 security/integrity/digsig.c       |  8 ++++++--
 3 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c
index 48457c6f33f9..276bdb627498 100644
--- a/crypto/asymmetric_keys/restrict.c
+++ b/crypto/asymmetric_keys/restrict.c
@@ -140,6 +140,8 @@ int restrict_link_by_ca(struct key *dest_keyring,
 		return -ENOKEY;
 	if (!test_bit(KEY_EFLAG_KEYCERTSIGN, &pkey->key_eflags))
 		return -ENOKEY;
+	if (!IS_ENABLED(CONFIG_INTEGRITY_CA_MACHINE_KEYRING_MAX))
+		return 0;
 	if (test_bit(KEY_EFLAG_DIGITALSIG, &pkey->key_eflags))
 		return -ENOKEY;
 
diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 599429f99f99..ec6e0d789da1 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -68,13 +68,34 @@ config INTEGRITY_MACHINE_KEYRING
 	depends on INTEGRITY_ASYMMETRIC_KEYS
 	depends on SYSTEM_BLACKLIST_KEYRING
 	depends on LOAD_UEFI_KEYS
-	depends on !IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
 	help
 	 If set, provide a keyring to which Machine Owner Keys (MOK) may
 	 be added. This keyring shall contain just MOK keys.  Unlike keys
 	 in the platform keyring, keys contained in the .machine keyring will
 	 be trusted within the kernel.
 
+config INTEGRITY_CA_MACHINE_KEYRING
+	bool "Enforce Machine Keyring CA Restrictions"
+	depends on INTEGRITY_MACHINE_KEYRING
+	default n
+	help
+	  The .machine keyring can be configured to enforce CA restriction
+	  on any key added to it.  By default no restrictions are in place
+	  and all Machine Owner Keys (MOK) are added to the machine keyring.
+	  If enabled only CA keys are added to the machine keyring, all
+	  other MOK keys load into the platform keyring.
+
+config INTEGRITY_CA_MACHINE_KEYRING_MAX
+	bool "Only CA keys without DigitialSignature usage set"
+	depends on INTEGRITY_CA_MACHINE_KEYRING
+	default n
+	help
+	  When selected, only load CA keys are loaded into the machine
+	  keyring that contain the CA bit set along with the keyCertSign
+	  Usage field.  Keys containing the digitialSignature Usage field
+	  will not be loaded. The remaining MOK keys are loaded into the
+	  .platform keyring.
+
 config LOAD_UEFI_KEYS
        depends on INTEGRITY_PLATFORM_KEYRING
        depends on EFI
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index f2193c531f4a..6f31ffe23c48 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -132,7 +132,8 @@ int __init integrity_init_keyring(const unsigned int id)
 		| KEY_USR_READ | KEY_USR_SEARCH;
 
 	if (id == INTEGRITY_KEYRING_PLATFORM ||
-	    id == INTEGRITY_KEYRING_MACHINE) {
+	    (id == INTEGRITY_KEYRING_MACHINE &&
+	    !IS_ENABLED(CONFIG_INTEGRITY_CA_MACHINE_KEYRING))) {
 		restriction = NULL;
 		goto out;
 	}
@@ -144,7 +145,10 @@ int __init integrity_init_keyring(const unsigned int id)
 	if (!restriction)
 		return -ENOMEM;
 
-	restriction->check = restrict_link_to_ima;
+	if (id == INTEGRITY_KEYRING_MACHINE)
+		restriction->check = restrict_link_by_ca;
+	else
+		restriction->check = restrict_link_to_ima;
 
 	/*
 	 * MOK keys can only be added through a read-only runtime services
-- 
2.27.0

Re: [PATCH v5 6/6] integrity: machine keyring CA configuration

[Mimi Zohar]

On Thu, 2023-03-02 at 11:46 -0500, Eric Snowberg wrote:
> Add machine keyring CA restriction options to control the type of
> keys that may be added to it. The motivation is separation of
> certificate signing from code signing keys. Subsquent work will
> limit certificates being loaded into the IMA keyring to code
> signing keys used for signature verification.
> 
> When no restrictions are selected, all Machine Owner Keys (MOK) are added
> to the machine keyring.  When CONFIG_INTEGRITY_CA_MACHINE_KEYRING is
> selected, the CA bit must be true.  Also the key usage must contain
> keyCertSign, any other usage field may be set as well.
> 
> When CONFIG_INTEGRITY_CA_MACHINE_KEYRING_MAX is selected, the CA bit must
> be true. Also the key usage must contain keyCertSign and the
> digitialSignature usage may not be set.
> 
> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>

Thanks, Eric.

Acked-by: Mimi Zohar <zohar@linux.ibm.com>