[LU-6784] Defects in SELinux support

• Type: Bug
• Resolution: Fixed
• Priority: Blocker
• Affects Version/s: Lustre 2.8.0
• Severity: 3
• Assignee: WC Triage
• Reporter: Andrew Perepechko

Description

I was asked to file a ticket and summarize defects in the landed implementation of SELinux support which had been reported prior to landing:

1) [performance] create and setxattr are separate RPCs which makes things slow
2) [recovery] create and setxattr are separate RPCs, if the client crashes in between of create and setxattr, the file will not get a security label; client kernels will use default security labels in this case, but default label concept is not to work around file system bugs and no default label will be consistent with SELinux security model
3) [atomicity] create and setxattr are separate RPCs, if another client accesses the same file, it will see no security label, it will raise the same issues as in (2)
4) [consistent file system view from different clients] although initial versions of the landed patch made attempts to synchronize relabel operations among clients, the final patch does not implement any synchronization, so inodes in memory will keep old security labels in inode->i_security (this is documented in LU-5560)

Issue Links

• is blocking: LU-5560 SELinux support on the client side
• is related to: LU-9193 Multiple hangs observed with many open/getattr
• is related to: LU-13742 Provide an selinux_is_enabled for newer kernels
• is related to: LU-5074 Connection flag reservation for Secure Data Appliance
• is related to: LU-8955 Send SELinux policy info to server

Comments

[Christopher Morrone (Inactive)]

Intel blocked new connection flag landing when there was discussion on porting SDA code upstream in later development phases.

That is not accurate. On the contrary, Intel employees gave the patch positive reviews and were ready to land it. I was the person who blocked that patch. I am an employee of LLNL and a member of the leadership within OpenSFS.

[Alexey Lyashkov]

We expect a fair play if you are employed by OpenSFS to work as GateKepeer and do development for OpenSFS, but i don't see it for long time.

[Alexey Lyashkov]

Andreas,

may i remember you many my patches which you forbid to land because it isn't fully finished?
Why you land bad patched which needs for Intel and say "we are free to contribute fixes" and don't like to land other?
if feature isn't ready and produce a regressions - it feature should be forbid to land.. isn't ?

[Andrew Perepechko]

I don't see how SELinux support in SDA, if there is any, is related to this ticket. Intel blocked new connection flag landing when there was discussion on porting SDA code upstream in later development phases. The patch for the connection flag is still in gerrit, I believe. So, it's clearly Intel who broke compatibility, if there IS any SELinux support in SDA. (I'm not part of the SDA team and not aware of the current status of the development)

The reference to time and resources is weird. All defects listed above are known since September 2014, for 10 months already (issue 4 was described by the author in detail, others were described in gerrit). In order to help improve the code, I shared our experience from design phase in gerrit comments, though I wasn't formally asked to review the patch and had no formal obligation to comment it. I believe, only my comment about setstripe creation path was taken into account.

This ticket is not about preventing moving SELinux support forward. It just states what needs to be addressed to move forward and make the feature usable.

Actually, quite insignificant defects many times were the reason of rejecting a patch at the Intel side. However, this time there is tolerance and optimistic belief that there will be some movement forward, that we are preventing by just speaking of defects, after 10 months of not moving forward. Double standard?

[Andreas Dilger]

My understanding is that the atomic setting of the SELinux xattr is something that Seagate/Xyratex has already implemented for SDA feature LU-5074, and we would welcome your contribution of those patches to the GPL LICENSED CODE THAT YOU ARE SHIPPING TO CUSTOMERS.

In the absence of those patches, we are moving forward with the SELinux functionality as time and resources permit, and it will very likely be incompatible with whatever you have currently implemented. If you aren't willing to contribute code that you already have to fix this, then please don't just provide objections to prevent it moving forward independently.

[Oleg Drokin]

It should be noted that the items #2 and #3 are not new and a very similar issue is present in Android implementation.
The way they address it there is to have default context for all unlabelled files to be super restrictive such that nothing can access them. Then we allow the creator to change that to something else and make it actually accessible.
That way half-done objects are at least safe from other people accessing them.
This would need to be explained in documentation and all that of course.

[Vitaly Fertman]

2 points:
1) the review revealed the recovery defects which were not resolved before the land; we should try to avoid such a practice;
2) the patch suggests to use a not reliable security (in the recovery case); however, this is not about some attributes, this is about security, who wants to use a not reliable security?
=> should we probably revert a patch or document somewhere “the selinux support is not reliable, will be fixed in 2.9” would be enough ?

[Andrew Perepechko]

Also, though not a terrible defect, it could be beneficial to avoid prepping dentry and inode in ll_dir_setstripe() if SELinux is disabled.