[LU-17454] nodemap: enable root squash user even if deny_unknown=0

• Status: Open
• Project: Lustre
• Affects Version/s: Lustre 2.14.0
• Type: Improvement
• Priority: Minor
• Reporter: Louis Douriez
• Assignee: Sebastien Buisson
• Resolution: Unresolved
• Severity: 3

Description

Root user squashed is denied access if deny_unknown is set:

The properties squash_uid, squash_gid and squash_projid define the default UID, GID and PROJID respectively that users will be squashed to if unmapped, unless the deny_unknown flag is set, in which case access will still be denied.

It means that root user can't be used on the client side unless:

• root is not squashed at all (unsecure)
• or all user are allowed (deny_unknown=0)

Comments

[Gerrit Updater]

"Sebastien Buisson <[e-mail]>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/53781
Subject: LU-17454 nodemap: root_squash independent of deny_unknown
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: f38cbe58178ea94c12fea4e7e0296f637e2f9943

[Andreas Dilger]

Does it make sense in this case to add a mapping for root to some other ID rather than always allowing access for root? Otherwise it now seems that root cannot be blocked from accessing the filesystem. Or somehow distinguish between a "squashed unknown ID" from "squashed root" and have a separate nodemap flag that indicates whether squashed root should be blocked?

[Sebastien Buisson]

Patch "LU-17454 nodemap: root_squash independent of deny_unknown" makes nodemap behavior consistent with what has been documented from the beginning:

The property admin defines whether root is squashed on the policy group. By default, it is squashed, unless this property is enabled.

So either root remains root (admin property set to 1), or it is squashed to the values squash_uid/squash_gid (admin property set to 0).

I am not sure the intention was to ever 'block' root access. And by doing so, it is just not possible to mount the client.

Maybe the less confusing could be to add properties to let admins specify dedicated squash uid/gid for root, so that root is squashed to a uid/gid different than the regular users. But I imagine it can be error prone to have two distinct squashings.

If we look at the on-disk data structure, it is already full (32 bytes):

struct nodemap_cluster_rec {
	char			ncr_name[LUSTRE_NODEMAP_NAME_LENGTH + 1];
	enum nm_flag_bits	ncr_flags:8;
	enum nm_flag2_bits	ncr_flags2:8;
	__u8			ncr_padding1;
	__u32			ncr_squash_projid;
	__u32			ncr_squash_uid;
	__u32			ncr_squash_gid;
};

We would have to create a new sub-key in order to introduce a new record sub-type, as we did for the role-based-access-control property:

/* sub-keys for records of type NODEMAP_CLUSTER_IDX */
enum nodemap_cluster_rec_subid {
	NODEMAP_CLUSTER_REC = 0,   /* nodemap_cluster_rec */
	NODEMAP_CLUSTER_ROLES = 1, /* nodemap_cluster_roles_rec */
};

Or maybe a better way would be to just add a new role, such as 'allow_squashed_root' (in this way, as new caps are necessarily ON by default). By default it would be allowed, but if this capability is removed, then we would deny access to root if the admin property is not set, instead of squashing it.
I think it would have the advantage of being less confusing, and simpler to implement, while still addressing the requirement of being able to 'block' root access.

[Matt Rásó-Barnett]

If I understand the patch correctly, I like this approach without adding further flags which I thing would be confusing.

With Patch, the new behaviour with admin=0, deny_unknown=1 would still allow root in a tenant to mount the filesystem, which is the critical problem addressed in this ticket, but root would be squashed to squash_uid and be unable to then access the filesystem. The admin can then optionally create a mapping for the tenant's root user in that nodemap in the usual way if desired.

This seems reasonable behaviour to me without requiring additional flags. I'm not sure if I prefer an 'allow_squashed_root' vs just creating an idmap rule for the root user.

[Sebastien Buisson]

Thanks for your feedback Matt!

This seems reasonable behaviour to me without requiring additional flags. I'm not sure if I prefer an 'allow_squashed_root' vs just creating an idmap rule for the root user.

To me, creating an idmap rule for the root user would be confusing because of the admin property that controls whether root is squashed or not. Today, the definition is:

The property admin defines whether root is squashed on the policy group.
By default, it is squashed, unless this property is enabled.

If we also accept an idmap rule for the root user, that would become something like this:

The property admin defines whether root is mapped/squashed on the policy group.
If this property is enabled, root on the client remains root on the file system on server side.
If this property is disabled, then root is mapped if an idmap rule for the root user is defined.
Otherwise root is squashed to squash_uid/gid.

That would makes things much more complicated IMO, while still not giving the ability to block root access.

[Andreas Dilger]

OK, I'm happy to keep it simpler if that works for everyone.

[Matt Rásó-Barnett]

Hi Sebastien, sorry for the delay in responding to your comment.

I actually still prefer the second formulation that you outlined above, the description given for the effect of admin + idmap rule reads to me quite logically, but that's just my perspective on this.

My opinion is:
1) We separate the ability to mount the filesystem from root-squash as your patch is doing. So we always allow the ability of local root to mount the filesystem, if the nodemap policy permits it for this client.

2) Then for what access the local root user has, the 'admin' property always controls whether root is squashed. There must be no way around that, so if an idmap rule is permitted for the local root user, it must not be able to override this (eg: --idmap 0:0).

So then the behaviour is: if no idmap rule is specified for local root user, then it will be squashed to the the squash_uid/gid value as currently.

If 'deny_unknown' is 1, then the local root user will be blocked from accessing the mount. If the admin wants to allow local root access, while still preventing other local user UIDs from accessing the mount, then the admin can create an idmap rule for the local root user, and control their access with file permissions as with any other mapped user.

[Sebastien Buisson]

Thinking further about the requirements, I think there are two different ideas, which are not completely compatible:
1. give the ability to block root. For this, I think the solution that would be both easier to understand for admins, and efficient to implement, would be to rely on a new rbac role such as 'allow_squashed_root'.
2. map root to a specific uid/gid, different from the ones applied for regular users. In this case, it would be better to allow an idmap for '0'. But as you say, the admin property would always have precedence over this mapping: if a mapping for '0' is defined, but admin is 1, then this mapping is ignored. And a mapping for '0' would always have precedence over the squash uid/gid.

To implement the second point, we would ideally want to have this behavior for root, that mimics what we have for regular users:

• if admin is set, root remains root.
• if admin is not set, an idmap for '0' is taken into account.
• if admin is not set and there is not idmap for '0' and deny_unknown is not set, root is squashed to the squash uid/gid.
• if admin is not set and there is not idmap for '0' and deny_unknown is set, root is blocked.

I am trying to find potential issues with existing nodemap configurations at customer sites. But after all it seems that implementing this would not break existing sites, in a sense that it would not change nodemap's behavior after code update, because today idmaps for '0' are not possible.

mrasobarnett does this sound good to you?
adilger do you see any pitfalls?

[Andreas Dilger]

Sebastien, I think this proposal is better than changing the current behavior. Allowing a mapping for root totally makes sense - I thought this was always possible, TBH.

[Matt Rásó-Barnett]

This proposal looks good to me Sebastien, thanks.

[Gerrit Updater]

"Sebastien Buisson <[e-mail]>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/53870
Subject: LU-17454 nodemap: allow mapping for root
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: c7de298dd4bad97d43a3e542eda66c61ecfe324d

[Sebastien Buisson]

Actually it seems mapping for root has never been taken into account, according to this code comment in the header of nodemap_map_id, from the initial git push of the feature:

 * if the id to be looked up is 0, check that root access is allowed and if it
 * is, return 0. Otherwise, return the squash uid or gid.

I submitted patch "LU-17454 nodemap: allow mapping for root" https://review.whamcloud.com/53870 to allow mapping for root, according to the description in comment https://jira.whamcloud.com/browse/LU-17454?focusedId=401850&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-401850.

Note that map_mode remains ignored for root, as it does not really make sense. Also, capabilities are not dropped for root when mapped, just like it is done for regular users. If admins want to drop root capabilities, root must be squashed.

So I am going to abandon patch "LU-17454 nodemap: root_squash independent of deny_unknown" https://review.whamcloud.com/53781.

[Andreas Dilger]

mrasobarnett I don't see that you have an account in Gerrit to add as a reviewer to patch https://review.whamcloud.com/53870 "LU-17454 nodemap: allow mapping for root". It would be useful for you to review, if possible.