[v3,0/5] Smack transmute fixes

Message

Roberto Sassu Nov. 16, 2023, 9:01 a.m. UTC

From: Roberto Sassu <[e-mail]>

The first two patches are obvious fixes, the first restricts setting the
SMACK64TRANSMUTE xattr only for directories, and the second makes it
possible to set SMACK64TRANSMUTE if the filesystem does not support xattrs
(e.g. ramfs).

The remaining fixes are optional, and only required if we want filesystems
without xattr support behave like those with xattr support. Since we have
the inode_setsecurity and inode_getsecurity hooks to make the first group
work, it seems useful to fix inode creation too (SELinux should be fine).

The third patch is merely a code move out of the 'if (xattr)' condition.
The fourth updates the security field of the in-memory inode directly in
smack_inode_init_security() and marks the inode as instantiated, and the
fifth adds a security_inode_init_security() call in ramfs to initialize the
security field of the in-memory inodes (needed to test transmuting
directories).

Both the Smack (on xfs) and IMA test suite succeed with all patches
applied. Tests were not executed on v3 (trivial changes).

By executing the tests in a ramfs, the results are:

Without the patches:
86 Passed, 9 Failed, 90% Success rate

With the patches:
93 Passed, 2 Failed, 97% Success rate

The remaining two failures are:
2151  ioctl(4, BTRFS_IOC_CLONE or FICLONE, 3) = -1 EOPNOTSUPP (Operation not supported)
2152  lsetxattr("./targets/proc-attr-Snap", "security.SMACK64EXEC", "Pop", 3, 0) = -1 EOPNOTSUPP (Operation not supported)

The first one is likely due ramfs lack of support for ioctl() while the
second could be fixed by handling SMACK64EXEC in smack_inode_setsecurity().

The patch set applies on top of lsm/dev, commit e246777e2a03 ("MAINTAINERS:
update the LSM entry").

The ramfs patch potentially could be useful to correctly initialize the
label of new inodes in the initramfs, assuming that it will be fully
labeled with support for xattrs in the cpio image:

https://lore.kernel.org/linux-integrity/[e-mail]/

Ramfs inode labels will be set from xattrs with the inode_setsecurity hook.

Changelog

v2:
- Replace return with goto in the ramfs patch, for better maintainability
  (suggested by Andrew Morton)

v1:
- Rebase on top of latest lsm/next
- Remove -EOPNOTSUPP check in patch 5 (cannot happen)

Roberto Sassu (5):
  smack: Set SMACK64TRANSMUTE only for dirs in smack_inode_setxattr()
  smack: Handle SMACK64TRANSMUTE in smack_inode_setsecurity()
  smack: Always determine inode labels in smack_inode_init_security()
  smack: Initialize the in-memory inode in smack_inode_init_security()
  ramfs: Initialize security of in-memory inodes

 fs/ramfs/inode.c           | 32 ++++++++++++-
 security/smack/smack_lsm.c | 95 ++++++++++++++++++++++----------------
 2 files changed, 86 insertions(+), 41 deletions(-)

Comments

Casey Schaufler Jan. 24, 2024, 10:31 p.m. UTC | #1

On 11/16/2023 1:01 AM, Roberto Sassu wrote:
> From: Roberto Sassu <[e-mail]>
>
> The first two patches are obvious fixes, the first restricts setting the
> SMACK64TRANSMUTE xattr only for directories, and the second makes it
> possible to set SMACK64TRANSMUTE if the filesystem does not support xattrs
> (e.g. ramfs).
>
> The remaining fixes are optional, and only required if we want filesystems
> without xattr support behave like those with xattr support. Since we have
> the inode_setsecurity and inode_getsecurity hooks to make the first group
> work, it seems useful to fix inode creation too (SELinux should be fine).
>
> The third patch is merely a code move out of the 'if (xattr)' condition.
> The fourth updates the security field of the in-memory inode directly in
> smack_inode_init_security() and marks the inode as instantiated,

I have taken patches 1-4 in smack-next. I'm still waiting on a convincing
approval for patch 5.


>  and the
> fifth adds a security_inode_init_security() call in ramfs to initialize the
> security field of the in-memory inodes (needed to test transmuting
> directories).
>
> Both the Smack (on xfs) and IMA test suite succeed with all patches
> applied. Tests were not executed on v3 (trivial changes).
>
> By executing the tests in a ramfs, the results are:
>
> Without the patches:
> 86 Passed, 9 Failed, 90% Success rate
>
> With the patches:
> 93 Passed, 2 Failed, 97% Success rate
>
> The remaining two failures are:
> 2151  ioctl(4, BTRFS_IOC_CLONE or FICLONE, 3) = -1 EOPNOTSUPP (Operation not supported)
> 2152  lsetxattr("./targets/proc-attr-Snap", "security.SMACK64EXEC", "Pop", 3, 0) = -1 EOPNOTSUPP (Operation not supported)
>
> The first one is likely due ramfs lack of support for ioctl() while the
> second could be fixed by handling SMACK64EXEC in smack_inode_setsecurity().
>
> The patch set applies on top of lsm/dev, commit e246777e2a03 ("MAINTAINERS:
> update the LSM entry").
>
> The ramfs patch potentially could be useful to correctly initialize the
> label of new inodes in the initramfs, assuming that it will be fully
> labeled with support for xattrs in the cpio image:
>
> https://lore.kernel.org/linux-integrity/[e-mail]/
>
> Ramfs inode labels will be set from xattrs with the inode_setsecurity hook.
>
> Changelog
>
> v2:
> - Replace return with goto in the ramfs patch, for better maintainability
>   (suggested by Andrew Morton)
>
> v1:
> - Rebase on top of latest lsm/next
> - Remove -EOPNOTSUPP check in patch 5 (cannot happen)
>
> Roberto Sassu (5):
>   smack: Set SMACK64TRANSMUTE only for dirs in smack_inode_setxattr()
>   smack: Handle SMACK64TRANSMUTE in smack_inode_setsecurity()
>   smack: Always determine inode labels in smack_inode_init_security()
>   smack: Initialize the in-memory inode in smack_inode_init_security()
>   ramfs: Initialize security of in-memory inodes
>
>  fs/ramfs/inode.c           | 32 ++++++++++++-
>  security/smack/smack_lsm.c | 95 ++++++++++++++++++++++----------------
>  2 files changed, 86 insertions(+), 41 deletions(-)
>

Roberto Sassu Jan. 25, 2024, 8:44 a.m. UTC | #2

On Wed, 2024-01-24 at 14:31 -0800, Casey Schaufler wrote:
> On 11/16/2023 1:01 AM, Roberto Sassu wrote:
> > From: Roberto Sassu <[e-mail]>
> > 
> > The first two patches are obvious fixes, the first restricts setting the
> > SMACK64TRANSMUTE xattr only for directories, and the second makes it
> > possible to set SMACK64TRANSMUTE if the filesystem does not support xattrs
> > (e.g. ramfs).
> > 
> > The remaining fixes are optional, and only required if we want filesystems
> > without xattr support behave like those with xattr support. Since we have
> > the inode_setsecurity and inode_getsecurity hooks to make the first group
> > work, it seems useful to fix inode creation too (SELinux should be fine).
> > 
> > The third patch is merely a code move out of the 'if (xattr)' condition.
> > The fourth updates the security field of the in-memory inode directly in
> > smack_inode_init_security() and marks the inode as instantiated,
> 
> I have taken patches 1-4 in smack-next. I'm still waiting on a convincing
> approval for patch 5.

Thanks Casey. I also hope to receive an Ack from the maintainers.

Roberto


> >  and the
> > fifth adds a security_inode_init_security() call in ramfs to initialize the
> > security field of the in-memory inodes (needed to test transmuting
> > directories).
> > 
> > Both the Smack (on xfs) and IMA test suite succeed with all patches
> > applied. Tests were not executed on v3 (trivial changes).
> > 
> > By executing the tests in a ramfs, the results are:
> > 
> > Without the patches:
> > 86 Passed, 9 Failed, 90% Success rate
> > 
> > With the patches:
> > 93 Passed, 2 Failed, 97% Success rate
> > 
> > The remaining two failures are:
> > 2151  ioctl(4, BTRFS_IOC_CLONE or FICLONE, 3) = -1 EOPNOTSUPP (Operation not supported)
> > 2152  lsetxattr("./targets/proc-attr-Snap", "security.SMACK64EXEC", "Pop", 3, 0) = -1 EOPNOTSUPP (Operation not supported)
> > 
> > The first one is likely due ramfs lack of support for ioctl() while the
> > second could be fixed by handling SMACK64EXEC in smack_inode_setsecurity().
> > 
> > The patch set applies on top of lsm/dev, commit e246777e2a03 ("MAINTAINERS:
> > update the LSM entry").
> > 
> > The ramfs patch potentially could be useful to correctly initialize the
> > label of new inodes in the initramfs, assuming that it will be fully
> > labeled with support for xattrs in the cpio image:
> > 
> > https://lore.kernel.org/linux-integrity/[e-mail]/
> > 
> > Ramfs inode labels will be set from xattrs with the inode_setsecurity hook.
> > 
> > Changelog
> > 
> > v2:
> > - Replace return with goto in the ramfs patch, for better maintainability
> >   (suggested by Andrew Morton)
> > 
> > v1:
> > - Rebase on top of latest lsm/next
> > - Remove -EOPNOTSUPP check in patch 5 (cannot happen)
> > 
> > Roberto Sassu (5):
> >   smack: Set SMACK64TRANSMUTE only for dirs in smack_inode_setxattr()
> >   smack: Handle SMACK64TRANSMUTE in smack_inode_setsecurity()
> >   smack: Always determine inode labels in smack_inode_init_security()
> >   smack: Initialize the in-memory inode in smack_inode_init_security()
> >   ramfs: Initialize security of in-memory inodes
> > 
> >  fs/ramfs/inode.c           | 32 ++++++++++++-
> >  security/smack/smack_lsm.c | 95 ++++++++++++++++++++++----------------
> >  2 files changed, 86 insertions(+), 41 deletions(-)
> >