Re: Building IP header

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Your checksum code is incorrect (See inline).

S Ramesh wrote:

> Hi,
> 

[snip]

> 
> uint16_t checksum(uint16_t *data,uint32_t len) {
> 
> /* data is the one for which we are calculating checksum and its size
> len in bytes */
> 
> register uint32_t sum=0;
> uint16_t ans;
> register uint16_t *temp=data;
> register uint32_t odd = len;
> 
> /* adding 16 bits every time to sum */
> while( odd > 1) {
>  sum += *temp;
>  odd -= 2;
> }
> 
> /* if length is odd the last 8 bits will also get added here */
> if( odd )
>  sum += *(uint8_t *)temp;
> 
> sum=(sum >> 16) + (sum & 0xffff);


This adds the overflow bits back into the low 16 (as it should).

> sum += (sum >> 16);


This does the same thing again, so if there were carries into the upper 
16 bits, your checksum is wrong.  You should delete the first line and 
leave the second one.

> ans=~sum; /* truncating to 16 bits */
> return (ans);
> 
> }
> 


Ironically, this bug lies latent since the Linux kernel sets the 
checksum on packets sent on RAW sockets, even when using IP_HDRINCL, so 
you don't need to set ip->check when you send packets.  If you try to 
verify the checksum on received packets, though, you will run into 
trouble if you don't fix this code.

 
> 
> int build_iphdr(void *buffer, uint32_t len) {
> 
> /* Pointer to the allocated buffer for IP hdr structure. len is the
> length if the data to be encap in IP */
> /* In this fn the tot length field is data len + ip hdr length */
> /* id will be the current user id running the process */
> 
> 
> struct iphdr *ip=buffer;
> 
> /*
> if( (ip=(struct iphdr *)calloc(1,sizeof(struct iphdr)) ) == NULL ) {
>  perror("calloc failed:");
>  return -1;
> }
> */
> 
> ip->version=4;
> ip->ihl=5;
> ip->tos=0;
> ip->tot_len=sizeof(struct iphdr) + len;


This is also (technically) incorrect.  ip->tot_len is a 16-bit field, so 
you need to htons() the length before setting it.  Fortunately, 
ip->tot_len, like ip->check, is one of the few IP header entries that 
the Linux kernel sets for you on RAW sockets, so this bug is also latent.

> ip->id=htons(getuid());


If you are sending more than one packet every few minutes, this may be a 
problem.  IP requires that the tuple of (source address, destination 
address, ID) be unique for some reasonable time period.  If you violate 
that rule, routers are allowed to do terrible things to you -- like 
reassembling fragments from different packets and delivering the result.
If you just set ip->id to 0, the kernel will pick an appropriate ID for you.

[snip]

> =======================================================
> The following is the code which is waiting to receive  the IP packet
> what i sent. The display api will display the content of the packet and
> i want to
> see the IP address 1.2.3.4 here. But its not receiving the packet
> 
> =======================================================
> 
> #include <netinet/in.h>
> #include <netinet/ip.h>
> #include <stdio.h>
> #include <sys/types.h>
> #include <errno.h>
> #include <fcntl.h>
> 
> 
> void display(void *buf, int bytes)
> { int i;
>  struct iphdr *ip = buf;
> 
> 
>  printf("----------------\n");
>  for ( i = 0; i < bytes; i++ )
>  {
>   if ( !(i & 15) ) printf("\n%04X:  ", i);
>   printf("%02X ", ((unsigned char*)buf)[i]);
>  }
>  printf("\n");
>  printf("IPv%d: hdr-size=%d pkt-size=%d protocol=%d TTL=%d src=%s ",
>   ip->version, ip->ihl*4, ntohs(ip->tot_len), ip->protocol,
>   ip->ttl, inet_ntoa(ip->saddr));
>  printf("dst=%s\n", inet_ntoa(ip->daddr));
> }
> 
> main()
> { int sd,sock_opt=1;
>  struct sockaddr_in addr;
>  unsigned char buf[1024];
> 
>  sd = socket(AF_INET, SOCK_RAW, IPPROTO_RAW);
>  if ( sd < 0 )
>  {
>   perror("socket");
>   exit(0);
>  }
> 
> 
>  if( fcntl(sd,F_SETFL,O_NONBLOCK) != 0 ) {
>   perror("NON block IO fail:");
>   exit(1);
>  }
>  for (;;)
>  { int bytes, len=sizeof(addr);
> 
>   bzero(buf, sizeof(buf));
>   bytes = recvfrom(sd, buf, sizeof(buf), 0, (struct sockaddr *)&addr,
> &len);
>   if ( bytes > 0 )
>    display(buf, bytes);
>  }


Why are you setting this socket non-blocking, and then busy-waiting for 
packets in a loop?

>  exit(0);
> }
> ======================================================
> 
> Kindly help me where am missing.
> 
> Thanks
> Ramesh
> 
> -
> : send the line "unsubscribe linux-net" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Casey Carter
Casey@Carter.net
ccarter@uiuc.edu
AIM: cartec69

-
: send the line "unsubscribe linux-net" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Netdev]     [Ethernet Bridging]     [Linux 802.1Q VLAN]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Git]     [Bugtraq]     [Yosemite News and Information]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux PCI]     [Linux Admin]     [Samba]

  Powered by Linux