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/ > Style comments:
1. There are many unnecessary whitespace changes.
2. Don't use the `register' keyword.
3. Double `const' doesn't do any good. (I was once confused about this too.)
4. ip_fw.c part of the patch has some cruft in it.
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.
2. Comment for ipf_rtaddr() is bogus -- the function returns
the pointer to an interface not address.
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().
General notes:
Ingress filtering in unacceptable in many cases. 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.
Cheers,
--
Ruslan Ermilov Sysadmin and DBA,
[EMAIL PROTECTED] Sunbay Software AG,
[EMAIL PROTECTED] FreeBSD committer,
+380.652.512.251 Simferopol, Ukraine
http://www.FreeBSD.org The Power To Serve
http://www.oracle.com Enabling The Information Age
msg05854/pgp00000.pgp
Description: PGP signature
