Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 29 Oct 2002 15:01:57 -0500 (EST)
From:      Andriy Gapon <avg@icyb.net.ua>
To:        Luigi Rizzo <rizzo@icir.org>
Cc:        FreeBSD-gnats-submit@FreeBSD.ORG, freebsd-ipfw@FreeBSD.ORG
Subject:   Re: kern/44417: ipfw layer2 rules are not checked for ether_output_frame() on bridged interface
Message-ID:  <20021029140617.R5584-100000@edge.foundation.invalid>
In-Reply-To: <20021029095521.A12933@carp.icir.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 29 Oct 2002, Luigi Rizzo wrote:

> let me think about it, i am not 100% sure that it is the correct fix.
> Could you summarise the bug and the logic for the fix ?

Luigi,

url for this PR is:
http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/44417

more detailed explanation of the problem and the proposed patch follows.
it appears that a packet going out through a bridged interface will never
be checked against ipfw rules in layer2 (this applies only to ipfw2 of
course).

I will be speaking about 4.7-RELEASE code.
In ether_output_frame():

    394         if (BDG_ACTIVE(ifp) ) {
    395                 struct ether_header *eh; /* a ptr suffices */
    396
    397                 m->m_pkthdr.rcvif = NULL;
    398                 eh = mtod(m, struct ether_header *);
    399                 m_adj(m, ETHER_HDR_LEN);
    400                 m = bdg_forward_ptr(m, eh, ifp);
    401                 if (m != NULL)
    402                         m_freem(m);
    403                 return (0);
    404         }

i.e. a packet is handed off to bdg_forward() at line 400 without any
ipfw checks and its rcvif is set to NULL at line 397.
Then in bdg_forward():

    853     /*
    854      * Do filtering in a very similar way to what is done in ip_output.
    855      * Only if firewall is loaded, enabled, and the packet is not
    856      * from ether_output() (src==NULL, or we would filter it twice).
    857      * Additional restrictions may apply e.g. non-IP, short packets,
    858      * and pkts already gone through a pipe.
    859      */
    860     if (src != NULL && (
    861 #ifdef PFIL_HOOKS
    862         ((pfh = pfil_hook_get(PFIL_IN, &inetsw[ip_protox[IPPROTO_IP]].pr_pfh)) != NULL && bdg_ipf !=0) ||
    863 #endif
    864         (IPFW_LOADED && bdg_ipfw != 0))) {


condition at line 860 makes sure that firewall rules are not checked here
too. That means that layer2-specific rules are never applied to the said
packet. The change to bridge.c was intended to fix that. I believe that it
shouldn't break something else.

I've made the change to if_ethersubr.c, because I didn't quite understand
what this code:

    386         /* Extract info from dummynet tag, ignore others */
    387         for (; m->m_type == MT_TAG; m = m->m_next)
    388                 if (m->m_flags == PACKET_TAG_DUMMYNET)
    389                         rule = ((struct dn_pkt *)m)->rule;
    390
    391         if (rule)       /* packet was already bridged */
    392                 goto no_bridge;

was doing. The condition appears to check if a packet was passed through a
pipe or queue, but the action is to by-pass bridge-specific code.
My vision was that the said packet should go again to bdg_forward(), where
it will be taken care of by:

    868         if (args.rule != NULL) /* packet already partially processed */
    869             goto forward; /* HACK! I should obey the fw_one_pass */

If the interface in question is not bridged, the patch should not make any
difference at all.

I have a system that has 3 interfaces, does bridging between two of them
and routing with the third one, and also limits traffic to/from a particular
host on one of the bridged LANs based on its MAC address. Everything seems
to work properly with this patch. I don't see any strange/unexpected
behaviour. But I understand that this fact alone is not a sufficient
proof, I'm not sure right now that I'm not doing an extra pass somewhere.

Hope this explains the problem and the idea for the patches.
Thanks a lot.

-- 
Andriy Gapon
*
"Never try to outstubborn a cat." Lazarus Long, "Time Enough for Love"



To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-ipfw" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20021029140617.R5584-100000>