Date: Sun, 8 Jul 2001 20:45:27 +0800 From: Igor Podlesny <poige@morning.ru> To: freebsd-isp@FreeBSD.ORG Cc: freebsd-hackers@FreeBSD.ORG Subject: Re: Flight of the rat, living wreck..... Message-ID: <335722238.20010708204527@morning.ru> In-Reply-To: <1595443006.20010630190139@morning.ru> References: <1595443006.20010630190139@morning.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?335722238.20010708204527>