NIAP PP_OS certification requests that OS need to 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.
v9:
- Rename the eku element in public_key structure to ext_key_usage.
- Fix selftest.c
v8:
- Fixed the bug of is_key_on_revocation_list() when
CONFIG_SYSTEM_REVOCATION_LIST is not set.
v7:
- Fixed the broken function call in is_key_on_revocation_list().
(be found by kernel test robot)
- Use a general name check_eku_by_usage() instead of check_codesign_eku().
v6:
- Add more length checking when parsing extKeyUsage and EKU's OID blob.
- Add 'usage' parameter to the comment of pkcs7_validate_trust function.
v5:
Fixed the wording in module-signing.rst.
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/blacklist.c | 5 ++-
certs/default_x509.genkey | 1 +
certs/system_keyring.c | 4 +-
crypto/asymmetric_keys/Kconfig | 9 ++++
crypto/asymmetric_keys/pkcs7_trust.c | 43 ++++++++++++++++++--
crypto/asymmetric_keys/selftest.c | 2 +-
crypto/asymmetric_keys/x509_cert_parser.c | 25 ++++++++++++
include/crypto/pkcs7.h | 4 +-
include/crypto/public_key.h | 1 +
include/keys/system_keyring.h | 7 +++-
include/linux/oid_registry.h | 5 +++
12 files changed, 101 insertions(+), 11 deletions(-)
--
2.26.2
This patch adds the logic for parsing the CodeSign extended key usage
extension in X.509. The parsing result will be set to the ext_key_usage
flag which is carried by public key. It can be used in the PKCS#7
verification.
Signed-off-by: "Lee, Chun-Yi" <[email protected]>
---
crypto/asymmetric_keys/x509_cert_parser.c | 25 +++++++++++++++++++++++
include/crypto/public_key.h | 1 +
include/linux/oid_registry.h | 5 +++++
3 files changed, 31 insertions(+)
diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index 2899ed80bb18..1f67e0adef65 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -554,6 +554,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);
@@ -583,6 +585,29 @@ int x509_process_extension(void *context, size_t hdrlen,
return 0;
}
+ if (ctx->last_oid == OID_extKeyUsage) {
+ if (vlen < 2 ||
+ 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 ((i + 10) > vlen || v[i] != ASN1_OID || v[i + 1] != 8)
+ return -EBADMSG;
+
+ oid = look_up_OID(v + i + 2, v[i + 1]);
+ if (oid == OID_codeSigning) {
+ ctx->cert->pub->ext_key_usage |= EKU_codeSigning;
+ }
+ i += 10;
+ }
+ pr_debug("extKeyUsage: %d\n", ctx->cert->pub->ext_key_usage);
+ return 0;
+ }
+
return 0;
}
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index 68f7aa2a7e55..72c0fcc39d0f 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 ext_key_usage : 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 0f4a8903922a..460135c2d918 100644
--- a/include/linux/oid_registry.h
+++ b/include/linux/oid_registry.h
@@ -140,9 +140,14 @@ enum OID {
OID_TPMImportableKey, /* 2.23.133.10.1.4 */
OID_TPMSealedData, /* 2.23.133.10.1.5 */
+ /* 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 parse_OID(const void *data, size_t datasize, enum OID *oid);
extern int sprint_oid(const void *, size_t, char *, size_t);
--
2.26.2
This patch adds the logic for checking the CodeSigning extended
key usage when verifying signature of kernel module or
kexec PE binary in PKCS#7.
Signed-off-by: "Lee, Chun-Yi" <[email protected]>
---
certs/blacklist.c | 5 ++--
certs/system_keyring.c | 4 +--
crypto/asymmetric_keys/Kconfig | 9 ++++++
crypto/asymmetric_keys/pkcs7_trust.c | 43 ++++++++++++++++++++++++++--
crypto/asymmetric_keys/selftest.c | 2 +-
include/crypto/pkcs7.h | 4 ++-
include/keys/system_keyring.h | 7 +++--
7 files changed, 63 insertions(+), 11 deletions(-)
diff --git a/certs/blacklist.c b/certs/blacklist.c
index 41f10601cc72..fa41454055be 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -282,11 +282,12 @@ int add_key_to_revocation_list(const char *data, size_t size)
* is_key_on_revocation_list - Determine if the key for a PKCS#7 message is revoked
* @pkcs7: The PKCS#7 message to check
*/
-int is_key_on_revocation_list(struct pkcs7_message *pkcs7)
+int is_key_on_revocation_list(struct pkcs7_message *pkcs7,
+ enum key_being_used_for usage)
{
int ret;
- ret = pkcs7_validate_trust(pkcs7, blacklist_keyring);
+ ret = pkcs7_validate_trust(pkcs7, blacklist_keyring, usage, false);
if (ret == 0)
return -EKEYREJECTED;
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 5042cc54fa5e..66737bfb26de 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -263,13 +263,13 @@ int verify_pkcs7_message_sig(const void *data, size_t len,
goto error;
}
- ret = is_key_on_revocation_list(pkcs7);
+ ret = is_key_on_revocation_list(pkcs7, usage);
if (ret != -ENOKEY) {
pr_devel("PKCS#7 platform key is on revocation list\n");
goto error;
}
}
- ret = pkcs7_validate_trust(pkcs7, trusted_keys);
+ ret = pkcs7_validate_trust(pkcs7, trusted_keys, usage, true);
if (ret < 0) {
if (ret == -ENOKEY)
pr_devel("PKCS#7 signature not signed with a trusted key\n");
diff --git a/crypto/asymmetric_keys/Kconfig b/crypto/asymmetric_keys/Kconfig
index 3df3fe4ed95f..189536bd0f9a 100644
--- a/crypto/asymmetric_keys/Kconfig
+++ b/crypto/asymmetric_keys/Kconfig
@@ -85,4 +85,13 @@ config FIPS_SIGNATURE_SELFTEST
depends on ASYMMETRIC_KEY_TYPE
depends on PKCS7_MESSAGE_PARSER
+config CHECK_CODESIGN_EKU
+ bool "Check codeSigning extended key usage"
+ depends on PKCS7_MESSAGE_PARSER=y
+ depends on SYSTEM_DATA_VERIFICATION
+ help
+ This option provides support for checking the codeSigning extended
+ key usage when verifying the signature in PKCS#7. It affects kernel
+ module verification and kexec PE binary verification.
+
endif # ASYMMETRIC_KEY_TYPE
diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
index 9a87c34ed173..087d3761d9a8 100644
--- a/crypto/asymmetric_keys/pkcs7_trust.c
+++ b/crypto/asymmetric_keys/pkcs7_trust.c
@@ -16,12 +16,40 @@
#include <crypto/public_key.h>
#include "pkcs7_parser.h"
+#ifdef CONFIG_CHECK_CODESIGN_EKU
+static bool check_eku_by_usage(struct key *key, enum key_being_used_for usage)
+{
+ struct public_key *public_key = key->payload.data[asym_crypto];
+ bool ret = true;
+
+ switch (usage) {
+ case VERIFYING_MODULE_SIGNATURE:
+ case VERIFYING_KEXEC_PE_SIGNATURE:
+ ret = !!(public_key->ext_key_usage & EKU_codeSigning);
+ if (!ret)
+ pr_warn("The signer '%s' key is not CodeSigning\n",
+ key->description);
+ break;
+ default:
+ break;
+ }
+ return ret;
+}
+#else
+static bool check_eku_by_usage(struct key *key, enum key_being_used_for usage)
+{
+ return true;
+}
+#endif
+
/*
* Check the trust on one PKCS#7 SignedInfo block.
*/
static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
struct pkcs7_signed_info *sinfo,
- struct key *trust_keyring)
+ struct key *trust_keyring,
+ enum key_being_used_for usage,
+ bool check_eku)
{
struct public_key_signature *sig = sinfo->sig;
struct x509_certificate *x509, *last = NULL, *p;
@@ -112,6 +140,10 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
return -ENOKEY;
matched:
+ if (check_eku && !check_eku_by_usage(key, usage)) {
+ key_put(key);
+ return -ENOKEY;
+ }
ret = verify_signature(key, sig);
key_put(key);
if (ret < 0) {
@@ -135,6 +167,8 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
* pkcs7_validate_trust - Validate PKCS#7 trust chain
* @pkcs7: The PKCS#7 certificate to validate
* @trust_keyring: Signing certificates to use as starting points
+ * @usage: The use to which the key is being put.
+ * @check_eku: Check EKU (Extended Key Usage)
*
* Validate that the certificate chain inside the PKCS#7 message intersects
* keys we already know and trust.
@@ -156,7 +190,9 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
* May also return -ENOMEM.
*/
int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
- struct key *trust_keyring)
+ struct key *trust_keyring,
+ enum key_being_used_for usage,
+ bool check_eku)
{
struct pkcs7_signed_info *sinfo;
struct x509_certificate *p;
@@ -167,7 +203,8 @@ int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
p->seen = false;
for (sinfo = pkcs7->signed_infos; sinfo; sinfo = sinfo->next) {
- ret = pkcs7_validate_trust_one(pkcs7, sinfo, trust_keyring);
+ ret = pkcs7_validate_trust_one(pkcs7, sinfo, trust_keyring,
+ usage, check_eku);
switch (ret) {
case -ENOKEY:
continue;
diff --git a/crypto/asymmetric_keys/selftest.c b/crypto/asymmetric_keys/selftest.c
index fa0bf7f24284..756e9f224d8a 100644
--- a/crypto/asymmetric_keys/selftest.c
+++ b/crypto/asymmetric_keys/selftest.c
@@ -212,7 +212,7 @@ int __init fips_signature_selftest(void)
if (ret < 0)
panic("Certs selftest %d: pkcs7_verify() = %d\n", i, ret);
- ret = pkcs7_validate_trust(pkcs7, keyring);
+ ret = pkcs7_validate_trust(pkcs7, keyring, VERIFYING_MODULE_SIGNATURE, false);
if (ret < 0)
panic("Certs selftest %d: pkcs7_validate_trust() = %d\n", i, ret);
diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
index 38ec7f5f9041..5d87b8a02f79 100644
--- a/include/crypto/pkcs7.h
+++ b/include/crypto/pkcs7.h
@@ -30,7 +30,9 @@ extern int pkcs7_get_content_data(const struct pkcs7_message *pkcs7,
* pkcs7_trust.c
*/
extern int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
- struct key *trust_keyring);
+ struct key *trust_keyring,
+ enum key_being_used_for usage,
+ bool check_eku);
/*
* pkcs7_verify.c
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index 91e080efb918..bb33b527240e 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -9,6 +9,7 @@
#define _KEYS_SYSTEM_KEYRING_H
#include <linux/key.h>
+#include <keys/asymmetric-type.h>
enum blacklist_hash_type {
/* TBSCertificate hash */
@@ -81,13 +82,15 @@ static inline int is_binary_blacklisted(const u8 *hash, size_t hash_len)
#ifdef CONFIG_SYSTEM_REVOCATION_LIST
extern int add_key_to_revocation_list(const char *data, size_t size);
-extern int is_key_on_revocation_list(struct pkcs7_message *pkcs7);
+extern int is_key_on_revocation_list(struct pkcs7_message *pkcs7,
+ enum key_being_used_for usage);
#else
static inline int add_key_to_revocation_list(const char *data, size_t size)
{
return 0;
}
-static inline int is_key_on_revocation_list(struct pkcs7_message *pkcs7)
+static inline int is_key_on_revocation_list(struct pkcs7_message *pkcs7,
+ enum key_being_used_for usage)
{
return -ENOKEY;
}
--
2.26.2
Add codeSigning EKU to the default X.509 key generation config for the
build time autogenerated kernel key.
Signed-off-by: "Lee, Chun-Yi" <[email protected]>
---
certs/default_x509.genkey | 1 +
1 file changed, 1 insertion(+)
diff --git a/certs/default_x509.genkey b/certs/default_x509.genkey
index d4c6628cb8e5..53be501ce57a 100644
--- a/certs/default_x509.genkey
+++ b/certs/default_x509.genkey
@@ -15,3 +15,4 @@ basicConstraints=critical,CA:FALSE
keyUsage=digitalSignature
subjectKeyIdentifier=hash
authorityKeyIdentifier=keyid
+extendedKeyUsage=codeSigning
--
2.26.2
Add an openssl command option example for generating CodeSign extended
key usage in X.509 when CONFIG_CHECK_CODESIGN_EKU is enabled.
Signed-off-by: "Lee, Chun-Yi" <[email protected]>
---
Documentation/admin-guide/module-signing.rst | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/admin-guide/module-signing.rst b/Documentation/admin-guide/module-signing.rst
index 7d7c7c8a545c..ca3b8f19466c 100644
--- a/Documentation/admin-guide/module-signing.rst
+++ b/Documentation/admin-guide/module-signing.rst
@@ -170,6 +170,12 @@ generate the public/private key files::
-config x509.genkey -outform PEM -out kernel_key.pem \
-keyout kernel_key.pem
+When ``CONFIG_CHECK_CODESIGN_EKU`` option is enabled, the following openssl
+command option should be added where for generating CodeSign extended key usage
+in X.509::
+
+ -addext "extendedKeyUsage=codeSigning"
+
The full pathname for the resulting kernel_key.pem file can then be specified
in the ``CONFIG_MODULE_SIG_KEY`` option, and the certificate and key therein will
be used instead of an autogenerated keypair.
--
2.26.2
On Thu, Aug 25, 2022 at 10:23:11PM +0800, Lee, Chun-Yi wrote:
> This patch adds the logic for parsing the CodeSign extended key usage
It's *not* a patch once it is applied.
And isn't the identifier actually "codeSign", not "CodeSign"? Please,
format identifier correctly in order not to cause confusion.
So, how I would rewrite the first sentence, would be:
Add the logic for parsing codeSign extended key usage field, as
described in RFC2459, section "4.2.1.13 Extended key usage
field.
E.g. it took me 15 minutes to review the commit message alone
because I could not remember the RFC number off top of my head.
> extension in X.509. The parsing result will be set to the
> ext_key_usage
> flag which is carried by public key. It can be used in the PKCS#7
> verification.
>
> Signed-off-by: "Lee, Chun-Yi" <[email protected]>
> ---
> crypto/asymmetric_keys/x509_cert_parser.c | 25 +++++++++++++++++++++++
> include/crypto/public_key.h | 1 +
> include/linux/oid_registry.h | 5 +++++
> 3 files changed, 31 insertions(+)
>
> diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
> index 2899ed80bb18..1f67e0adef65 100644
> --- a/crypto/asymmetric_keys/x509_cert_parser.c
> +++ b/crypto/asymmetric_keys/x509_cert_parser.c
> @@ -554,6 +554,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;
I'd reorder the declarations (suggestion).
>
> pr_debug("Extension: %u\n", ctx->last_oid);
>
> @@ -583,6 +585,29 @@ int x509_process_extension(void *context, size_t hdrlen,
> return 0;
> }
>
> + if (ctx->last_oid == OID_extKeyUsage) {
> + if (vlen < 2 ||
> + 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 ((i + 10) > vlen || v[i] != ASN1_OID || v[i + 1] != 8)
> + return -EBADMSG;
> +
> + oid = look_up_OID(v + i + 2, v[i + 1]);
> + if (oid == OID_codeSigning) {
> + ctx->cert->pub->ext_key_usage |= EKU_codeSigning;
> + }
> + i += 10;
> + }
> + pr_debug("extKeyUsage: %d\n", ctx->cert->pub->ext_key_usage);
> + return 0;
> + }
> +
> return 0;
> }
>
> diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> index 68f7aa2a7e55..72c0fcc39d0f 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 ext_key_usage : 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 0f4a8903922a..460135c2d918 100644
> --- a/include/linux/oid_registry.h
> +++ b/include/linux/oid_registry.h
> @@ -140,9 +140,14 @@ enum OID {
> OID_TPMImportableKey, /* 2.23.133.10.1.4 */
> OID_TPMSealedData, /* 2.23.133.10.1.5 */
>
> + /* 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 parse_OID(const void *data, size_t datasize, enum OID *oid);
> extern int sprint_oid(const void *, size_t, char *, size_t);
> --
> 2.26.2
>
BR, Jarkko
On Thu, Aug 25, 2022 at 10:23:10PM +0800, Lee, Chun-Yi wrote:
> NIAP PP_OS certification requests that OS need to 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.
Might be cutting hairs here but you don't really explain
why we want to support it. It's not a counter argument
to add the feature. It's a counter argument against adding
undocumented features.
BR, Jarkko
On Thu, Aug 25, 2022 at 10:23:14PM +0800, Lee, Chun-Yi wrote:
> Add an openssl command option example for generating CodeSign extended
> key usage in X.509 when CONFIG_CHECK_CODESIGN_EKU is enabled.
>
> Signed-off-by: "Lee, Chun-Yi" <[email protected]>
> ---
> Documentation/admin-guide/module-signing.rst | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/admin-guide/module-signing.rst b/Documentation/admin-guide/module-signing.rst
> index 7d7c7c8a545c..ca3b8f19466c 100644
> --- a/Documentation/admin-guide/module-signing.rst
> +++ b/Documentation/admin-guide/module-signing.rst
> @@ -170,6 +170,12 @@ generate the public/private key files::
> -config x509.genkey -outform PEM -out kernel_key.pem \
> -keyout kernel_key.pem
>
> +When ``CONFIG_CHECK_CODESIGN_EKU`` option is enabled, the following openssl
> +command option should be added where for generating CodeSign extended key usage
You have:
1. codeSign
2. CodeSign
3. CodeSigning
Why this ambiguity?
> +in X.509::
> +
> + -addext "extendedKeyUsage=codeSigning"
> +
> The full pathname for the resulting kernel_key.pem file can then be specified
> in the ``CONFIG_MODULE_SIG_KEY`` option, and the certificate and key therein will
> be used instead of an autogenerated keypair.
> --
> 2.26.2
>
BR, Jarkko
On Thu, Aug 25, 2022 at 10:23:12PM +0800, Lee, Chun-Yi wrote:
> This patch adds the logic for checking the CodeSigning extended
> key usage when verifying signature of kernel module or
> kexec PE binary in PKCS#7.
Pretty much the same feedback as for 1/4.
>
> Signed-off-by: "Lee, Chun-Yi" <[email protected]>
> ---
> certs/blacklist.c | 5 ++--
> certs/system_keyring.c | 4 +--
> crypto/asymmetric_keys/Kconfig | 9 ++++++
> crypto/asymmetric_keys/pkcs7_trust.c | 43 ++++++++++++++++++++++++++--
> crypto/asymmetric_keys/selftest.c | 2 +-
> include/crypto/pkcs7.h | 4 ++-
> include/keys/system_keyring.h | 7 +++--
> 7 files changed, 63 insertions(+), 11 deletions(-)
>
> diff --git a/certs/blacklist.c b/certs/blacklist.c
> index 41f10601cc72..fa41454055be 100644
> --- a/certs/blacklist.c
> +++ b/certs/blacklist.c
> @@ -282,11 +282,12 @@ int add_key_to_revocation_list(const char *data, size_t size)
> * is_key_on_revocation_list - Determine if the key for a PKCS#7 message is revoked
> * @pkcs7: The PKCS#7 message to check
> */
> -int is_key_on_revocation_list(struct pkcs7_message *pkcs7)
> +int is_key_on_revocation_list(struct pkcs7_message *pkcs7,
> + enum key_being_used_for usage)
> {
> int ret;
>
> - ret = pkcs7_validate_trust(pkcs7, blacklist_keyring);
> + ret = pkcs7_validate_trust(pkcs7, blacklist_keyring, usage, false);
>
> if (ret == 0)
> return -EKEYREJECTED;
> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> index 5042cc54fa5e..66737bfb26de 100644
> --- a/certs/system_keyring.c
> +++ b/certs/system_keyring.c
> @@ -263,13 +263,13 @@ int verify_pkcs7_message_sig(const void *data, size_t len,
> goto error;
> }
>
> - ret = is_key_on_revocation_list(pkcs7);
> + ret = is_key_on_revocation_list(pkcs7, usage);
> if (ret != -ENOKEY) {
> pr_devel("PKCS#7 platform key is on revocation list\n");
> goto error;
> }
> }
> - ret = pkcs7_validate_trust(pkcs7, trusted_keys);
> + ret = pkcs7_validate_trust(pkcs7, trusted_keys, usage, true);
> if (ret < 0) {
> if (ret == -ENOKEY)
> pr_devel("PKCS#7 signature not signed with a trusted key\n");
> diff --git a/crypto/asymmetric_keys/Kconfig b/crypto/asymmetric_keys/Kconfig
> index 3df3fe4ed95f..189536bd0f9a 100644
> --- a/crypto/asymmetric_keys/Kconfig
> +++ b/crypto/asymmetric_keys/Kconfig
> @@ -85,4 +85,13 @@ config FIPS_SIGNATURE_SELFTEST
> depends on ASYMMETRIC_KEY_TYPE
> depends on PKCS7_MESSAGE_PARSER
>
> +config CHECK_CODESIGN_EKU
> + bool "Check codeSigning extended key usage"
> + depends on PKCS7_MESSAGE_PARSER=y
> + depends on SYSTEM_DATA_VERIFICATION
> + help
> + This option provides support for checking the codeSigning extended
> + key usage when verifying the signature in PKCS#7. It affects kernel
> + module verification and kexec PE binary verification.
> +
> endif # ASYMMETRIC_KEY_TYPE
> diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
> index 9a87c34ed173..087d3761d9a8 100644
> --- a/crypto/asymmetric_keys/pkcs7_trust.c
> +++ b/crypto/asymmetric_keys/pkcs7_trust.c
> @@ -16,12 +16,40 @@
> #include <crypto/public_key.h>
> #include "pkcs7_parser.h"
>
> +#ifdef CONFIG_CHECK_CODESIGN_EKU
> +static bool check_eku_by_usage(struct key *key, enum key_being_used_for usage)
> +{
> + struct public_key *public_key = key->payload.data[asym_crypto];
> + bool ret = true;
> +
> + switch (usage) {
> + case VERIFYING_MODULE_SIGNATURE:
> + case VERIFYING_KEXEC_PE_SIGNATURE:
> + ret = !!(public_key->ext_key_usage & EKU_codeSigning);
> + if (!ret)
> + pr_warn("The signer '%s' key is not CodeSigning\n",
> + key->description);
> + break;
> + default:
> + break;
> + }
> + return ret;
> +}
> +#else
> +static bool check_eku_by_usage(struct key *key, enum key_being_used_for usage)
> +{
> + return true;
> +}
> +#endif
> +
> /*
> * Check the trust on one PKCS#7 SignedInfo block.
> */
> static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
> struct pkcs7_signed_info *sinfo,
> - struct key *trust_keyring)
> + struct key *trust_keyring,
> + enum key_being_used_for usage,
> + bool check_eku)
> {
> struct public_key_signature *sig = sinfo->sig;
> struct x509_certificate *x509, *last = NULL, *p;
> @@ -112,6 +140,10 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
> return -ENOKEY;
>
> matched:
> + if (check_eku && !check_eku_by_usage(key, usage)) {
> + key_put(key);
> + return -ENOKEY;
> + }
Why you don't just open code this to those rare call
sites where it is needed?
I counted that there is exactly one call site.
> ret = verify_signature(key, sig);
> key_put(key);
> if (ret < 0) {
> @@ -135,6 +167,8 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
> * pkcs7_validate_trust - Validate PKCS#7 trust chain
> * @pkcs7: The PKCS#7 certificate to validate
> * @trust_keyring: Signing certificates to use as starting points
> + * @usage: The use to which the key is being put.
> + * @check_eku: Check EKU (Extended Key Usage)
> *
> * Validate that the certificate chain inside the PKCS#7 message intersects
> * keys we already know and trust.
> @@ -156,7 +190,9 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
> * May also return -ENOMEM.
> */
> int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
> - struct key *trust_keyring)
> + struct key *trust_keyring,
> + enum key_being_used_for usage,
> + bool check_eku)
> {
> struct pkcs7_signed_info *sinfo;
> struct x509_certificate *p;
> @@ -167,7 +203,8 @@ int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
> p->seen = false;
>
> for (sinfo = pkcs7->signed_infos; sinfo; sinfo = sinfo->next) {
> - ret = pkcs7_validate_trust_one(pkcs7, sinfo, trust_keyring);
> + ret = pkcs7_validate_trust_one(pkcs7, sinfo, trust_keyring,
> + usage, check_eku);
> switch (ret) {
> case -ENOKEY:
> continue;
> diff --git a/crypto/asymmetric_keys/selftest.c b/crypto/asymmetric_keys/selftest.c
> index fa0bf7f24284..756e9f224d8a 100644
> --- a/crypto/asymmetric_keys/selftest.c
> +++ b/crypto/asymmetric_keys/selftest.c
> @@ -212,7 +212,7 @@ int __init fips_signature_selftest(void)
> if (ret < 0)
> panic("Certs selftest %d: pkcs7_verify() = %d\n", i, ret);
>
> - ret = pkcs7_validate_trust(pkcs7, keyring);
> + ret = pkcs7_validate_trust(pkcs7, keyring, VERIFYING_MODULE_SIGNATURE, false);
> if (ret < 0)
> panic("Certs selftest %d: pkcs7_validate_trust() = %d\n", i, ret);
>
> diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
> index 38ec7f5f9041..5d87b8a02f79 100644
> --- a/include/crypto/pkcs7.h
> +++ b/include/crypto/pkcs7.h
> @@ -30,7 +30,9 @@ extern int pkcs7_get_content_data(const struct pkcs7_message *pkcs7,
> * pkcs7_trust.c
> */
> extern int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
> - struct key *trust_keyring);
> + struct key *trust_keyring,
> + enum key_being_used_for usage,
> + bool check_eku);
>
> /*
> * pkcs7_verify.c
> diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
> index 91e080efb918..bb33b527240e 100644
> --- a/include/keys/system_keyring.h
> +++ b/include/keys/system_keyring.h
> @@ -9,6 +9,7 @@
> #define _KEYS_SYSTEM_KEYRING_H
>
> #include <linux/key.h>
> +#include <keys/asymmetric-type.h>
>
> enum blacklist_hash_type {
> /* TBSCertificate hash */
> @@ -81,13 +82,15 @@ static inline int is_binary_blacklisted(const u8 *hash, size_t hash_len)
>
> #ifdef CONFIG_SYSTEM_REVOCATION_LIST
> extern int add_key_to_revocation_list(const char *data, size_t size);
> -extern int is_key_on_revocation_list(struct pkcs7_message *pkcs7);
> +extern int is_key_on_revocation_list(struct pkcs7_message *pkcs7,
> + enum key_being_used_for usage);
> #else
> static inline int add_key_to_revocation_list(const char *data, size_t size)
> {
> return 0;
> }
> -static inline int is_key_on_revocation_list(struct pkcs7_message *pkcs7)
> +static inline int is_key_on_revocation_list(struct pkcs7_message *pkcs7,
> + enum key_being_used_for usage)
> {
> return -ENOKEY;
> }
> --
> 2.26.2
>
BR, Jarkko
Hi Jarkko,
Thanks for your review!
On Fri, Aug 26, 2022 at 11:23:00PM +0300, Jarkko Sakkinen wrote:
> On Thu, Aug 25, 2022 at 10:23:11PM +0800, Lee, Chun-Yi wrote:
> > This patch adds the logic for parsing the CodeSign extended key usage
>
> It's *not* a patch once it is applied.
>
> And isn't the identifier actually "codeSign", not "CodeSign"? Please,
> format identifier correctly in order not to cause confusion.
>
> So, how I would rewrite the first sentence, would be:
>
> Add the logic for parsing codeSign extended key usage field, as
> described in RFC2459, section "4.2.1.13 Extended key usage
> field.
>
> E.g. it took me 15 minutes to review the commit message alone
> because I could not remember the RFC number off top of my head.
>
Thanks for your suggestion. As your rewrited sentence, Adding RFC2459
and section is better for reader of patch descriptor.
I will change the patch descriptor.
> > extension in X.509. The parsing result will be set to the
> > ext_key_usage
> > flag which is carried by public key. It can be used in the PKCS#7
> > verification.
> >
> > Signed-off-by: "Lee, Chun-Yi" <[email protected]>
> > ---
> > crypto/asymmetric_keys/x509_cert_parser.c | 25 +++++++++++++++++++++++
> > include/crypto/public_key.h | 1 +
> > include/linux/oid_registry.h | 5 +++++
> > 3 files changed, 31 insertions(+)
> >
> > diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
> > index 2899ed80bb18..1f67e0adef65 100644
> > --- a/crypto/asymmetric_keys/x509_cert_parser.c
> > +++ b/crypto/asymmetric_keys/x509_cert_parser.c
> > @@ -554,6 +554,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;
>
> I'd reorder the declarations (suggestion).
>
I will change the order.
Thanks a lot!
Joey Lee
Hi Jarkko,
On Sun, Aug 28, 2022 at 06:34:26AM +0300, Jarkko Sakkinen wrote:
> On Thu, Aug 25, 2022 at 10:23:12PM +0800, Lee, Chun-Yi wrote:
> > This patch adds the logic for checking the CodeSigning extended
> > key usage when verifying signature of kernel module or
> > kexec PE binary in PKCS#7.
>
> Pretty much the same feedback as for 1/4.
>
I will also change this description.
Thanks
Joey Lee
> >
> > Signed-off-by: "Lee, Chun-Yi" <[email protected]>
> > ---
> > certs/blacklist.c | 5 ++--
> > certs/system_keyring.c | 4 +--
> > crypto/asymmetric_keys/Kconfig | 9 ++++++
> > crypto/asymmetric_keys/pkcs7_trust.c | 43 ++++++++++++++++++++++++++--
> > crypto/asymmetric_keys/selftest.c | 2 +-
> > include/crypto/pkcs7.h | 4 ++-
> > include/keys/system_keyring.h | 7 +++--
> > 7 files changed, 63 insertions(+), 11 deletions(-)
> >
> > diff --git a/certs/blacklist.c b/certs/blacklist.c
> > index 41f10601cc72..fa41454055be 100644
> > --- a/certs/blacklist.c
> > +++ b/certs/blacklist.c
> > @@ -282,11 +282,12 @@ int add_key_to_revocation_list(const char *data, size_t size)
> > * is_key_on_revocation_list - Determine if the key for a PKCS#7 message is revoked
> > * @pkcs7: The PKCS#7 message to check
> > */
> > -int is_key_on_revocation_list(struct pkcs7_message *pkcs7)
> > +int is_key_on_revocation_list(struct pkcs7_message *pkcs7,
> > + enum key_being_used_for usage)
> > {
> > int ret;
> >
> > - ret = pkcs7_validate_trust(pkcs7, blacklist_keyring);
> > + ret = pkcs7_validate_trust(pkcs7, blacklist_keyring, usage, false);
> >
> > if (ret == 0)
> > return -EKEYREJECTED;
> > diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> > index 5042cc54fa5e..66737bfb26de 100644
> > --- a/certs/system_keyring.c
> > +++ b/certs/system_keyring.c
> > @@ -263,13 +263,13 @@ int verify_pkcs7_message_sig(const void *data, size_t len,
> > goto error;
> > }
> >
> > - ret = is_key_on_revocation_list(pkcs7);
> > + ret = is_key_on_revocation_list(pkcs7, usage);
> > if (ret != -ENOKEY) {
> > pr_devel("PKCS#7 platform key is on revocation list\n");
> > goto error;
> > }
> > }
> > - ret = pkcs7_validate_trust(pkcs7, trusted_keys);
> > + ret = pkcs7_validate_trust(pkcs7, trusted_keys, usage, true);
> > if (ret < 0) {
> > if (ret == -ENOKEY)
> > pr_devel("PKCS#7 signature not signed with a trusted key\n");
> > diff --git a/crypto/asymmetric_keys/Kconfig b/crypto/asymmetric_keys/Kconfig
> > index 3df3fe4ed95f..189536bd0f9a 100644
> > --- a/crypto/asymmetric_keys/Kconfig
> > +++ b/crypto/asymmetric_keys/Kconfig
> > @@ -85,4 +85,13 @@ config FIPS_SIGNATURE_SELFTEST
> > depends on ASYMMETRIC_KEY_TYPE
> > depends on PKCS7_MESSAGE_PARSER
> >
> > +config CHECK_CODESIGN_EKU
> > + bool "Check codeSigning extended key usage"
> > + depends on PKCS7_MESSAGE_PARSER=y
> > + depends on SYSTEM_DATA_VERIFICATION
> > + help
> > + This option provides support for checking the codeSigning extended
> > + key usage when verifying the signature in PKCS#7. It affects kernel
> > + module verification and kexec PE binary verification.
> > +
> > endif # ASYMMETRIC_KEY_TYPE
> > diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
> > index 9a87c34ed173..087d3761d9a8 100644
> > --- a/crypto/asymmetric_keys/pkcs7_trust.c
> > +++ b/crypto/asymmetric_keys/pkcs7_trust.c
> > @@ -16,12 +16,40 @@
> > #include <crypto/public_key.h>
> > #include "pkcs7_parser.h"
> >
> > +#ifdef CONFIG_CHECK_CODESIGN_EKU
> > +static bool check_eku_by_usage(struct key *key, enum key_being_used_for usage)
> > +{
> > + struct public_key *public_key = key->payload.data[asym_crypto];
> > + bool ret = true;
> > +
> > + switch (usage) {
> > + case VERIFYING_MODULE_SIGNATURE:
> > + case VERIFYING_KEXEC_PE_SIGNATURE:
> > + ret = !!(public_key->ext_key_usage & EKU_codeSigning);
> > + if (!ret)
> > + pr_warn("The signer '%s' key is not CodeSigning\n",
> > + key->description);
> > + break;
> > + default:
> > + break;
> > + }
> > + return ret;
> > +}
> > +#else
> > +static bool check_eku_by_usage(struct key *key, enum key_being_used_for usage)
> > +{
> > + return true;
> > +}
> > +#endif
> > +
> > /*
> > * Check the trust on one PKCS#7 SignedInfo block.
> > */
> > static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
> > struct pkcs7_signed_info *sinfo,
> > - struct key *trust_keyring)
> > + struct key *trust_keyring,
> > + enum key_being_used_for usage,
> > + bool check_eku)
> > {
> > struct public_key_signature *sig = sinfo->sig;
> > struct x509_certificate *x509, *last = NULL, *p;
> > @@ -112,6 +140,10 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
> > return -ENOKEY;
> >
> > matched:
> > + if (check_eku && !check_eku_by_usage(key, usage)) {
> > + key_put(key);
> > + return -ENOKEY;
> > + }
>
> Why you don't just open code this to those rare call
> sites where it is needed?
>
> I counted that there is exactly one call site.
>
> > ret = verify_signature(key, sig);
> > key_put(key);
> > if (ret < 0) {
> > @@ -135,6 +167,8 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
> > * pkcs7_validate_trust - Validate PKCS#7 trust chain
> > * @pkcs7: The PKCS#7 certificate to validate
> > * @trust_keyring: Signing certificates to use as starting points
> > + * @usage: The use to which the key is being put.
> > + * @check_eku: Check EKU (Extended Key Usage)
> > *
> > * Validate that the certificate chain inside the PKCS#7 message intersects
> > * keys we already know and trust.
> > @@ -156,7 +190,9 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
> > * May also return -ENOMEM.
> > */
> > int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
> > - struct key *trust_keyring)
> > + struct key *trust_keyring,
> > + enum key_being_used_for usage,
> > + bool check_eku)
> > {
> > struct pkcs7_signed_info *sinfo;
> > struct x509_certificate *p;
> > @@ -167,7 +203,8 @@ int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
> > p->seen = false;
> >
> > for (sinfo = pkcs7->signed_infos; sinfo; sinfo = sinfo->next) {
> > - ret = pkcs7_validate_trust_one(pkcs7, sinfo, trust_keyring);
> > + ret = pkcs7_validate_trust_one(pkcs7, sinfo, trust_keyring,
> > + usage, check_eku);
> > switch (ret) {
> > case -ENOKEY:
> > continue;
> > diff --git a/crypto/asymmetric_keys/selftest.c b/crypto/asymmetric_keys/selftest.c
> > index fa0bf7f24284..756e9f224d8a 100644
> > --- a/crypto/asymmetric_keys/selftest.c
> > +++ b/crypto/asymmetric_keys/selftest.c
> > @@ -212,7 +212,7 @@ int __init fips_signature_selftest(void)
> > if (ret < 0)
> > panic("Certs selftest %d: pkcs7_verify() = %d\n", i, ret);
> >
> > - ret = pkcs7_validate_trust(pkcs7, keyring);
> > + ret = pkcs7_validate_trust(pkcs7, keyring, VERIFYING_MODULE_SIGNATURE, false);
> > if (ret < 0)
> > panic("Certs selftest %d: pkcs7_validate_trust() = %d\n", i, ret);
> >
> > diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
> > index 38ec7f5f9041..5d87b8a02f79 100644
> > --- a/include/crypto/pkcs7.h
> > +++ b/include/crypto/pkcs7.h
> > @@ -30,7 +30,9 @@ extern int pkcs7_get_content_data(const struct pkcs7_message *pkcs7,
> > * pkcs7_trust.c
> > */
> > extern int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
> > - struct key *trust_keyring);
> > + struct key *trust_keyring,
> > + enum key_being_used_for usage,
> > + bool check_eku);
> >
> > /*
> > * pkcs7_verify.c
> > diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
> > index 91e080efb918..bb33b527240e 100644
> > --- a/include/keys/system_keyring.h
> > +++ b/include/keys/system_keyring.h
> > @@ -9,6 +9,7 @@
> > #define _KEYS_SYSTEM_KEYRING_H
> >
> > #include <linux/key.h>
> > +#include <keys/asymmetric-type.h>
> >
> > enum blacklist_hash_type {
> > /* TBSCertificate hash */
> > @@ -81,13 +82,15 @@ static inline int is_binary_blacklisted(const u8 *hash, size_t hash_len)
> >
> > #ifdef CONFIG_SYSTEM_REVOCATION_LIST
> > extern int add_key_to_revocation_list(const char *data, size_t size);
> > -extern int is_key_on_revocation_list(struct pkcs7_message *pkcs7);
> > +extern int is_key_on_revocation_list(struct pkcs7_message *pkcs7,
> > + enum key_being_used_for usage);
> > #else
> > static inline int add_key_to_revocation_list(const char *data, size_t size)
> > {
> > return 0;
> > }
> > -static inline int is_key_on_revocation_list(struct pkcs7_message *pkcs7)
> > +static inline int is_key_on_revocation_list(struct pkcs7_message *pkcs7,
> > + enum key_being_used_for usage)
> > {
> > return -ENOKEY;
> > }
> > --
> > 2.26.2
> >
>
> BR, Jarkko
On Sun, Aug 28, 2022 at 06:35:24AM +0300, Jarkko Sakkinen wrote:
> On Thu, Aug 25, 2022 at 10:23:14PM +0800, Lee, Chun-Yi wrote:
> > Add an openssl command option example for generating CodeSign extended
> > key usage in X.509 when CONFIG_CHECK_CODESIGN_EKU is enabled.
> >
> > Signed-off-by: "Lee, Chun-Yi" <[email protected]>
> > ---
> > Documentation/admin-guide/module-signing.rst | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/admin-guide/module-signing.rst b/Documentation/admin-guide/module-signing.rst
> > index 7d7c7c8a545c..ca3b8f19466c 100644
> > --- a/Documentation/admin-guide/module-signing.rst
> > +++ b/Documentation/admin-guide/module-signing.rst
> > @@ -170,6 +170,12 @@ generate the public/private key files::
> > -config x509.genkey -outform PEM -out kernel_key.pem \
> > -keyout kernel_key.pem
> >
> > +When ``CONFIG_CHECK_CODESIGN_EKU`` option is enabled, the following openssl
> > +command option should be added where for generating CodeSign extended key usage
>
> You have:
>
> 1. codeSign
> 2. CodeSign
> 3. CodeSigning
>
> Why this ambiguity?
>
Sorry for the ambiguity. I will use codeSigning in document and patch
description to sync with RFC2459 and openssl.
Thanks a lot!
Joey Lee
> > +in X.509::
> > +
> > + -addext "extendedKeyUsage=codeSigning"
> > +
> > The full pathname for the resulting kernel_key.pem file can then be specified
> > in the ``CONFIG_MODULE_SIG_KEY`` option, and the certificate and key therein will
> > be used instead of an autogenerated keypair.
> > --
> > 2.26.2
> >
>
> BR, Jarkko
Hi Jarkko,
On Sun, Aug 28, 2022 at 06:30:23AM +0300, Jarkko Sakkinen wrote:
> On Thu, Aug 25, 2022 at 10:23:10PM +0800, Lee, Chun-Yi wrote:
> > NIAP PP_OS certification requests that OS need to 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.
>
> Might be cutting hairs here but you don't really explain
> why we want to support it. It's not a counter argument
> to add the feature. It's a counter argument against adding
> undocumented features.
>
In some cases, a organization may publish different certificates for
difference purposes. When a certificate for a specific purpose is
leaked, it will not affect other certificates.
The function for using a code signing certificate to verify kernel
binary or module can restrict the purpose of the certificate to avoid
attacker uses other leaked non-codeSigning certificate for signing.
Thanks a lot!
Joey Lee