Improving FreeBSD-SA-07:01.jail fix [was: HEADS UP: Re: FreeBSD Security Advisory FreeBSD-SA-07:01.jail]

[Alexander Leidinger]

Quoting Pawel Jakub Dawidek <pjd at FreeBSD.org> (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 <pjd at FreeBSD.org> (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.net    Alexander @ Leidinger.net: PGP ID = B0063FE7
http://www.FreeBSD.org       netchild @ FreeBSD.org  : PGP ID = 72077137