Received-SPF: pass (google.com: domain of linux-kernel+bounces-99372-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <553f0f09-45b4-4f4b-9b91-b3c2fe5d6030@linux.microsoft.com>
Date: Mon, 11 Mar 2024 11:34:22 -0700
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [RFC PATCH v14 05/19] initramfs|security: Add a security hook to
 do_populate_rootfs()
Content-Language: en-CA
To: Roberto Sassu <roberto.sassu@huaweicloud.com>, corbet@lwn.net,
 zohar@linux.ibm.com, jmorris@namei.org, serge@hallyn.com, tytso@mit.edu,
 ebiggers@kernel.org, axboe@kernel.dk, agk@redhat.com, snitzer@kernel.org,
 eparis@redhat.com, paul@paul-moore.com
Cc: linux-doc@vger.kernel.org, linux-integrity@vger.kernel.org,
 linux-security-module@vger.kernel.org, linux-fscrypt@vger.kernel.org,
 linux-block@vger.kernel.org, dm-devel@lists.linux.dev,
 audit@vger.kernel.org, linux-kernel@vger.kernel.org
References: <1709768084-22539-1-git-send-email-wufan@linux.microsoft.com>
 <1709768084-22539-6-git-send-email-wufan@linux.microsoft.com>
 <cff886eef84ced5b4dfac1be7572dc8d06b63792.camel@huaweicloud.com>
From: Fan Wu <wufan@linux.microsoft.com>
In-Reply-To: <cff886eef84ced5b4dfac1be7572dc8d06b63792.camel@huaweicloud.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit


On 3/11/2024 7:53 AM, Roberto Sassu wrote:
> On Wed, 2024-03-06 at 15:34 -0800, Fan Wu wrote:
>> This patch introduces a new hook to notify security system that the
>> content of initramfs has been unpacked into the rootfs.
>>
>> Upon receiving this notification, the security system can activate
>> a policy to allow only files that originated from the initramfs to
>> execute or load into kernel during the early stages of booting.
>>
>> This approach is crucial for minimizing the attack surface by
>> ensuring that only trusted files from the initramfs are operational
>> in the critical boot phase.
>>
>> Signed-off-by: Fan Wu <wufan@linux.microsoft.com>
>>
>> ---
>> v1-v11:
>>    + Not present
>>
>> v12:
>>    + Introduced
>>
>> v13:
>>    + Rename the hook name to initramfs_populated()
>>
>> v14:
>>    + No changes
>> ---
>>   include/linux/lsm_hook_defs.h |  2 ++
>>   include/linux/security.h      |  8 ++++++++
>>   init/initramfs.c              |  3 +++
>>   security/security.c           | 10 ++++++++++
>>   4 files changed, 23 insertions(+)
>>
>> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
>> index 76458b6d53da..e0f50789a18f 100644
>> --- a/include/linux/lsm_hook_defs.h
>> +++ b/include/linux/lsm_hook_defs.h
>> @@ -425,3 +425,5 @@ LSM_HOOK(int, 0, uring_override_creds, const struct cred *new)
>>   LSM_HOOK(int, 0, uring_sqpoll, void)
>>   LSM_HOOK(int, 0, uring_cmd, struct io_uring_cmd *ioucmd)
>>   #endif /* CONFIG_IO_URING */
>> +
>> +LSM_HOOK(void, LSM_RET_VOID, initramfs_populated, void)
> 
> I don't know, but why there is no super_block as parameter?
> 
> And, wouldn't be better to rely on existing hooks to identify inodes in
> the initial ram disk?
> 
> (gdb) p *file->f_path.dentry->d_inode->i_sb->s_type
> $3 = {name = 0xffffffff826058a9 "rootfs"
> 
> That could also help if you want to enforce action based on the
> filesystem name (and why not on the UUID too).
> 
> Roberto
>
We are not passing any parameter here because when populating the 
initramfs, the rootfs can be accessed via current->fs->root. In the next 
patch, we use ipe_sb(current->fs->root.mnt->mnt_sb)->initramfs = true; 
to mark the initramfs via a security blob.

The hook here is used only to signal that files are unpacked into the 
rootfs. The hook won't be triggered if initramfs is not enabled.

Regarding the approach of using the filesystem's name attribute to 
identify files in initramfs, it will treat all files from a filesystem 
named "rootfs" as initramfs files, which could potentially be exploited 
by malicious users. For example, an attacker could override the name of 
an existing filesystem and then load malicious kernel modules from it. 
The LSM would think the kernel modules are from initramfs and allow 
them, which is not what we want.

Also, the rootfs is a ramfs, which doesn't have a UUID, so we couldn't 
use UUID to identify it.

-Fan

>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index d0eb20f90b26..619e17e59532 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -2167,4 +2167,12 @@ static inline int security_uring_cmd(struct io_uring_cmd *ioucmd)
>>   #endif /* CONFIG_SECURITY */
>>   #endif /* CONFIG_IO_URING */
>>   
>> +#ifdef CONFIG_SECURITY
>> +extern void security_initramfs_populated(void);
>> +#else
>> +static inline void security_initramfs_populated(void)
>> +{
>> +}
>> +#endif /* CONFIG_SECURITY */
>> +
>>   #endif /* ! __LINUX_SECURITY_H */
>> diff --git a/init/initramfs.c b/init/initramfs.c
>> index 76deb48c38cb..140619a583ff 100644
>> --- a/init/initramfs.c
>> +++ b/init/initramfs.c
>> @@ -18,6 +18,7 @@
>>   #include <linux/init_syscalls.h>
>>   #include <linux/task_work.h>
>>   #include <linux/umh.h>
>> +#include <linux/security.h>
>>   
>>   static __initdata bool csum_present;
>>   static __initdata u32 io_csum;
>> @@ -720,6 +721,8 @@ static void __init do_populate_rootfs(void *unused, async_cookie_t cookie)
>>   #endif
>>   	}
>>   
>> +	security_initramfs_populated();
>> +
>>   done:
>>   	/*
>>   	 * If the initrd region is overlapped with crashkernel reserved region,
>> diff --git a/security/security.c b/security/security.c
>> index f168bc30a60d..26c28db211fd 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -5619,3 +5619,13 @@ int security_uring_cmd(struct io_uring_cmd *ioucmd)
>>   	return call_int_hook(uring_cmd, 0, ioucmd);
>>   }
>>   #endif /* CONFIG_IO_URING */
>> +
>> +/**
>> + * security_initramfs_populated() - Notify LSMs that initramfs has been loaded
>> + *
>> + * Tells the LSMs the initramfs has been unpacked into the rootfs.
>> + */
>> +void security_initramfs_populated(void)
>> +{
>> +	call_void_hook(initramfs_populated);
>> +}