diff mbox series

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

Message ID 20240130214620.3155380-6-stefanb@linux.ibm.com
State New
Headers
Series evm: Support signatures on stacked filesystem |

Commit Message

Stefan Berger Jan. 30, 2024, 9:46 p.m. UTC 
  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(-)

Comments

Amir Goldstein Jan. 31, 2024, 2:06 p.m. UTC | #1 
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?

Thanks,
Amir.
Mimi Zohar Feb. 1, 2024, 5:53 p.m. UTC | #2 
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
diff mbox series

Patch

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;
 
 	/* if status is not PASS, try to check again - against -ENOMEM */
@@ -262,7 +266,8 @@  static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 				evm_status = INTEGRITY_PASS_IMMUTABLE;
 			} else if (!IS_RDONLY(inode) &&
 				   !(inode->i_sb->s_readonly_remount) &&
-				   !IS_IMMUTABLE(inode)) {
+				   !IS_IMMUTABLE(inode) &&
+				   !is_unsupported_fs(dentry)) {
 				evm_update_evmxattr(dentry, xattr_name,
 						    xattr_value,
 						    xattr_value_len);
@@ -422,9 +427,6 @@  enum integrity_status evm_verifyxattr(struct dentry *dentry,
 	if (!evm_key_loaded() || !evm_protected_xattr(xattr_name))
 		return INTEGRITY_UNKNOWN;
 
-	if (is_unsupported_fs(dentry))
-		return INTEGRITY_UNKNOWN;
-
 	if (!iint) {
 		iint = integrity_iint_find(d_backing_inode(dentry));
 		if (!iint)