mbox series

[RFC,v3a,00/11] ima: support fs-verity digests and signatures (alternative)

Message ID 20220127184614.2837938-1-roberto.sassu@huawei.com (mailing list archive)
Headers show
Series ima: support fs-verity digests and signatures (alternative) | expand

Message

Roberto Sassu Jan. 27, 2022, 6:46 p.m. UTC
I wanted to propose a different approach for handling fsverity digests and
signatures, compared to:

https://lore.kernel.org/linux-integrity/20220126000658.138345-1-zohar@linux.ibm.com/

In the original proposal, a new signature version has been introduced (v3)
to allow the possibility of signing the digest of a more flexible data
structure, ima_file_id, which could also include the fsverity file digest.

While the new signature type would be sufficient to handle fsverity file
digests, the problem is that its format would not be compatible with the
signature format supported by the built-in verification module in fsverity.
The rpm package manager already has an extension to include fsverity
signatures, with the existing format, in the RPM header.

Given that the fsverity signature is in the PKCS#7 format, IMA has already
the capability of handling it with the existing code, more specifically the
modsig code. It would be sufficient to provide to modsig the correct data
to avoid introducing a new signature format.

This is what this alternative patch set does. Patches 1-5, 8 have been
omitted as they almost don't need modification. Patches 6-7 of this patch
set extend the fsverity API to get the necessary information to handle the
existing fsverity signatures. Patch 8 (which could be split in two parts,
moving the appraisal-specific part to a new patch) gets the fsverity
formatted digest and the signature, if present, and use the obtained
information for measurement, appraisal and audit.

Interference with the code dealing with modsig has been elimitated by
introducing the new function ima_modsig_is_verity(), from which that
code knows how to deal with the data structure.

Also, the fsverity method needs to be enabled with the policy (no change
from the original patch set) and is used only if the xattr and modsig
appraisal methods are not available.

Regarding the measurement part, the original patch set avoids the ambiguity
of d-ng, or with the new template field d-type, or with the new signature
type IMA_XATTR_DIGSIG in the sig field. This patch set removes the
ambiguity by linking d-ng with d-modsig: if d-modsig is the digest of the
formatted digest including d-ng, sig is an fsverity signature, otherwise it
is a modsig signature.

Finally, this patch set addresses also the EVM part. Since the link of an
EVM portable signature/HMAC is not done anymore with the IMA xattr, as in
the original patch set, EVM directly fetches the formatted digest from
fsverity, and includes it in the HMAC/digest calculation. This behavior is
disabled by default and needs to be enabled in the kernel configuration.
A new function has been exposed to tell to IMA whether or not the fsverity
formatted digest is protected.

Remaining work would probably be to introduce new template fields to
specifically store the fsverity formatted digest and signature (instead of
d-modsig and modsig).

Mimi Zohar (6):
  ima: rename IMA_ACTION_FLAGS to IMA_NONACTION_FLAGS
  ima: define ima_max_digest_data struct without a flexible array
    variable
  fs-verity: define a function to return the integrity protected file
    digest
  ima: define a new template field 'd-type' and a new template
    'ima-ngv2'
  ima: include fsverity's file digests in the IMA measurement list
  fsverity: update the documentation

Roberto Sassu (5):
  fsverity: Introduce fsverity_get_formatted_digest()
  fsverity: Introduce fsverity_get_signature()
  fsverity: Completely disable signature verification if not requested
  ima: Add support for fsverity signatures
  evm: Include fsverity formatted digest in the HMAC/digest calculation

 Documentation/ABI/testing/ima_policy      |  17 +++
 Documentation/filesystems/fsverity.rst    |  22 ++--
 Documentation/security/IMA-templates.rst  |  13 ++-
 fs/verity/Kconfig                         |   1 +
 fs/verity/fsverity_private.h              |   7 --
 fs/verity/measure.c                       | 123 ++++++++++++++++++++++
 fs/verity/signature.c                     |  12 +--
 include/linux/evm.h                       |   9 ++
 include/linux/fsverity.h                  |  37 +++++++
 security/integrity/evm/Kconfig            |  15 +++
 security/integrity/evm/evm_crypto.c       |  18 ++++
 security/integrity/evm/evm_main.c         |   4 +
 security/integrity/ima/ima.h              |  21 +++-
 security/integrity/ima/ima_api.c          |  19 +++-
 security/integrity/ima/ima_appraise.c     |  67 ++++++++++--
 security/integrity/ima/ima_crypto.c       |   2 +-
 security/integrity/ima/ima_init.c         |   9 +-
 security/integrity/ima/ima_main.c         |  34 +++++-
 security/integrity/ima/ima_modsig.c       |  75 +++++++++++++
 security/integrity/ima/ima_policy.c       |  40 ++++++-
 security/integrity/ima/ima_template.c     |   3 +
 security/integrity/ima/ima_template_lib.c |  23 +++-
 security/integrity/ima/ima_template_lib.h |   2 +
 security/integrity/integrity.h            |  30 +++++-
 24 files changed, 553 insertions(+), 50 deletions(-)

Comments

Eric Biggers Jan. 27, 2022, 7:35 p.m. UTC | #1
On Thu, Jan 27, 2022 at 07:46:09PM +0100, Roberto Sassu wrote:
> I wanted to propose a different approach for handling fsverity digests and
> signatures, compared to:
> 
> https://lore.kernel.org/linux-integrity/20220126000658.138345-1-zohar@linux.ibm.com/
> 
> In the original proposal, a new signature version has been introduced (v3)
> to allow the possibility of signing the digest of a more flexible data
> structure, ima_file_id, which could also include the fsverity file digest.
> 
> While the new signature type would be sufficient to handle fsverity file
> digests, the problem is that its format would not be compatible with the
> signature format supported by the built-in verification module in fsverity.
> The rpm package manager already has an extension to include fsverity
> signatures, with the existing format, in the RPM header.
> 
> Given that the fsverity signature is in the PKCS#7 format, IMA has already
> the capability of handling it with the existing code, more specifically the
> modsig code. It would be sufficient to provide to modsig the correct data
> to avoid introducing a new signature format.

I think it would be best to get people moved off of the fs-verity built-in
signatures, rather than further extend the use of it.  PKCS#7 is a pretty
terrible signature format.  The IMA one is better, though it's unfortunate that
IMA still relies on X.509 for keys.

- Eric
Eric Biggers Jan. 27, 2022, 7:39 p.m. UTC | #2
On Thu, Jan 27, 2022 at 11:35:12AM -0800, Eric Biggers wrote:
> On Thu, Jan 27, 2022 at 07:46:09PM +0100, Roberto Sassu wrote:
> > I wanted to propose a different approach for handling fsverity digests and
> > signatures, compared to:
> > 
> > https://lore.kernel.org/linux-integrity/20220126000658.138345-1-zohar@linux.ibm.com/
> > 
> > In the original proposal, a new signature version has been introduced (v3)
> > to allow the possibility of signing the digest of a more flexible data
> > structure, ima_file_id, which could also include the fsverity file digest.
> > 
> > While the new signature type would be sufficient to handle fsverity file
> > digests, the problem is that its format would not be compatible with the
> > signature format supported by the built-in verification module in fsverity.
> > The rpm package manager already has an extension to include fsverity
> > signatures, with the existing format, in the RPM header.
> > 
> > Given that the fsverity signature is in the PKCS#7 format, IMA has already
> > the capability of handling it with the existing code, more specifically the
> > modsig code. It would be sufficient to provide to modsig the correct data
> > to avoid introducing a new signature format.
> 
> I think it would be best to get people moved off of the fs-verity built-in
> signatures, rather than further extend the use of it.  PKCS#7 is a pretty
> terrible signature format.  The IMA one is better, though it's unfortunate that
> IMA still relies on X.509 for keys.

Note, the only reason that support for fs-verity built-in signatures was added
to RPM is that people didn't want to use IMA:
https://lore.kernel.org/linux-fscrypt/b49b4367-51e7-f62a-6209-b46a6880824b@gmail.com

If people are going to use IMA anyway, then there would be no point.

- Eric
Roberto Sassu Jan. 28, 2022, 9:05 a.m. UTC | #3
> From: Eric Biggers [mailto:ebiggers@kernel.org]
> Sent: Thursday, January 27, 2022 8:40 PM
> On Thu, Jan 27, 2022 at 11:35:12AM -0800, Eric Biggers wrote:
> > On Thu, Jan 27, 2022 at 07:46:09PM +0100, Roberto Sassu wrote:
> > > I wanted to propose a different approach for handling fsverity digests and
> > > signatures, compared to:
> > >
> > > https://lore.kernel.org/linux-integrity/20220126000658.138345-1-
> zohar@linux.ibm.com/
> > >
> > > In the original proposal, a new signature version has been introduced (v3)
> > > to allow the possibility of signing the digest of a more flexible data
> > > structure, ima_file_id, which could also include the fsverity file digest.
> > >
> > > While the new signature type would be sufficient to handle fsverity file
> > > digests, the problem is that its format would not be compatible with the
> > > signature format supported by the built-in verification module in fsverity.
> > > The rpm package manager already has an extension to include fsverity
> > > signatures, with the existing format, in the RPM header.
> > >
> > > Given that the fsverity signature is in the PKCS#7 format, IMA has already
> > > the capability of handling it with the existing code, more specifically the
> > > modsig code. It would be sufficient to provide to modsig the correct data
> > > to avoid introducing a new signature format.
> >
> > I think it would be best to get people moved off of the fs-verity built-in
> > signatures, rather than further extend the use of it.  PKCS#7 is a pretty
> > terrible signature format.  The IMA one is better, though it's unfortunate that
> > IMA still relies on X.509 for keys.
> 
> Note, the only reason that support for fs-verity built-in signatures was added
> to RPM is that people didn't want to use IMA:
> https://lore.kernel.org/linux-fscrypt/b49b4367-51e7-f62a-6209-
> b46a6880824b@gmail.com
> 
> If people are going to use IMA anyway, then there would be no point.

Hi Eric

I thought that the solution I came with could satisfy multiple needs.

For people that don't want to use IMA, they could still continue
to use the existing signature format, and wait for an LSM that
satisfy their needs. They also have the option to migrate to the
new signature format you are defining. But will those people be
willing to switch to something IMA-specific?

For people that use IMA, they could benefit from the effort
of people creating packages with the original fsverity signature.

For people that are skeptical about IMA, they could be interested
in trying the full solution, which would probably be more easily
available if the efforts from both sides converge.

If, as you say, you have concerns about the existing signature
format, wouldn't it be better that you address them from the
fsverity side, so that all users of fsverity can benefit from it?

Currently, fsverity hashes the formatted digest whose format
is FSVerity<digest algo><digest size><digest>. Couldn't IMA
hash the same data as well?

An idea could be to always sign the formatted digest, and have
a selector for the signature format: IMA, PKCS#7 or PGP.

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua
Eric Biggers Jan. 28, 2022, 8:25 p.m. UTC | #4
On Fri, Jan 28, 2022 at 09:05:01AM +0000, Roberto Sassu wrote:
> > From: Eric Biggers [mailto:ebiggers@kernel.org]
> > Sent: Thursday, January 27, 2022 8:40 PM
> > On Thu, Jan 27, 2022 at 11:35:12AM -0800, Eric Biggers wrote:
> > > On Thu, Jan 27, 2022 at 07:46:09PM +0100, Roberto Sassu wrote:
> > > > I wanted to propose a different approach for handling fsverity digests and
> > > > signatures, compared to:
> > > >
> > > > https://lore.kernel.org/linux-integrity/20220126000658.138345-1-
> > zohar@linux.ibm.com/
> > > >
> > > > In the original proposal, a new signature version has been introduced (v3)
> > > > to allow the possibility of signing the digest of a more flexible data
> > > > structure, ima_file_id, which could also include the fsverity file digest.
> > > >
> > > > While the new signature type would be sufficient to handle fsverity file
> > > > digests, the problem is that its format would not be compatible with the
> > > > signature format supported by the built-in verification module in fsverity.
> > > > The rpm package manager already has an extension to include fsverity
> > > > signatures, with the existing format, in the RPM header.
> > > >
> > > > Given that the fsverity signature is in the PKCS#7 format, IMA has already
> > > > the capability of handling it with the existing code, more specifically the
> > > > modsig code. It would be sufficient to provide to modsig the correct data
> > > > to avoid introducing a new signature format.
> > >
> > > I think it would be best to get people moved off of the fs-verity built-in
> > > signatures, rather than further extend the use of it.  PKCS#7 is a pretty
> > > terrible signature format.  The IMA one is better, though it's unfortunate that
> > > IMA still relies on X.509 for keys.
> > 
> > Note, the only reason that support for fs-verity built-in signatures was added
> > to RPM is that people didn't want to use IMA:
> > https://lore.kernel.org/linux-fscrypt/b49b4367-51e7-f62a-6209-
> > b46a6880824b@gmail.com
> > 
> > If people are going to use IMA anyway, then there would be no point.
> 
> Hi Eric
> 
> I thought that the solution I came with could satisfy multiple needs.
> 
> For people that don't want to use IMA, they could still continue
> to use the existing signature format, and wait for an LSM that
> satisfy their needs. They also have the option to migrate to the
> new signature format you are defining. But will those people be
> willing to switch to something IMA-specific?
> 
> For people that use IMA, they could benefit from the effort
> of people creating packages with the original fsverity signature.
> 
> For people that are skeptical about IMA, they could be interested
> in trying the full solution, which would probably be more easily
> available if the efforts from both sides converge.
> 
> If, as you say, you have concerns about the existing signature
> format, wouldn't it be better that you address them from the
> fsverity side, so that all users of fsverity can benefit from it?
> 
> Currently, fsverity hashes the formatted digest whose format
> is FSVerity<digest algo><digest size><digest>. Couldn't IMA
> hash the same data as well?
> 
> An idea could be to always sign the formatted digest, and have
> a selector for the signature format: IMA, PKCS#7 or PGP.

Adding support for the new IMA signature format to fsverity_verify_signature()
*might* make sense.  (When I added this code, my understanding was that it was
just verifying signatures the way the kernel usually verifies signatures.  I
don't think I realized there was a more direct, PKCS#7-less way to do it and
that IMA used that way.)  However, it would be better to use this as an
opportunity to move people off of the built-in signatures entirely, either by
switching to a full userspace solution or by switching to IMA.

Part of the problem with IMA is that no one wants to use it because it has
terrible documentation.  It sounds like it's really complicated, and tied to
specific TCG standards and to TPMs.  I think if it was documented better, people
would find it more attractive and wouldn't be trying to avoid it at all costs.

- Eric
Roberto Sassu Jan. 31, 2022, 3:12 p.m. UTC | #5
> From: Eric Biggers [mailto:ebiggers@kernel.org]
> Sent: Friday, January 28, 2022 9:26 PM
> On Fri, Jan 28, 2022 at 09:05:01AM +0000, Roberto Sassu wrote:
> > > From: Eric Biggers [mailto:ebiggers@kernel.org]
> > > Sent: Thursday, January 27, 2022 8:40 PM
> > > On Thu, Jan 27, 2022 at 11:35:12AM -0800, Eric Biggers wrote:
> > > > On Thu, Jan 27, 2022 at 07:46:09PM +0100, Roberto Sassu wrote:
> > > > > I wanted to propose a different approach for handling fsverity digests
> and
> > > > > signatures, compared to:
> > > > >
> > > > > https://lore.kernel.org/linux-integrity/20220126000658.138345-1-
> > > zohar@linux.ibm.com/
> > > > >
> > > > > In the original proposal, a new signature version has been introduced (v3)
> > > > > to allow the possibility of signing the digest of a more flexible data
> > > > > structure, ima_file_id, which could also include the fsverity file digest.
> > > > >
> > > > > While the new signature type would be sufficient to handle fsverity file
> > > > > digests, the problem is that its format would not be compatible with the
> > > > > signature format supported by the built-in verification module in fsverity.
> > > > > The rpm package manager already has an extension to include fsverity
> > > > > signatures, with the existing format, in the RPM header.
> > > > >
> > > > > Given that the fsverity signature is in the PKCS#7 format, IMA has already
> > > > > the capability of handling it with the existing code, more specifically the
> > > > > modsig code. It would be sufficient to provide to modsig the correct data
> > > > > to avoid introducing a new signature format.
> > > >
> > > > I think it would be best to get people moved off of the fs-verity built-in
> > > > signatures, rather than further extend the use of it.  PKCS#7 is a pretty
> > > > terrible signature format.  The IMA one is better, though it's unfortunate
> that
> > > > IMA still relies on X.509 for keys.
> > >
> > > Note, the only reason that support for fs-verity built-in signatures was added
> > > to RPM is that people didn't want to use IMA:
> > > https://lore.kernel.org/linux-fscrypt/b49b4367-51e7-f62a-6209-
> > > b46a6880824b@gmail.com
> > >
> > > If people are going to use IMA anyway, then there would be no point.
> >
> > Hi Eric
> >
> > I thought that the solution I came with could satisfy multiple needs.
> >
> > For people that don't want to use IMA, they could still continue
> > to use the existing signature format, and wait for an LSM that
> > satisfy their needs. They also have the option to migrate to the
> > new signature format you are defining. But will those people be
> > willing to switch to something IMA-specific?
> >
> > For people that use IMA, they could benefit from the effort
> > of people creating packages with the original fsverity signature.
> >
> > For people that are skeptical about IMA, they could be interested
> > in trying the full solution, which would probably be more easily
> > available if the efforts from both sides converge.
> >
> > If, as you say, you have concerns about the existing signature
> > format, wouldn't it be better that you address them from the
> > fsverity side, so that all users of fsverity can benefit from it?
> >
> > Currently, fsverity hashes the formatted digest whose format
> > is FSVerity<digest algo><digest size><digest>. Couldn't IMA
> > hash the same data as well?
> >
> > An idea could be to always sign the formatted digest, and have
> > a selector for the signature format: IMA, PKCS#7 or PGP.
> 
> Adding support for the new IMA signature format to fsverity_verify_signature()
> *might* make sense.  (When I added this code, my understanding was that it
> was
> just verifying signatures the way the kernel usually verifies signatures.  I

Ok. Do we need something more to sign other than the fsverity
formatted digest? If not, this could be the same for any method
we support.

> don't think I realized there was a more direct, PKCS#7-less way to do it and
> that IMA used that way.)  However, it would be better to use this as an
> opportunity to move people off of the built-in signatures entirely, either by
> switching to a full userspace solution or by switching to IMA.

If what we sign remains the same, then we could support multiple
methods and use a selector to let fsverity_verify_signature() know
how it should verify the signature. I don't know what would be a
proper place for the selector.

PKCS#7 seems ok, as it is used for kernel modules. IMA would be
also ok, as it can verify the signature more directly. I would also
be interested in supporting PGP, to avoid the requirement for
Linux distributions to manage a secondary key. I have a small
extension for rpmsign, that I would like to test in the Fedora
infrastructure.

Both the PKCS#7 and the PGP methods don't require additional
support from outside, the functions verify_pkcs7_signature()
and verify_pgp_signature() (proposed, not yet in the upstream
kernel) would be sufficient.

The IMA method instead would require the signature_v2_hdr
structure to be exported to user space, so that rpm could
produce a blob that can be interpreted by the kernel (this
work could also be done by evmctl). Also, IMA should pass
its .ima keyring to fsverity for signature verification, or should
simply get the signature and do the verification internally.

Given that fsverity has already the capability of managing the
signature blob, it would make sense to still keep it. Adding it
in an xattr could be possible, but it would introduce more
constraints (requiring the filesystem to support xattrs). And,
an user of fsverity willing to use the IMA method would have
to look at security.ima.

To summarize: I would prefer a method that relies on an
existing signature verification mechanism (PKCS#7) or that
has an equivalent API and simplify support for Linux distributions
(PGP). If we add the IMA method, available outside IMA, we
need to also add support for user space so that it can produces
the signature in the desired format, and preferably should use
the fsverity way of getting the signature. If the IMA method
would be used by IMA only, then IMA could store the signature
in its xattr and do the verification independently.

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

> Part of the problem with IMA is that no one wants to use it because it has
> terrible documentation.  It sounds like it's really complicated, and tied to
> specific TCG standards and to TPMs.  I think if it was documented better, people
> would find it more attractive and wouldn't be trying to avoid it at all costs.
> 
> - Eric
Stefan Berger Jan. 31, 2022, 7:29 p.m. UTC | #6
On 1/31/22 10:12, Roberto Sassu wrote:
>> From: Eric Biggers [mailto:ebiggers@kernel.org]
>> Sent: Friday, January 28, 2022 9:26 PM
>> On Fri, Jan 28, 2022 at 09:05:01AM +0000, Roberto Sassu wrote:
>>>> From: Eric Biggers [mailto:ebiggers@kernel.org]
>>>> Sent: Thursday, January 27, 2022 8:40 PM
>>>> On Thu, Jan 27, 2022 at 11:35:12AM -0800, Eric Biggers wrote:
>>>>> On Thu, Jan 27, 2022 at 07:46:09PM +0100, Roberto Sassu wrote:
>>>>>> I wanted to propose a different approach for handling fsverity digests
>> and
>>>>>> signatures, compared to:
>>>>>>
>>>>>> https://lore.kernel.org/linux-integrity/20220126000658.138345-1-
>>>> zohar@linux.ibm.com/
>>>>>> In the original proposal, a new signature version has been introduced (v3)
>>>>>> to allow the possibility of signing the digest of a more flexible data
>>>>>> structure, ima_file_id, which could also include the fsverity file digest.
>>>>>>
>>>>>> While the new signature type would be sufficient to handle fsverity file
>>>>>> digests, the problem is that its format would not be compatible with the
>>>>>> signature format supported by the built-in verification module in fsverity.
>>>>>> The rpm package manager already has an extension to include fsverity
>>>>>> signatures, with the existing format, in the RPM header.
>>>>>>
>>>>>> Given that the fsverity signature is in the PKCS#7 format, IMA has already
>>>>>> the capability of handling it with the existing code, more specifically the
>>>>>> modsig code. It would be sufficient to provide to modsig the correct data
>>>>>> to avoid introducing a new signature format.
>>>>> I think it would be best to get people moved off of the fs-verity built-in
>>>>> signatures, rather than further extend the use of it.  PKCS#7 is a pretty
>>>>> terrible signature format.  The IMA one is better, though it's unfortunate
>> that
>>>>> IMA still relies on X.509 for keys.
>>>> Note, the only reason that support for fs-verity built-in signatures was added
>>>> to RPM is that people didn't want to use IMA:
>>>> https://lore.kernel.org/linux-fscrypt/b49b4367-51e7-f62a-6209-
>>>> b46a6880824b@gmail.com
>>>>
>>>> If people are going to use IMA anyway, then there would be no point.
>>> Hi Eric
>>>
>>> I thought that the solution I came with could satisfy multiple needs.
>>>
>>> For people that don't want to use IMA, they could still continue
>>> to use the existing signature format, and wait for an LSM that
>>> satisfy their needs. They also have the option to migrate to the
>>> new signature format you are defining. But will those people be
>>> willing to switch to something IMA-specific?
>>>
>>> For people that use IMA, they could benefit from the effort
>>> of people creating packages with the original fsverity signature.
>>>
>>> For people that are skeptical about IMA, they could be interested
>>> in trying the full solution, which would probably be more easily
>>> available if the efforts from both sides converge.
>>>
>>> If, as you say, you have concerns about the existing signature
>>> format, wouldn't it be better that you address them from the
>>> fsverity side, so that all users of fsverity can benefit from it?
>>>
>>> Currently, fsverity hashes the formatted digest whose format
>>> is FSVerity<digest algo><digest size><digest>. Couldn't IMA
>>> hash the same data as well?
>>>
>>> An idea could be to always sign the formatted digest, and have
>>> a selector for the signature format: IMA, PKCS#7 or PGP.
>> Adding support for the new IMA signature format to fsverity_verify_signature()
>> *might* make sense.  (When I added this code, my understanding was that it
>> was
>> just verifying signatures the way the kernel usually verifies signatures.  I
> Ok. Do we need something more to sign other than the fsverity
> formatted digest? If not, this could be the same for any method
> we support.
>
>> don't think I realized there was a more direct, PKCS#7-less way to do it and
>> that IMA used that way.)  However, it would be better to use this as an
>> opportunity to move people off of the built-in signatures entirely, either by
>> switching to a full userspace solution or by switching to IMA.
> If what we sign remains the same, then we could support multiple
> methods and use a selector to let fsverity_verify_signature() know
> how it should verify the signature. I don't know what would be a
> proper place for the selector.
>
> PKCS#7 seems ok, as it is used for kernel modules. IMA would be
> also ok, as it can verify the signature more directly. I would also
> be interested in supporting PGP, to avoid the requirement for
> Linux distributions to manage a secondary key. I have a small
> extension for rpmsign, that I would like to test in the Fedora
> infrastructure.
>
> Both the PKCS#7 and the PGP methods don't require additional
> support from outside, the functions verify_pkcs7_signature()
> and verify_pgp_signature() (proposed, not yet in the upstream
> kernel) would be sufficient.

FYI: An empty file signed with pkcs7 and an ecc key for NIST p256 
generates a signature of size 817 bytes. If an RPM needs to carry such 
signatures on a per-file basis we are back to the size increase of 
nearly an RSA signature. I would say for packages this is probably too 
much size increase.. and this is what drove the implementation of ECC 
support.
Eric Biggers Jan. 31, 2022, 8:24 p.m. UTC | #7
On Mon, Jan 31, 2022 at 02:29:19PM -0500, Stefan Berger wrote:
> > > don't think I realized there was a more direct, PKCS#7-less way to do it and
> > > that IMA used that way.)  However, it would be better to use this as an
> > > opportunity to move people off of the built-in signatures entirely, either by
> > > switching to a full userspace solution or by switching to IMA.
> > If what we sign remains the same, then we could support multiple
> > methods and use a selector to let fsverity_verify_signature() know
> > how it should verify the signature. I don't know what would be a
> > proper place for the selector.
> > 
> > PKCS#7 seems ok, as it is used for kernel modules. IMA would be
> > also ok, as it can verify the signature more directly. I would also
> > be interested in supporting PGP, to avoid the requirement for
> > Linux distributions to manage a secondary key. I have a small
> > extension for rpmsign, that I would like to test in the Fedora
> > infrastructure.
> > 
> > Both the PKCS#7 and the PGP methods don't require additional
> > support from outside, the functions verify_pkcs7_signature()
> > and verify_pgp_signature() (proposed, not yet in the upstream
> > kernel) would be sufficient.
> 
> FYI: An empty file signed with pkcs7 and an ecc key for NIST p256 generates
> a signature of size 817 bytes. If an RPM needs to carry such signatures on a
> per-file basis we are back to the size increase of nearly an RSA signature.
> I would say for packages this is probably too much size increase.. and this
> is what drove the implementation of ECC support.

I am getting 256 bytes for an ECC signature in PKCS#7 format:

	cd src/fsverity-utils
	make
	openssl ecparam -name prime256v1 -genkey -noout -out key.pem
	openssl req -new -x509 -key key.pem -out cert.pem -days 360
	touch file
	./fsverity sign file file.sig --key=key.pem --cert=cert.pem
	stat -c %s file.sig

Probably you accidentally included the whole certificate in the PKCS#7 message.
That's not required here.

There are definitely problems with PKCS#7, and it does have space overhead.  But
the space overhead is not as bad as you state.

- Eric
Eric Biggers Jan. 31, 2022, 8:31 p.m. UTC | #8
On Mon, Jan 31, 2022 at 03:12:42PM +0000, Roberto Sassu wrote:
> > From: Eric Biggers [mailto:ebiggers@kernel.org]
> > Sent: Friday, January 28, 2022 9:26 PM
> > On Fri, Jan 28, 2022 at 09:05:01AM +0000, Roberto Sassu wrote:
> > > > From: Eric Biggers [mailto:ebiggers@kernel.org]
> > > > Sent: Thursday, January 27, 2022 8:40 PM
> > > > On Thu, Jan 27, 2022 at 11:35:12AM -0800, Eric Biggers wrote:
> > > > > On Thu, Jan 27, 2022 at 07:46:09PM +0100, Roberto Sassu wrote:
> > > > > > I wanted to propose a different approach for handling fsverity digests
> > and
> > > > > > signatures, compared to:
> > > > > >
> > > > > > https://lore.kernel.org/linux-integrity/20220126000658.138345-1-
> > > > zohar@linux.ibm.com/
> > > > > >
> > > > > > In the original proposal, a new signature version has been introduced (v3)
> > > > > > to allow the possibility of signing the digest of a more flexible data
> > > > > > structure, ima_file_id, which could also include the fsverity file digest.
> > > > > >
> > > > > > While the new signature type would be sufficient to handle fsverity file
> > > > > > digests, the problem is that its format would not be compatible with the
> > > > > > signature format supported by the built-in verification module in fsverity.
> > > > > > The rpm package manager already has an extension to include fsverity
> > > > > > signatures, with the existing format, in the RPM header.
> > > > > >
> > > > > > Given that the fsverity signature is in the PKCS#7 format, IMA has already
> > > > > > the capability of handling it with the existing code, more specifically the
> > > > > > modsig code. It would be sufficient to provide to modsig the correct data
> > > > > > to avoid introducing a new signature format.
> > > > >
> > > > > I think it would be best to get people moved off of the fs-verity built-in
> > > > > signatures, rather than further extend the use of it.  PKCS#7 is a pretty
> > > > > terrible signature format.  The IMA one is better, though it's unfortunate
> > that
> > > > > IMA still relies on X.509 for keys.
> > > >
> > > > Note, the only reason that support for fs-verity built-in signatures was added
> > > > to RPM is that people didn't want to use IMA:
> > > > https://lore.kernel.org/linux-fscrypt/b49b4367-51e7-f62a-6209-
> > > > b46a6880824b@gmail.com
> > > >
> > > > If people are going to use IMA anyway, then there would be no point.
> > >
> > > Hi Eric
> > >
> > > I thought that the solution I came with could satisfy multiple needs.
> > >
> > > For people that don't want to use IMA, they could still continue
> > > to use the existing signature format, and wait for an LSM that
> > > satisfy their needs. They also have the option to migrate to the
> > > new signature format you are defining. But will those people be
> > > willing to switch to something IMA-specific?
> > >
> > > For people that use IMA, they could benefit from the effort
> > > of people creating packages with the original fsverity signature.
> > >
> > > For people that are skeptical about IMA, they could be interested
> > > in trying the full solution, which would probably be more easily
> > > available if the efforts from both sides converge.
> > >
> > > If, as you say, you have concerns about the existing signature
> > > format, wouldn't it be better that you address them from the
> > > fsverity side, so that all users of fsverity can benefit from it?
> > >
> > > Currently, fsverity hashes the formatted digest whose format
> > > is FSVerity<digest algo><digest size><digest>. Couldn't IMA
> > > hash the same data as well?
> > >
> > > An idea could be to always sign the formatted digest, and have
> > > a selector for the signature format: IMA, PKCS#7 or PGP.
> > 
> > Adding support for the new IMA signature format to fsverity_verify_signature()
> > *might* make sense.  (When I added this code, my understanding was that it
> > was
> > just verifying signatures the way the kernel usually verifies signatures.  I
> 
> Ok. Do we need something more to sign other than the fsverity
> formatted digest? If not, this could be the same for any method
> we support.
> 
> > don't think I realized there was a more direct, PKCS#7-less way to do it and
> > that IMA used that way.)  However, it would be better to use this as an
> > opportunity to move people off of the built-in signatures entirely, either by
> > switching to a full userspace solution or by switching to IMA.
> 
> If what we sign remains the same, then we could support multiple
> methods and use a selector to let fsverity_verify_signature() know
> how it should verify the signature. I don't know what would be a
> proper place for the selector.
> 
> PKCS#7 seems ok, as it is used for kernel modules. IMA would be
> also ok, as it can verify the signature more directly. I would also
> be interested in supporting PGP, to avoid the requirement for
> Linux distributions to manage a secondary key. I have a small
> extension for rpmsign, that I would like to test in the Fedora
> infrastructure.
> 
> Both the PKCS#7 and the PGP methods don't require additional
> support from outside, the functions verify_pkcs7_signature()
> and verify_pgp_signature() (proposed, not yet in the upstream
> kernel) would be sufficient.
> 
> The IMA method instead would require the signature_v2_hdr
> structure to be exported to user space, so that rpm could
> produce a blob that can be interpreted by the kernel (this
> work could also be done by evmctl). Also, IMA should pass
> its .ima keyring to fsverity for signature verification, or should
> simply get the signature and do the verification internally.
> 
> Given that fsverity has already the capability of managing the
> signature blob, it would make sense to still keep it. Adding it
> in an xattr could be possible, but it would introduce more
> constraints (requiring the filesystem to support xattrs). And,
> an user of fsverity willing to use the IMA method would have
> to look at security.ima.
> 
> To summarize: I would prefer a method that relies on an
> existing signature verification mechanism (PKCS#7) or that
> has an equivalent API and simplify support for Linux distributions
> (PGP). If we add the IMA method, available outside IMA, we
> need to also add support for user space so that it can produces
> the signature in the desired format, and preferably should use
> the fsverity way of getting the signature. If the IMA method
> would be used by IMA only, then IMA could store the signature
> in its xattr and do the verification independently.
> 
> Roberto
> 

I think you are conflating the signatures themselves from where they are stored.
The fs-verity built-in signatures feature could be extended to support the same
signatures as IMA, while still storing the signature in the same way the
fs-verity built-in signatures are currently stored (which doesn't use xattrs).

But as I said, I don't think it makes sense to continue building on the
fs-verity built-in signatures feature, as opposed to guiding users towards a
full userspace solution or to IMA instead.

- Eric
Stefan Berger Jan. 31, 2022, 8:51 p.m. UTC | #9
On 1/31/22 15:24, Eric Biggers wrote:
> On Mon, Jan 31, 2022 at 02:29:19PM -0500, Stefan Berger wrote:
>>>> don't think I realized there was a more direct, PKCS#7-less way to do it and
>>>> that IMA used that way.)  However, it would be better to use this as an
>>>> opportunity to move people off of the built-in signatures entirely, either by
>>>> switching to a full userspace solution or by switching to IMA.
>>> If what we sign remains the same, then we could support multiple
>>> methods and use a selector to let fsverity_verify_signature() know
>>> how it should verify the signature. I don't know what would be a
>>> proper place for the selector.
>>>
>>> PKCS#7 seems ok, as it is used for kernel modules. IMA would be
>>> also ok, as it can verify the signature more directly. I would also
>>> be interested in supporting PGP, to avoid the requirement for
>>> Linux distributions to manage a secondary key. I have a small
>>> extension for rpmsign, that I would like to test in the Fedora
>>> infrastructure.
>>>
>>> Both the PKCS#7 and the PGP methods don't require additional
>>> support from outside, the functions verify_pkcs7_signature()
>>> and verify_pgp_signature() (proposed, not yet in the upstream
>>> kernel) would be sufficient.
>> FYI: An empty file signed with pkcs7 and an ecc key for NIST p256 generates
>> a signature of size 817 bytes. If an RPM needs to carry such signatures on a
>> per-file basis we are back to the size increase of nearly an RSA signature.
>> I would say for packages this is probably too much size increase.. and this
>> is what drove the implementation of ECC support.
> I am getting 256 bytes for an ECC signature in PKCS#7 format:
>
> 	cd src/fsverity-utils
> 	make
> 	openssl ecparam -name prime256v1 -genkey -noout -out key.pem
> 	openssl req -new -x509 -key key.pem -out cert.pem -days 360
> 	touch file
> 	./fsverity sign file file.sig --key=key.pem --cert=cert.pem
> 	stat -c %s file.sig
>
> Probably you accidentally included the whole certificate in the PKCS#7 message.
> That's not required here.
>
> There are definitely problems with PKCS#7, and it does have space overhead.  But
> the space overhead is not as bad as you state.

You are right. I used openssl cms without -nocerts and -noattr 
(unintentionately). Though 256 bytes is RSA 2048 signature size again. 
ECDSA with NIST p256 key is around 70 bytes.


>
> - Eric