Re: [PATCH 5/5] evm: Enforce signatures on unsupported filesystem for EVM_INIT_X509

From: Mimi Zohar <zohar@linux.ibm.com>
To: Amir Goldstein <amir73il@gmail.com>,
	Stefan Berger <stefanb@linux.ibm.com>
Cc: linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-unionfs@vger.kernel.org, linux-kernel@vger.kernel.org,
	paul@paul-moore.com, jmorris@namei.org, serge@hallyn.com,
	roberto.sassu@huawei.com, miklos@szeredi.hu,
	Christian Brauner <brauner@kernel.org>
Subject: Re: [PATCH 5/5] evm: Enforce signatures on unsupported filesystem for EVM_INIT_X509
Date: Thu, 01 Feb 2024 12:53:31 -0500	[thread overview]
Message-ID: <42a1c6674ed61fe3cd77cab1709d253be4862778.camel@linux.ibm.com> (raw)
In-Reply-To: <CAOQ4uxikngPf5t9zmJqV3SKkdmMm6ZwF095uoa6HLN-yAkdnSQ@mail.gmail.com>

On Wed, 2024-01-31 at 16:06 +0200, Amir Goldstein wrote:
> On Tue, Jan 30, 2024 at 11:46 PM Stefan Berger <stefanb@linux.ibm.com
> > wrote:
> > Unsupported filesystems currently do not enforce any signatures.
> > Add
> > support for signature enforcement of the "original" and "portable &
> > immutable" signatures when EVM_INIT_X509 is enabled.
> > 
> > The "original" signature type contains filesystem specific
> > metadata.
> > Thus it cannot be copied up and verified. However with
> > EVM_INIT_X509
> > and EVM_ALLOW_METADATA_WRITES enabled, the "original" file
> > signature
> > may be written.
> > 
> > When EVM_ALLOW_METADATA_WRITES is not set or once it is removed
> > from
> > /sys/kernel/security/evm by setting EVM_INIT_HMAC for example, it
> > is not
> > possible to write or remove xattrs on the overlay filesystem.
> > 
> > This change still prevents EVM from writing HMAC signatures on
> > unsupported filesystem when EVM_INIT_HMAC is enabled.
> > 
> > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> > ---
> >  security/integrity/evm/evm_main.c | 12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> > 
> > diff --git a/security/integrity/evm/evm_main.c
> > b/security/integrity/evm/evm_main.c
> > index e96d127b48a2..f49609dfcbc7 100644
> > --- a/security/integrity/evm/evm_main.c
> > +++ b/security/integrity/evm/evm_main.c
> > @@ -192,7 +192,11 @@ static enum integrity_status
> > evm_verify_hmac(struct dentry *dentry,
> >                      iint->evm_status == INTEGRITY_PASS_IMMUTABLE))
> >                 return iint->evm_status;
> > 
> > -       if (is_unsupported_fs(dentry))
> > +       /*
> > +        * On unsupported filesystems with EVM_INIT_X509 not
> > enabled, skip
> > +        * signature verification.
> > +        */
> > +       if (!(evm_initialized & EVM_INIT_X509) &&
> > is_unsupported_fs(dentry))
> >                 return INTEGRITY_UNKNOWN;
> > 
> 
> Are the names is_unsupported_fs() and SB_I_EVM_UNSUPPORTED still
> a good description of what overlayfs is after this change?
> Is EVM really not supported on overlayfs after this change?
> 
> Would you consider a better descriptive name, for the helper and
> flag,
> at least as descriptive as SB_I_IMA_UNVERIFIABLE_SIGNATURE?

The EVM "portable & immutable" signature can be copied up, because it
does not contain filesystem specific metadata.  Support for the
"original" EVM signature is limited, since it contains filesystem
specific metadata, but it could be used to sign the overlay filesystem
during a "setup" stage.

Like the "original" EVM signatues, the EVM HMAC contains filesystem
specific metadata.  For this reason, they too cannot be copied up.  

In addition, without first verifying the file's EVM HMAC on the lower
filesystem, calculating and writing the EVM HMAC on the overlay could
result in making the lower level file with an invalid HMAC, valid. 

SB_I_EVM_UNSUPPORTED could be renamed SB_I_EVM_HMAC_UNSUPPORTED.

Mimi