Re: Nping BETA2 - a fix to the packet recv handler?

["Luis M." <luis.mgarc () gmail com>]

Hi Jack,

Thank you very much for your patch. I am very busy today but I will
examine it first thing tomorrow and send you an email with my thoughts.

Regards,

Luis.




Jack Grahl wrote:
> This patch hopefully does the right thing to fix the code which prints output and tallies stats when a packet is 
> received that got through the right pcap filters - I realized that the previous code had more problems than I thought 
> because iinm if for example nping receives a ICMP packet in response to a TCP probe it tries to find the port number 
> of the ICMP packet, and if it fails (which it will) it simply doesnt process the packet further.
>
> What I decided I thought was the correct solution here was not to branch based on probe type but simply on response 
> type. If we receive an ICMP response, it doesnt matter what the probe was at this place in the code. Of course we 
> shouldnt get a TCP reply to a ICMP probe etc but if we did it doesn't matter - it goes through the same checks.
>
> To implement this I added several more 'helper functions' to utils_net.cc.
>
> Hope this seems reasonable. Based on this patch it should be possible to change the setProbeRecvICMP function to 
> reflect what we discussed before, however I didnt do this yet as I didn't want to confuse the two patches.
>
> Jack
>
> ------------------
> patch starts here.
> ------------------
>
> --- nping-0.1BETA2/nping.cc    2009-08-20 19:01:10.000000000 +0100
> +++ nping-build2/nping.cc    2009-08-22 02:42:32.000000000 +0100
> @@ -1661,6 +1661,7 @@
>   static struct timeval prevtime;
>   NpingTarget *trg=NULL;
>   u16 *prt=NULL;
> + u8 proto=NULL;
>  
>   outPrint(DBG_4, "nping_event_handler(): Received callback of type %s with status %s",
>                    nse_type2str(type), nse_status2str(status));
> @@ -1725,27 +1726,54 @@
>              else
>                  outError(QT_1, "Unsupported EtherType %02X", *ethtype);
>  
> -            outPrint(VB_0, "RCVD (%.4fs) %s", o.timeSinceStart(t), buffer );
> -            if( o.getVerbosity() >= VB_3 )
> -                luis_hdump((char*)packet, packetlen);
> -            prevtime=pcaptime;
> -
> -            /* Statistics */
> -            o.stats.addRecvPacket(packetlen);
> -            trg=o.targets.findTarget( getSrcSockAddrFromIPPacket((u8*)packet, packetlen) );
> -
> -            if( o.getMode()==TCP || o.getMode()==UDP){
> -                if(trg != NULL){
> -                    prt=getSrcPortFromIPPacket((u8*)packet, packetlen);
> -                    if( prt!=NULL )              
> -                        trg->setProbeRecvTCP(*prt, 0);
> -                }
> -            }else if (o.getMode()==ICMP){
> -                if(trg != NULL){
> -                    trg->setProbeRecvICMP(0, 0);
> -                }
> -            }
> -       break;
> +        proto = getProtoFromIPPacket((u8*)packet, packetlen);
> +
> +        if (proto == IPPROTO_UDP || proto == IPPROTO_TCP){
> +          /* for UDP/TCP we print out and update the global total straight away
> +         since we know that pcap only found packets from connections that we 
> +         opened */
> +          outPrint(VB_0, "RCVD (%.4fs) %s", o.timeSinceStart(t), buffer );
> +          if( o.getVerbosity() >= VB_3 )
> +        luis_hdump((char*)packet, packetlen);
> +          prevtime=pcaptime;
> +          
> +          /* Statistics */
> +          o.stats.addRecvPacket(packetlen);
> +
> +          /* then we check for a target and a port and do the individual stats */
> +          trg=o.targets.findTarget( getSrcSockAddrFromIPPacket((u8*)packet, packetlen) );
> +          
> +          if(trg != NULL){
> +        prt=getSrcPortFromIPPacket((u8*)packet, packetlen);
> +        if( prt!=NULL )              
> +          trg->setProbeRecvTCP(*prt, 0);
> +          }
> +
> +        }else if (proto==IPPROTO_ICMP){
> +          /* we look for a target based on first src addr and second the dest addr of 
> +         the packet header which is returned in the ICMP packet */
> +          trg=o.targets.findTarget( getSrcSockAddrFromIPPacket((u8*)packet, packetlen) );
> +          if(trg == NULL){
> +        trg=o.targets.findTarget( getDestAddrFromICMPPacket((u8*)packet, packetlen));
> +          }
> +
> +          /* In the case of ICMP we only do any printing and stats if we 
> +         found a target - otherwise it could be a packet that is nothing 
> +         to do with us */
> +          if(trg!=NULL){
> +          outPrint(VB_0, "RCVD (%.4fs) %s", o.timeSinceStart(t), buffer );
> +          if( o.getVerbosity() >= VB_3 )
> +        luis_hdump((char*)packet, packetlen);
> +          prevtime=pcaptime;
> +          
> +          o.stats.addRecvPacket(packetlen);
> +          
> +          trg->setProbeRecvICMP(0, 0);
> +          }
> +          
> +        }
> +        
> +        break;
>  
>         /* In theory we should never get these kind of events in this handler
>          * because no code schedules them */  
> diff -u -r nping-0.1BETA2/utils_net.cc nping-build2/utils_net.cc
> --- nping-0.1BETA2/utils_net.cc    2009-08-20 19:01:10.000000000 +0100
> +++ nping-build2/utils_net.cc    2009-08-22 02:39:10.000000000 +0100
> @@ -1360,12 +1360,65 @@
>  } /* End of getSrcSockAddrFromPacket() */
>  
>  
> +struct sockaddr_storage *getDestAddrFromICMPPacket(u8 *pkt, size_t pktLen){
>  
> +  static struct sockaddr_storage ss;
> +  struct sockaddr_in *s_ip4=(struct sockaddr_in *)&ss;
> +  struct ip *i4=(struct ip*)pkt;
> +  memset(&ss, 0, sizeof(struct sockaddr_storage));
>  
> +  if(pkt==NULL || pktLen < 48)
> +    return NULL;
> +
> +  if( i4->ip_v == 4 ){
> +    s_ip4->sin_family=AF_INET;
> +    memcpy(&(s_ip4->sin_addr.s_addr), pkt + 24 + i4->ip_len * 4, 4);
> +
> +    return &ss;
> +  }
> +  else {
> +    return NULL;
> +  }
> +
> +}
> +  /* End of getDestAddrFromICMPPacket */
>  
>  
>  
>  
> +
> +u8 *getUDPheaderLocation(u8 *pkt, size_t pktLen){
> +
> +struct ip *i4=(struct ip*)pkt;
> +  if(pkt==NULL || pktLen < 40)
> +    return NULL;
> +
> +  /* Packet is IPv4 */
> +  if( i4->ip_v == 4 ){
> +    if (i4->ip_p == IPPROTO_UDP) { 
> +        if( pktLen >= ((size_t)(i4->ip_hl*4 + 8)) ) /* We have a full IP+UDP packet */
> +            return pkt+(i4->ip_hl*4);    
> +    }
> +    else
> +        return NULL;
> +  }
> +  /* Packet is IPv6 */
> +  else if(i4->ip_v == 6 ){
> +    if(pktLen<40 + 8 ) /* We possible have a full IPv6+UDP packet */
> +        return NULL;
> +    if( pkt[6] == IPPROTO_UDP ) /* Next Header is TCP? */
> +        return pkt+40;
> +    else /* Extension headers not supported, return NULL TODO: support it? */
> +        return NULL;
> +  }
> +  else{
> +      return NULL;
> +  }
> +
> +  return NULL;
> +}
> +  /* end of getUDPheaderLocation */
> +
>  u8 *getTCPheaderLocation(u8 *pkt, size_t pktLen){
>  
>  struct ip *i4=(struct ip*)pkt;
> @@ -1401,6 +1454,30 @@
>  
>  
>  
> +/* Returns the IP protocol of the packet or -1 in case of failure */
> +u8 getProtoFromIPPacket(u8 *pkt, size_t pktLen){
> +struct ip *i4=(struct ip*)pkt;
> +static u8 proto;
> +
> +  if(pkt==NULL || pktLen < 28)
> +    return -1;
> +
> +  /* Packet is IPv4 */
> +  if( i4->ip_v == 4 ){
> +    proto = i4->ip_p;
> +    return proto;
> +  }
> +
> +  /* Packet is IPv6 */
> +  else if(i4->ip_v == 6 ){
> +    proto = pkt[6];
> +    return proto;
> +  }
> +
> +  return -1;
> +
> +} /* End of getProtoFromIPPacket() */
> +
>  
>  
>  /** @warning Returns pointer to an internal static buffer
> @@ -1409,19 +1486,23 @@
>  
>  static u16 port;
>  u16 *pnt=NULL;
> -u8 *tcp=NULL;
> +u8 *header=NULL;
>  
> -  if(pkt==NULL || pktLen < 40)
> +  if(pkt==NULL || pktLen < 28)
>      return NULL;
>  
> -  if((tcp=getTCPheaderLocation(pkt, pktLen))==NULL)
> -    return NULL;
> +  if((header=getTCPheaderLocation(pkt, pktLen))==NULL){
> +    if ((header=getUDPheaderLocation(pkt, pktLen))==NULL)
> +      return NULL;
> +    
> +  }
>  
> -  pnt=(u16*)&(tcp[0]);
> +  pnt=(u16*)&(header[0]);
>    port= ntohs(*pnt);
> -
> +  
>    return &port;
>    
> +  
>  } /* End of getSrcPortFromIPPacket() */
>  
>  
> diff -u -r nping-0.1BETA2/utils_net.h nping-build2/utils_net.h
> --- nping-0.1BETA2/utils_net.h    2009-08-20 19:01:10.000000000 +0100
> +++ nping-build2/utils_net.h    2009-08-22 02:13:45.000000000 +0100
> @@ -129,7 +129,10 @@
>  int print_dnet_interface(const struct intf_entry *entry, void *arg) ;
>  int print_interfaces_dnet();
>  struct sockaddr_storage *getSrcSockAddrFromIPPacket(u8 *pkt, size_t pktLen);
> +struct sockaddr_storage *getDestAddrFromICMPPacket(u8 *pkt, size_t pktLen);
> +u8 *getUDPheaderLocation(u8 *pkt, size_t pktLen);
>  u8 *getTCPheaderLocation(u8 *pkt, size_t pktLen);
> +u8 getProtoFromIPPacket(u8 *pkt, size_t pktLen);
>  u16 *getSrcPortFromIPPacket(u8 *pkt, size_t pktLen);
>  u16 *getDstPortFromIPPacket(u8 *pkt, size_t pktLen);
>  u16 *getDstPortFromTCPHeader(u8 *pkt, size_t pktLen);
>
>
>       
>
> _______________________________________________
> Sent through the nmap-dev mailing list
> http://cgi.insecure.org/mailman/listinfo/nmap-dev
> Archived at http://SecLists.Org
>
>   


_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://SecLists.Org