Re: Re: TCP-packet with PUSH flag with wrong payload data in LOCAL_OUT

Re: Re: TCP-packet with PUSH flag with wrong payload data in LOCAL_OUT

[Кобылянский Владимир]

>>  kent@lissi.ru
>>    SMTP error from remote mail server after end of data:
>>    host 194.84.136.194 [194.84.136.194]: 550 5.7.1 Message rejected.
>
>Should - have - known.

Oops. It is some troubles in our hoster DNS config. (something in MX records...)
Now I will post from current mail. Sorry.


>>
>>I see in my module 3 handshake packets - they all normal.
>>Then I see first packet with payload - it is TCP-packet with PSH and ACK
>>flags and it is not normal at all.
>
>It could be something in your code (which seem to be absent here).
>
>>In this packet in tcp-data area I MUST see such string:
>>"GET / HTTP/1.1..."
>>or in HEX
>>"4745 5420 2f20 4854 5450 2f31 2e31 ..."
>>BUT I see such data in it:
>>"0200 0100 0100 0000 0100 0000 0000 ....".
>
>Could be TCP options. Payload could be begin in a later packet maybe.
>Or somewhere in your code you have a wrong pointer.
>I don't read glass spheres so...


Sorry. 
There are my sources.
(As I undestood mail-list don't support attachments. Am I write?)

==============================================

#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/netdevice.h>
#include <linux/inetdevice.h>
#include <linux/netfilter.h>
#include <linux/netfilter_ipv4.h>
#include <net/route.h>
#include <linux/ip.h>
#include <linux/tcp.h>
#include <linux/spinlock_types.h>
#include <linux/in_route.h>
#include <net/ip.h>
#include <linux/etherdevice.h>
#include <linux/vmalloc.h>


#define SYSLOG_ID "my_fw"
# define IP_PRINTF(addr) ((addr) & 0xff), (((addr) >> 8) & 0xff), (((addr) >> 16) & 0xff), (((addr) >> 24) & 0xff)
#define info(format, arg...)\
		do {\
			printk(KERN_INFO "%s: %s(): " format "\n" , SYSLOG_ID, __FUNCTION__, ## arg);\
		} while (0)


static struct nf_hook_ops nfho;
static struct nf_hook_ops nfho_in;

int ip_packet_from_local_host(struct iphdr *iph)
{
	struct net_device *dev   = NULL;
	struct in_device *in_dev = NULL;
	struct in_ifaddr *ifaddr = NULL;
	
	for (dev = dev_base; dev; dev = dev->next)
	{
		if (!dev->ip_ptr)
		{
			continue;
		}
		in_dev = (struct in_device *)dev->ip_ptr;
		ifaddr = in_dev->ifa_list;
		while (ifaddr)
		{
			if (iph->saddr == ifaddr->ifa_address)
			{
				return 1;
			}
			ifaddr = ifaddr->ifa_next;
		}
	}
	
	return 0;
}

struct net_device* ip_packet_to_local_host(struct iphdr *iph)
{
	struct net_device *dev   = NULL;
	struct in_device *in_dev = NULL;
	struct in_ifaddr *ifaddr = NULL;
	
	for (dev = dev_base; dev; dev = dev->next)
	{
		if (!dev->ip_ptr)
		{
			continue;
		}
		in_dev = (struct in_device *)dev->ip_ptr;
		ifaddr = in_dev->ifa_list;
		while (ifaddr)
		{
			if (iph->daddr == ifaddr->ifa_address)
			{
				return dev;
			}
			ifaddr = ifaddr->ifa_next;
		}
	}
	return NULL;
}

static unsigned int check_packet(struct sk_buff	*skb, short in_out)
{
    if(skb->nh.iph->protocol == IPPROTO_TCP)
    {
	    struct tcphdr	*tcp;
	    char		*tcp_data=NULL;
	    char		tcp_flags[4];
	    unsigned int	tcp_data_off=0;
	    
	    tcp = (struct tcphdr *)((char*)skb->nh.iph + skb->nh.iph->ihl * 4);
	    tcp_data_off = (tcp->doff)*4;
	    tcp_data = (char *)tcp + tcp_data_off;
	    tcp_flags[0]=' ';
	    tcp_flags[1]=' ';
	    tcp_flags[2]=' ';
	    tcp_flags[3]='\0';

	    if(tcp->syn)
		    tcp_flags[0] = 'S';
	    if(tcp->ack)
		    tcp_flags[1] = 'A';
	    if(tcp->psh)
		    tcp_flags[2] = 'P';
	    info("=== HOOK_PACKET: packet src_addr=%u.%u.%u.%u:%u dst_addr=%u.%u.%u.%u:%u [%s] protocol=%u, IN_OUT=%d, DATA_OFF = %u",
			    IP_PRINTF(skb->nh.iph->saddr),ntohs(tcp->source),IP_PRINTF(skb->nh.iph->daddr),ntohs(tcp->dest), 
			    tcp_flags, skb->nh.iph->protocol, in_out, tcp_data_off);
	    if(tcp->psh && ntohs(tcp->dest) == 80)
	    {
		    info("=== HOOK_PACKET: DATA=%02x %02x %02x %02x %02x %02x", tcp_data[0],tcp_data[1],tcp_data[2],tcp_data[3],tcp_data[4],tcp_data[5]);
//		    return NF_DROP;
	    }
    }


    return NF_ACCEPT;
}

unsigned int hook_func(unsigned int hooknum,
                                struct sk_buff **pskb,
                                const struct net_device *in,
                                const struct net_device *out,
                                int (*okfn)(struct sk_buff *))
{
    struct sk_buff * skb = * pskb;
    int ret;
    if(skb == NULL){
	info("HOOK skb==NULL");
	return NF_DROP;
    }	
    ret = check_packet(skb, (short)1); //1 - out
    return(ret);
}

unsigned int hook_func_in(unsigned int hooknum,
                                struct sk_buff **pskb,
                                const struct net_device *in,
                                const struct net_device *out,
                                int (*okfn)(struct sk_buff *))
{
    struct sk_buff * skb = * pskb;
    int ret;
    if(skb == NULL){
	info("HOOK_IN skb==NULL");
	return NF_DROP;
    }	
    if(skb->pkt_type == PACKET_OUTGOING)  
        ret = check_packet(skb, (short)1); //1 - out
    else
	ret = check_packet(skb, (short)0); //0 - in
    return(ret);
}


static int __init fw_init(void)
{
    int ret;

    nfho.hook     	= hook_func;
    nfho.hooknum  	= NF_IP_LOCAL_OUT;
    nfho.pf       	= PF_INET;
    nfho.priority 	= NF_IP_PRI_FIRST;
    
    nfho_in.hook     	= hook_func_in;
    nfho_in.hooknum 	= NF_IP_PRE_ROUTING;
    nfho_in.pf       	= PF_INET;
    nfho_in.priority 	= NF_IP_PRI_FIRST;
  
    nf_register_hook(&nfho);
    nf_register_hook(&nfho_in);

    info("Driver my_fw started");	    
    return 0;
}
        
static void __exit fw_cleanup(void)
{
    nf_unregister_hook(&nfho);
    nf_unregister_hook(&nfho_in);
    info("Driver my_fw stoped");	    
}
MODULE_LICENSE("GPL");
MODULE_AUTHOR("KENTLINUX");
MODULE_DESCRIPTION("KENTLINUX");
module_init(fw_init);
module_exit(fw_cleanup);
==============================================================


And syslog:

====================================
Jan 22 11:35:52 FW_EXT kernel: my_fw: fw_init(): Driver my_fw started
Jan 22 11:35:54 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.198:41924 dst_addr=192.168.0.132:80 [S  ] protocol=6, IN_OUT=1, DATA_OFF = 40
Jan 22 11:35:54 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.132:80 dst_addr=192.168.0.198:41924 [SA ] protocol=6, IN_OUT=0, DATA_OFF = 40
Jan 22 11:35:54 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.198:41924 dst_addr=192.168.0.132:80 [ A ] protocol=6, IN_OUT=1, DATA_OFF = 32
Jan 22 11:35:54 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.198:41924 dst_addr=192.168.0.132:80 [ AP] protocol=6, IN_OUT=1, DATA_OFF = 32
Jan 22 11:35:54 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: DATA=02 00 01 00 01 00
Jan 22 11:35:54 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.132:80 dst_addr=192.168.0.198:41924 [ A ] protocol=6, IN_OUT=0, DATA_OFF = 32
Jan 22 11:35:54 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.132:80 dst_addr=192.168.0.198:41924 [ AP] protocol=6, IN_OUT=0, DATA_OFF = 32
Jan 22 11:35:54 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.198:41924 dst_addr=192.168.0.132:80 [ A ] protocol=6, IN_OUT=1, DATA_OFF = 32
Jan 22 11:35:54 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.198:41924 dst_addr=192.168.0.132:80 [ A ] protocol=6, IN_OUT=1, DATA_OFF = 32
Jan 22 11:35:54 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.132:80 dst_addr=192.168.0.198:41924 [ A ] protocol=6, IN_OUT=0, DATA_OFF = 32
Jan 22 11:35:54 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.198:41924 dst_addr=192.168.0.132:80 [ A ] protocol=6, IN_OUT=1, DATA_OFF = 32
Jan 22 11:36:01 FW_EXT kernel: my_fw: fw_cleanup(): Driver my_fw stoped

====================================

Re: Re: TCP-packet with PUSH flag with wrong payload data in LOCAL_OUT

[Jan Engelhardt]

On Thursday 2009-01-22 14:53, Кобылянский Владимир wrote:
>Sorry. 
>There are my sources.
>(As I undestood mail-list don't support attachments. Am I write?)

Inline is easier to reply on:


># define IP_PRINTF(addr) ((addr) & 0xff), (((addr) >> 8) & 0xff), (((addr) >> 16) & 0xff), (((addr) >> 24) & 0xff)

Use NIPQUAD and NIPQUAD_FMT.


>int ip_packet_from_local_host(struct iphdr *iph)
>{
>	struct net_device *dev   = NULL;
>	struct in_device *in_dev = NULL;
>	struct in_ifaddr *ifaddr = NULL;
>	
>	for (dev = dev_base; dev; dev = dev->next)
>	{
>		if (!dev->ip_ptr)
>		{
>			continue;
>		}
>		in_dev = (struct in_device *)dev->ip_ptr;

Don't cast this.

>		ifaddr = in_dev->ifa_list;
>		while (ifaddr)
>		{
>			if (iph->saddr == ifaddr->ifa_address)
>			{
>				return 1;
>			}
>			ifaddr = ifaddr->ifa_next;
>		}
>	}
>	
>	return 0;
>}
>
>struct net_device* ip_packet_to_local_host(struct iphdr *iph)
>{
>	struct net_device *dev   = NULL;
>	struct in_device *in_dev = NULL;
>	struct in_ifaddr *ifaddr = NULL;
>	
>	for (dev = dev_base; dev; dev = dev->next)
>	{
>		if (!dev->ip_ptr)
>		{
>			continue;
>		}
>		in_dev = (struct in_device *)dev->ip_ptr;

Nor this.

>		ifaddr = in_dev->ifa_list;
>		while (ifaddr)
>		{
>			if (iph->daddr == ifaddr->ifa_address)
>			{
>				return dev;
>			}
>			ifaddr = ifaddr->ifa_next;
>		}
>	}
>	return NULL;
>}
>
>static unsigned int check_packet(struct sk_buff	*skb, short in_out)
>{
>    if(skb->nh.iph->protocol == IPPROTO_TCP)
>    {
>	    struct tcphdr	*tcp;
>	    char		*tcp_data=NULL;
>	    char		tcp_flags[4];
>	    unsigned int	tcp_data_off=0;
>	    
>	    tcp = (struct tcphdr *)((char*)skb->nh.iph + skb->nh.iph->ihl * 4);
>	    tcp_data_off = (tcp->doff)*4;
>	    tcp_data = (char *)tcp + tcp_data_off;
>	    tcp_flags[0]=' ';
>	    tcp_flags[1]=' ';
>	    tcp_flags[2]=' ';
>	    tcp_flags[3]='\0';
>
>	    if(tcp->syn)
>		    tcp_flags[0] = 'S';
>	    if(tcp->ack)
>		    tcp_flags[1] = 'A';
>	    if(tcp->psh)
>		    tcp_flags[2] = 'P';
>	    info("=== HOOK_PACKET: packet src_addr=%u.%u.%u.%u:%u dst_addr=%u.%u.%u.%u:%u [%s] protocol=%u, IN_OUT=%d, DATA_OFF = %u",
>			    IP_PRINTF(skb->nh.iph->saddr),ntohs(tcp->source),IP_PRINTF(skb->nh.iph->daddr),ntohs(tcp->dest), 
>			    tcp_flags, skb->nh.iph->protocol, in_out, tcp_data_off);
>	    if(tcp->psh && ntohs(tcp->dest) == 80)
>	    {
>		    info("=== HOOK_PACKET: DATA=%02x %02x %02x %02x %02x %02x", tcp_data[0],tcp_data[1],tcp_data[2],tcp_data[3],tcp_data[4],tcp_data[5]);
>//		    return NF_DROP;
>	    }
>    }

Now consider this - tcp_data[0] invokes undefined behavior when

 * there is no payload

 * or the packet is fragmented (a corner case, though)


Most likely the former is your case.

>unsigned int hook_func(unsigned int hooknum,
>                                struct sk_buff **pskb,
>                                const struct net_device *in,
>                                const struct net_device *out,
>                                int (*okfn)(struct sk_buff *))
>{
>    struct sk_buff * skb = * pskb;
>    int ret;
>    if(skb == NULL){
>	info("HOOK skb==NULL");
>	return NF_DROP;
>    }	
>    ret = check_packet(skb, (short)1); //1 - out

Do not do redundant casts.

>    return(ret);
>}
>
>unsigned int hook_func_in(unsigned int hooknum,
>                                struct sk_buff **pskb,
>                                const struct net_device *in,
>                                const struct net_device *out,
>                                int (*okfn)(struct sk_buff *))
>{
>    struct sk_buff * skb = * pskb;
>    int ret;
>    if(skb == NULL){
>	info("HOOK_IN skb==NULL");
>	return NF_DROP;
>    }	
>    if(skb->pkt_type == PACKET_OUTGOING)  
>        ret = check_packet(skb, (short)1); //1 - out
>    else
>	ret = check_packet(skb, (short)0); //0 - in

Stop it! >:)

>    return(ret);
>}
>
>
>Jan 22 11:35:52 FW_EXT kernel: my_fw: fw_init(): Driver my_fw started
>Jan 22 11:35:54 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.198:41924 dst_addr=192.168.0.132:80 [S  ] protocol=6, IN_OUT=1, DATA_OFF = 40
>Jan 22 11:35:54 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.132:80 dst_addr=192.168.0.198:41924 [SA ] protocol=6, IN_OUT=0, DATA_OFF = 40
>Jan 22 11:35:54 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.198:41924 dst_addr=192.168.0.132:80 [ A ] protocol=6, IN_OUT=1, DATA_OFF = 32
>Jan 22 11:35:54 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.198:41924 dst_addr=192.168.0.132:80 [ AP] protocol=6, IN_OUT=1, DATA_OFF = 32
>Jan 22 11:35:54 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: DATA=02 00 01 00 01 00
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: TCP-packet with PUSH flag with wrong payload data in LOCAL_OUT

[Vladimir Kobylyanskiy]

22.01.09, 17:27, "Jan Engelhardt" <jengelh@medozas.de>:

> ># define IP_PRINTF(addr) ((addr) & 0xff), (((addr) >> 8) & 0xff), (((addr) >> 16) & 0xff), (((addr) >> 24) & 0xff)
> Use NIPQUAD and NIPQUAD_FMT.

Ok. Thank you.

> >int ip_packet_from_local_host(struct iphdr *iph)
> >{
> >	struct net_device *dev   = NULL;
> >	struct in_device *in_dev = NULL;
> >	struct in_ifaddr *ifaddr = NULL;
> >	
> >	for (dev = dev_base; dev; dev = dev->next)
> >	{
> >		if (!dev->ip_ptr)
> >		{
> >			continue;
> >		}
> >		in_dev = (struct in_device *)dev->ip_ptr;
> Don't cast this.

Why?


> Now consider this - tcp_data[0] invokes undefined behavior when
>  * there is no payload
>  * or the packet is fragmented (a corner case, though)
> Most likely the former is your case.

> >    ret = check_packet(skb, (short)1); //1 - out
> Do not do redundant casts.
Ok. :)


I add more debug-print to function "check_packet" - to show You, that the packet has payload data and it is not fragmented:

Rewrited function:
==================================================================

static unsigned int check_packet(struct sk_buff *skb, short in_out)
{
    if(skb->nh.iph->protocol == IPPROTO_TCP)
    {
            struct tcphdr       *tcp;
            char                *tcp_data=NULL;
            char                tcp_flags[4];
            unsigned int        tcp_data_off=0;
            unsigned int        tcp_data_len=0;
            unsigned int        if_fragmented = 0;
            unsigned int        flag_MORE_FRAGMENTS = 0;
            unsigned int        FRAGMENT_OFFSET = 0;

            flag_MORE_FRAGMENTS = ntohs(skb->nh.iph->frag_off) & 0x2000;
            FRAGMENT_OFFSET = ntohs(skb->nh.iph->frag_off) & 0x1fff;

            if( (flag_MORE_FRAGMENTS == 1) || (FRAGMENT_OFFSET != 0))
            {
                    if_fragmented = 1;
            }

            tcp = (struct tcphdr *)((char*)skb->nh.iph + skb->nh.iph->ihl * 4);
            tcp_data_off = (tcp->doff)*4;

            tcp_data_len = ntohs(skb->nh.iph->tot_len) - (skb->nh.iph->ihl * 4) - tcp_data_off;

            tcp_data = (char *)tcp + tcp_data_off;
            tcp_flags[0]=' ';
            tcp_flags[1]=' ';
            tcp_flags[2]=' ';
            tcp_flags[3]='\0';

            if(tcp->syn)
                    tcp_flags[0] = 'S';
            if(tcp->ack)
                    tcp_flags[1] = 'A';
            if(tcp->psh)
                    tcp_flags[2] = 'P';
            info("=== HOOK_PACKET: packet src_addr=%u.%u.%u.%u:%u dst_addr=%u.%u.%u.%u:%u [%s] protocol=%u, IN_OUT=%d, DATA_OFF = %u, TCP_DATA_LEN = %u, if_fragmented = %u",
                            IP_PRINTF(skb->nh.iph->saddr),ntohs(tcp->source),IP_PRINTF(skb->nh.iph->daddr),ntohs(tcp->dest),
                            tcp_flags, skb->nh.iph->protocol, in_out, tcp_data_off, tcp_data_len, if_fragmented);
            if(tcp->psh && ntohs(tcp->dest) == 80)
            {
                    info("=== HOOK_PACKET: DATA=%02x %02x %02x %02x %02x %02x", tcp_data[0],tcp_data[1],tcp_data[2],tcp_data[3],tcp_data[4],tcp_data[5]);
//                  return NF_ACCEPT;
            }
    }

==================================================================






Syslog output:
=============
Jan 22 17:27:42 FW_EXT kernel: my_fw: fw_init(): Driver my_fw started
Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.198:37118 dst_addr=192.168.0.132:80 [S  ] protocol=6, IN_OUT=1, DATA_OFF = 40, TCP_DATA_LEN = 0, if_fragmented = 0
Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.132:80 dst_addr=192.168.0.198:37118 [SA ] protocol=6, IN_OUT=0, DATA_OFF = 40, TCP_DATA_LEN = 0, if_fragmented = 0
Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.198:37118 dst_addr=192.168.0.132:80 [ A ] protocol=6, IN_OUT=1, DATA_OFF = 32, TCP_DATA_LEN = 0, if_fragmented = 0
Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.198:37118 dst_addr=192.168.0.132:80 [ AP] protocol=6, IN_OUT=1, DATA_OFF = 32, TCP_DATA_LEN = 101, if_fragmented = 0
Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: DATA=02 00 01 00 01 00

You see - it has 101 bytes of payload and it is not fragmented!
May be when TCP packet with PUSH flag sended from localhost, payload data stored is some special buffer?
Or kernel copy payload data after all firewall checks? (may be for some optimisation reasons )


Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.132:80 dst_addr=192.168.0.198:37118 [ A ] protocol=6, IN_OUT=0, DATA_OFF = 32, TCP_DATA_LEN = 0, if_fragmented = 0
Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.132:80 dst_addr=192.168.0.198:37118 [ AP] protocol=6, IN_OUT=0, DATA_OFF = 32, TCP_DATA_LEN = 842, if_fragmented = 0
Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.198:37118 dst_addr=192.168.0.132:80 [ A ] protocol=6, IN_OUT=1, DATA_OFF = 32, TCP_DATA_LEN = 0, if_fragmented = 0
Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.198:37118 dst_addr=192.168.0.132:80 [ A ] protocol=6, IN_OUT=1, DATA_OFF = 32, TCP_DATA_LEN = 0, if_fragmented = 0
Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.132:80 dst_addr=192.168.0.198:37118 [ A ] protocol=6, IN_OUT=0, DATA_OFF = 32, TCP_DATA_LEN = 0, if_fragmented = 0
Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.198:37118 dst_addr=192.168.0.132:80 [ A ] protocol=6, IN_OUT=1, DATA_OFF = 32, TCP_DATA_LEN = 0, if_fragmented = 0
Jan 22 17:27:49 FW_EXT kernel: my_fw: fw_cleanup(): Driver my_fw stoped

=============

Re: TCP-packet with PUSH flag with wrong payload data in LOCAL_OUT

[Jan Engelhardt]

On Thursday 2009-01-22 16:40, Vladimir Kobylyanskiy wrote:

>> >		in_dev = (struct in_device *)dev->ip_ptr;
>> Don't cast this.
>
>Why?

Because it is not needed. ip_ptr is a void * (at least
in [dr]ecent kernels).

>I add more debug-print to function "check_packet" - to show You,
> that the packet has payload data and it is not fragmented:
>
>            info("=== HOOK_PACKET: packet src_addr=%u.%u.%u.%u:%u dst_addr=%u.%u.%u.%u:%u [%s] protocol=%u, IN_OUT=%d, DATA_OFF = %u, TCP_DATA_LEN = %u, if_fragmented = %u",
>                            IP_PRINTF(skb->nh.iph->saddr),ntohs(tcp->source),IP_PRINTF(skb->nh.iph->daddr),ntohs(tcp->dest),
>                            tcp_flags, skb->nh.iph->protocol, in_out, tcp_data_off, tcp_data_len, if_fragmented);
>            if(tcp->psh && ntohs(tcp->dest) == 80)
>            {
>                    info("=== HOOK_PACKET: DATA=%02x %02x %02x %02x %02x %02x", tcp_data[0],tcp_data[1],tcp_data[2],tcp_data[3],tcp_data[4],tcp_data[5]);
>//                  return NF_ACCEPT;
>            }

>Jan 22 17:27:42 FW_EXT kernel: my_fw: fw_init(): Driver my_fw started
>Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.198:37118 dst_addr=192.168.0.132:80 [S  ] protocol=6, IN_OUT=1, DATA_OFF = 40, TCP_DATA_LEN = 0, if_fragmented = 0
>Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.132:80 dst_addr=192.168.0.198:37118 [SA ] protocol=6, IN_OUT=0, DATA_OFF = 40, TCP_DATA_LEN = 0, if_fragmented = 0
>Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.198:37118 dst_addr=192.168.0.132:80 [ A ] protocol=6, IN_OUT=1, DATA_OFF = 32, TCP_DATA_LEN = 0, if_fragmented = 0
>Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.198:37118 dst_addr=192.168.0.132:80 [ AP] protocol=6, IN_OUT=1, DATA_OFF = 32, TCP_DATA_LEN = 101, if_fragmented = 0
>Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: DATA=02 00 01 00 01 00
>
>You see - it has 101 bytes of payload and it is not fragmented!

Very well. But there still is another case I can think of -
non-linear skb.

	struct tcphdr tcpbuf;
	const struct tcphdr *th;
	
	th = skb_header_pointer(skb, ip_hdrlen(skb), sizeof(tcpbuf), &tcpbuf);
	if (th->psh && ntohs(tcp->dest) == 80) {
		char n[4];

		if (skb_copy_bits(skb, ip_hdrlen(skb) + 4*tcp->doff,
		    n, sizeof(n)) >= 0)
			printk("data: %02x %02x %02x %02x\n", n[0], n[1],
			       n[2], n[3]);
	}

>May be when TCP packet with PUSH flag sended from localhost, payload data stored is some special buffer?

I do not think there is a special buffer (it's not the "urgent"
tcp thing), but it may be at an alternate location.
Using skb_header_pointer/copy_bits should give some
more insight.

>Or kernel copy payload data after all firewall checks? (may be for some optimisation reasons )
>
>
>Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.132:80 dst_addr=192.168.0.198:37118 [ A ] protocol=6, IN_OUT=0, DATA_OFF = 32, TCP_DATA_LEN = 0, if_fragmented = 0
>Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.132:80 dst_addr=192.168.0.198:37118 [ AP] protocol=6, IN_OUT=0, DATA_OFF = 32, TCP_DATA_LEN = 842, if_fragmented = 0
>Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.198:37118 dst_addr=192.168.0.132:80 [ A ] protocol=6, IN_OUT=1, DATA_OFF = 32, TCP_DATA_LEN = 0, if_fragmented = 0
>Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.198:37118 dst_addr=192.168.0.132:80 [ A ] protocol=6, IN_OUT=1, DATA_OFF = 32, TCP_DATA_LEN = 0, if_fragmented = 0
>Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.132:80 dst_addr=192.168.0.198:37118 [ A ] protocol=6, IN_OUT=0, DATA_OFF = 32, TCP_DATA_LEN = 0, if_fragmented = 0
>Jan 22 17:27:45 FW_EXT kernel: my_fw: check_packet(): === HOOK_PACKET: packet src_addr=192.168.0.198:37118 dst_addr=192.168.0.132:80 [ A ] protocol=6, IN_OUT=1, DATA_OFF = 32, TCP_DATA_LEN = 0, if_fragmented = 0
>Jan 22 17:27:49 FW_EXT kernel: my_fw: fw_cleanup(): Driver my_fw stoped
>
>=============
>

Re: TCP-packet with PUSH flag with wrong payload data in LOCAL_OUT

[Vladimir Kobylyanskiy]

22.01.09, 19:11, "Jan Engelhardt" <jengelh@medozas.de>:

> Very well. But there still is another case I can think of -
> non-linear skb.
> 	struct tcphdr tcpbuf;
> 	const struct tcphdr *th;
> 	
> 	th = skb_header_pointer(skb, ip_hdrlen(skb), sizeof(tcpbuf), &tcpbuf);
> 	if (th->psh && ntohs(tcp->dest) == 80) {
> 		char n[4];
> 		if (skb_copy_bits(skb, ip_hdrlen(skb) + 4*tcp->doff,
> 		    n, sizeof(n)) >= 0)
> 			printk("data: %02x %02x %02x %02x\n", n[0], n[1],
> 			       n[2], n[3]);
> 	}

Yes! It is non-linear skb! All works! THANK YOU VERY MUCH!
I try to fix this bug for about 2 weeks. And You fix it for 15 minutes!
Non of my books tell me about non-linear skb. Google was useless!
Man, You are the best! :)

I want to give You a little present - bottle of Russian vodka (old Russian tradition :) ). 
How can I pass it to You? Give me your address please!

Re: TCP-packet with PUSH flag with wrong payload data in LOCAL_OUT

[Jan Engelhardt]

On Thursday 2009-01-22 17:54, Vladimir Kobylyanskiy wrote:
>
>> Very well. But there still is another case I can think of -
>> non-linear skb.
>> 	struct tcphdr tcpbuf;
>> 	const struct tcphdr *th;
>> 	
>> 	th = skb_header_pointer(skb, ip_hdrlen(skb), sizeof(tcpbuf), &tcpbuf);
>> 	if (th->psh && ntohs(tcp->dest) == 80) {
>> 		char n[4];
>> 		if (skb_copy_bits(skb, ip_hdrlen(skb) + 4*tcp->doff,
>> 		    n, sizeof(n)) >= 0)
>> 			printk("data: %02x %02x %02x %02x\n", n[0], n[1],
>> 			       n[2], n[3]);
>> 	}
>
>Yes! It is non-linear skb! All works! THANK YOU VERY MUCH!
>I try to fix this bug for about 2 weeks. And You fix it for 15 minutes!
>Non of my books tell me about non-linear skb.

I suggest my very own 
http://jengelh.medozas.de/Netfilter_Modules.pdf

supplemental to that one might want to read "Linux Networking Internals" 
as published by O'Reilly, though, it is based on 2.6.14 and one needs 
to think a fair bit to make it applicable to 14 releases later that we 
had.

Re: TCP-packet with PUSH flag with wrong payload data in LOCAL_OUT

[James King]

On Thu, Jan 22, 2009 at 8:11 AM, Jan Engelhardt <jengelh@medozas.de> wrote:
> Very well. But there still is another case I can think of -
> non-linear skb.
>
>        struct tcphdr tcpbuf;
>        const struct tcphdr *th;
>
>        th = skb_header_pointer(skb, ip_hdrlen(skb), sizeof(tcpbuf), &tcpbuf);
>        if (th->psh && ntohs(tcp->dest) == 80) {
>                char n[4];
>
>                if (skb_copy_bits(skb, ip_hdrlen(skb) + 4*tcp->doff,
>                    n, sizeof(n)) >= 0)
>                        printk("data: %02x %02x %02x %02x\n", n[0], n[1],
>                               n[2], n[3]);
>        }

Wouldn't

if ((skb_linearize(skb) != 0) && net_ratelimit())
      printk("skb is non-linear, but failed to linearize");

be more preferable?

Re: TCP-packet with PUSH flag with wrong payload data in LOCAL_OUT

[Jan Engelhardt]

On Thursday 2009-01-22 19:15, James King wrote:
>On Thu, Jan 22, 2009 at 8:11 AM, Jan Engelhardt <jengelh@medozas.de> wrote:
>> Very well. But there still is another case I can think of -
>> non-linear skb.
>>
>>        struct tcphdr tcpbuf;
>>        const struct tcphdr *th;
>>
>>        th = skb_header_pointer(skb, ip_hdrlen(skb), sizeof(tcpbuf), &tcpbuf);
>>        if (th->psh && ntohs(tcp->dest) == 80) {
>>                char n[4];
>>
>>                if (skb_copy_bits(skb, ip_hdrlen(skb) + 4*tcp->doff,
>>                    n, sizeof(n)) >= 0)
>>                        printk("data: %02x %02x %02x %02x\n", n[0], n[1],
>>                               n[2], n[3]);
>>        }
>
>Wouldn't
>
>if ((skb_linearize(skb) != 0) && net_ratelimit())
>      printk("skb is non-linear, but failed to linearize");
>
>be more preferable?

I don't think you want to be spending the CPU cycles for that.
skb_copy_.. has a reason, at least for when you only need
parts of the packet. A whole packet can be large --
the MTU for lo is slightly above 16K, and that's a pretty big
allocation already. The larger the allocation (come to think
of jumbo packets), the sooner the fail.