Want to release 1.2.5 soon

[Martin Josefsson]

On Fri, 11 Jan 2002, Jozsef Kadlecsik wrote:

> On Fri, 11 Jan 2002, Jozsef Kadlecsik wrote:
> 
> > But! NAT sets the NAT helper always on the NATed port and this is
> > confusing. Shouldn't NAT set it's helper on the original port?
> >
> > What's your opinion?
> 
> Correctly: Shouldn't NAT set it's helper the same way as alter_reply
> does in your patch? I.e if there is no helper on the NATed port, try to
> find a helper on the original port.

I think that sounds like a good idea. but there's still a possibility that
a related connection get's assigned a conntrack-helper and ie. all
ftp-data goes through the irc conntrack helper. Would changing it so it
doesn't change the helper if it's a related connection or if it's a normal
connection it only changes it if it finds a new helper be a good idea?

either change would work with my module, and I think that combining them
could be a good idea. you said that the related connections in your h323
conntrack-helper uses dynamic ports but still uses a helper, and the
helper is unregistered as the related connection uses dynamic ports.

so in this case it's related and we shouldn't change the helper.

The big question is if there exists a protocol which needs a helper on the
related connection and the related connection uses fixed ports so that a
helper can be registered for it. or should we make it so that if you
design a helper for such a protocol you have to assign the helper
for the related connections manually in the main helper as you are doing
in the h323 helper?

not looking up the helper after NAT has been performed is a nightmare for
certain admins such as me. I have a router that performas SNAT for 60-100
clients. And some clients want's to have a ftp-server and they run the
ftpserver on port 21 on their machines. All I have to do currently is to
forward a port into port 21 on their machine and ip_{conntrack,nat}_ftp
takes care of everything. It would certainly ruin my day if I had to add
all external ports on the router to the ftp modules.

I vote for a combined approach of these two designs and force all helpers
which have related connections that also need helpers to manually assign
the helper. And this way a related connection which is a ftp-data
connection can't be assigned the irc-helper by mistake as the helper isn't
looked up for expected connections.

Now I only have one small problem left... my patch changes

+               conntrack->helper = expected->expectant->helper;

and 

-       if (ret != NF_DROP && ct->helper) {
+       if (ret != NF_DROP && !(ct->status & IPS_EXPECTED) && ct->helper) {
                ret = ct->helper->help((*pskb)->nh.iph, (*pskb)->len,
                                       ct, ctinfo);

in ip_conntrack_core.c so that the expected connection get's assigned
the same helper as the expecting connection. and then it changes it so
that the ftp helper isn't called if it's a related connection... this
would obviously break h323 if this is done the same way in newnat.

All I'm using in the helper struct is the name field to identify packets
so this could possibly be moved to some other place. I don't want to move
the name directly into struct ip_conntrack as the maxlength of the name is
set to 30 bytes. but one solution would be to add a pointer to struct
ip_conntrack and allocate 32 bytes (kmalloc doesn't allocate anything
smaller than that) when a helper wants to set it's name. And then copy
that pointer when a expected connection comes in. This way the expected
connection doesn't get a helper but it still has a name to identify it by.

what's your opinion about all of this?

/Martin

Never argue with an idiot. They drag you down to their level, then beat you with experience.