Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 24 May 2012 20:39:07 -0600
From:      Jamie Gritton <jamie@FreeBSD.org>
To:        Mateusz Guzik <mjguzik@gmail.com>
Cc:        Mike Jakubik <mike.jakubik@intertainservices.com>, "freebsd-stable@freebsd.org" <freebsd-stable@FreeBSD.org>
Subject:   Re: Jail startup/shutdown broken on latest 9-STABLE
Message-ID:  <4FBEF0CB.8090005@FreeBSD.org>
In-Reply-To: <20120525012326.GA27314@dft-labs.eu>
References:  <1337887134.1908.20.camel@mike-PC> <20120524212219.GA17579@dft-labs.eu> <1337897210.1908.24.camel@mike-PC> <20120524221353.GB17579@dft-labs.eu> <1337898015.1908.27.camel@mike-PC> <20120524223004.GC17579@dft-labs.eu> <4FBEBA5C.7050804@FreeBSD.org> <20120524225931.GD17579@dft-labs.eu> <4FBED6A2.2060602@FreeBSD.org> <20120525012326.GA27314@dft-labs.eu>

next in thread | previous in thread | raw e-mail | index | archive | help
On 05/24/12 19:23, Mateusz Guzik wrote:
> On Thu, May 24, 2012 at 06:47:30PM -0600, Jamie Gritton wrote:
>> On 05/24/12 16:59, Mateusz Guzik wrote:
>>> On Thu, May 24, 2012 at 04:46:52PM -0600, Jamie Gritton wrote:
>>>> I'll get the patch to jail(8) in - thanks for catching that. But I
>>>> wonder about the patch to /etc/rc.d/jail. It looks correct, but I'm
>>>> going to see if it's /etc/rc.d/jail that needs changing, or if my recent
>>>> changes to jail(8) have changed the order in which things are written.
>>>>
>>>
>>> Yeah, was not sure whether I should change the order or the script. :)
>>>
>>> Would not it be better to just create empty persistent jail as first step?
>>> Since in this case only one line will be generated (jid), rc.d script
>>> will be able to just take the output - this seems much less fragile
>>> than the current method. Then of course it would proceed with jexec
>>> running /etc/rc and in the end drop persist flag.
>>>
>>> It looks like rc.d script still uses old syntax so this actually may be
>>> less trivial than it sounds. That being said, if this is idea sounds
>>> okay, I can try to come up with a patch this weekend.
>>
>>
>> There definitely is a difference between old and new jail behavior, not
>> just in the order of things:
>>
>> 	glorfindel# jail -i -c name=foo command="foo"
>> 	5
>> 	jail: execvp: foo: No such file or directory
>>
>> vs
>>
>> 	glorfindel# /usr/obj`pwd`/jail -i -c name=foo command="foo"
>> 	jail: exec foo: No such file or directory
>> 	jail: foo: failed
>> 	-1
>>
>> The jail id given back used to correspond to a jail that was created but
>> no longer exists by the time it's printed (or shortly thereafter). Now
>> it's a -1 indicating that no jail exists. I think the -1 is more
>> correct, but perhaps better for CURRENT but not STABLE? And the extra
>> "foo: failed" is printed by jail, as a generic message when a command
>> doesn't work out (for the case where the command itself doesn't print
>> a message).
>>
>> Hmm ... I'll be pondering this one while I get a bite to eat :-).
>>
>
> Note that my proposal with persistent jail creation as first step doesn't
> conflict with new behavior of jail(8) regarding jid printing.
>
> Also, I think that head ->  tail change for rc.d script that I proposed is
> broken.
>
> I think that as a short-term solution you should restore old behavior
> (jid printed before everything else) for -STABLE. The reason is that
> currently you have to take the jid from the last line, but you cannot be
> sure that jailed processes didn't mess with the output, so you actually
> cannot trust the last line, so you don't know the actual jid.
>
> That is:
> rc.d/jail reads the last line
>
> jail(8) just writes jid to very same file that contains output of jailed
> /etc/rc. So if the last line written by jailed processes doesn't end with
> newline character, jail(8) will just append jid to the line, so
> the actual content of /var/run/jail_foo.id will consist of characters
> written by possibly malicious script and jid.
>
> This could be used to store jid of another jail (assuming jid 2, /etc/rc
> consisting of echo -n 1 would result in stored jid 12 etc.) or random
> content that in some conditions could lead to code execution.

I think I should restore the old behavior for CURRENT as well. The -i
option really only exists for /etc/rc.d/jail, and should behave the way
it expects it to. And if anything else uses it, all the more reason not
to change it.

- Jamie



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4FBEF0CB.8090005>