[Snort-devel] small patch for snort's PPP decoder

Thomas Moestl tmoestl at ...224...
Sun Jan 21 13:05:57 EST 2001


Hi,

while toying around with snort on a dialup line, I have noticed that
some incoming packets would get handled in a incorrect way, some
generating spurious alerts.

The reason is simple: DecodePppPkt treats all incoming packets as
IP packets, but ppp has it's own protocol field and may encapsulate
different protocols. The way things are currently handled, snort
might also interpret LCP, IPCP and authorization packets wrongly,
because those will also be received on raw sockets (at least on
FreeBSD). I have attached a small patch that makes DecodePppPkt
honor the PPP protocol fields: packets of unknown types are discarded
(eg LCP and IPCP packets) and IP and IPX packets are decoded 
accordingly. Uncompressed VJ  (Van Jacobson compression) packets 
are also handled; the compression in this case just alters the protocol 
field to hold some state, so it only needs to be set to IPPROTO_TCP 
(only TCP packets can be VJ compressed). I also print a warning when
the first actual compressed VJ frame is received, so that users know
that those packets cannot be decoded and don't feel safe. Full-blown
VJ decompression would probably need some kind of PPP state machine
(the compression is stateful).

I have not taken much time to familiarize myself with the snort
coding style, but I hope the patch is acceptable anway ;-)

	- thomas

PS: please CC me when answering to the list.

-------------- next part --------------
diff -cr --exclude=*~ snort-1.7/decode.c snort-new/decode.c
*** snort-1.7/decode.c	Tue Jan  2 20:46:25 2001
--- snort-new/decode.c	Sun Jan 21 19:02:04 2001
***************
*** 430,442 ****
--- 430,445 ----
   */
  void DecodePppPkt(Packet * p, struct pcap_pkthdr * pkthdr, u_int8_t * pkt)
  {
+     static int had_vj = 0;
      u_int32_t len;
      u_int32_t cap_len;
+     struct ppp_header *ppphdr;
  
      bzero((char *) p, sizeof(Packet));
  
      p->pkth = pkthdr;
      p->pkt = pkt;
+     ppphdr = (struct ppp_header *)pkt;
  
      len = pkthdr->len;
      cap_len = pkthdr->caplen;
***************
*** 452,459 ****
                       cap_len);
          return;
      }
! 
!     DecodeIP(p->pkt + PPP_HDRLEN, cap_len - PPP_HDRLEN, p);
  }
  
  
--- 455,488 ----
                       cap_len);
          return;
      }
!     /* 
!      * We only handle uncompressed packets. Handling VJ compression would mean
!      * to implement a PPP state machine.
!      */
!     switch (ntohs(ppphdr->protocol)) {
!     case PPP_VJ_COMP:
! 	    if (!had_vj)
! 		    ErrorMessage("PPP link seems to use VJ compression, cannot handle compressed packets!\n");
! 	    had_vj = 1;
! 	    break;
!     case PPP_VJ_UCOMP:
! 	    /* VJ compression modifies the protocol field. It must be set
! 	     * to tcp (only TCP packets can be VJ compressed) */
! 	    if(cap_len < PPP_HDRLEN + IP_HEADER_LEN)
! 	    {
! 		    ErrorMessage("PPP VJ min packet length > captured len! (%d bytes)\n",
! 				 cap_len);
! 		    return;
! 	    }
! 	    ((IPHdr *)(p->pkt + PPP_HDRLEN))->ip_proto = IPPROTO_TCP;
! 	    /* fall through */
!     case PPP_IP:
! 	    DecodeIP(p->pkt + PPP_HDRLEN, cap_len - PPP_HDRLEN, p);
! 	    break;
!     case PPP_IPX:
! 	    DecodeIPX(p->pkt + PPP_HDRLEN, cap_len - PPP_HDRLEN);
! 	    break;
!     }
  }
  
  
diff -cr --exclude=*~ snort-1.7/decode.h snort-new/decode.h
*** snort-1.7/decode.h	Tue Jan  2 09:06:00 2001
--- snort-new/decode.h	Sun Jan 21 16:45:42 2001
***************
*** 66,74 ****
  #define TOKENRING_LLC_LEN                8
  #define SLIP_HEADER_LEN                 16
  
  #ifndef PPP_HDRLEN
!     #define PPP_HDRLEN                       4
  #endif
  
  /* otherwise defined in /usr/include/ppp_defs.h */
  #ifndef PPP_MTU
--- 66,85 ----
  #define TOKENRING_LLC_LEN                8
  #define SLIP_HEADER_LEN                 16
  
+ /* ppp header structure */
+ struct ppp_header {
+ 	unsigned char  address;
+ 	unsigned char  control;
+ 	unsigned short protocol;
+ };
+ 
  #ifndef PPP_HDRLEN
!     #define PPP_HDRLEN          sizeof(struct ppp_header)
  #endif
+ #define PPP_IP		0x0021		/* Internet Protocol */
+ #define PPP_VJ_COMP	0x002d		/* VJ compressed TCP/IP */
+ #define PPP_VJ_UCOMP	0x002f		/* VJ uncompressed TCP/IP */
+ #define PPP_IPX		0x002b		/* Novell IPX Protocol */
  
  /* otherwise defined in /usr/include/ppp_defs.h */
  #ifndef PPP_MTU


More information about the Snort-devel mailing list