From owner-freebsd-hackers Sun Jul 8 5:45:26 2001 Delivered-To: freebsd-hackers@freebsd.org Received: from roulen-gw.morning.ru (roulen-gw.morning.ru [195.161.98.242]) by hub.freebsd.org (Postfix) with ESMTP id 2CA8737B401; Sun, 8 Jul 2001 05:45:14 -0700 (PDT) (envelope-from poige@morning.ru) Received: from NIC1 (seven.ld [192.168.11.7]) by roulen-gw.morning.ru (Postfix) with ESMTP id CF9773D; Sun, 8 Jul 2001 20:45:09 +0800 (KRAST) Date: Sun, 8 Jul 2001 20:45:27 +0800 From: Igor Podlesny X-Mailer: The Bat! (v1.52 Beta/7) UNREG / CD5BF9353B3B7091 Organization: Morning Network X-Priority: 3 (Normal) Message-ID: <335722238.20010708204527@morning.ru> To: freebsd-isp@FreeBSD.ORG Cc: freebsd-hackers@FreeBSD.ORG Subject: Re: Flight of the rat, living wreck..... In-Reply-To: <1595443006.20010630190139@morning.ru> References: <1595443006.20010630190139@morning.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Sender: owner-freebsd-hackers@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG > Hello everybody! > This is relative to 4.3 for yet ;) so if you're using something older > you can skip it easily. well, I came up with a patch (http://www.morning.ru/~poige/patchzone/ip_fw.c.patch) > How it was started > ------------------ > For a long time I've been looking forward (and even trying to learn > freebsd internals enough to implement it by myself :) for newly > implemented ipfw's feature allowing easy filtering of non-transit > ip-packets, i.e., packets with destination address of one of the > interfaces. (You know in Linux it is done now with netfilter, which > separates ip flow into 3 different chains, BSDi's ipfw looks like a > programming language :) which allows such things for ages, if I'm not > mistaken ;). In short -- the feature is cool, and I get prepared to > start using it. At first it seemed to be okay, I felt security > comparable to "deny ip from any to any" ;)), but than, noticed that > something was going wrong. > And this was with Point-to-point interfaces. Everything was as if > remote peer ip-address matched 'me'. It's certainly wrong as far as I > can guess, so after applying fixes to my IPFW's rules allowing easy > going (passing) for packets to such addresses I started digging the > code. > ip_fw.c looks okay, but in_var.h with its INADDR_TO_IFP definition > which is a core for 'me'-feature >> if (f->fw_flg & IP_FW_F_SME) { >> INADDR_TO_IFP(src_ip, tif); >> if (tif == NULL) >> continue; >> } >> if (f->fw_flg & IP_FW_F_DME) { >> INADDR_TO_IFP(dst_ip, tif); >> if (tif == NULL) >> continue; > doesn't: >> /* >> * Macro for finding the interface (ifnet structure) corresponding to one >> * of our IP addresses. >> */ >> #define INADDR_TO_IFP(addr, ifp) \ >> /* struct in_addr addr; */ \ >> /* struct ifnet *ifp; */ \ >> { \ >> register struct in_ifaddr *ia; \ >> \ >> for (ia = in_ifaddrhead.tqh_first; \ > // so here we start looking through the queue >> ia != NULL > // sanity (I'd have written just (ia)) >> && ((ia->ia_ifp->if_flags & IFF_POINTOPOINT)? \ > // hm. special case if the interface is PTP >> IA_DSTSIN(ia):IA_SIN(ia))->sin_addr.s_addr != (addr).s_addr; \ > // so it is like: if it is PTP, then we using DST address in comparison > // with addr.s_addr > // it is the time I started to ask myself why it is so? why we're (ok, > // they're) checking for remote ip-address if the head comment > // says: > // * Macro for finding the interface (ifnet structure) corresponding to one > // * of our IP addresses. > // ^^^ > // ^^^ >> ia = ia->ia_link.tqe_next) \ >> continue; \ > // as it's seen, the algo is: checking addresses of our ifaces or > // our remote ends in case of PTP until we get the matching or reach the end > // this is like vice versa: looking through the queue for exact matching > // and in case only ia is NULL after the first search. Also, this > // it's taking into consideration only PTP interfaces and only local > // addresses of them. >> if (ia == NULL) \ >> for (ia = in_ifaddrhead.tqh_first; \ >> ia != NULL; \ >> ia = ia->ia_link.tqe_next) \ >> if (ia->ia_ifp->if_flags & IFF_POINTOPOINT && \ >> IA_SIN(ia)->sin_addr.s_addr == (addr).s_addr) \ >> break; \ > // the terminator: if we have found something we would come up with > // ia_ifp, or with NULL at least. >> (ifp) = (ia == NULL) ? NULL : ia->ia_ifp; \ >> } > Now, getting down to IPFW's 'me'-keyword business: > IMHO, it breaks the sense in this way: > on first cycle-pass, the matching is found and ia isn't NULL. so the > second is skipped. and we got the matching, although we shouldn't. > I deem this is wrong. > Now, in conclusion > ------------------ > I'm a man who hasn't very deep knowledge of the BSD's bones, still be > learning it. So I can't say that the code INADDR_TO_IFP is completely > wrong because of lack of knowledge and all I say is just it doesn't > fit the purpose of IPFW's 'me'-keyword and the solution is to avoid > using it there. > Your ideas and opinions are really appreciated. > Good luck everybody and thank you in advance. -- Igor mailto:poige@morning.ru To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-hackers" in the body of the message