Message-ID: <e0c1dfe84c64e50112bf46b8b906b76f2ff49391.camel@huaweicloud.com>
Date: Fri, 06 Dec 2024 16:26:38 +0100
From: Roberto Sassu <roberto.sassu@...weicloud.com>
To: Eric Snowberg <eric.snowberg@...cle.com>
Cc: Mimi Zohar <zohar@...ux.ibm.com>, Dmitry Kasatkin
 <dmitry.kasatkin@...il.com>, "corbet@....net" <corbet@....net>, 
 "mcgrof@...nel.org" <mcgrof@...nel.org>, "petr.pavlu@...e.com"
 <petr.pavlu@...e.com>,  "samitolvanen@...gle.com"
 <samitolvanen@...gle.com>, "da.gomez@...sung.com" <da.gomez@...sung.com>, 
 Andrew Morton <akpm@...ux-foundation.org>, "paul@...l-moore.com"
 <paul@...l-moore.com>, "jmorris@...ei.org" <jmorris@...ei.org>,
 "serge@...lyn.com" <serge@...lyn.com>, "shuah@...nel.org"
 <shuah@...nel.org>, "mcoquelin.stm32@...il.com"
 <mcoquelin.stm32@...il.com>,  "alexandre.torgue@...s.st.com"
 <alexandre.torgue@...s.st.com>, "linux-integrity@...r.kernel.org"
 <linux-integrity@...r.kernel.org>, "linux-doc@...r.kernel.org"
 <linux-doc@...r.kernel.org>, "linux-kernel@...r.kernel.org"
 <linux-kernel@...r.kernel.org>, "linux-api@...r.kernel.org"
 <linux-api@...r.kernel.org>, "linux-modules@...r.kernel.org"
 <linux-modules@...r.kernel.org>, "linux-security-module@...r.kernel.org"
 <linux-security-module@...r.kernel.org>, "linux-kselftest@...r.kernel.org"
 <linux-kselftest@...r.kernel.org>, "wufan@...ux.microsoft.com"
 <wufan@...ux.microsoft.com>, "pbrobinson@...il.com" <pbrobinson@...il.com>,
  "zbyszek@...waw.pl" <zbyszek@...waw.pl>, "hch@....de" <hch@....de>,
 "mjg59@...f.ucam.org" <mjg59@...f.ucam.org>, "pmatilai@...hat.com"
 <pmatilai@...hat.com>,  "jannh@...gle.com" <jannh@...gle.com>,
 "dhowells@...hat.com" <dhowells@...hat.com>,  "jikos@...nel.org"
 <jikos@...nel.org>, "mkoutny@...e.com" <mkoutny@...e.com>,
 "ppavlu@...e.com" <ppavlu@...e.com>, "petr.vorel@...il.com"
 <petr.vorel@...il.com>,  "mzerqung@...inter.de" <mzerqung@...inter.de>,
 "kgold@...ux.ibm.com" <kgold@...ux.ibm.com>, Roberto Sassu
 <roberto.sassu@...wei.com>
Subject: Re: [PATCH v6 00/15] integrity: Introduce the Integrity Digest Cache

On Fri, 2024-12-06 at 15:15 +0000, Eric Snowberg wrote:
> 
> > On Dec 6, 2024, at 3:06 AM, Roberto Sassu <roberto.sassu@...weicloud.com> wrote:
> > 
> > On Thu, 2024-12-05 at 19:41 +0000, Eric Snowberg wrote:
> > > 
> > > > On Dec 5, 2024, at 9:16 AM, Roberto Sassu <roberto.sassu@...weicloud.com> wrote:
> > > > 
> > > > On Thu, 2024-12-05 at 09:53 +0100, Roberto Sassu wrote:
> > > > > On Thu, 2024-12-05 at 00:57 +0000, Eric Snowberg wrote:
> > > > > > 
> > > > > > > On Dec 4, 2024, at 3:44 AM, Roberto Sassu <roberto.sassu@...weicloud.com> wrote:
> > > > > > > 
> > > > > > > On Tue, 2024-12-03 at 20:06 +0000, Eric Snowberg wrote:
> > > > > > > > 
> > > > > > > > > On Nov 26, 2024, at 3:41 AM, Roberto Sassu <roberto.sassu@...weicloud.com> wrote:
> > > > > > > > > 
> > > > > > > > > On Tue, 2024-11-26 at 00:13 +0000, Eric Snowberg wrote:
> > > > > > > > > > 
> > > > > > > > > > > On Nov 19, 2024, at 3:49 AM, Roberto Sassu <roberto.sassu@...weicloud.com> wrote:
> > > > > > > > > > > 
> > > > > > > > > > > From: Roberto Sassu <roberto.sassu@...wei.com>
> > > > > > > > > > > 
> > > > > > > > > > > The Integrity Digest Cache can also help IMA for appraisal. IMA can simply
> > > > > > > > > > > lookup the calculated digest of an accessed file in the list of digests
> > > > > > > > > > > extracted from package headers, after verifying the header signature. It is
> > > > > > > > > > > sufficient to verify only one signature for all files in the package, as
> > > > > > > > > > > opposed to verifying a signature for each file.
> > > > > > > > > > 
> > > > > > > > > > Is there a way to maintain integrity over time?  Today if a CVE is discovered 
> > > > > > > > > > in a signed program, the program hash can be added to the blacklist keyring. 
> > > > > > > > > > Later if IMA appraisal is used, the signature validation will fail just for that 
> > > > > > > > > > program.  With the Integrity Digest Cache, is there a way to do this?  
> > > > > > > > > 
> > > > > > > > > As far as I can see, the ima_check_blacklist() call is before
> > > > > > > > > ima_appraise_measurement(). If it fails, appraisal with the Integrity
> > > > > > > > > Digest Cache will not be done.
> > > > > > > > 
> > > > > > > > 
> > > > > > > > It is good the program hash would be checked beforehand and fail if it is 
> > > > > > > > contained on the list. 
> > > > > > > > 
> > > > > > > > The .ima keyring may contain many keys.  If one of the keys was later 
> > > > > > > > revoked and added to the .blacklist, wouldn't this be missed?  It would 
> > > > > > > > be caught during signature validation when the file is later appraised, but 
> > > > > > > > now this step isn't taking place.  Correct?
> > > > > > > 
> > > > > > > For files included in the digest lists, yes, there won't be detection
> > > > > > > of later revocation of a key. However, it will still work at package
> > > > > > > level/digest list level, since they are still appraised with a
> > > > > > > signature.
> > > > > > > 
> > > > > > > We can add a mechanism (if it does not already exist) to invalidate the
> > > > > > > integrity status based on key revocation, which can be propagated to
> > > > > > > files verified with the affected digest lists.
> > > > > > > 
> > > > > > > > With IMA appraisal, it is easy to maintain authenticity but challenging to 
> > > > > > > > maintain integrity over time. In user-space there are constantly new CVEs.  
> > > > > > > > To maintain integrity over time, either keys need to be rotated in the .ima 
> > > > > > > > keyring or program hashes need to be frequently added to the .blacklist.   
> > > > > > > > If neither is done, for an end-user on a distro, IMA-appraisal basically 
> > > > > > > > guarantees authenticity.
> > > > > > > > 
> > > > > > > > While I understand the intent of the series is to increase performance, 
> > > > > > > > have you considered using this to give the end-user the ability to maintain 
> > > > > > > > integrity of their system?  What I mean is, instead of trying to import anything 
> > > > > > > > from an RPM, just have the end-user provide this information in some format 
> > > > > > > > to the Digest Cache.  User-space tools could be built to collect and format 
> > > > > > > 
> > > > > > > This is already possible, digest-cache-tools
> > > > > > > (https://github.com/linux-integrity/digest-cache-tools) already allow
> > > > > > > to create a digest list with the file a user wants.
> > > > > > > 
> > > > > > > But in this case, the user is vouching for having taken the correct
> > > > > > > measure of the file at the time it was added to the digest list. This
> > > > > > > would be instead automatically guaranteed by RPMs or other packages
> > > > > > > shipped with Linux distributions.
> > > > > > > 
> > > > > > > To mitigate the concerns of CVEs, we can probably implement a rollback
> > > > > > > prevention mechanism, which would not allow to load a previous version
> > > > > > > of a digest list.
> > > > > > 
> > > > > > IMHO, pursuing this with the end-user being in control of what is contained 
> > > > > > within the Digest Cache vs what is contained in a distro would provide more
> > > > > > value. Allowing the end-user to easily update their Digest Cache in some way 
> > > > > > without having to do any type of revocation for both old and vulnerable 
> > > > > > applications with CVEs would be very beneficial.
> > > > > 
> > > > > Yes, deleting the digest list would invalidate any integrity result
> > > > > done with that digest list.
> > > > > 
> > > > > I developed also an rpm plugin that synchronizes the digest lists with
> > > > > installed software. Old vulnerable software cannot be verified anymore
> > > > > with the Integrity Digest Cache, since the rpm plugin deletes the old
> > > > > software digest lists.
> > > > > 
> > > > > https://github.com/linux-integrity/digest-cache-tools/blob/main/rpm-plugin/digest_cache.c
> > > > > 
> > > > > The good thing is that the Integrity Digest Cache can be easily
> > > > > controlled with filesystem operations (it works similarly to security
> > > > > blobs attached to kernel objects, like inodes and file descriptors).
> > > > > 
> > > > > As soon as something changes (e.g. digest list written, link to the
> > > > > digest lists), this triggers a reset in the Integrity Digest Cache, so
> > > > > digest lists and files need to be verified again. Deleting the digest
> > > > > list causes the in-kernel digest cache to be wiped away too (when the
> > > > > reference count reaches zero).
> > > > > 
> > > > > > Is there a belief the Digest Cache would be used without signed kernel 
> > > > > > modules?  Is the performance gain worth changing how kernel modules 
> > > > > > get loaded at boot?  Couldn't this part just be dropped for easier acceptance?  
> > > > > > Integrity is already maintained with the current model of appended signatures. 
> > > > > 
> > > > > I don't like making exceptions in the design, and I recently realized
> > > > > that it should not be task of the users of the Integrity Digest Cache
> > > > > to limit themselves.
> > > > 
> > > > Forgot to mention that your use case is possible. The usage of the
> > > > Integrity Digest Cache must be explicitly enabled in the IMA policy. It
> > > > will be used if the matching rule has 'digest_cache=data' (its foreseen
> > > > to be used also for metadata).
> > > 
> > > I see a lot of benefit if metadata integrity could be maintained, but in the 
> > > current form of this series, I don't think that is possible.  The Digest Cache 
> > > doesn't contain or enforce the file path, which would be necessary to 
> > > maintain integrity.  Here is an example of why it would be needed, say 
> > > you have two applications that need a configuration file to start.  The first 
> > > application has an empty file where no configuration options are currently 
> > > defined. Now there is a hash for an empty file in the Digest Cache.  The 
> > > second application can be started with an empty configuration file, however 
> > > the end-user has added some options to it.  If the configuration file for the 
> > > second application is replaced with an empty file, it will not be detected, 
> > > since the Digest Cache would see the empty file hash in its cache.
> > 
> > I was thinking more to store in the digest cache digests of metadata
> > (including for example the expected SELinux label), that EVM can
> > lookup.
> > 
> > In that way, the problem you foresee cannot happen: if you replace the
> > file belonging to app2_t with the one belonging to app1_t, SELinux
> > would deny the permission to access; if you change the SELinux label of
> > the file, EVM will deny the access.
> 
> If two different applications have config files in /etc, wouldn't both files 
> have the same SELinux label?

Likely, unless there is an application-specific policy.

> > You can still go back to the initial state, for that a rollback
> > prevention mechanism is needed (maybe EVM can remove the digest of the
> > initial state from the digest cache when it sees an update?).
> > 
> > In general, the Integrity Digest Cache should be considered as an
> > alternative mechanism to validate immutable files, or the initial state
> > of mutable files. For mutable files, EVM HMAC will protect further
> > updates.
> 
> In the example above, from a distro standpoint, most files contained in /etc 
> are viewed as being mutable.  However an end-user that wants to maintain 
> integrity on their system wouldn't view it that way.  They don't want config 
> changes they have made to be backed out. In the current form they would 
> view this series as an Authenticity Digest Cache. I'm just trying to show that 
> this could be a lot more valuable to the end-user if some things were changed.

I agree, I think the current patch set contains the minimum necessary,
and it can grow depending on use cases/requirements from the community.

Thanks

Roberto

Powered by blists - more mailing lists