On Fri, Apr 26, 2002 at 12:16:20PM +0300, Ruslan Ermilov wrote: > On Sun, Apr 14, 2002 at 06:04:47PM +0800, Igor M Podlesny wrote: > > > > Hello! > > > > I'd like to know your opinion about this patch > > > > http://www.morning.ru/~poige/patchzone/ingressfiltering.patch > > > > which is mine attempt to implement an ingress filter being inspired by > > RFC2827 "Network Ingress Filtering: Defeating Denial of Service Attacks > > which employ IP Source Address Spoofing". > > > > (http://www.ietf.org/rfc/rfc2827.txt) > > > > It should be mentioned IMHO that this code makes another one in ip_input.c a > > kind of redundant -- I mean code checking/blocking the 127/8 network "on > > wire". BTW, I suggest if not removing it completely then adding (sys)logging > > into, -- 127/8-spoofing certainly should be logged. :) > > > > Another thing to pay an attention to: I deem it'd be better if a such filter > > was built-in into ip_fw.c, allowing such syntax for ipfw(8): > > > > deny log ip from any to any in via fxp0 spoofed > > > > But AFAIS in ip_fw.h: > > > > #define IP_FW_F_IN 0x00000100 > > ... > > #define IP_FW_F_DME 0x40000000 /* destination = me */ > > > > #define IP_FW_F_MASK 0x7FFFFFFF /* All possible flag bits mask */ > > > > and u_int32_t fw_flg; > > > > there is no free space for any additional flags... > > > > So, I was a bit unsure whether should I expand fw_flg to u_int64_t, and do > > any other extensions. For now I decided just to wrote something like a > > draft, test it (it seems to be working ;), and asking you, people, for your > > comments/ideas on it. > > > > P.S. A bit more info on this patch is at http://www.morning.ru/~poige/patchzone/ > >
At first, thank you, Ruslan for your answer, it's quite fertile! > Style comments: I should had read the manual or just had known it by heart before writing out the patch in case I was a commiter going to commit it into the repository. But I'm not. Moreover, I don't suggest using _this_ code AS IS in FreeBSD. I said before -- this is a draft. Other people also agree this should be better done (ingress filtering I mean) at ip_fw.c not ip_input.c. So yeah, it works someway, but again -- it's a draft. ;) Well, thank you for time you spent telling me (us all, reading that) about style, but I think that was done in a wrong time. And just one question to you (if we're anyway have been talking about the style) as to a really knowledgeable person: is the whole FreeBSD's kernel code is style(9)d from A to Z?... :) > 1. There are many unnecessary whitespace changes. (I consider using whitespaces similar to commenting, BTW. It's a good C-style I heard. ;)) > 2. Don't use the `register' keyword. (Haven't found anything bout it in style(9)...) > 3. Double `const' doesn't do any good. (I was once confused about this too.) (const char *const ptr? Why? I deem `const' can't make a code worse, only better, cause it makes an additional description of variables/functions/code/algo...) > 4. ip_fw.c part of the patch has some cruft in it. Namely what/where? > Functional comments: > > 1. The use and externalization of ipfw_report() wasn't a good > idea. Your patch makes ingressfilter dependent on `options > IPFIREWALL' because ip_fw.c is only compiled if this option > is present. Not such a big deal cause this can be easily solved in various ways dependent on what we want... (yeah, that's again about `draft'-approach :) > 2. Comment for ipf_rtaddr() is bogus -- the function returns > the pointer to an interface not address. (A pointer is a value keeping an address. Am I wrong?) > 3. Function name is not good either, the more natural name > would be ip_rtifp(). I would also suggest reimplementing > the already existing ip_rtaddr() into ip_rtifp(), and > implementing ip_rtaddr() in terms of ip_rtifp(). (This'd be a good thing, I also thought about it, but this wasn't the main point of the patch -- the main idea stills be the same and it is ingress filtering. :)) > General notes: > > Ingress filtering in unacceptable in many cases. And is acceptable in many others :) Don't you agree with that? > For example, > our site is connected to two ISPs, ISP-A and ISP-B. Each ISP > has allocated a network (NET-A and NET-B). Both channels are > connected to a single gateway box, and are reachable through > interfaces IF-A and IF-B, respectively. The `default' route > on this box point to ISP-A through IF-A. > > Now imagine that you want to ping(8) one of our addresses in > NET-B. This packet will appear on our gateway box through > IF-B, but ingress filter would discard it because ip_rtifp() > lookup would return the IF-A interface for your address > (ip_src in the packet). > We solve the problem with multiple default routes with `ipfw > fwd'. All outgoing packets with the source IP address in > NET-B we forward through the IF-B interface. 1) in case of ip_fw.c integrated version you could easily specify not using ingress filtering on such NICs. 2) this patch _AS_ _IS_ is already useful for both back bone routers making up an internal network infrastructure and border gateways with single _default_ route. I wrote about asymmetric routing incompatible with the patch, and this point is quite similar to the situation, represented by you. > Cheers, P.S. I'd be very grateful if you could point out reasonable ways of integration ingress filter with ip_fw.c... Or you consider this is not worth doing at all?... -- Igor M Podlesny a.k.a. Poige http://WwW.MorninG.RU/~poige To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-net" in the body of the message
