From owner-freebsd-stable@FreeBSD.ORG Fri May 25 02:39:12 2012 Return-Path: Delivered-To: freebsd-stable@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 336EE1065670 for ; Fri, 25 May 2012 02:39:12 +0000 (UTC) (envelope-from jamie@FreeBSD.org) Received: from m2.gritton.org (gritton.org [199.192.164.235]) by mx1.freebsd.org (Postfix) with ESMTP id 076D58FC0C for ; Fri, 25 May 2012 02:39:11 +0000 (UTC) Received: from glorfindel.gritton.org (c-174-52-130-208.hsd1.ut.comcast.net [174.52.130.208]) (authenticated bits=0) by m2.gritton.org (8.14.5/8.14.5) with ESMTP id q4P2d9Qc072990; Thu, 24 May 2012 20:39:09 -0600 (MDT) (envelope-from jamie@FreeBSD.org) Message-ID: <4FBEF0CB.8090005@FreeBSD.org> Date: Thu, 24 May 2012 20:39:07 -0600 From: Jamie Gritton User-Agent: Mozilla/5.0 (X11; U; FreeBSD amd64; en-US; rv:1.9.2.24) Gecko/20120129 Thunderbird/3.1.16 MIME-Version: 1.0 To: Mateusz Guzik 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> In-Reply-To: <20120525012326.GA27314@dft-labs.eu> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Mike Jakubik , "freebsd-stable@freebsd.org" Subject: Re: Jail startup/shutdown broken on latest 9-STABLE X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 May 2012 02:39:12 -0000 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