Differential D21939

Enter chroot before looking up user or group to change to
AbandonedPublic
Actions

Authored by thj on Oct 8 2019, 8:35 AM.

Details

Reviewers

jilles

Group Reviewers

manpages

Summary

Before r345132 chroot(8) stated:

If the -u, -g or -G options are given, the user, group and group
list of the process are set to these values after the chroot has
taken place.

chroot looks up the user, grop or group list before entering the chroot.
This leads to usernames that exist in the chroot not being found by the
command on the host and chroot erroring. Instead chroot uses the uid of
the username found on the host as the uid to use in the chroot. This
association is probably not correct and is working unintentionally.

Enter the chroot before looking up the user, group or group list.
Restore the previous sentence to the man page that describes this
behaviour.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 26923
Build 25227: arc lint + arc unit

Event Timeline

thj created this revision.Oct 8 2019, 8:35 AM

Harbormaster completed remote builds in B26923: Diff 63034.Oct 8 2019, 8:35 AM

bcr added a subscriber: bcr.Jul 16 2020, 2:11 PM

bcr added inline comments.

usr.sbin/chroot/chroot.8
68	Oxford comma here?
73	s/group/group,/ (Oxford called, they want their comma set properly ;-))

This change may cause breakage in setups where the chroot does not have /etc/pwd.db and related files. Given that chroot(8) has worked like this since 2003, why change it now?

The text that used to be in the manual page may be a bit misleading but it is still correct, since chroot(2) requires root privileges and therefore the actual privilege change occurs after it. The lookup of UIDs and GIDs is not explicitly mentioned.

Swapping the order can be done with the existing tools by executing su(1) or another chroot(8) in the chroot. The former has the advantage of also setting various other items such as the login class.

This revision now requires changes to proceed.Jul 16 2020, 7:55 PM

In D21939#568632, @jilles wrote:

This change may cause breakage in setups where the chroot does not have /etc/pwd.db and related files. Given that chroot(8) has worked like this since 2003, why change it now?

The text that used to be in the manual page may be a bit misleading but it is still correct, since chroot(2) requires root privileges and therefore the actual privilege change occurs after it. The lookup of UIDs and GIDs is not explicitly mentioned.

Swapping the order can be done with the existing tools by executing su(1) or another chroot(8) in the chroot. The former has the advantage of also setting various other items such as the login class.

Interestingly, chroot() no longer requires root privileges. Does that change things here?

In D21939#769857, @allanjude wrote:

Interestingly, chroot() no longer requires root privileges. Does that change things here?

This may give some more flexibility, but it is not a drop-in replacement since chroot() by non-root can only be allowed securely if NO_NEW_PRIVS is in effect (procctl(2)).

It may certainly require the man page to be formulated more precisely.

Language in manual page change LGTM.

usr.sbin/chroot/chroot.8
31	Will need bumping again.

I actually thought I had abandoned this, If I remember gnu chroot looks up the user before and after the chroot so it can use the correct user id. I was told when creating the diff that this is an awful idea and I should use other tools.