Re: jail and chflags [patch]

On Sat, 15 May 2004, Robert Watson wrote:

> On Sat, 15 May 2004, Pawel Jakub Dawidek wrote:
>
> > +> +> comments? yeahs? neys?
> > +>
> > +> Whoa! This looks very serious.
> >
> > Ok, false alarm:) After discussion with rwatson_at_ and cperciva_at_, it looks
> > that changing those flags is permitted due to per-jail securelevels,
> > which were intruduced in 5.x.
>
> However, we might want to think about setting the securelevel in a jail to
> at least 1 on creation for compatibility reasons...

We should also think about fixing the comment so that it matches the code,
and fixing other bugs in the comment and code.

% 		/*
% 		 * Unprivileged processes and privileged processes in
% 		 * jail() are not permitted to unset system flags, or
% 		 * modify flags if any system flags are set.

Nope:
(1) Privileged processes in jail are permitted to unset system flags,
    depending on the securelevel in the same was as for privileged
    processes not in jail
(2) Privileged processes in jail are permitted modify flags if a system
    flag is set, depending on the securelevel...
(3) Privileged processes (in jail or not) are permitted to modify flags
    if only certain system flags that don't restrict changing flags are
    set:
    (a) SF_ARCHIVED and SF_SNAPSHOT don't restrict changing flags (except
	SF_SNAPSHOT itself is always immutable) and work right
    (b) SF_APPEND and SF_IMMUTABLE restrict changing flags and work right
    (c) SF_NOUNLINK shouldn't restrict changing flags (it should only
	restrict unlinking), but does.

% 		 * Privileged non-jail processes may not modify system flags
% 		 * if securelevel > 0 and any existing system flags are set.
% 		 */

This of course applies to jailed processes too, except it mis-echoes the
the code by misspelling "securelevel_gt(cred, 0)".

% 		if (!suser_cred(cred, PRISON_ROOT)) {
% 			if (ip->i_flags
% 			    & (SF_NOUNLINK | SF_IMMUTABLE | SF_APPEND)) {
% 				error = securelevel_gt(cred, 0);
% 				if (error)
% 					return (error);
% 			}

The unprivileged case has larger bugs related to existing system flags.
Except for the one with SF_NOUNLINK, these are fixed in rev.1.1 of
chflags.2 and in NetBSD's rev.1.24 of ufs_vnops.c.  Lite2 moved the
bugs around a bit so that the man page is broken and doesn't match the
code in a different way, and turned NetBSD's fix into nonsense.  The
result is that applications wanting to change user flags while preserving
system flags must actually reqest clearing the system flags.  chflags(1)
is not in the small set of applications which understands this full
brokenness of chflags(2), so it doesn't work for changing user flags
if a harmless system flag like SF_ARCHIVED is set.  OTOH, SF_ARCHIVED
is only harmless because it has no effect except to get in the way
here.  It should probably be cleared by any modification to a file or
its metadata, and then unprivileged requests to change the user flags
would have to be rejected if SF_ARCHIVED is set since they would also
change the system flag SF_ARCHIVED.

Bruce