settling serious conflicts between MAC and IPSEC
Robert Watson
rwatson at FreeBSD.org
Tue Mar 28 10:30:16 UTC 2006
On Tue, 28 Mar 2006, zhouyi zhou wrote:
> It is my pleasure, is any one willing to settle the mbuf without label
> initialized problem in function ipfw_tick? if there is none, I am willing to
> do it.
I took a quick glance at that one, need to look at it some more. The tricky
thing is figuring out what label to assign. ipfw_tick() appears to use
send_pkt() to generate keeplives; this function is also used to generate
RST's. In the RST case, we have existing MAC entry points to generate the
label of a replying RST to a TCP segment, and should use that. In the case
where a keepalive is spontaneously generated by the firewall rule, that's a
little more tricky. One possibility is that ipfw needs to learn about
associating MAC labels with IPFW dynamic rules. Another possibility is that
IPFW should use the a label assigned for spontaneously generated packets.
The former is certainly more complicated to implement, but is more what one
actually wants, whereas the latter is easy to implement, but means that the
keepalive might not actually be delivered to the end socket because the label
might not match. If you have time to look at this, that would be great -- I'm
pretty occupied for the next few days, and it would be very good to get a fix
into the RELENG_5 and RELENG_6 trees before their respective releases in the
next couple of weeks.
I'm not entirely sure why ipfw2 is generating keepalives -- normally, it
strikes me that that is something the two end hosts would do, not the
intermediate firewall.
Thanks,
Robert N M Watson
> On Tue, 28 Mar 2006 10:02:39 +0000 (GMT)
> Robert Watson <rwatson at FreeBSD.org> wrote:
>
>>
>> On Mon, 27 Mar 2006, zhouyi zhou wrote:
>>
>>> High everyone, there exists a serious bug in function ipsec_copypkt(m) of
>>> netinet6/ipsec.c in FreeBSD 5.4, FreeBSD 6.0 and FreeBSD 7.0
>>>
>>> 3469 MGETHDR(mnew, M_DONTWAIT, MT_HEADER);
>>> 3470 if (mnew == NULL)
>>> 3471 goto fail;
>>> 3472 mnew->m_pkthdr = n->m_pkthdr;
>>> 3473 #if 0
>>> 3474 /* XXX: convert to m_tag or delete? */
>>> 3475 if (n->m_pkthdr.aux) {
>>> 3476 mnew->m_pkthdr.aux =
>>> 3477 m_copym(n->m_pkthdr.aux,
>>> 3478 0, M_COPYALL, M_DONTWAIT);
>>> 3479 }
>>> 3480 #endif
>>> 3481 M_MOVE_PKTHDR(mnew, n);
>>>
>>> On line 3472, mnew->m_pkthdr is assigned n->m_pkthdr, and on line 3481, in
>>> function m_move_pkthdr, mnew's tag list will be delete (and the n's tag of
>>> cause). This will cause system to crash.
>>>
>>> After commenting out line 3472, everything is OK.
>>
>> Thanks for this report! The M_MOVE_PKTHDR() should do all the necessary work,
>> including copying the fields referenced in 3472, as well as handling existing
>> m_tags right. I've attached a patch with your proposal, which looks and
>> sounds good to me, and CC'd George and Bjoern in the hopes that one of them
>> will give it a node of approval before I commit it -- hopefully we can get
>> this MFC'd for 6.1-RELEASE.
>>
>> Robert N M Watson
>>
>> Index: ipsec.c
>> ===================================================================
>> RCS file: /home/ncvs/src/sys/netinet6/ipsec.c,v
>> retrieving revision 1.43
>> diff -u -r1.43 ipsec.c
>> --- ipsec.c 25 Jul 2005 12:31:42 -0000 1.43
>> +++ ipsec.c 28 Mar 2006 09:58:54 -0000
>> @@ -3469,15 +3469,6 @@
>> MGETHDR(mnew, M_DONTWAIT, MT_HEADER);
>> if (mnew == NULL)
>> goto fail;
>> - mnew->m_pkthdr = n->m_pkthdr;
>> -#if 0
>> - /* XXX: convert to m_tag or delete? */
>> - if (n->m_pkthdr.aux) {
>> - mnew->m_pkthdr.aux =
>> - m_copym(n->m_pkthdr.aux,
>> - 0, M_COPYALL, M_DONTWAIT);
>> - }
>> -#endif
>> M_MOVE_PKTHDR(mnew, n);
>> }
>> else {
>>
>
More information about the freebsd-bugs
mailing list