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

Alessandro Salvatori <[email protected]> Tue, 25 May 2004 21:18:12 +0200
Newsgroups gmane.comp.security.libnet
Message-ID <[email protected]>
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.

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 ;)
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?):
        
        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?
          
        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...)

B) SIZES, SIZES, SIZES!

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:

   p->h_len  <--->  HEADER LEN
       ===> WILL DISAPPEAR, BUT FOR THE MOMENT I'D LIKE TO KEEP IT FOR 
COMPARISON/TESTING THROUGH ASSERT()IONS
   p->b_len  <--->  BUFFER LEN
       ===> STAYS THE SAME
   p->jth    <--->  BOOLEAN VALUE: "just the header"
       ===> TELLS WHETHER JUST THE HEADER OR ALSO THE PAYLOAD SHOULD BE 
CHECKSUMMED
   p->c_len  <--->  CHECKSUM LEN
       ===> for each pblock p lower than the modified one, and whichever 
its stack level is, it will be updated if p->jth!=0
   p->f_len  <--->  FURTHER LEN ===> this pblock plus all the following 
---> updated in chain to the left

1)
when we change the 'size' of a pblock then
the _payload size_ for all the lower blocks increases/decreases as well, 
and of the same amount

[THIS IS MORE OR LESS TRUE, BUT NOT ALWAYS... for this reason i suggest 
we should explicitly
manage the stack of layers, but maybe this is not the only solution]

we should update the payload size for every one of the "lower level" 
pblocks...

[TILL PBLOCKS THAT ARE NOT AFFECTED! -> stack management]

but, then, what about the _header lenght for checksumming_? should it
be always updated or not? what if the only thing that matters was
the header? and in that case, what if what changed size was a header
option pblock???

well BTW, it seems to me that there is an other little bug:
p->h_len should not always be updated as it happens in libnet_pblock.c!!!

it should be updated only for those pblocks that have their checksum 
computed
over the header and payload or only the header but have a variable 
header lenght.
In this case it would be slightly tricky, but this case is just the case 
of ipv4...

2) some changes (at the moment they are not consistent at all!!!)

new function: libnet_update_lowers(p, offset)

  offset is the amount of bytes the new pblock is bigger (if +) or 
smaller (if -) than
  the previous one.

  p is the pblock.

  the function updates p's fields and then p->next's fields in a loop

this version is not yet stack-aware... by stack-aware i mean... we'll
you will see in the end of this message...

    +-------------------------------+
    | libnet_pblock_update_lowers() |
8<--+-------------------------------+----------------------
void
libnet_pblock_update_lowers(libnet_pblock_t *p, int32_t offset){ /**** 
WORK IN PROGRESS!!! ****/
    p->h_len += offset; /* new length for checksums */ /* THIS WAS THERE 
BUT IT IS BUGGY! */
    if(!p->jth){
        p->c_len += offset; /* new length for checksums */
    }
    p->f_len += offset; /* new further length from here on */
    l->total_size += offset;

    while(NULL!=(p=p->next)){
        p->h_len += offset; /* new length for checksums */ /* THIS WAS 
THERE BUT IT IS BUGGY! */
    if(!p->jth){
            p->c_len += offset; /* new length for checksums */
        }
    p->f_len += offset; /* new further length from here on */
        l->total_size += offset;
    }
}
8<---------------------------------------------------------

new function: _further_len(p)

a function you can use to know how much data there's in the payload...
once more, this version is not yet stack-aware...

    +-----------------------------+
    | libnet_pblock_further_len() |
8<--+-----------------------------+------------------------
u_int32_t /* SHOULD BECOME VOID */
libnet_pblock_further_len(libnet_pblock_t *p){ /**** WORK IN PROGRESS!!! 
****/
    p->f_len=p->b_len;
    if(p->prev){
    p->f_len+=p->prev->f_len;
        if(!(p->jth)){ /* if not just the header */
        p->c_len+=p->f_len;
          /* WE UPDATE THIS EVEN IF THERE IS NO CHECKSUM TO COMPUTE,
             SINCE THIS COULD BE TOGGLED AFTERWORDS OR IN SUCCESSIVE
             CALLS TO libnet_build_whatever()!!! */
#if 0
/* THIS ELSE BRANCH SEEMS USELESS AT THE MOMENT SINCE WE ARE LUCKY IN
   ALL THE FOUR CASES BELOW! */
        } else {/* if this header has got header options in an other 
pblock and doesn't checksum the payload too */
                switch(p->type){
                case LIBNET_PBLOCK_IPV4_H:
                    /* THE CHECSUM COVERS THE HEADER ONLY */
                            /* THEN WE HAVE TO DO WITH OPTIONS!!! */

                            /* ***** THIS PART IS NOT READY YET! ***** */

                    break;
                case LIBNET_PBLOCK_IPV6_H:
                    /* I THINK IPV6 HAS NO CHECKSUM INDEED (YEP THAT WAS 
AN OLD "FEATURE" OF LIBNET :) */
                            /* SO WHO CARES?!! */
                    break;
                case LIBNET_PBLOCK_TCP_H:
                    /* THE CHECKSUM IS COMPUTED OVER THE WHOLE HEADER 
AND SEGMENT + 2 32bit words from pseudeheader */
                                /* SO WE DEFINITELY SHOULDN'T BE IN THIS 
BRANCH OF THE IF */
                            /* from rfc793 */
                            /*    The checksum field is the 16 bit one's 
complement of the one's
                                complement sum of all 16 bit words in 
the header and text.  If a
                                segment contains an odd number of header 
and text octets to be
                                checksummed, the last octet is padded on 
the right with zeros to
                                form a 16 bit word for checksum 
purposes.  The pad is not
                                transmitted as part of the segment.  
While computing the checksum,
                                the checksum field itself is replaced 
with zeros.
                
                                  The checksum also covers a 96 bit 
pseudo header conceptually
                                prefixed to the TCP header.  This pseudo 
header contains the Source
                                Address, the Destination Address, the 
Protocol, and TCP length.
                                This gives the TCP protection against 
misrouted segments.  This
                                information is carried in the Internet 
Protocol and is transferred
                                across the TCP/Network interface in the 
arguments or results of
                                calls by the TCP on the IP.
                            */
                    break;
                case LIBNET_PBLOCK_UDP_H:
                    /* THE CHECKSUM IS COMPUTED OVER THE WHOLE HEADER 
AND PAYLOAD + 2 32bit words from pseudeheader */
                                /* SO WE DEFINITELY SHOULDN'T BE IN THIS 
BRANCH OF THE IF */
                            /* from rfc768 */
                            /*    Checksum is the 16-bit one's 
complement of the one's complement sum of a
                                pseudo header of information from the IP 
header, the UDP header, and the
                                data,  padded  with zero octets  at the 
end (if  necessary)  to  make  a
                                multiple of two octets.

                                  The pseudo  header  conceptually 
prefixed to the UDP header contains the
                                source  address,  the destination  
address,  the protocol,  and the  UDP
                                length.   This information gives 
protection against misrouted datagrams.
                                This checksum procedure is the same as 
is used in TCP.
                             */
                    break;
                }
#endif
    }
    }
    return c_len; /* SHOULD DISAPPEAR */
}
8<---------------------------------------------------------

then these would be the changes to any libnet_build_icmpXXX():

8<---------------------------------------------------------
@@ -86,7 +86,6 @@
     }
 
     n = LIBNET_ICMPV4_ECHO_H + payload_s;        /* size of memory block */
-    h = LIBNET_ICMPV4_ECHO_H + payload_s;        /* hl for checksum */
 
     /*
      *  Find the existing protocol block if a ptag is specified, or create
@@ -97,6 +96,7 @@
     {
         return (-1);
     }
+    h = LIBNET_ICMPV4_ECHO_H + payload_s + 
libnet_pblock_further_len(p);        /* hl for checksum */
 
     memset(&icmp_hdr, 0, sizeof(icmp_hdr));
     icmp_hdr.icmp_type = type;          /* packet type */
8<---------------------------------------------------------

h is then use by libnet_pblock_update(), which is called only if this is 
a brand new pblock.
in the case the pblock is recycled, then the libnet_pblock_probe() will 
call libnet_pblock_update_lowers();

    +-----------------------+
    | libnet_pblock_probe() |
8<--+-----------------------+------------------------------
libnet_pblock_t *
libnet_pblock_probe(libnet_t *l, libnet_ptag_t ptag, u_int32_t n, 
u_int8_t type)
{
    int offset;
    libnet_pblock_t *p;

    if (ptag == LIBNET_PTAG_INITIALIZER)
    {
        /*
         *  Create a new pblock and enough buffer space for the packet.
         */
        p = libnet_pblock_new(l, n);
        if (p == NULL)
        {
            /* err msg set in libnet_pblock_new() */
            return (NULL);
        }
    }
    else
    {
        /*
         *  Update this pblock, don't create a new one.  Note that if the
         *  new packet size is larger than the old one we will do a malloc.
         */
        p = libnet_pblock_find(l, ptag);

        if (p == NULL)
        {
            /* err msg set in libnet_pblock_find() */
            return (NULL);
        }
        if (p->type != type)
        {
            snprintf(l->err_buf, LIBNET_ERRBUF_SIZE,
               "%s(): ptag refers to different type than expected (%d != 
%d)",
               __func__, p->type, type);
            return (NULL);
        }
        /*
         *  If size is greater than the original block of memory, we need
         *  to malloc more memory.  Should we use realloc?
         */
    if(n != p->b_len){ /* because in that case we would follow the 
linked list loosing time with no reason! */
            if (n > p->b_len)
        {
                offset = n - p->b_len;  /* how many bytes larger new 
pblock is */
            free(p->buf);
                p->buf = malloc(n);
            if (p->buf == NULL)
                {
                snprintf(l->err_buf, LIBNET_ERRBUF_SIZE,
                            "%s(): can't resize pblock buffer: %s\n", 
__func__,
                        strerror(errno));
                    return (NULL);
            }
                memset(p->buf, 0, n);
            /* the payload size for all the lower blocks has increased */
            /* we should update it for every one of them... but, then, */
        /* what about the header lenght for checksumming? should it */
        /* be always updated or not? what if the only thing that */
        /* matters was the header? and in that case, what if what */
            /* changed size was a header option pblock??? */
       
                libnet_pblock_update_lowers(p, offset);
        p->b_len = n;       /* new buf len */
            }
        else
            {
            offset = p->b_len - n;
        libnet_pblock_update_lowers(p, -offset);
            p->b_len = n;       /* new buf len */
            }
    }
        p->copied = 0;      /* reset copied counter */
    }
    return (p);
}
8<---------------------------------------------------------


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.

 +--------+-------+--------+-------------+--------------+
 |        |       |        |             |              |
 |  ETH   |  IP   |  ICMP  |  some shit  | some more... |
 |        |       |        |             |              |
 +--------+-------+--------+-------------+--------------+

what if IP->TOTAL_LEN points to the end of some shit and then
there is some more shit after? the ip checksum is ok (as it relies on
the field IP->TOTAL_LEN) but the icmp header is not! it should checksum
till "some shit"!

we should provide a trick to take into account those nasty tricks
when doing something advanced! and in a way that allows for matrioshkas!
moreover we *MUST* *NOT* *BREAK* *EXISTING* *CODE*

libnet_build_some_more_shit();
 libnet_stack_push();
  libnet_stack_push();
  libnet_build_some_shit();
  libnet_build_icmp();
 libnet_stack_pop();
 libnet_build_ip();
libnet_stack_pop();
libnet_build_eth();

                  +--------+-------------+
                  |        |             |
                  |  ICMP  |  some shit  |
                  |        |             |
          +-------+--------+-------------+
          |       |        |             |
          |  IP   |  ICMP  |  some shit  |
          |       |        |             |
 +--------+-------+--------+-------------+--------------+
 |        |       |        |             |              |
 |  ETH   |  IP   |  ICMP  |  some shit  | some more... |
 |        |       |        |             |              |
 +--------+-------+--------+-------------+--------------+

obviously our context l should be enhanced with some more dope to cope with
this stuff...

just an other example:

                  +--------+-------------+                      
+--------+-------------+
                  |        |             |                      |        
|             |
                  |  ICMP  |  some shit  |                      |  ICMP  
|  some shit  |
                  |        |             |                      |        
|             |
          +-------+--------+-------------+              
+-------+--------+-------------+
          |       |        |             |              |       |        
|             |
          |  IP   |  ICMP  |  some shit  |              |  IP   |  ICMP  
|  some shit  |
          |       |        |             |              |       |        
|             |
 +--------+-------+--------+-------------+--------------+-------+--------+-------------+--------------+
 |        |       |        |             |              |       |        
|             |              |
 |  ETH   |  IP   |  ICMP  |  some shit  | some more... |  IP   |  ICMP  
|  some shit  | some more... |
 |        |       |        |             |              |       |        
|             |              |
 +--------+-------+--------+-------------+--------------+-------+--------+-------------+--------------+

for(int i=0; i<2; ++i){
 libnet_build_some_more_shit();
  libnet_stack_push();
   libnet_stack_push();
   libnet_build_some_shit();
   libnet_build_icmp();
  libnet_stack_pop();
  libnet_build_ip();
 libnet_stack_pop();
}
libnet_build_eth();

^_^

 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!!!

maybe two/four functions to navigate up/down the stack would be useful 
too, but could also make things much more complicate, so
i really don't know if it would be worth to implement them...
(two for steps of single pblocks, other two for steps of one layer...)

this should not break existing code but we still MUST REMOVE THOSE BUGS 
that are there...
(i refer to these lines i wrote:
             well BTW, it seems to me that there is an other little bug:
             p->h_len should not always be updated as it happens in 
libnet_pblock.c!!!

             it should be updated only for those pblocks that have their 
checksum computed
             over the header and payload or only the header but have a 
variable header lenght.
             In this case it would be slightly tricky, but this case is 
just the case of ipv4...
)

cheers
sandr8)