[LU-3866] missing permission checks on HSM operations

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

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, ...

Comments

[Peter Jones]

Bruno

Could you please look into this one?

Thanks

Peter

[John Hammond]

Also the CT ioctls LL_IOC_HSM_CT_START, LL_IOC_HSM_PROGRESS, LL_IOC_HSM_COPY_START, LL_IOC_HSM_COPY_END are completely unrestricted. A non-root user with access to a lustre mount point can register as a copytool. And the same user running on the same node as a CT, with access to any lustre mount point on that node can unregister the CT by crafting a KUC with the CT's PID and group KUC_GRP_HSM.

[Malcolm Cowe (Inactive)]

I would suggest a restrictive set of permissions for all operations except restore. I make an exception for restore, as anyone with read privileges on a file should reasonably expect to be able to access the file, even if it has been released. It’s implied in the nature of the restore operation (whether invoked explicitly or implicitly).

Other commands could be left as super-user commands, with sudo or equivalent providing the necessary privilege escalation, at least as a stop-gap. If we documented this adequately, it would provide a reasonable implementation.

I agree with you that the copytools must only be managed by the super-user.

I’ve included some other notes:

Archive Files

• Super-user can archive files
• Owner can archive files
• ? Group members with write access can archive files?
• Other users cannot archive files

Release files

• Super-user can release files
• Other users, whether file owners, group members or other, cannot release files

Restore Files

• Any user with read permission can restore a file (implicit restore effectively mandates this behaviour)
• Restore requests cannot change ownership, permissions or other metadata

Remove files

• Super-user can remove files from the archive
• Owner can remove files from archive
• ? Group members with write access can remove files from archive?
• Other users cannot remove files from archive

Cancel

• Super-user can cancel all requests
• Requester can cancel their own requests
• Users cannot cancel requests made by other users

Questions

• Apply ACLs to allow alternative uses? For example "group_write_can_archive" – group members can archive files when the group write bit is set. Could effectively allow privilege escalation based on ACLs for files. So, very restrictive by default, but users can modify permissions to allow further options
• Create an MDT parameter to assign "super-user" privilege or otherwise delegate operations for HSM command set?
  • sudo could cover this, which means we could leave the commands as requiring super-user privileges, then demonstrate how one could use Sudo to delegate privileges to admin users
• MDT could potentially provide finer-grained access control but would need to weigh cost/benefit against alternatives

[jacques-charles lafoucriere]

I think we must have a simple tunnable (through CDT policy flags => managed by CDT?) like control on/off. Many sites will not care of who is doing what.

• "Requester can cancel their own requests": today we do not memorize the request owner in llog. Do we replace this by file owner or who is able to read file ?
• "Users cannot cancel requests made by other users": even on their file ?

[Bruno Faccini (Inactive)]

Malcolm, thanks for the list that we can use as a start point for the "high-level" control for this task. I already see that we need to also add notes about the eXecution right/bit, do we allow for explicit/implicit restore when execution bit is set ?

John, J-C, what do you think if, we start with implementation of 3 modes, no-control/root-only/enhanced ?

Also and as very 1st step, for root-only mode, what do you think would be the best/simplest way to implement it, in ll_xx_ioctl() with just a tunable/proc to set the mode ?

[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.

[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

[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.

[John Hammond]

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

[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 ??

[Aurelien Degremont (Inactive)]

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

[Peter Jones]

Landed for 2.5