mbox series

[v3,00/14] KEYS: Add support for PGP keys and signatures

Message ID 20240911122911.1381864-1-roberto.sassu@huaweicloud.com (mailing list archive)
Headers show
Series KEYS: Add support for PGP keys and signatures | expand

Message

Roberto Sassu Sept. 11, 2024, 12:28 p.m. UTC
From: Roberto Sassu <roberto.sassu@huawei.com>

Support for PGP keys and signatures was proposed by David long time ago,
before the decision of using PKCS#7 for kernel modules signatures
verification was made. After that, there has been not enough interest to
support PGP too.

Lately, there is renewed interest in supporting PGP keys and signatures for
the following use cases:

- EFI variables in modern Lenovo laptops are in PGP form:
  https://lore.kernel.org/keyrings/2493611.1723748374@warthog.procyon.org.uk/

- Appraisal of RPM package headers for integrity check
  https://lore.kernel.org/linux-integrity/20240905152512.3781098-1-roberto.sassu@huaweicloud.com/


To make these use cases possible, introduce support for PGP keys and
signatures in the kernel, and load provided PGP keys (e.g. Linux
distribution own PGP keys) in the built-in keyring and .ima keyrings.

This feature does not interfere with existing signature verification
mechanisms, such as the one for kernel modules. One has to explicitly call
verify_pgp_signature() to take advantage of it.

For the envisioned use cases, PGP operations cannot be done in user space,
since the consumers are in the kernel itself (Integrity Digest Cache and
IMA). Also they cannot be done in a trusted initial ram disk, since PGP
operations can occur also while the system is running (e.g. after software
package installation).

In addition to the original version of the patch set, also introduce
support for signature verification of PGP keys, so that those keys can be
added to keyrings with a signature-based restriction (e.g. .ima). PGP keys
are searched with partial IDs, provided with signature subtype 16 (Issuer).
However, due to the possibility of ID collisions, the key_or_keyring
restriction is not supported.

The patch set includes two preliminary patches: patch 1 introduces
mpi_key_length(), to get the number of bits and bytes of an MPI; patch 2
introduces rsa_parse_priv_key_raw() and rsa_parse_pub_key_raw(), to parse
an RSA key in RAW format if the ASN.1 parser returns an error.

Patches 3-5 introduce the library necessary to parse PGP keys and
signatures, whose support is added with patches 6-10. Patch 11 introduces
verify_pgp_signature() to be used by kernel subsystems (e.g. IMA). Patch 12
is for testing of PGP signatures. Finally, patches 13-14 allow loading a
set of PGP keys from a supplied blob at boot time.

Changelog

v2 [3]:
- Fix description of verify_pgp_signature()
- Change references from RFC 4880 to RFC 9580 (suggested by Jonathan
  McDowell)
- Remove support for v2 and v3 PGP keys (suggested by Jonathan McDowell)
- Explain better CONFIG_PGP_TEST_KEY
- Add MODULE_DESCRIPTION() for all kernel modules (suggested by Jeff
  Johnson)
- Don't fill capabilities and MPIs for unsupported key algorithms
- Check if there is enough data when parsing PGP key MPIs and RSA RAW keys
- Fix style issues
- Fix debug messages in pgp_request_asymmetric_key()
- Search verification key in the secondary keyring for the pgp_test key
  type

v1 [2]:
- Remove quiet_cmd_extract_certs (redundant, likely leftover from
  conflict resolution)
- Load PGP keys embedded in the kernel image within load_module_cert()
  and load_system_certificate_list(), instead of using a separate initcall
- Style bug fixes found by checkpatch.pl
- Add <crypto/pgp.h> include in crypto/asymmetric_keys/pgp_preload.c, to
  remove no previous prototype warning
- Correctly check returned tfm in pgp_generate_fingerprint()
- Fix printing message in pgp_generate_fingerprint()
- Don't create a public key if the key blob does not contain a PGP key
  packet
- Remove unused pgp_pubkey_hash array
- Set KEY_EFLAG_DIGITALSIG key flag if the key has the capability
- Allow PGP_SIG_GENERAL_CERT_OF_UID_PUBKEY signature type (for key sigs)
- Add is_key_sig parameter to pgp_sig_get_sig() to ensure the key
  signature type is PGP_SIG_GENERAL_CERT_OF_UID_PUBKEY or
  PGP_SIG_POSTITIVE_CERT_OF_UID_PUBKEY

v0 [1]:
- style fixes
- move include/linux/pgp.h and pgplib.h to crypto/asymmetric_keys
- introduce verify_pgp_signature()
- replace KEY_ALLOC_TRUSTED flag with KEY_ALLOC_BUILT_IN
- don't fetch PGP subkeys
- drop support for DSA
- store number of MPIs in pgp_key_algo_p_num_mpi array
- replace dynamic memory allocations with static ones in
  pgp_generate_fingerprint()
- store only keys with capability of verifying signatures
- remember selection of PGP signature packet and don't repeat parsing
- move search of the PGP key to verify the signature from the beginning
  to the end of the verification process (to be similar with PKCS#7)
- don't retry key search in the session keyring from the signature
  verification code, let the caller pass the desired keyring
- for the PGP signature test key type, retry the key search in the session
  keyring
- retry key search in restrict_link_by_signature() with a partial ID
  (provided in the PGP signature)

[1] https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-modsign.git/log/?h=pgp-parser
[2] https://lore.kernel.org/linux-integrity/20220111180318.591029-1-roberto.sassu@huawei.com/
[3] https://lore.kernel.org/linux-integrity/20240818165756.629203-1-roberto.sassu@huaweicloud.com/

David Howells (8):
  PGPLIB: PGP definitions (RFC 9580)
  PGPLIB: Basic packet parser
  PGPLIB: Signature parser
  KEYS: PGP data parser
  KEYS: Provide PGP key description autogeneration
  KEYS: PGP-based public key signature verification
  PGP: Provide a key type for testing PGP signatures
  KEYS: Provide a function to load keys from a PGP keyring blob

Roberto Sassu (6):
  mpi: Introduce mpi_key_length()
  rsa: add parser of raw format
  KEYS: Retry asym key search with partial ID in
    restrict_link_by_signature()
  KEYS: Calculate key digest and get signature of the key
  verification: introduce verify_pgp_signature()
  KEYS: Introduce load_pgp_public_keyring()

 MAINTAINERS                             |   1 +
 certs/Kconfig                           |  11 +
 certs/Makefile                          |   7 +
 certs/system_certificates.S             |  18 +
 certs/system_keyring.c                  |  94 ++++
 crypto/asymmetric_keys/Kconfig          |  39 ++
 crypto/asymmetric_keys/Makefile         |  13 +
 crypto/asymmetric_keys/pgp.h            | 216 +++++++++
 crypto/asymmetric_keys/pgp_library.c    | 610 ++++++++++++++++++++++++
 crypto/asymmetric_keys/pgp_parser.h     |  18 +
 crypto/asymmetric_keys/pgp_preload.c    | 111 +++++
 crypto/asymmetric_keys/pgp_public_key.c | 482 +++++++++++++++++++
 crypto/asymmetric_keys/pgp_signature.c  | 510 ++++++++++++++++++++
 crypto/asymmetric_keys/pgp_test_key.c   | 131 +++++
 crypto/asymmetric_keys/pgplib.h         |  74 +++
 crypto/asymmetric_keys/restrict.c       |  10 +-
 crypto/rsa.c                            |  14 +-
 crypto/rsa_helper.c                     |  83 +++-
 include/crypto/internal/rsa.h           |   6 +
 include/crypto/pgp.h                    |  36 ++
 include/linux/mpi.h                     |   2 +
 include/linux/verification.h            |  23 +
 lib/crypto/mpi/mpicoder.c               |  33 +-
 23 files changed, 2527 insertions(+), 15 deletions(-)
 create mode 100644 crypto/asymmetric_keys/pgp.h
 create mode 100644 crypto/asymmetric_keys/pgp_library.c
 create mode 100644 crypto/asymmetric_keys/pgp_parser.h
 create mode 100644 crypto/asymmetric_keys/pgp_preload.c
 create mode 100644 crypto/asymmetric_keys/pgp_public_key.c
 create mode 100644 crypto/asymmetric_keys/pgp_signature.c
 create mode 100644 crypto/asymmetric_keys/pgp_test_key.c
 create mode 100644 crypto/asymmetric_keys/pgplib.h
 create mode 100644 include/crypto/pgp.h

Comments

Herbert Xu Sept. 13, 2024, 4:45 a.m. UTC | #1
Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
>
> For the envisioned use cases, PGP operations cannot be done in user space,
> since the consumers are in the kernel itself (Integrity Digest Cache and
> IMA). Also they cannot be done in a trusted initial ram disk, since PGP
> operations can occur also while the system is running (e.g. after software
> package installation).

Does this address Linus's objections? If not then we cannot proceed.

Personally I don't think the argument above holds water.  With
IPsec we had a similar issue of authenticating untrusted peers
using public key cryptography.  In that case we successfully
delegated the task to user-space and it is still how it works
to this day.

A user-space daemon dedicated to public key crypto seems equally
applicable to your scenario.

The original application that brought public key crypto into the
kernel was module loading.  If we really wanted to we could extend
the user-space verification to modules too and perhaps kick all
public key crypto out of the kernel.

The complexity and lack of reviewer attention in this area means
that we're more likely to introduce security holes into the kernel
with such code.

Cheers,
Roberto Sassu Sept. 13, 2024, 8:30 a.m. UTC | #2
On Fri, 2024-09-13 at 12:45 +0800, Herbert Xu wrote:
> Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:

+ linux-security-module

> > 
> > For the envisioned use cases, PGP operations cannot be done in user space,
> > since the consumers are in the kernel itself (Integrity Digest Cache and
> > IMA). Also they cannot be done in a trusted initial ram disk, since PGP
> > operations can occur also while the system is running (e.g. after software
> > package installation).
> 
> Does this address Linus's objections? If not then we cannot proceed.

I hope to get an answer from him.

> Personally I don't think the argument above holds water.  With
> IPsec we had a similar issue of authenticating untrusted peers
> using public key cryptography.  In that case we successfully
> delegated the task to user-space and it is still how it works
> to this day.

That makes sense, since it is not the kernel holding secrets on behalf
of user space, it is user space passing the crypto material to the
kernel (if I remember IPSEC correctly). Failure of user space to hold
its secrets or to tamper with the task has only effect in user space.

With my understanding, I'm citing a source enumerating the requirements
of a secure system:

James P. Anderson: Computer Security Technology Planning Study

The first requirement of a component enforcing a security policy on a
Target of Evaluation (TOE), aka the reference monitor, is that it must
be tamperproof [1].

The security policy I want to enforce is: all code that the system
executes has been built by a trusted source (e.g. a Linux
distribution).

I want to leverage the kernel to enforce such security policy, and I
assume that the kernel can be fortified enough (for example through the
lockdown LSM) to be considered tamperproof against the TOE (the user
space processes).

The first problem I see in delegating the public crypto task to user
space is that it is at the same time part of the reference monitor
(since it is used to enforce the security policy) and it is a TOE too.

The second problem is, assuming that the task is verified through other
means other than PGP (but again, we are still relying on the public
crypto functionality to be performed by the kernel, for this to work),
that I didn't get a confirmation that user space can have equivalent
isolation guarantees as the kernel:

https://lore.kernel.org/linux-integrity/eb31920bd00e2c921b0aa6ebed8745cb0130b0e1.camel@huaweicloud.com/


Please, keep in mind that I already proposed what you suggested:

https://lore.kernel.org/linux-kernel/20230317145240.363908-1-roberto.sassu@huaweicloud.com/#r


After discussing with some kernel developers, the outcome was that a
better choice would be to put the code in the kernel, if I want
reasonable tamperproof guarantees.

Thanks

Roberto


[1] https://seclab.cs.ucdavis.edu/projects/history/papers/ande72a.pdf (page 17)

> A user-space daemon dedicated to public key crypto seems equally
> applicable to your scenario.
> 
> The original application that brought public key crypto into the
> kernel was module loading.  If we really wanted to we could extend
> the user-space verification to modules too and perhaps kick all
> public key crypto out of the kernel.
> 
> The complexity and lack of reviewer attention in this area means
> that we're more likely to introduce security holes into the kernel
> with such code.
> 
> Cheers,
Herbert Xu Sept. 13, 2024, 9 a.m. UTC | #3
On Fri, Sep 13, 2024 at 10:30:11AM +0200, Roberto Sassu wrote:
>
> The second problem is, assuming that the task is verified through other
> means other than PGP (but again, we are still relying on the public
> crypto functionality to be performed by the kernel, for this to work),
> that I didn't get a confirmation that user space can have equivalent
> isolation guarantees as the kernel:
> 
> https://lore.kernel.org/linux-integrity/eb31920bd00e2c921b0aa6ebed8745cb0130b0e1.camel@huaweicloud.com/
> 
> 
> Please, keep in mind that I already proposed what you suggested:
> 
> https://lore.kernel.org/linux-kernel/20230317145240.363908-1-roberto.sassu@huaweicloud.com/#r
> 
> 
> After discussing with some kernel developers, the outcome was that a
> better choice would be to put the code in the kernel, if I want
> reasonable tamperproof guarantees.

Where is this discussion? I clicked through the two links above
and everyone seems to agree that putting it in user-space is a good
idea.

Thanks,
David Howells Sept. 13, 2024, 9:32 a.m. UTC | #4
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> Personally I don't think the argument above holds water.  With
> IPsec we had a similar issue of authenticating untrusted peers
> using public key cryptography.  In that case we successfully
> delegated the task to user-space and it is still how it works
> to this day.

It transpires that we do actually need at least a PGP parser in the kernel -
and it needs to be used prior to loading any modules: some Lenovo Thinkpads,
at least, may have EFI variables holding a list of keys in PGP form, not X.509
form.

For example, in dmesg, you might see:

May 16 04:01:01 localhost kernel: integrity: Loading X.509 certificate: UEFI:MokListRT (MOKvar table)
May 16 04:01:01 localhost kernel: integrity: Problem loading X.509 certificate -126

On my laptop, if I dump this variable:

	efivar -e /tmp/q --name=605dab50-e046-4300-abb6-3dd810dd8b23-MokListRT

And then looking at the data exported:

	file /tmp/q

I see:

	/tmp/q: PGP Secret Sub-key -

The kernel doesn't currently have a PGP parser.  I've checked and the value
doesn't parse as ASN.1:

	openssl asn1parse -in /tmp/q -inform DER
	    0:d=0  hl=2 l=  21 prim: cont [ 23 ]       
	Error in encoding
	001EBA93B67F0000:error:0680007B:asn1 encoding routines:ASN1_get_object:header too long:crypto/asn1/asn1_lib.c:105:

which would suggest that it isn't X.509.

David
Ard Biesheuvel Sept. 13, 2024, 10:46 a.m. UTC | #5
On Fri, 13 Sept 2024 at 11:32, David Howells <dhowells@redhat.com> wrote:
>
> Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> > Personally I don't think the argument above holds water.  With
> > IPsec we had a similar issue of authenticating untrusted peers
> > using public key cryptography.  In that case we successfully
> > delegated the task to user-space and it is still how it works
> > to this day.

This is slightly disingenuous. The kernel itself has no need to trust
the peer - it only manages the network pipe and authenticates/decrypts
the packets on behalf of user space.

The situation would be radically different if the kernel itself would
communicate over IPsec (or HTTPS) directly.

Likewise for module loading: there is no way the authentication can be
delegated to user space, unless that user space component is
authenticated by the kernel (and runs in a special, hardened context).
>
> It transpires that we do actually need at least a PGP parser in the kernel -
> and it needs to be used prior to loading any modules: some Lenovo Thinkpads,
> at least, may have EFI variables holding a list of keys in PGP form, not X.509
> form.
>
> For example, in dmesg, you might see:
>
> May 16 04:01:01 localhost kernel: integrity: Loading X.509 certificate: UEFI:MokListRT (MOKvar table)
> May 16 04:01:01 localhost kernel: integrity: Problem loading X.509 certificate -126
>

MokListRT is a shim construct, which is a component in the downstream
distro boot chain. It is not part of EFI, and in your case, this is
unlikely to be specific to Lenovo systems.

> On my laptop, if I dump this variable:
>
>         efivar -e /tmp/q --name=605dab50-e046-4300-abb6-3dd810dd8b23-MokListRT
>
> And then looking at the data exported:
>
>         file /tmp/q
>
> I see:
>
>         /tmp/q: PGP Secret Sub-key -
>
> The kernel doesn't currently have a PGP parser.  I've checked and the value
> doesn't parse as ASN.1:
>
>         openssl asn1parse -in /tmp/q -inform DER
>             0:d=0  hl=2 l=  21 prim: cont [ 23 ]
>         Error in encoding
>         001EBA93B67F0000:error:0680007B:asn1 encoding routines:ASN1_get_object:header too long:crypto/asn1/asn1_lib.c:105:
>
> which would suggest that it isn't X.509.
>

This should be fixed in shim not the kernel.
Jarkko Sakkinen Sept. 14, 2024, 11:29 a.m. UTC | #6
On Fri Sep 13, 2024 at 7:45 AM EEST, Herbert Xu wrote:
> Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
> >
> > For the envisioned use cases, PGP operations cannot be done in user space,
> > since the consumers are in the kernel itself (Integrity Digest Cache and
> > IMA). Also they cannot be done in a trusted initial ram disk, since PGP
> > operations can occur also while the system is running (e.g. after software
> > package installation).
>
> Does this address Linus's objections? If not then we cannot proceed.

I don't get why integrity digest cache is brought up in patch set
discussion in the first place. It is RFC patch set. It is misleading
to bring up everywhere as it was something we would need to take in
the account.

Let's worry about that once it is even a feature.

BR, Jarkko
Linus Torvalds Sept. 15, 2024, 7:11 a.m. UTC | #7
On Fri, 13 Sept 2024 at 10:30, Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
>
> On Fri, 2024-09-13 at 12:45 +0800, Herbert Xu wrote:
> >
> > Does this address Linus's objections? If not then we cannot proceed.
>
> I hope to get an answer from him.

So honestly, just the series adding pgp key verification I have no
objection to. The use case where some firmware uses pgp to validate
allowed keys in EFI variables etc sounds like a "ok, then we need to
parse them".

The objections I had were against the whole "start doing policy in
kernel", with what sounded like actually parsing and unpacking rpm
contents and verifying them with a pgp key. *That* still sounds like a
disaster to me, and is the part that made me go "why isn't that done
in user space together with then generating the fsverifty
information"?

The argument that the kernel is the only part of the system you trust
is bogus. The kernel does nothing on its own (apart from device
enumeration etc of course), so if you have no trustworthy user space,
then you might as well just give up entirely. At a *minimum* you have
initrd, and that can then be the start of a chain of user space trust.

Parsing rpm files in the kernel really sounds horrendous. But that
doesn't mean that I hate *this* series that just adds pgp key handling
in case there are other valid uses for it.

But maybe I misunderstood the original suggestion from Roberto.

              Linus
Herbert Xu Sept. 15, 2024, 8:07 a.m. UTC | #8
On Sun, Sep 15, 2024 at 09:11:04AM +0200, Linus Torvalds wrote:
>
> So honestly, just the series adding pgp key verification I have no
> objection to. The use case where some firmware uses pgp to validate
> allowed keys in EFI variables etc sounds like a "ok, then we need to
> parse them".

The use-case for EFI variables appears to be invalid:

https://lore.kernel.org/all/CAMj1kXH8nWtAzX+9xc2tLyy5d0w==JNQCMJBAbL=LdcF+XrYkw@mail.gmail.com/

> The objections I had were against the whole "start doing policy in
> kernel", with what sounded like actually parsing and unpacking rpm
> contents and verifying them with a pgp key. *That* still sounds like a
> disaster to me, and is the part that made me go "why isn't that done
> in user space together with then generating the fsverifty
> information"?

If the aformentioned EFI use-case is bogus, then distro package
verification is going to be the only application for PGP keys in
the kernel.  

Cheers,
Linus Torvalds Sept. 15, 2024, 8:40 a.m. UTC | #9
On Sun, 15 Sept 2024 at 10:08, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> If the aformentioned EFI use-case is bogus, then distro package
> verification is going to be the only application for PGP keys in
> the kernel.

So I haven't actually seen _that_ series, but as mentioned it does
smell pretty conceptually broken to me.

But hey, code talks, bullshit walks. People can most certainly try to
convince me.

                   Linus
Herbert Xu Sept. 15, 2024, 9:15 a.m. UTC | #10
On Sun, Sep 15, 2024 at 10:40:15AM +0200, Linus Torvalds wrote:
> 
> So I haven't actually seen _that_ series, but as mentioned it does
> smell pretty conceptually broken to me.
> 
> But hey, code talks, bullshit walks. People can most certainly try to
> convince me.

Roberto, correct me if I'm wrong but your intended use case is
the following patch series, right?

https://lore.kernel.org/linux-integrity/20240905152512.3781098-1-roberto.sassu@huaweicloud.com/

Thanks,
Herbert Xu Sept. 15, 2024, 9:31 a.m. UTC | #11
On Sun, Sep 15, 2024 at 05:15:25PM +0800, Herbert Xu wrote:
>
> Roberto, correct me if I'm wrong but your intended use case is
> the following patch series, right?

Actually the meat of the changes is in the following series:

https://lore.kernel.org/linux-integrity/20240905150543.3766895-1-roberto.sassu@huaweicloud.com/

Cheers,
Roberto Sassu Sept. 15, 2024, 10:51 a.m. UTC | #12
On 9/15/2024 9:11 AM, Linus Torvalds wrote:
> On Fri, 13 Sept 2024 at 10:30, Roberto Sassu

[...]

> The objections I had were against the whole "start doing policy in
> kernel", with what sounded like actually parsing and unpacking rpm
> contents and verifying them with a pgp key. *That* still sounds like a
> disaster to me, and is the part that made me go "why isn't that done
> in user space together with then generating the fsverifty
> information"?

In my opinion, trusting root in this situation is not ideal. Trusting 
root means trusting all applications that root can run, that they will 
verify PGP signatures of fsverity digests with Linux distribution keys. 
In order to trust them, we would need to check the integrity of all 
those applications, in particular file read and IPC with the rest of the 
system.

A safer way to achieve the same goal is to let the kernel verify PGP 
signatures, assuming that the kernel is more privileged and cannot be 
tampered by root (for example, by using the 'lockdown' LSM). Since the 
PGP keys of the Linux distribution would be embedded in the kernel image 
(or certified by the embedded ones), trusting the system would require 
only to verify the kernel image itself (for example, with the boot loader).

Kernel-based policy enforcement is currently adopted by other LSMs, such 
as SELinux. SELinux also parses and enforces a policy sent from user 
space in the kernel. This does not mean that the policy itself is in the 
kernel, but that the kernel is the only component in the position of 
enforcing the policy without trusting all applications that root can run.

Roberto

> The argument that the kernel is the only part of the system you trust
> is bogus. The kernel does nothing on its own (apart from device
> enumeration etc of course), so if you have no trustworthy user space,
> then you might as well just give up entirely. At a *minimum* you have
> initrd, and that can then be the start of a chain of user space trust.
> 
> Parsing rpm files in the kernel really sounds horrendous. But that
> doesn't mean that I hate *this* series that just adds pgp key handling
> in case there are other valid uses for it.
> 
> But maybe I misunderstood the original suggestion from Roberto.
> 
>                Linus
Roberto Sassu Sept. 15, 2024, 5:52 p.m. UTC | #13
On 9/15/2024 11:31 AM, Herbert Xu wrote:
> On Sun, Sep 15, 2024 at 05:15:25PM +0800, Herbert Xu wrote:
>>
>> Roberto, correct me if I'm wrong but your intended use case is
>> the following patch series, right?
> 
> Actually the meat of the changes is in the following series:
> 
> https://lore.kernel.org/linux-integrity/20240905150543.3766895-1-roberto.sassu@huaweicloud.com/

Yes, correct. The idea is to verify the authenticity of RPM headers, 
extract the file digests from them, and use those file digests as 
reference values for integrity checking of files accessed by user space 
processes.

If the calculated digest of a file being accessed matches one extracted 
from the RPM header, access is granted otherwise it is denied.

Roberto
Dr. Greg Sept. 17, 2024, 11:27 a.m. UTC | #14
On Sun, Sep 15, 2024 at 07:52:13PM +0200, Roberto Sassu wrote:

Good morning, I hope the day is starting well for everyone.

> On 9/15/2024 11:31 AM, Herbert Xu wrote:
> >On Sun, Sep 15, 2024 at 05:15:25PM +0800, Herbert Xu wrote:
> >>
> >>Roberto, correct me if I'm wrong but your intended use case is
> >>the following patch series, right?
> >
> >Actually the meat of the changes is in the following series:
> >
> >https://lore.kernel.org/linux-integrity/20240905150543.3766895-1-roberto.sassu@huaweicloud.com/

> Yes, correct. The idea is to verify the authenticity of RPM headers,
> extract the file digests from them, and use those file digests as
> reference values for integrity checking of files accessed by user
> space processes.
>
> If the calculated digest of a file being accessed matches one
> extracted from the RPM header, access is granted otherwise it is
> denied.

Based on the above response and your comment:

"The security policy I want to enforce is: all code that the system
executes has been built by a trusted source (e.g. a Linux
distribution)."

From the following URL:

https://lore.kernel.org/linux-integrity/b4a3e55650a9e9f2302cf093e5cc8e739b4ac98f.camel@huaweicloud.com/#t

What you are advocating for then, with this patch series and the
digest cache series, is a security policy that requires signed code
execution, correct?

Nothing wrong with that, it is a reasonable security desire, but it
seems wrong to conflate that with the implementation of the digest
cache.  There is a great deal of utility in a digest cache but it
doesn't require the need to parse RPM header information and/or TLV
sequences in the kernel.

That would only appear to be a requirement if your goal is a signed
executable policy that is implemented through a packaging medium,
correct?

To wit:

If I have security infrastructure that gives me confidence in the
integrity of the files on my media, I can populate a digest cache with
a simple ASCII list of pathnames fed into the kernel at boot time.

If I don't have confidence in the integrity of the files on my media I
could append a known good checksum to each pathname with the last
entry in the list being a PGP signature over the input stream.

I brought the following issue up in the patch series that Herbert
links to above, but will do so here, since I believe it has relevance
to this conversation as well.

If the goal is to have the digest cache be relevant from an integrity
perspective, particularly a signed code policy, you have to physically
read every file that has a digest value in the RPM digest list.
Otherwise the scheme is vulnerable to a Time Of Measurement Time Of
Use (TOMTOU) vulnerability scenario, correct?

This requires that one needs to experience a latency hit at least
once, presumably at boot when you prime the digest cache, correct?

An alternative approach may be to separate the RPM/TLV parsing code
from the digest code and implement RPM/Debian/whatever parsing in a
loadable module that would in turn populate the digest cache.

That may be a more acceptable strategy since it places the potential
security vulnerabilities of a parser into something that an entity
that is interested in a signed code policy would consider to be an
acceptable tradeoff from a security perspective.

> Roberto

Hopefully the above comments and clarifications will be helpful in
furthering additional discussion.

Have a good day.

As always,
Dr. Greg

The Quixote Project - Flailing at the Travails of Cybersecurity
              https://github.com/Quixote-Project
Roberto Sassu Sept. 26, 2024, 9:41 a.m. UTC | #15
On Sun, 2024-09-15 at 10:40 +0200, Linus Torvalds wrote:
> On Sun, 15 Sept 2024 at 10:08, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > 
> > If the aformentioned EFI use-case is bogus, then distro package
> > verification is going to be the only application for PGP keys in
> > the kernel.
> 
> So I haven't actually seen _that_ series, but as mentioned it does
> smell pretty conceptually broken to me.
> 
> But hey, code talks, bullshit walks. People can most certainly try to
> convince me.

The solution has three parts.

1. The kernel verifies the RPM header with a PGP key embedded in the
kernel, and provided by the Linux distribution vendor.

2. The Integrity Digest Cache parses the verified RPM header in the
kernel and feeds one of the existing LSMs (IMA, IPE and BPF LSM) with
the digests extracted from the RPM header.

3. The LSMs compare the fsverity digest they find in the filesystem
with the authenticated ones from the RPM header, and might deny access
to the file if the digests don't match.

At this point, RPM headers don't contain fsverity digests, only file
content digests, but this is an orthogonal problem.


I had a look at previous threads on similar topics, to find your
position on the matter.

I got that you would not be probably against (1), and maybe not (3).

However, we still need a source telling whether the fsverity digest in
the filesystem is the same of one calculated by Linux distributions
during build. That is what the Integrity Digest Cache provides.

Regarding (2), maybe I'm missing something fundamental, but isn't
parsing the ELF format of kernel modules from the kernel similar?

Cannot really go to user space at this point, since the authenticated
fsverity digests are directly consumed by LSMs. Also, as David pointed
out in this thread [1], there is no obligation for user space to call
any signature verification function before executing a file, this task
must be done by an LSM.

I'm aware that we should not run unnecessary code in the kernel. I
tried to mitigate this issue by striping the parsing functionality to
the minimum (220 LOC), and formally verifying it with the Frama-C
static analyzer. The parser is available here [2].

I'm also aware that this is not the long term solution, but I didn't
find much support on the alternatives, like a trustworthy user mode
driver [3][4] (isolated from other root processes) and signed eBPF
programs [5].

What it would be the right way to proceed, in your opinion?

Thanks

Roberto

[1] https://lore.kernel.org/linux-kernel/32081.1171560770@redhat.com/
[2] https://lore.kernel.org/linux-integrity/20240905150543.3766895-9-roberto.sassu@huaweicloud.com/
[3] https://lore.kernel.org/lkml/20230317145240.363908-1-roberto.sassu@huaweicloud.com/#t
[4] https://lore.kernel.org/linux-integrity/eb31920bd00e2c921b0aa6ebed8745cb0130b0e1.camel@huaweicloud.com/
[5] https://lwn.net/Articles/853489/
Dr. Greg Sept. 27, 2024, 1:25 a.m. UTC | #16
On Thu, Sep 26, 2024 at 11:41:51AM +0200, Roberto Sassu wrote:

Good evening, I hope the week has gone well for everyone.

> On Sun, 2024-09-15 at 10:40 +0200, Linus Torvalds wrote:
> > On Sun, 15 Sept 2024 at 10:08, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > > 
> > > If the aformentioned EFI use-case is bogus, then distro package
> > > verification is going to be the only application for PGP keys in
> > > the kernel.
> > 
> > So I haven't actually seen _that_ series, but as mentioned it does
> > smell pretty conceptually broken to me.
> > 
> > But hey, code talks, bullshit walks. People can most certainly try to
> > convince me.

> The solution has three parts.
> 
> 1. The kernel verifies the RPM header with a PGP key embedded in the
> kernel, and provided by the Linux distribution vendor.
> 
> 2. The Integrity Digest Cache parses the verified RPM header in the
> kernel and feeds one of the existing LSMs (IMA, IPE and BPF LSM) with
> the digests extracted from the RPM header.
> 
> 3. The LSMs compare the fsverity digest they find in the filesystem
> with the authenticated ones from the RPM header, and might deny access
> to the file if the digests don't match.
> 
> At this point, RPM headers don't contain fsverity digests, only file
> content digests, but this is an orthogonal problem.

So from the above, can it be assumed that the RPM parsing code isn't
useful until the RPM packages contain fsverity root hashes?

In addition, and we mentioned this previously in this thread, it seems
that one needs to 'eat' a full read of a file, at least once, in order
to generate an fsverity digest that is consistent with the actual
on-disk contents of the file.

So, once again, the notion of the implementation of a generic digest
cache would seem to be orthogonal to the issue of verifying that the
digest values in the cache are from a 'known good' source.

> I had a look at previous threads on similar topics, to find your
> position on the matter.
> 
> I got that you would not be probably against (1), and maybe not (3).
> 
> However, we still need a source telling whether the fsverity digest in
> the filesystem is the same of one calculated by Linux distributions
> during build. That is what the Integrity Digest Cache provides.
> 
> Regarding (2), maybe I'm missing something fundamental, but isn't
> parsing the ELF format of kernel modules from the kernel similar?
> 
> Cannot really go to user space at this point, since the authenticated
> fsverity digests are directly consumed by LSMs. Also, as David pointed
> out in this thread [1], there is no obligation for user space to call
> any signature verification function before executing a file, this task
> must be done by an LSM.
> 
> I'm aware that we should not run unnecessary code in the kernel. I
> tried to mitigate this issue by striping the parsing functionality to
> the minimum (220 LOC), and formally verifying it with the Frama-C
> static analyzer. The parser is available here [2].
> 
> I'm also aware that this is not the long term solution, but I didn't
> find much support on the alternatives, like a trustworthy user mode
> driver [3][4] (isolated from other root processes) and signed eBPF
> programs [5].

We mentioned this previously in the related threads you cite, our TSEM
LSM implementation allows the kernel to determine whether or not a
userspace process or the entirety of userspace should be 'trusted' at
any point in time.

The security footprint/model of a digest 'priming' workload is going
to be extremely small.  If the priming workload is invoked early in
the boot process the kernel can have assurance that neither the
priming workload or the surrounding userspace has commited any actions
that would be considered to make the system untrusted.

> What it would be the right way to proceed, in your opinion?

Alternatively, we had suggested previously that the RPM parsing code
may be a good candidate for implementation with a separate kernel
loadable module.  That places the parsing code in the kernel to meet
your security requirement and there is standardized infrastructure for
module signing that would ensure the integrity of the parser.

The module surfaces a pseudo-file through securityfs that userspace
can use to feed the RPM header into the kernel to drive the population
of the digest cache.  The parser only needs to be resident and
operative long enough to initialize the digest cache of a trusted
system and a kernel loadable module would seem to be consistent with
that model.

Once again, priming the digest cache with known good digests seems to
be an issue orthogonal to the implementation of the digest cache
itself.

We would be interested in knowing if we are misunderstanding the model
you are trying to achieve.

> Thanks
> 
> Roberto
> 
> [1] https://lore.kernel.org/linux-kernel/32081.1171560770@redhat.com/
> [2] https://lore.kernel.org/linux-integrity/20240905150543.3766895-9-roberto.sassu@huaweicloud.com/
> [3] https://lore.kernel.org/lkml/20230317145240.363908-1-roberto.sassu@huaweicloud.com/#t
> [4] https://lore.kernel.org/linux-integrity/eb31920bd00e2c921b0aa6ebed8745cb0130b0e1.camel@huaweicloud.com/
> [5] https://lwn.net/Articles/853489/

Have a good weekend.

As always,
Dr. Greg

The Quixote Project - Flailing at the Travails of Cybersecurity
              https://github.com/Quixote-Project
Roberto Sassu Oct. 4, 2024, 10:42 a.m. UTC | #17
On 9/26/2024 11:41 AM, Roberto Sassu wrote:
> On Sun, 2024-09-15 at 10:40 +0200, Linus Torvalds wrote:
>> On Sun, 15 Sept 2024 at 10:08, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>>>
>>> If the aformentioned EFI use-case is bogus, then distro package
>>> verification is going to be the only application for PGP keys in
>>> the kernel.
>>
>> So I haven't actually seen _that_ series, but as mentioned it does
>> smell pretty conceptually broken to me.
>>
>> But hey, code talks, bullshit walks. People can most certainly try to
>> convince me.
> 
> The solution has three parts.
> 
> 1. The kernel verifies the RPM header with a PGP key embedded in the
> kernel, and provided by the Linux distribution vendor.
> 
> 2. The Integrity Digest Cache parses the verified RPM header in the
> kernel and feeds one of the existing LSMs (IMA, IPE and BPF LSM) with
> the digests extracted from the RPM header.
> 
> 3. The LSMs compare the fsverity digest they find in the filesystem
> with the authenticated ones from the RPM header, and might deny access
> to the file if the digests don't match.
> 
> At this point, RPM headers don't contain fsverity digests, only file
> content digests, but this is an orthogonal problem.
> 
> 
> I had a look at previous threads on similar topics, to find your
> position on the matter.
> 
> I got that you would not be probably against (1), and maybe not (3).
> 
> However, we still need a source telling whether the fsverity digest in
> the filesystem is the same of one calculated by Linux distributions
> during build. That is what the Integrity Digest Cache provides.
> 
> Regarding (2), maybe I'm missing something fundamental, but isn't
> parsing the ELF format of kernel modules from the kernel similar?
> 
> Cannot really go to user space at this point, since the authenticated
> fsverity digests are directly consumed by LSMs. Also, as David pointed
> out in this thread [1], there is no obligation for user space to call
> any signature verification function before executing a file, this task
> must be done by an LSM.
> 
> I'm aware that we should not run unnecessary code in the kernel. I
> tried to mitigate this issue by striping the parsing functionality to
> the minimum (220 LOC), and formally verifying it with the Frama-C
> static analyzer. The parser is available here [2].
> 
> I'm also aware that this is not the long term solution, but I didn't
> find much support on the alternatives, like a trustworthy user mode
> driver [3][4] (isolated from other root processes) and signed eBPF
> programs [5].
> 
> What it would be the right way to proceed, in your opinion?

If I remove the parsers completely from the kernel, and attach them 
dynamically with eBPF, would you reconsider my patch set?

Thanks

Roberto