xfs
[Top] [All Lists]

Re: [PATCH v3 2/4] xfs: initialize inode security on tmpfile creation

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH v3 2/4] xfs: initialize inode security on tmpfile creation
From: Stephen Smalley <sds@xxxxxxxxxxxxx>
Date: Wed, 16 Apr 2014 08:51:38 -0400
Cc: Brian Foster <bfoster@xxxxxxxxxx>, linux-fsdevel@xxxxxxxxxxxxxxx, linux-security-module@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx, Eric Paris <eparis@xxxxxxxxxxxxxx>, Paul Moore <paul@xxxxxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <534D94E4.8070606@xxxxxxxxxxxxx>
Organization: National Security Agency
References: <1397578706-5385-1-git-send-email-bfoster@xxxxxxxxxx> <1397578706-5385-3-git-send-email-bfoster@xxxxxxxxxx> <20140415175033.GB26404@xxxxxxxxxxxxx> <534D90D0.9090805@xxxxxxxxxxxxx> <20140415202222.GA10928@xxxxxxxxxxxxx> <534D94E4.8070606@xxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0
On 04/15/2014 04:21 PM, Stephen Smalley wrote:
> On 04/15/2014 04:22 PM, Christoph Hellwig wrote:
>> On Tue, Apr 15, 2014 at 04:04:32PM -0400, Stephen Smalley wrote:
>>> Is there a reason that xfs_init_security() isn't called from the inode
>>> allocation function (e.g. xfs_ialloc), as in ext4 (__ext4_new_inode
>>> calls ext4_init_security and also calls ext4_init_acl)?  That would have
>>> ensured that tmpfile inodes would have been labeled without requiring a
>>> separate change and more generally ensures complete coverage for all inodes.
>>
>> Really just code structuring - we don't like callouts to high level VFS
>> functions from deep down in the guts of the filesystem.
>>
>>> For SELinux, we need the tmpfile inodes to be labeled at creation time,
>>> not just if linked into the namespace, since they may be shared via
>>> local socket IPC or inherited across a label-changing exec and since we
>>> revalidate access on transfer or use.
>>>
>>> Labeling based on the provided directory could be a bit random, although
>>> it will work out with current policy if the provided directory
>>> corresponds to existing tmpfile locations (e.g. /tmp, /var/tmp) and
>>> therefore already has a label associated with temporary files.
>>> Otherwise we might want some indication that it is a tmpfile passed into
>>> security_inode_init_security() so that we can always select a stable
>>> label irrespective of the directory.
>>
>> Just check for I_LINKABLE in i_flags.
> 
> Thanks, that should allow us to handle it cleanly in the security modules!

Maybe I spoke too soon.  IIUC, I_LINKABLE doesn't necessarily
distinguish tmpfiles from other files, as some tmpfiles may be linkable
and others not.  But what we want is a way to identify all tmpfiles when
security_inode_init_security() is called if we are going to label them
independently of the provided dir.

Also, in that situation, we would need to likewise distinguish them
during the create-time checking, i.e. when security_inode_create() is
called (from may_o_create), as we have to determine the label that will
be applied at that point for permission checking.  And there we do not
have the inode yet so we do not even have I_LINKABLE as a distinguisher.

So I think we need __O_TMPFILE or similar flag passed down to
may_o_create() -> security_inode_create() and to
security_inode_init_security() if we are going to label these files
independently of the provided directory.

<Prev in Thread] Current Thread [Next in Thread>