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
_______________________________________________
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"
