Re: Improving FreeBSD-SA-07:01.jail fix [was: HEADS UP: Re: FreeBSD Security Advisory FreeBSD-SA-07:01.jail]
On 2007.01.13 12:29:37 +0100, Pawel Jakub Dawidek wrote: > On Thu, Jan 11, 2007 at 04:51:02PM -0800, Colin Percival wrote: > > Hello Everyone, > > > > I usually let security advisories speak for themselves, but I want to call > > special attention to this one: If you use jails, READ THE ADVISORY, in > > particular the "NOTE WELL" part below; and if you have problems after > > applying > > the security patch, LET US KNOW -- we do everything we can to make sure > > that security updates will never cause problems, but in this case we could > > not fix the all of the security issues without either making assumptions > > about how systems are configured or reducing functionality. > > > > In the end we opted to reduce functionality (the jail startup process is > > no longer logged to /var/log/console.log inside the jail), make an > > assumption > > about how systems are configured (filesystems which are mounted via per-jail > > fstab files should not be mounted on symlinks -- if you do this, adjust your > > fstab files to give the real, non-symlinked, path to the mount point), and > > leave a potential security problem unfixed (if you mount any filesystems via > > per-jail fstab files on mount points which are visible within multiple > > jails, > > there are problems -- don't do this). So, I have been putting off replying to this thread, but I guess it seems like I should reply... :-) > I don't like the way it was fixed. I do know it wasn't easy to fix. I don't like it either, but it was the best of bad solutions. My hope while developing the patch, and cursing computers in general :-), was that after the Security Advisory went out somebody would implement a fix which sucks less possibly by modifying some of the support tools. Your suggestion with modifying realpath to use chroot(2) certainly sounds like it could work, but I haven't thought about it in great detail if there are problems. The Security Team does not hold a lock on trying to improve the fix in src/etc/rc.d/jail, but anyone that does change the fix from the Security Advisory should be really really really really (did I mention "really"?) sure the fix is safe and have at least a few people with security clue review patches. It is very easy to get this wrong (my first patch did). Also, whatever fix is made should be in -CURRENT for a while (3 weeks min. IMO) before being MFC'ed, both because it gives more time for people to think about the fix and because -CURRENT isn't supported wrt. security issues, so if the fix is wrong we don't have to issue an advisory. BTW. with regard to the console.log file I really don't think it should be put back inside the jail unless it's possible to make the generation of the file entirely inside the jail since it's just not worth the risk/complexity. I think it should be possible to do this with jail(8) in -CURRENT (see -J flag), but: Note that it will probably be at least a couple of weeks before I feel like going anywhere near the jail rc.d script again (except for the warning comment I plan to add...), so don't wait for me with regard to improving this. And in case anyone were in doubt: Computers still suck :-). -- Simon L. Nielsen ___ freebsd-stable@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-stable To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: Improving FreeBSD-SA-07:01.jail fix [was: HEADS UP: Re: FreeBSD Security Advisory FreeBSD-SA-07:01.jail]
On Sat, Jan 20, 2007 at 01:24:33PM +0100, Simon L. Nielsen wrote: [...] > BTW. with regard to the console.log file I really don't think it > should be put back inside the jail unless it's possible to make the > generation of the file entirely inside the jail since it's just not > worth the risk/complexity. I think it should be possible to do this > with jail(8) in -CURRENT (see -J flag), but: When -J operates on a file inside a jail, it create the same security hole as the one from security advisory, because it opens a file before calling jail(2). I fully agree that console.log should be outside a jail. At least noone proposed safe solution so far, which also means it's not an easy fix. -- Pawel Jakub Dawidek http://www.wheel.pl [EMAIL PROTECTED] http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! pgp1bmSk2cQlL.pgp Description: PGP signature
Re: Improving FreeBSD-SA-07:01.jail fix [was: HEADS UP: Re: FreeBSD Security Advisory FreeBSD-SA-07:01.jail]
On 2007.01.20 14:03:08 +0100, Pawel Jakub Dawidek wrote: > On Sat, Jan 20, 2007 at 01:24:33PM +0100, Simon L. Nielsen wrote: > [...] > > BTW. with regard to the console.log file I really don't think it > > should be put back inside the jail unless it's possible to make the > > generation of the file entirely inside the jail since it's just not > > worth the risk/complexity. I think it should be possible to do this > > with jail(8) in -CURRENT (see -J flag), but: > > When -J operates on a file inside a jail, it create the same security > hole as the one from security advisory, because it opens a file before > calling jail(2). My thought with using -J was not place the info about jid in a file outside the jail root, basically (pseudo code): _tmpfile=`mktemp...` jail -J $_tmpfile "sh /etc/rc > /var/log/console.log" _jid=`cat $_tmpfile | something` At least that was what I thought might be possible with the -J switch when I noticed it existed. In any case, actually coding this, verifying that it works and is safe is left up to anyone who cares about having console.log inside the jail. -- Simon L. Nielsen ___ freebsd-stable@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-stable To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: Improving FreeBSD-SA-07:01.jail fix [was: HEADS UP: Re: FreeBSD Security Advisory FreeBSD-SA-07:01.jail]
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Pawel Jakub Dawidek wrote: > When -J operates on a file inside a jail, it create the same security > hole as the one from security advisory, because it opens a file before > calling jail(2). > I fully agree that console.log should be outside a jail. At least noone > proposed safe solution so far, which also means it's not an easy fix. I still suggest using "pwd -P" to get the real path and using the shell's CWD as a lock. That works safely with mount(8) at least. Comments? erdgeist -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.3 (Darwin) iD8DBQFFsiGzImmQdUyYEgkRAlKcAJ4izD1J4x6jDDfvrtr5J+bcmSxK/ACfRpwn x5yVH4uJIN7CWEgYtATKDE0= =sQq3 -END PGP SIGNATURE- ___ freebsd-stable@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-stable To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: Improving FreeBSD-SA-07:01.jail fix [was: HEADS UP: Re: FreeBSD Security Advisory FreeBSD-SA-07:01.jail]
Quoting Pawel Jakub Dawidek <[EMAIL PROTECTED]> (Sat, 20 Jan 2007 14:03:08
+0100):
> I fully agree that console.log should be outside a jail. At least noone
> proposed safe solution so far, which also means it's not an easy fix.
What's unsafe about my proposal? I did had a look at the code now, and
it should work (with minor mods).
Original:
---snip---
_tmp_jail=${_tmp_dir}/jail.$$
eval jail ${_flags} -i ${_rootdir} ${_hostname} \
${_ip} ${_exec_start} > ${_tmp_jail} 2>&1
if [ "$?" -eq 0 ] ; then
_jail_id=$(head -1 ${_tmp_jail})
i=1
while [ true ]; do
eval out=\"\${_exec_afterstart${i}:-''}\"
if [ -z "$out" ]; then
break;
fi
jexec "${_jail_id}" ${out}
i=$((i + 1))
done
echo -n " $_hostname"
tail +2 ${_tmp_jail} >${_consolelog}
echo ${_jail_id} > /var/run/jail_${_jail}.id
---snip---
Pseudocode proposal, not tested (changes prefixed with 'x'):
---snip---
_tmp_jail=${_tmp_dir}/jail.$$
x # assuming safe _consolelog (inside chroot) according
to the
x # previous mails here in the thread
x eval (echo "" ; \
x jail ${_flags} -I /var/run/jail_${_jail}.id \
x ${_rootdir} ${_hostname} {_ip} ${_exec_start}) \
x > ${_consolelog} 2>&1
if [ "$?" -eq 0 ] ; then
x _jail_id=$(cat /var/run/jail_${_jail}.id)
i=1
while [ true ]; do
eval out=\"\${_exec_afterstart${i}:-''}\"
if [ -z "$out" ]; then
break;
fi
jexec "${_jail_id}" ${out}
i=$((i + 1))
done
echo -n " $_hostname"
x
x
---snip---
Repeating my points:
- sanitize the consolelog path like discussed in this thread
- the jail is not running, so nobody can create a link (jail
root within FS space of another jail still prohibited)
- subshell to group echo and jail
- 'echo ""' to make sure the file exists when the jail starts
- (new) additional flag to jail to write a jid file
- redirect to the consolelog, it is still open from the echo
when the jail starts so there's no race
I did test "(echo 1; sleep 60 ; echo 2) >/tmp/test" in /bin/sh, and it
is line buffered, so the above works.
Where's the security problem in the above?
Bye,
Alexander.
--
I wore my extra loose pants for nothing. Nothing!
-- Homer Simpson
New Kid on the Block
http://www.Leidinger.net Alexander @ Leidinger.net: PGP ID = B0063FE7
http://www.FreeBSD.org netchild @ FreeBSD.org : PGP ID = 72077137
___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: Improving FreeBSD-SA-07:01.jail fix [was: HEADS UP: Re: FreeBSD Security Advisory FreeBSD-SA-07:01.jail]
On Sat, Jan 20, 2007 at 03:24:23PM +0100, Alexander Leidinger wrote:
> Quoting Pawel Jakub Dawidek <[EMAIL PROTECTED]> (Sat, 20 Jan 2007 14:03:08
> +0100):
>
> > I fully agree that console.log should be outside a jail. At least noone
> > proposed safe solution so far, which also means it's not an easy fix.
>
> What's unsafe about my proposal? I did had a look at the code now, and
> it should work (with minor mods).
>
> Original:
> ---snip---
> _tmp_jail=${_tmp_dir}/jail.$$
> eval jail ${_flags} -i ${_rootdir} ${_hostname} \
> ${_ip} ${_exec_start} > ${_tmp_jail} 2>&1
>
> if [ "$?" -eq 0 ] ; then
> _jail_id=$(head -1 ${_tmp_jail})
> i=1
> while [ true ]; do
> eval out=\"\${_exec_afterstart${i}:-''}\"
>
> if [ -z "$out" ]; then
> break;
> fi
>
> jexec "${_jail_id}" ${out}
> i=$((i + 1))
> done
>
> echo -n " $_hostname"
> tail +2 ${_tmp_jail} >${_consolelog}
> echo ${_jail_id} > /var/run/jail_${_jail}.id
> ---snip---
>
> Pseudocode proposal, not tested (changes prefixed with 'x'):
> ---snip---
> _tmp_jail=${_tmp_dir}/jail.$$
> x # assuming safe _consolelog (inside chroot) according
> to the
> x # previous mails here in the thread
> x eval (echo "" ; \
> x jail ${_flags} -I /var/run/jail_${_jail}.id \
> x ${_rootdir} ${_hostname} {_ip} ${_exec_start}) \
> x > ${_consolelog} 2>&1
>
> if [ "$?" -eq 0 ] ; then
> x _jail_id=$(cat /var/run/jail_${_jail}.id)
> i=1
> while [ true ]; do
> eval out=\"\${_exec_afterstart${i}:-''}\"
>
> if [ -z "$out" ]; then
> break;
> fi
>
> jexec "${_jail_id}" ${out}
> i=$((i + 1))
> done
>
> echo -n " $_hostname"
> x
> x
> ---snip---
>
> Repeating my points:
> - sanitize the consolelog path like discussed in this thread
> - the jail is not running, so nobody can create a link (jail
>root within FS space of another jail still prohibited)
> - subshell to group echo and jail
> - 'echo ""' to make sure the file exists when the jail starts
> - (new) additional flag to jail to write a jid file
> - redirect to the consolelog, it is still open from the echo
>when the jail starts so there's no race
>
> I did test "(echo 1; sleep 60 ; echo 2) >/tmp/test" in /bin/sh, and it
> is line buffered, so the above works.
>
> Where's the security problem in the above?
It looks like it may work, but I still find it a bit risky. If sh(1) can
reopen the file under some conditions or someone in the future will
modify sh(1) in that way (because he won't be aware that such a change
may have impact on system security) we will have a security hole.
Chances are small, but I'm not going to be the one who will accept that
change:)
--
Pawel Jakub Dawidek http://www.wheel.pl
[EMAIL PROTECTED] http://www.FreeBSD.org
FreeBSD committer Am I Evil? Yes, I Am!
pgpxJyediGS02.pgp
Description: PGP signature
Re: Improving FreeBSD-SA-07:01.jail fix [was: HEADS UP: Re: FreeBSD Security Advisory FreeBSD-SA-07:01.jail]
Quoting Pawel Jakub Dawidek <[EMAIL PROTECTED]> (from Tue, 23 Jan 2007
12:34:44 +0100):
On Sat, Jan 20, 2007 at 03:24:23PM +0100, Alexander Leidinger wrote:
Quoting Pawel Jakub Dawidek <[EMAIL PROTECTED]> (Sat, 20 Jan 2007
14:03:08 +0100):
> I fully agree that console.log should be outside a jail. At least noone
> proposed safe solution so far, which also means it's not an easy fix.
What's unsafe about my proposal? I did had a look at the code now, and
it should work (with minor mods).
Original:
---snip---
_tmp_jail=${_tmp_dir}/jail.$$
eval jail ${_flags} -i ${_rootdir} ${_hostname} \
${_ip} ${_exec_start} > ${_tmp_jail} 2>&1
if [ "$?" -eq 0 ] ; then
_jail_id=$(head -1 ${_tmp_jail})
i=1
while [ true ]; do
eval out=\"\${_exec_afterstart${i}:-''}\"
if [ -z "$out" ]; then
break;
fi
jexec "${_jail_id}" ${out}
i=$((i + 1))
done
echo -n " $_hostname"
tail +2 ${_tmp_jail} >${_consolelog}
echo ${_jail_id} > /var/run/jail_${_jail}.id
---snip---
Pseudocode proposal, not tested (changes prefixed with 'x'):
---snip---
_tmp_jail=${_tmp_dir}/jail.$$
x # assuming safe _consolelog (inside chroot) according
to the
x # previous mails here in the thread
x eval (echo "" ; \
x jail ${_flags} -I /var/run/jail_${_jail}.id \
x ${_rootdir} ${_hostname} {_ip} ${_exec_start}) \
x > ${_consolelog} 2>&1
if [ "$?" -eq 0 ] ; then
x _jail_id=$(cat /var/run/jail_${_jail}.id)
i=1
while [ true ]; do
eval out=\"\${_exec_afterstart${i}:-''}\"
if [ -z "$out" ]; then
break;
fi
jexec "${_jail_id}" ${out}
i=$((i + 1))
done
echo -n " $_hostname"
x
x
---snip---
Repeating my points:
- sanitize the consolelog path like discussed in this thread
- the jail is not running, so nobody can create a link (jail
root within FS space of another jail still prohibited)
- subshell to group echo and jail
- 'echo ""' to make sure the file exists when the jail starts
- (new) additional flag to jail to write a jid file
- redirect to the consolelog, it is still open from the echo
when the jail starts so there's no race
I did test "(echo 1; sleep 60 ; echo 2) >/tmp/test" in /bin/sh, and it
is line buffered, so the above works.
Where's the security problem in the above?
It looks like it may work, but I still find it a bit risky. If sh(1) can
reopen the file under some conditions or someone in the future will
modify sh(1) in that way (because he won't be aware that such a change
may have impact on system security) we will have a security hole.
Chances are small, but I'm not going to be the one who will accept that
change:)
The spawned subshell is like a command. It doesn't make sense to
reopen the file for a command. It's like saying we open and close the
file for each line. I didn't calculated the probability of this to
happen, but I would be very surprised if it is significant. Just think
about the performance of such behavior (or a more complex logic which
open()/close()es in a more complex way). And if you think about such
unlikely stuff to happen, you should also think about some other stuff
we are not prepared to survive. But feel free to propose a better
solution for the problem.
Bye,
Alexander.
--
In Newark the laundromats are open 24 hours a day!
http://www.Leidinger.netAlexander @ Leidinger.net: PGP ID = B0063FE7
http://www.FreeBSD.org netchild @ FreeBSD.org : PGP ID = 72077137
___
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: Improving FreeBSD-SA-07:01.jail fix [was: HEADS UP: Re: FreeBSD Security Advisory FreeBSD-SA-07:01.jail]
On Tue, Jan 23, 2007 at 01:25:08PM +0100, Alexander Leidinger wrote: > Quoting Pawel Jakub Dawidek <[EMAIL PROTECTED]> (from Tue, 23 Jan 2007 > 12:34:44 +0100): > >It looks like it may work, but I still find it a bit risky. If sh(1) can > >reopen the file under some conditions or someone in the future will > >modify sh(1) in that way (because he won't be aware that such a change > >may have impact on system security) we will have a security hole. > >Chances are small, but I'm not going to be the one who will accept that > >change:) > > The spawned subshell is like a command. It doesn't make sense to reopen the > file for a command. It's like saying we open and close the file for each > line. I didn't > calculated the probability of this to happen, but I would be very surprised > if it is significant. Just think about the performance of such behavior (or a > more complex logic > [...] And if you think about such unlikely stuff to happen, you should also > think about some other stuff we are not prepared to > survive. [...] Come on, this argument always stands. I only wanted to point out that we should be extra careful with building security on top of tools that are not intended for this purpose. > [...] But feel free to propose a better solution for the problem. The solution was proposed already - keep console.log outside of jail. Don't read my comment as a "no" vote for your solution. If secteam@ decide there is nothing to be worry about - fine by me. -- Pawel Jakub Dawidek http://www.wheel.pl [EMAIL PROTECTED] http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! pgpq64ac0jGwG.pgp Description: PGP signature
