[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1030200: linux-image-6.1.0-3-amd64: .platform keyring (EFI DB variable) no longer trusted to sign modules, regression against 6.0



Control: retitle -1 linux-image-6.1.0-3-amd64: .platform keyring (EFI DB variable) no longer trusted to sign modules, regression against 6.0
Control: tags -1 + patch

In many ways I went wrong by limiting my search to the linux checkout,
and not the source package.

Upstream kernel/module/signing.c#mod_verify_sig() ends with:
          return verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
                                        VERIFY_USE_SECONDARY_KEYRING,
                                        VERIFYING_MODULE_SIGNATURE,
                                        NULL, NULL);
  }

In 6.1.20-1 it ends with:
          ret = verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
                                        VERIFY_USE_SECONDARY_KEYRING,
                                        VERIFYING_MODULE_SIGNATURE,
                                        NULL, NULL);
          pr_devel("verify_pkcs7_signature() = %d\n", ret);
  
          /* checking hash of module is in blacklist */
          if (!ret)
                  ret = mod_is_hash_blacklisted(mod, wholelen);
  
          return ret;
  }

But in 6.0.12-1 it ended with (excuse patch format):
  @@ -116,6 +116,13 @@ int mod_verify_sig(const void *mod, stru
                                        VERIFYING_MODULE_SIGNATURE,
                                        NULL, NULL);
          pr_devel("verify_pkcs7_signature() = %d\n", ret);
  +       if (ret == -ENOKEY && IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING)) {
  +               ret = verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
  +                               VERIFY_USE_PLATFORM_KEYRING,
  +                               VERIFYING_MODULE_SIGNATURE,
  +                               NULL, NULL);
  +                pr_devel("verify_pkcs7_signature() = %d\n", ret);
  +       }
  
          /* checking hash of module is in blacklist */
          if (!ret)

So in 6.0.x, debian carried a patch
(features/all/db-mok-keyring/KEYS-Make-use-of-platform-keyring-for-module-signature.patch),
which was dropped in
  commit 9aa15a22634335db058256092572fd2725ca674b
  Author: Luca Boccassi <bluca@debian.org>
  Date:   Thu Oct 13 23:23:28 2022 +0100
  
      Drop obsolete MODSIGN patches
  
      Upstream loads keys from MoK, no need for the out-of-tree patches anymore
and that commit is included in all debian/6.1* tags on salsa.

On Fri, Mar 31, 2023 at 01:45:08AM +0200, наб wrote:
>   keys-6.0:19c0bfbd I------     1 perm 1f0f0000     0     0 keyring   .secondary_trusted_keys: 1
>   keys-6.1:29a13614 I------     1 perm 1f0f0000     0     0 keyring   .secondary_trusted_keys: 2
> 
> Note how there's somehow a second key in .secondary_trusted_keys.
> Whether this means something or is an accounting difference
> is unclear to me at this time.

That patch was added for #935945, which also notes a method of having
keyctl dump keyring contents, which I couldn't figure out.

However, on 6.1 (dumped from the initrd):
  # keyctl list %:.builtin_trusted_keys
  2 keys in keyring:
  672487752: ---lswrv     0     0 asymmetric: Debian Secure Boot CA: 6ccece7e4c6c0d1f6149f3dd27dfcc5cbb419ea1
  874879581: ---lswrv     0     0 asymmetric: Debian Secure Boot Signer 2022 - linux: 14011249c2675ea8e5148542202005810584b25f
  # keyctl list %:.platform
  4 keys in keyring:
  374796544: ---lswrv     0     0 asymmetric: babtop.nabijaczleweli.xyz: 82b7fc21cc3f583ac4a7b05712d95377f41fbdd6
  116765678: ---lswrv     0     0 asymmetric: babtop.nabijaczleweli.xyz SecureBoot DB 2023: 00befacaa0
   20694596: ---lswrv     0     0 asymmetric: babtop.nabijaczleweli.xyz SecureBoot CA: 00befa30fa
  946336801: ---lswrv     0     0 asymmetric: Debian Secure Boot CA: 6ccece7e4c6c0d1f6149f3dd27dfcc5cbb419ea1
  # keyctl list %:.secondary_trusted_keys
  2 keys in keyring:
  624245999: ---lswrv     0     0 keyring: .builtin_trusted_keys
  996684961: ---lswrv     0     0 keyring: .machine
  # keyctl list %:.machine
  keyring is empty

And on 6.0:
  # keyctl list %:.builtin_trusted_keys
  Please touch the device.
  2 keys in keyring:
  417373165: ---lswrv     0     0 asymmetric: Debian Secure Boot CA: 6ccece7e4c6c0d1f6149f3dd27dfcc5cbb419ea1
  726407570: ---lswrv     0     0 asymmetric: Debian Secure Boot Signer 2022 - linux: 14011249c2675ea8e5148542202005810584b25f
  # keyctl list %:.platform
  4 keys in keyring:
  699875236: ---lswrv     0     0 asymmetric: babtop.nabijaczleweli.xyz: 82b7fc21cc3f583ac4a7b05712d95377f41fbdd6
  885300684: ---lswrv     0     0 asymmetric: babtop.nabijaczleweli.xyz SecureBoot DB 2023: 00befacaa0
  927553785: ---lswrv     0     0 asymmetric: babtop.nabijaczleweli.xyz SecureBoot CA: 00befa30fa
  707478853: ---lswrv     0     0 asymmetric: Debian Secure Boot CA: 6ccece7e4c6c0d1f6149f3dd27dfcc5cbb419ea1
  # keyctl list %:.secondary_trusted_keys
  1 key in keyring:
  889210333: ---lswrv     0     0 keyring: .builtin_trusted_keys
  # keyctl list %:.machine
  Can't find 'keyring:.machine'

So, to summarise:
  6.0 upstream trusts the built-in keys,
      and carries a patch to trust the platform
  6.1 upstream trusts the built-in keys and MOK
      (debian just enables trusting MOK always)

See certs/system_keyring.c:
  static __init struct key_restriction *get_builtin_and_secondary_restriction(void)
  {
          struct key_restriction *restriction;
  
          restriction = kzalloc(sizeof(struct key_restriction), GFP_KERNEL);
  
          if (!restriction)
                  panic("Can't allocate secondary trusted keyring restriction\n");
  
          if (IS_ENABLED(CONFIG_INTEGRITY_MACHINE_KEYRING))
                  restriction->check = restrict_link_by_builtin_secondary_and_machine;
          else
                  restriction->check = restrict_link_by_builtin_and_secondary_trusted;
  
          return restriction;
  }
  
  void __init set_machine_trusted_keys(struct key *keyring)
  {
          machine_trusted_keys = keyring;
  
          if (key_link(secondary_trusted_keys, machine_trusted_keys) < 0)
                  panic("Can't link (machine) trusted keyrings\n");
  }
  /**
   * restrict_link_by_builtin_secondary_and_machine - Restrict keyring addition.
   * @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, the secondary, or
   * the machine keyrings.
   */
  int restrict_link_by_builtin_secondary_and_machine(
          struct key *dest_keyring,
          const struct key_type *type,
          const union key_payload *payload,
          struct key *restrict_key)
  {
          if (machine_trusted_keys && type == &key_type_keyring &&
              dest_keyring == secondary_trusted_keys &&
              payload == &machine_trusted_keys->payload)
                  /* Allow the machine keyring to be added to the secondary */
                  return 0;
  
          return restrict_link_by_builtin_and_secondary_trusted(dest_keyring, type,
                                                                payload, restrict_key);
  }
security/integrity/digsig.c:
  static int __init __integrity_init_keyring(const unsigned int id,
                                             key_perm_t perm,
                                             struct key_restriction *restriction)
  {
          const struct cred *cred = current_cred();
          int err = 0;
  
          keyring[id] = keyring_alloc(keyring_name[id], KUIDT_INIT(0),
                                      KGIDT_INIT(0), cred, perm,
                                      KEY_ALLOC_NOT_IN_QUOTA, restriction, NULL);
          if (IS_ERR(keyring[id])) {
                  err = PTR_ERR(keyring[id]);
                  pr_info("Can't allocate %s keyring (%d)\n",
                          keyring_name[id], err);
                  keyring[id] = NULL;
          } else {
                  if (id == INTEGRITY_KEYRING_PLATFORM)
                          set_platform_trusted_keys(keyring[id]);
                  if (id == INTEGRITY_KEYRING_MACHINE && trust_moklist())
                          set_machine_trusted_keys(keyring[id]);
                  if (id == INTEGRITY_KEYRING_IMA)
                          load_module_cert(keyring[id]);
          }
  
          return err;
  }
(features/all/db-mok-keyring/trust-machine-keyring-by-default.patch
 makes trust_moklist() be always true, hence why I see .machine linked
 to secondary even though I don't use MOK).

And compare this with how the non-MOK restriction works:
  /**
   * restrict_link_by_builtin_and_secondary_trusted - Restrict keyring
   *   addition by both builtin and secondary keyrings
   *
   * 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
   * keyrings.
   */
  int restrict_link_by_builtin_and_secondary_trusted(
          struct key *dest_keyring,
          const struct key_type *type,
          const union key_payload *payload,
          struct key *restrict_key)
  {
          /* If we have a secondary trusted keyring, then that contains a link
           * through to the builtin keyring and the search will follow that link.
           */
          if (type == &key_type_keyring &&
              dest_keyring == secondary_trusted_keys &&
              payload == &builtin_trusted_keys->payload)
                  /* Allow the builtin keyring to be added to the secondary */
                  return 0;
  
          return restrict_link_by_signature(dest_keyring, type, payload,
                                            secondary_trusted_keys);
  }

  #ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
  void __init set_platform_trusted_keys(struct key *keyring)
  {
          platform_trusted_keys = keyring;
  }
  #endif

It's relatively trivial to port what's done with the machine keyring
here to the platform keyring, but that'd extend the reach of the
platform keyring more than desired
(DM_VERITY_VERIFY_ROOTHASH_SIG_SECONDARY_KEYRING,
 pkcs7_preparse()).

But most places that use VERIFY_USE_SECONDARY_KEYRING also check for
VERIFY_USE_PLATFORM_KEYRING right after, so dropping that patch is
baffling to me, since it's very much a regression, and it was spelled in
an upstream-endorsed way (indeed, look no further than
kexec_kernel_verify_pe_sig() which is exactly the same).

Attaching, essentially, a re-generated version of the old patch;
I'll build-test and post a Salsa MR tomorrow.

Best,
наб
From: Robert Holmes <robeholmes@gmail.com>
Date: Tue, 23 Apr 2019 07:39:29 +0000
Subject: [PATCH] KEYS: Make use of platform keyring for module signature
 verify
Bug-Debian: https://bugs.debian.org/935945
Bug-Debian: https://bugs.debian.org/1030200
Origin: https://src.fedoraproject.org/rpms/kernel/raw/master/f/KEYS-Make-use-of-platform-keyring-for-module-signature.patch

This patch completes commit 278311e417be ("kexec, KEYS: Make use of
platform keyring for signature verify") which, while adding the
platform keyring for bzImage verification, neglected to also add
this keyring for module verification.

As such, kernel modules signed with keys from the MokList variable
were not successfully verified.

Signed-off-by: Robert Holmes <robeholmes@gmail.com>
Signed-off-by: Jeremy Cline <jcline@redhat.com>
[bwh: Forward-ported to 5.19: adjust filename]
[наб: reinstate for 6.1;
      required for .platform (EFI DB) keys to also validate modules]
---
--- linux-6.1.20.orig/kernel/module/signing.c
+++ linux-6.1.20/kernel/module/signing.c
@@ -116,6 +116,13 @@ int mod_verify_sig(const void *mod, stru
 				      VERIFYING_MODULE_SIGNATURE,
 				      NULL, NULL);
 	pr_devel("verify_pkcs7_signature() = %d\n", ret);
+	if (ret == -ENOKEY && IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING)) {
+		ret = verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
+					      VERIFY_USE_PLATFORM_KEYRING,
+					      VERIFYING_MODULE_SIGNATURE,
+					      NULL, NULL);
+		pr_devel("verify_pkcs7_signature() = %d\n", ret);
+	}
 
 	/* checking hash of module is in blacklist */
 	if (!ret)

Attachment: signature.asc
Description: PGP signature


Reply to: