From owner-freebsd-stable@FreeBSD.ORG Fri May 25 01:23:33 2012 Return-Path: Delivered-To: freebsd-stable@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 009F3106564A; Fri, 25 May 2012 01:23:33 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-we0-f182.google.com (mail-we0-f182.google.com [74.125.82.182]) by mx1.freebsd.org (Postfix) with ESMTP id 5BFA08FC17; Fri, 25 May 2012 01:23:32 +0000 (UTC) Received: by werg1 with SMTP id g1so323932wer.13 for ; Thu, 24 May 2012 18:23:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=6AWsls2g+YZKUXpV7Mbnz1G6tNrd56qkvQ1cU5D2zaA=; b=noHHPLiro5GUYrIiaEAo7yAVnvkuZGQRZ1wr+L3PSGBcN8xNx/NJuTIeWDoQ8jNq43 wctGW+HNE5C/6DfwzfdF9JF+9iQXE+fz87z6wJZYPDbfFFOY6PgBI+udDyDuZyqz5Wil TnR1ty6nZd6zGAQvGmNXGOoYX3TWYJLL3rVNj8jXBpnsRIvLL9ZGBnrCXZeY626fB+GP UnRTT3biZ+9FwAkFk6vLXkG8tiWkdR6gdQxgGhUaW/qm/LWYSVvYSlH8xqkQQBXgBo+D OR7/j3OKhSP9nn60YY3kZP71Qzl47OE4yVfMBP23WanHmMerWXkEKoSJTQTngykssSVG KpeQ== Received: by 10.180.85.129 with SMTP id h1mr3074373wiz.2.1337909011385; Thu, 24 May 2012 18:23:31 -0700 (PDT) Received: from dft-labs.eu (dft-labs.eu. [80.87.128.179]) by mx.google.com with ESMTPS id dg2sm14379051wib.4.2012.05.24.18.23.29 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 24 May 2012 18:23:30 -0700 (PDT) Date: Fri, 25 May 2012 03:23:27 +0200 From: Mateusz Guzik To: Jamie Gritton Message-ID: <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> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <4FBED6A2.2060602@FreeBSD.org> User-Agent: Mutt/1.5.20 (2009-06-14) 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 01:23:33 -0000 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. -- Mateusz Guzik