[LU-3866] missing permission checks on HSM operations

• Type: Technical task
• Resolution: Fixed
• Priority: Blocker
• Fix Version/s: Lustre 2.5.0
• Affects Version/s: Lustre 2.5.0
• Labels: HSM
• Assignee: John Hammond
• Reporter: John Hammond

Description

We have two kinds of missing permission checks in HSM. I'll call them "bad" and "less-bad."

Here are the bad ones: By guessing its FID an unprivileged user can archive and restore an arbitrary file, even one which is otherwise inaccessible. An unprivileged user can also cancel running HSM actions on an arbitrary file, and send a remove command.

If write permission for other is set on the file then an unprivileged user can get about 1/2 through HSM release, failing at the call to mo_xattr_set() on the orphan, just before layout_swap.

The less-bad ones involve the rights to perform various HSM operations on a file owned by the user. In this case there is some disagreement about what should be allowed (some discussion on LU-3811).

Some questions: Which less-bad operations should be permitted by default? How should additional permissions be granted? (/proc tunable, setuid() binary, policy tool?) Should explicit restore be treated differently from implicit restore?

I propose that by default all explicit HSM actions require root or CAP_SYS_ADMIN. The next level can be enabled via /proc and allows explicit archive, release, and restore to the file owner.

Issue Links

• is related to: LU-3811 non-root users cannot archive files, root cannot archive non-root users' files

Comments

[Peter Jones]

Landed for 2.5

[Aurelien Degremont (Inactive)]

I'm OK with John's proposal. I think Gerrit #7565 will be fine for a first round.

[Bruno Faccini (Inactive)]

All,
Can we get an agreement to limit the scope of this ticket to the critical/minimum permission checks that we want to be integrated in 2.5, and which I strongly feel are already in John's patch ??

[John Hammond]

Please see http://review.whamcloud.com/7565.

[Malcolm Cowe (Inactive)]

Hi Thomas,

I agree with your comments: archive and release should be tuneable and remove should be a privileged operation (users can make mistakes after all). My original thought was just to make sure that these operations should be protected such that the owner of a file does not get any unwelcome surprises. I support your amendments.

[Thomas LEIBOVICI - CEA (Inactive)]

Thank you Malcolm for this work of listing all actions and the related permissions.

However, some of these permissions doesn't look appropriate for a production system,
so I think they should be tunable:

Archive
Owner can archive files

This should be a tunable, basically if we want to prevent users from archiving their files after each modification
and let the PolicyEngine schedule the copy to control/leverage the data flow to the archive.

Release
Other users, whether file owners, group members or other, cannot release files

This should be a tunable, this can be a way for the user to give an hint to the system
that he doesn't plan to read the file in a near future, so we can free space in Lustre for other 'hot' data.

Remove files
Owner can remove files from archive

This sounds dangerous. I don't want users to be able to invalidate copies in the backend an uncontrolled way.

Add the following case:

• Only root can hsm_remove fids that no longer exist. The main use case is the garbage collection of orphan entries in the HSM (for removed entries in Lustre).
  BTW, I think this case is not currently tested in sanity-hsm as lfs doesn't support it (this can only be done by directly calling the llapi).

Thomas

[John Hammond]

I started a patch on for this that would do the following.

1. Make LL_IOC_HSM_CT_START require CAP_SYS_ADMIN (the client side handler modifies the KUC table).
2. Add MUTABOR to the handler flags for all MDS_HSM opcodes except MDS_HSM_STATE_GET.
3. Require CAP_SYS_ADMIN for MDS_HSM_PROGRESS, MDS_HSM_CT_REGISTER, and MDS_HSM_CT_UNREGISTER.
4. Require CAP_SYS_ADMIN in mdt_hsm_state_set() for setting flags not in HSM_USER_MASK.
5. Add bit masks hsm_user_request_mask,hsm_group_request_mask,hsm_other_request_mask to control who
   can perform what HSM actions on file. For example, we check
   hsm_user_request_mask & (1 << HSMA_RELEASE) to see if the file owner
   (uc->uc_fsuid == la->la_uid) can release the file. By default, I set each mask to 1 << HSMA_RESTORE. Having CAP_SYS_ADMIN will override
   these masks of course.
6. Add /proc/fs/lustre/mdt/*/hsm/hsm_user,group,other_request_mask to set and set these bits.

This patch will not distinguish between implicit restore and explicit restore.

Begin the flames.