Re: [PATCH v3] fs: generalize anon_inode_make_secure_inode() and fix secretmem LSM bypass

On 7/7/2025 4:01 PM, Paul Moore wrote:
> On Fri, Jul 4, 2025 at 6:41 AM Shivank Garg <[email protected]> wrote:
>> On 7/3/2025 7:43 AM, Paul Moore wrote:
>>> On Jun 26, 2025 Shivank Garg <[email protected]> wrote:
>>
>> ...
>>
>>> Thanks again for your continued work on this!  I think the patch looks
>>> pretty reasonable, but it would be good to hear a bit about how you've
>>> tested this before ACK'ing the patch.  For example, have you tested this
>>> against any of the LSMs which provide anonymous inode support?
>>>
>>> At the very least, the selinux-testsuite has a basic secretmem test, it
>>> would be good to know if the test passes with this patch or if any
>>> additional work is needed to ensure compatibility.
>>>
>>> https://github.com/SELinuxProject/selinux-testsuite
>>
>> Hi Paul,
>>
>> Thank you for pointing me to the selinux-testsuite. I wasn't sure how to properly
>> test this patch, so your guidance was very helpful.
>>
>> With the current test policy (test_secretmem.te), I initially encountered the following failures:
>>
>> ~/selinux-testsuite/tests/secretmem# ./test
>> memfd_secret() failed:  Permission denied
>> 1..6
>> memfd_secret() failed:  Permission denied
>> ok 1
>> ftruncate failed:  Permission denied
>> unable to mmap secret memory:  Permission denied
>> not ok 2
> 
> ...
> 
>> To resolve this, I updated test_secretmem.te to add additional required
>> permissions {create, read, write, map}
>> With this change, all tests now pass successfully:
>>
>> diff --git a/policy/test_secretmem.te b/policy/test_secretmem.te
>> index 357f41d..4cce076 100644
>> --- a/policy/test_secretmem.te
>> +++ b/policy/test_secretmem.te
>> @@ -13,12 +13,12 @@ testsuite_domain_type_minimal(test_nocreate_secretmem_t)
>>   # Domain allowed to create secret memory with the own domain type
>>   type test_create_secretmem_t;
>>   testsuite_domain_type_minimal(test_create_secretmem_t)
>> -allow test_create_secretmem_t self:anon_inode create;
>> +allow test_create_secretmem_t self:anon_inode { create read write map };
>>
>>   # Domain allowed to create secret memory with the own domain type and allowed to map WX
>>   type test_create_wx_secretmem_t;
>>   testsuite_domain_type_minimal(test_create_wx_secretmem_t)
>> -allow test_create_wx_secretmem_t self:anon_inode create;
>> +allow test_create_wx_secretmem_t self:anon_inode { create read write map };
> 
> I believe this domain also needs the anon_inode/execute permission.
> 
>>   allow test_create_wx_secretmem_t self:process execmem;
>>
>>   # Domain not allowed to create secret memory via a type transition to a private type
>> @@ -30,4 +30,4 @@ type_transition test_nocreate_transition_secretmem_t test_nocreate_transition_se
>>   type test_create_transition_secretmem_t;
>>   testsuite_domain_type_minimal(test_create_transition_secretmem_t)
>>   type_transition test_create_transition_secretmem_t test_create_transition_secretmem_t:anon_inode test_secretmem_inode_t "[secretmem]";
>> -allow test_create_transition_secretmem_t test_secretmem_inode_t:anon_inode create;
>> +allow test_create_transition_secretmem_t test_secretmem_inode_t:anon_inode { create read write map };
>>
>> Does this approach look correct to you? Please let me know if my understanding
>> makes sense and what should be my next step for patch.
> 
> [NOTE: added selinux@vger and selinux-refpolicy@vger to the To/CC line]
> 
> Hi Shivank,
> 
> My apologies for not responding earlier, Friday was a holiday and I
> was away over the weekend.  Getting back at it this morning I ran into
> the same failures as you described, and had to make similar changes to
> the selinux-testsuite policy (see the anon_inode/execute comment
> above, I also added the capability/ipc_lock permission as needed).
> 
> Strictly speaking this is a regression in the kernel, even if the new
> behavior is correct.  I'm CC'ing the SELinux and Reference Policy
> lists so that the policy devs can take a look and see what impacts
> there might be to the various public SELinux policies.  If this looks
> like it may be a significant issue, we'll need to work around this
> with a SELinux "policy capability" or some other compatibility
> solution.

In refpolicy, there are 34 rules for anon_inode and they all have { 
create read write map } -- none of them have the execute permission.  Of 
these, only 4 are explict and could potentially be broken.  The 
remaining get it due to being unconfined, thus can be immediately fixed, 
since it's unconfined.

IMO, this is very low impact.

-- 
Chris PeBenito