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>