Re: h_len, pblock_probe() and checksums consinstency; push(), pop() enhancements (?)

Hello there,

On Tue, May 25, 2004 at 09:18:12PM +0200, Alessandro Salvatori wrote:
> Hi!
> i'm sorry this mail is quite long, but i'm going slightly mad because i 
> can't find any trivial solution to the checksum issue...
> the library is getting complex and i really don't know what would be the 
> best thing to do.
> 
> The better, IMHO, would be to make the best out of the interface of 
> libnet 1.1.2.1 (behaving in a fully consinstent way) and provide some 
> doped functions to enable the programmer to have some more control.

Agreed:
http://www.security-labs.org/index.php3?page=libnetng

> some changes are undergoing in my copy of the source code... they are 
> not in a consistent state, since
> i'm changing my mind too often and i definitely need suggestions... 
> furthermore this evening i had to issue a make buildworld on my laptop 
> and i'm still waiting after some hours ^_^
> 
> sorry i have no patch ready yet... i would really appreciate any 
> suggestion/opinion or help in terms of source code ;)

I have started to "port" what I talked earlier with Mike & Kirby to
the current libnet cvs. My main work until now has been to provide the
basic functions that will be needed by everything else.

Next step will be to provide a structure to handle the interfaces.

Then, last but not least, some small changes are needed in every builder.

To know if it's ok, my best test will be to compile the sample
directory without having changed any file in there.

> Mike, Fred, Kirby please tell me if these ideas go into the wrong direction.
> 
> MY DOUBTS:
> ==========
> 
> A) P->IP_OFFSET
> 
>        it's not clear to me the role of p->ip_offset, since i've got
>        some doubts on its behaviour (maybe it's a temporary
>        workaround?):

Its goal is to keep the position of the IP pblock containing a given
pblock. That is mandatory to avoid problems when you are building "IP
in IP" packets (GRE tunnels for instance).

I proposed this workaround when someone (sorry I am to lazy to look
for your name in my mailbox, but anyway thanks for the bug report)
reported a problem with ICMP builders: the embedded IP header's
checksum was badly coomputed.

This was for the same reason the IP header embeded in a GRE packet was
badly configured.

I proposed that as workaround, which is the right word as this
solution is really not very clean :(


>        1) it is set the same for every pblock...
>           (*) if this was the purpose why not to have one only 
>               l->ip_offset ?
>           (*) isn't that the purpose of p->ip_offset was to have different
>               ip_offset for pblocks inside one ip header and pblocks inside
>               an other ip header?
>              => in this example
>              
>              p1 libnet_build_something()
>           p2 p1 libnet_build_ipv4()                   ----> ok we  update p1->ip_offset
>        p3 p2 p1 libnet_build_something_else()
>         p4 p3 p2 p1 libnet_build_ipv4()               ----> we update {p1,p2,p3}->ip_offset
>         
>          is this what should happen? shouldn't  {p1,p2}->ip_offset
>          stay the same?

Yes, they should. I cant check it right now, but they definitely
should! Can someone check that with icmp builder or gre ones please?

>        2) in build_ipv4 p->ip_offset is set to l->total_size for any
>        p after the current p. this is good the first time p is
>        built...
>           (*) what happens when p is recycled?
>              => total_size will take into account also stuff outside the ip
>                 (id est link layer header...)

It's ten in the morning, I have only had one coffee, and you start
with so much annoying question ;-) Ok, I could have answered earlier
too...

Ok, I need to think about that some minutes ...

Shit, you're probably right !

That "recycling" is a real pain in the ass. That is exactly the reason
why I did not "port" it to my draft for next libnet. There, pblock
(called pbuf) will be very cheap, so we should not care about building
and deleting them all around.

> B) SIZES, SIZES, SIZES!

Dont trust girls: yes, size matters ;-)

> for what concerns the payload issue i encountered, i have in mind a
> possible solution, but i would really like to know your opinion
> about it, since there's the need to change some internal data
> structure and it would be good to do that in a reasonable way:

Too much to read right now: brain overflowed

But, yes, sizeS are difficult to handle.

> PERVERSE PACKETS:
> =================
> 
> Just an other idea that could solve the problem...
> Just ask people to add some magic function calls if they are playing some
> weird trick with packet lenght. if they do that without these calls, then
> the library gives no guaranty (at the moment) on the result.

... snip ...

> libnet_stack_push() and stack_pop() should somehow know which pblock 
> was referred
> as the last one (in the timeline). the context l can really help us. 
> otherwise a great idea would be to insert FAKE PBLOCKS
> THAT BEHAVE AS MARKERS!!!

I dont like that: it is too much complicated for both user and
internal development.

A packet (aka buffer) must be something really easy and cheap to
build. After all, that is just some bytes put after some other bytes.
Thus, there must be a simple solution to perform that.

As you noticed above, complexity raises when
size/checksums/tunneling is considered (and it MUST be). However, I am
not sure that allowing a user to manipulate, add, remove some weird
pblock is an option. It brings too much complexity for the real needs.
I may be mistaken, but rather to care with allowing the recycling of
pblocks and so on, I think we shoud focus on performances so that the
building of a packet ic easy and cheap (in memory, but mainly in
computation).

I mean most applications do not need to change all of a pblock at
once. In 90% of timpes, you only change one or two parameters (src ip,
IP ID, or whatever), so almost all the header is unaffected by that
change. Thus, this must be allowed. More in-depth change are much more
complicated and expensive ... but do not occur that often (at least,
that is true with most the the programs that use libnet that I have
seen: if I am mistaken, please, tell me because I'll need to change
things then).

However, where that has to be taken into consideration is when one
will want to import/export packets from other libraries (libpcap &
libdnet).


Ok, it's 10:49, and I definitly another coffee ;-)

	Fred Raynal