* MD5 SG fix @ 2008-07-01 21:16 Adam Langley 2008-07-01 21:39 ` Stephen Hemminger 0 siblings, 1 reply; 12+ messages in thread From: Adam Langley @ 2008-07-01 21:16 UTC (permalink / raw) To: Stephen Hemminger; +Cc: netdev I believe this patch fixes MD5 in the face of SG interfaces for IPv4. Stephen, could you test this because I don't get non-linear packets in my test network? The patch includes a debugging function which can be uncommented if you have issues. If this works for you, I'll clean it up and perform the same fix for IPv6 before submiting. Sadly, I couldn't just pass in the SKB to the md5_hash function because there are several places where we don't have an SKB to hand (generating ACKs and RSTs, for example). This patch is, obviously, semantically incongruent with [1] and [1] should be backed out before applying. Cheers, AGL [1] http://marc.info/?l=linux-netdev&m=121459157816964&w=2 --- include/net/tcp.h | 11 +++-- net/ipv4/tcp_ipv4.c | 113 +++++++++++++++++++++++++++++++++++++------------ net/ipv4/tcp_output.c | 14 ++++-- 3 files changed, 101 insertions(+), 37 deletions(-) diff --git a/include/net/tcp.h b/include/net/tcp.h index 633147c..4213379 100644 --- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -1110,6 +1110,7 @@ union tcp_md5sum_block { struct tcp_md5sig_pool { struct hash_desc md5_desc; union tcp_md5sum_block md5_blk; + struct scatterlist sg[MAX_SKB_FRAGS + 3]; }; #define TCP_MD5SIG_MAXKEYS (~(u32)0) /* really?! */ @@ -1120,9 +1121,10 @@ extern int tcp_v4_calc_md5_hash(char *md5_hash, struct sock *sk, struct dst_entry *dst, struct request_sock *req, - struct tcphdr *th, int protocol, - unsigned int tcplen); + struct tcphdr *th, + int data_off, int tcplen, + struct skb_shared_info *); extern struct tcp_md5sig_key *tcp_v4_md5_lookup(struct sock *sk, struct sock *addr_sk); @@ -1370,9 +1372,10 @@ struct tcp_sock_af_ops { struct sock *sk, struct dst_entry *dst, struct request_sock *req, - struct tcphdr *th, int protocol, - unsigned int len); + struct tcphdr *th, + int data_off, int tcplen, + struct skb_shared_info*frags); int (*md5_add) (struct sock *sk, struct sock *addr_sk, u8 *newkey, diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index cd601a8..5041e92 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -95,8 +95,9 @@ static struct tcp_md5sig_key *tcp_v4_md5_do_lookup(struct sock *sk, __be32 addr); static int tcp_v4_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, __be32 saddr, __be32 daddr, - struct tcphdr *th, int protocol, - unsigned int tcplen); + int protocol, + struct tcphdr *th, int data_off, int tcplen, + struct skb_shared_info *frags); #endif struct inet_hashinfo __cacheline_aligned tcp_hashinfo = { @@ -586,8 +587,9 @@ static void tcp_v4_send_reset(struct sock *sk, struct sk_buff *skb) key, ip_hdr(skb)->daddr, ip_hdr(skb)->saddr, - &rep.th, IPPROTO_TCP, - arg.iov[0].iov_len); + IPPROTO_TCP, &rep.th, + arg.iov[0].iov_len, arg.iov[0].iov_len, + NULL); } #endif arg.csum = csum_tcpudp_nofold(ip_hdr(skb)->daddr, @@ -680,8 +682,9 @@ static void tcp_v4_send_ack(struct tcp_timewait_sock *twsk, key, ip_hdr(skb)->daddr, ip_hdr(skb)->saddr, - &rep.th, IPPROTO_TCP, - arg.iov[0].iov_len); + IPPROTO_TCP, &rep.th, + arg.iov[0].iov_len, arg.iov[0].iov_len, + NULL); } #endif arg.csum = csum_tcpudp_nofold(ip_hdr(skb)->daddr, @@ -1004,20 +1007,56 @@ static int tcp_v4_parse_md5_keys(struct sock *sk, char __user *optval, newkey, cmd.tcpm_keylen); } +/* +static void +md5_dump(struct scatterlist *sglist, int nsg, unsigned nbytes, int data_off, int tcplen, char frags) { + struct scatterlist *sg; + unsigned e; + + printk(KERN_INFO "MD5: %d %d %d %d %d\n", nsg, nbytes, data_off, tcplen, frags); + for_each_sg(sglist, sg, nsg, e) { + u8 *buf = sg_virt(sg); + unsigned i; + + for (i = 0; i < sg->length; ++i) + printk(" %02x", buf[i]); + + printk("\n"); + } + + printk("\n"); +}*/ + +/** + * tcp_v4_do_calc_md5_hash - calculate an MD5 hash (RFC 2385) + * @md5_hash: (output) a 16 byte space into which the MD5 sig is written + * @key: the key that is appened to the hash input + * @saddr: source IP address for the packet + * @daddr: destination IP address for the packet + * @protocol: the protocol number in the IP header (see the RFC) + * @th: the TCP header, followed by options and (optional) data + * @data_off: the offset of the optional data (in bytes) from @th + * @tcplen: the length of the buffer (in bytes) pointed to by @th. If + * @tcplen == @data_off then there is no data following the header + * @frags: (maybe NULL) a list of additional fragments of data + * + * We don't always have the SKB when this function is called, thus the pointer + * to the TCP header and all the length arguments. + */ static int tcp_v4_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, - __be32 saddr, __be32 daddr, - struct tcphdr *th, int protocol, - unsigned int tcplen) + __be32 saddr, __be32 daddr, int protocol, + struct tcphdr *th, int data_off, int tcplen, + struct skb_shared_info *frags) { - struct scatterlist sg[4]; - __u16 data_len; + struct scatterlist *sg; + const __u16 head_data_len = tcplen - data_off; int block = 0; __sum16 old_checksum; struct tcp_md5sig_pool *hp; struct tcp4_pseudohdr *bp; struct hash_desc *desc; - int err; - unsigned int nbytes = 0; + int err, i; + unsigned int nbytes = tcplen; /* * Okay, so RFC2385 is turned on for this connection, @@ -1029,6 +1068,7 @@ static int tcp_v4_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, goto clear_hash_noput; bp = &hp->md5_blk.ip4; + sg = hp->sg; desc = &hp->md5_desc; /* @@ -1040,9 +1080,14 @@ static int tcp_v4_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, bp->daddr = daddr; bp->pad = 0; bp->protocol = protocol; - bp->len = htons(tcplen); + if (frags) + for (i = 0; i < frags->nr_frags; ++i) + nbytes += frags->frags[i].size; + bp->len = htons(nbytes); + nbytes = 0; - sg_init_table(sg, 4); + sg_init_table(sg, 3 + (head_data_len > 0) + + (frags ? frags->nr_frags : 0)); sg_set_buf(&sg[block++], bp, sizeof(*bp)); nbytes += sizeof(*bp); @@ -1056,11 +1101,21 @@ static int tcp_v4_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, nbytes += sizeof(struct tcphdr); /* 3. the TCP segment data (if any) */ - data_len = tcplen - (th->doff << 2); - if (data_len > 0) { - unsigned char *data = (unsigned char *)th + (th->doff << 2); - sg_set_buf(&sg[block++], data, data_len); - nbytes += data_len; + if (head_data_len > 0) { + unsigned char *data = (unsigned char *)th + data_off; + sg_set_buf(&sg[block++], data, head_data_len); + nbytes += head_data_len; + } + + if (frags) { + for (i = 0; i < frags->nr_frags; ++i) { + const struct skb_frag_struct *f = &frags->frags[i]; + sg_set_buf(&sg[block++], + ((unsigned char *) page_address(f->page)) + + f->page_offset, + f->size); + nbytes += f->size; + } } /* 4. an independently-specified key or password, known to both @@ -1069,7 +1124,7 @@ static int tcp_v4_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, sg_set_buf(&sg[block++], key->key, key->keylen); nbytes += key->keylen; - sg_mark_end(&sg[block - 1]); + /*md5_dump(sg, block, nbytes, data_off, tcplen, frags != NULL);*/ /* Now store the Hash into the packet */ err = crypto_hash_init(desc); @@ -1099,8 +1154,9 @@ int tcp_v4_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, struct sock *sk, struct dst_entry *dst, struct request_sock *req, - struct tcphdr *th, int protocol, - unsigned int tcplen) + int protocol, + struct tcphdr *th, int data_off, int tcplen, + struct skb_shared_info *frags) { __be32 saddr, daddr; @@ -1113,9 +1169,8 @@ int tcp_v4_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, saddr = rt->rt_src; daddr = rt->rt_dst; } - return tcp_v4_do_calc_md5_hash(md5_hash, key, - saddr, daddr, - th, protocol, tcplen); + return tcp_v4_do_calc_md5_hash(md5_hash, key, saddr, daddr, protocol, + th, data_off, tcplen, frags); } EXPORT_SYMBOL(tcp_v4_calc_md5_hash); @@ -1206,8 +1261,10 @@ done_opts: genhash = tcp_v4_do_calc_md5_hash(newhash, hash_expected, iph->saddr, iph->daddr, - th, sk->sk_protocol, - skb->len); + sk->sk_protocol, th, th->doff << 2, + skb_headlen(skb), + skb_is_nonlinear(skb) ? + skb_shinfo(skb) : NULL); if (genhash || memcmp(hash_location, newhash, 16) != 0) { if (net_ratelimit()) { diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index e399bde..40b2c72 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -606,9 +606,11 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it, tp->af_specific->calc_md5_hash(md5_hash_location, md5, sk, NULL, NULL, - tcp_hdr(skb), sk->sk_protocol, - skb->len); + th, th->doff << 2, + skb_headlen(skb), + skb_is_nonlinear(skb) ? + skb_shinfo(skb) : NULL); } #endif @@ -2263,9 +2265,11 @@ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst, if (md5) { tp->af_specific->calc_md5_hash(md5_hash_location, md5, - NULL, dst, req, - tcp_hdr(skb), sk->sk_protocol, - skb->len); + NULL, dst, req, sk->sk_protocol, + th, th->doff << 2, + skb_headlen(skb), + skb_is_nonlinear(skb) ? + skb_shinfo(skb) : NULL); } #endif ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: MD5 SG fix 2008-07-01 21:16 MD5 SG fix Adam Langley @ 2008-07-01 21:39 ` Stephen Hemminger 2008-07-01 21:48 ` Adam Langley 0 siblings, 1 reply; 12+ messages in thread From: Stephen Hemminger @ 2008-07-01 21:39 UTC (permalink / raw) To: Adam Langley; +Cc: netdev On Tue, 1 Jul 2008 14:16:34 -0700 "Adam Langley" <agl@imperialviolet.org> wrote: > I believe this patch fixes MD5 in the face of SG interfaces for IPv4. Stephen, > could you test this because I don't get non-linear packets in my test network? > The patch includes a debugging function which can be uncommented if you have > issues. > > If this works for you, I'll clean it up and perform the same fix for IPv6 > before submiting. > > Sadly, I couldn't just pass in the SKB to the md5_hash function because there > are several places where we don't have an SKB to hand (generating ACKs and > RSTs, for example). > > This patch is, obviously, semantically incongruent with [1] and [1] should be > backed out before applying. > > Cheers, > > > AGL > > > [1] http://marc.info/?l=linux-netdev&m=121459157816964&w=2 > > --- > > include/net/tcp.h | 11 +++-- > net/ipv4/tcp_ipv4.c | 113 +++++++++++++++++++++++++++++++++++++------------ > net/ipv4/tcp_output.c | 14 ++++-- > 3 files changed, 101 insertions(+), 37 deletions(-) > > diff --git a/include/net/tcp.h b/include/net/tcp.h > index 633147c..4213379 100644 > --- a/include/net/tcp.h > +++ b/include/net/tcp.h > @@ -1110,6 +1110,7 @@ union tcp_md5sum_block { > struct tcp_md5sig_pool { > struct hash_desc md5_desc; > union tcp_md5sum_block md5_blk; > + struct scatterlist sg[MAX_SKB_FRAGS + 3]; > }; > I would rather see the pool used less and the stack used more. Global context is more like FORTRAN common blocks. My suggestion would be to have two hash functions, one that takes a block for the cases of TCP SYN, etc. and another that takes an skb for data packets. You still need to have some of the places that reset sk_route_caps otherwise you will get TSO/GSO packets because of the resetting of route_caps after the SYN/ACK. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: MD5 SG fix 2008-07-01 21:39 ` Stephen Hemminger @ 2008-07-01 21:48 ` Adam Langley 2008-07-01 21:49 ` Adam Langley 0 siblings, 1 reply; 12+ messages in thread From: Adam Langley @ 2008-07-01 21:48 UTC (permalink / raw) To: Stephen Hemminger; +Cc: netdev On Tue, Jul 1, 2008 at 2:39 PM, Stephen Hemminger <shemminger@vyatta.com> wrote: > I would rather see the pool used less and the stack used more. > Global context is more like FORTRAN common blocks. Will do. My only concern was that it was 214 bytes which could easily not be on the stack. > My suggestion would be to have two hash functions, one that takes a block > for the cases of TCP SYN, etc. and another that takes an skb for data packets. You're probably right. If it works I'll break it out like that. > You still need to have some of the places that reset sk_route_caps otherwise > you will get TSO/GSO packets because of the resetting of route_caps after the SYN/ACK. I've copied the hunks from [1] where sk_route_caps were added (not just changed from ~GSO to ~(GSO|SG)). Hopefully those are the right ones. I'll resend in a sec. Cheers, AGL -- Adam Langley agl@imperialviolet.org http://www.imperialviolet.org ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: MD5 SG fix 2008-07-01 21:48 ` Adam Langley @ 2008-07-01 21:49 ` Adam Langley 2008-07-01 22:38 ` Adam Langley 0 siblings, 1 reply; 12+ messages in thread From: Adam Langley @ 2008-07-01 21:49 UTC (permalink / raw) To: Stephen Hemminger; +Cc: netdev Second cut: copy from hunks from [1] where route caps were set to disable GSO. Cheers, AGL [1] http://marc.info/?l=linux-netdev&m=121459157816964&w=2 --- include/net/tcp.h | 11 +++-- net/ipv4/tcp_ipv4.c | 114 +++++++++++++++++++++++++++++++++++++------------ net/ipv4/tcp_output.c | 18 +++++--- 3 files changed, 105 insertions(+), 38 deletions(-) diff --git a/include/net/tcp.h b/include/net/tcp.h index 633147c..4213379 100644 --- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -1110,6 +1110,7 @@ union tcp_md5sum_block { struct tcp_md5sig_pool { struct hash_desc md5_desc; union tcp_md5sum_block md5_blk; + struct scatterlist sg[MAX_SKB_FRAGS + 3]; }; #define TCP_MD5SIG_MAXKEYS (~(u32)0) /* really?! */ @@ -1120,9 +1121,10 @@ extern int tcp_v4_calc_md5_hash(char *md5_hash, struct sock *sk, struct dst_entry *dst, struct request_sock *req, - struct tcphdr *th, int protocol, - unsigned int tcplen); + struct tcphdr *th, + int data_off, int tcplen, + struct skb_shared_info *); extern struct tcp_md5sig_key *tcp_v4_md5_lookup(struct sock *sk, struct sock *addr_sk); @@ -1370,9 +1372,10 @@ struct tcp_sock_af_ops { struct sock *sk, struct dst_entry *dst, struct request_sock *req, - struct tcphdr *th, int protocol, - unsigned int len); + struct tcphdr *th, + int data_off, int tcplen, + struct skb_shared_info*frags); int (*md5_add) (struct sock *sk, struct sock *addr_sk, u8 *newkey, diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index cd601a8..1ac4b1f 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -95,8 +95,9 @@ static struct tcp_md5sig_key *tcp_v4_md5_do_lookup(struct sock *sk, __be32 addr); static int tcp_v4_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, __be32 saddr, __be32 daddr, - struct tcphdr *th, int protocol, - unsigned int tcplen); + int protocol, + struct tcphdr *th, int data_off, int tcplen, + struct skb_shared_info *frags); #endif struct inet_hashinfo __cacheline_aligned tcp_hashinfo = { @@ -586,8 +587,9 @@ static void tcp_v4_send_reset(struct sock *sk, struct sk_buff *skb) key, ip_hdr(skb)->daddr, ip_hdr(skb)->saddr, - &rep.th, IPPROTO_TCP, - arg.iov[0].iov_len); + IPPROTO_TCP, &rep.th, + arg.iov[0].iov_len, arg.iov[0].iov_len, + NULL); } #endif arg.csum = csum_tcpudp_nofold(ip_hdr(skb)->daddr, @@ -680,8 +682,9 @@ static void tcp_v4_send_ack(struct tcp_timewait_sock *twsk, key, ip_hdr(skb)->daddr, ip_hdr(skb)->saddr, - &rep.th, IPPROTO_TCP, - arg.iov[0].iov_len); + IPPROTO_TCP, &rep.th, + arg.iov[0].iov_len, arg.iov[0].iov_len, + NULL); } #endif arg.csum = csum_tcpudp_nofold(ip_hdr(skb)->daddr, @@ -1004,20 +1007,56 @@ static int tcp_v4_parse_md5_keys(struct sock *sk, char __user *optval, newkey, cmd.tcpm_keylen); } +/* +static void +md5_dump(struct scatterlist *sglist, int nsg, unsigned nbytes, int data_off, int tcplen, char frags) { + struct scatterlist *sg; + unsigned e; + + printk(KERN_INFO "MD5: %d %d %d %d %d\n", nsg, nbytes, data_off, tcplen, frags); + for_each_sg(sglist, sg, nsg, e) { + u8 *buf = sg_virt(sg); + unsigned i; + + for (i = 0; i < sg->length; ++i) + printk(" %02x", buf[i]); + + printk("\n"); + } + + printk("\n"); +}*/ + +/** + * tcp_v4_do_calc_md5_hash - calculate an MD5 hash (RFC 2385) + * @md5_hash: (output) a 16 byte space into which the MD5 sig is written + * @key: the key that is appened to the hash input + * @saddr: source IP address for the packet + * @daddr: destination IP address for the packet + * @protocol: the protocol number in the IP header (see the RFC) + * @th: the TCP header, followed by options and (optional) data + * @data_off: the offset of the optional data (in bytes) from @th + * @tcplen: the length of the buffer (in bytes) pointed to by @th. If + * @tcplen == @data_off then there is no data following the header + * @frags: (maybe NULL) a list of additional fragments of data + * + * We don't always have the SKB when this function is called, thus the pointer + * to the TCP header and all the length arguments. + */ static int tcp_v4_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, - __be32 saddr, __be32 daddr, - struct tcphdr *th, int protocol, - unsigned int tcplen) + __be32 saddr, __be32 daddr, int protocol, + struct tcphdr *th, int data_off, int tcplen, + struct skb_shared_info *frags) { - struct scatterlist sg[4]; - __u16 data_len; + struct scatterlist *sg; + const __u16 head_data_len = tcplen - data_off; int block = 0; __sum16 old_checksum; struct tcp_md5sig_pool *hp; struct tcp4_pseudohdr *bp; struct hash_desc *desc; - int err; - unsigned int nbytes = 0; + int err, i; + unsigned int nbytes = tcplen; /* * Okay, so RFC2385 is turned on for this connection, @@ -1029,6 +1068,7 @@ static int tcp_v4_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, goto clear_hash_noput; bp = &hp->md5_blk.ip4; + sg = hp->sg; desc = &hp->md5_desc; /* @@ -1040,9 +1080,14 @@ static int tcp_v4_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, bp->daddr = daddr; bp->pad = 0; bp->protocol = protocol; - bp->len = htons(tcplen); + if (frags) + for (i = 0; i < frags->nr_frags; ++i) + nbytes += frags->frags[i].size; + bp->len = htons(nbytes); + nbytes = 0; - sg_init_table(sg, 4); + sg_init_table(sg, 3 + (head_data_len > 0) + + (frags ? frags->nr_frags : 0)); sg_set_buf(&sg[block++], bp, sizeof(*bp)); nbytes += sizeof(*bp); @@ -1056,11 +1101,21 @@ static int tcp_v4_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, nbytes += sizeof(struct tcphdr); /* 3. the TCP segment data (if any) */ - data_len = tcplen - (th->doff << 2); - if (data_len > 0) { - unsigned char *data = (unsigned char *)th + (th->doff << 2); - sg_set_buf(&sg[block++], data, data_len); - nbytes += data_len; + if (head_data_len > 0) { + unsigned char *data = (unsigned char *)th + data_off; + sg_set_buf(&sg[block++], data, head_data_len); + nbytes += head_data_len; + } + + if (frags) { + for (i = 0; i < frags->nr_frags; ++i) { + const struct skb_frag_struct *f = &frags->frags[i]; + sg_set_buf(&sg[block++], + ((unsigned char *) page_address(f->page)) + + f->page_offset, + f->size); + nbytes += f->size; + } } /* 4. an independently-specified key or password, known to both @@ -1069,7 +1124,7 @@ static int tcp_v4_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, sg_set_buf(&sg[block++], key->key, key->keylen); nbytes += key->keylen; - sg_mark_end(&sg[block - 1]); + /*md5_dump(sg, block, nbytes, data_off, tcplen, frags != NULL);*/ /* Now store the Hash into the packet */ err = crypto_hash_init(desc); @@ -1099,8 +1154,9 @@ int tcp_v4_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, struct sock *sk, struct dst_entry *dst, struct request_sock *req, - struct tcphdr *th, int protocol, - unsigned int tcplen) + int protocol, + struct tcphdr *th, int data_off, int tcplen, + struct skb_shared_info *frags) { __be32 saddr, daddr; @@ -1113,9 +1169,8 @@ int tcp_v4_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, saddr = rt->rt_src; daddr = rt->rt_dst; } - return tcp_v4_do_calc_md5_hash(md5_hash, key, - saddr, daddr, - th, protocol, tcplen); + return tcp_v4_do_calc_md5_hash(md5_hash, key, saddr, daddr, protocol, + th, data_off, tcplen, frags); } EXPORT_SYMBOL(tcp_v4_calc_md5_hash); @@ -1206,8 +1261,10 @@ done_opts: genhash = tcp_v4_do_calc_md5_hash(newhash, hash_expected, iph->saddr, iph->daddr, - th, sk->sk_protocol, - skb->len); + sk->sk_protocol, th, th->doff << 2, + skb_headlen(skb), + skb_is_nonlinear(skb) ? + skb_shinfo(skb) : NULL); if (genhash || memcmp(hash_location, newhash, 16) != 0) { if (net_ratelimit()) { @@ -1456,6 +1513,7 @@ struct sock *tcp_v4_syn_recv_sock(struct sock *sk, struct sk_buff *skb, if (newkey != NULL) tcp_v4_md5_do_add(newsk, inet_sk(sk)->daddr, newkey, key->keylen); + newsk->sk_route_caps &= ~NETIF_F_GSO_MASK; } #endif diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index e399bde..c5f044b 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -542,8 +542,10 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it, * room for it. */ md5 = tp->af_specific->md5_lookup(sk, sk); - if (md5) + if (md5) { tcp_header_size += TCPOLEN_MD5SIG_ALIGNED; + sk->sk_route_caps &= ~NETIF_F_GSO_MASK; + } #endif skb_push(skb, tcp_header_size); @@ -606,9 +608,11 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it, tp->af_specific->calc_md5_hash(md5_hash_location, md5, sk, NULL, NULL, - tcp_hdr(skb), sk->sk_protocol, - skb->len); + th, th->doff << 2, + skb_headlen(skb), + skb_is_nonlinear(skb) ? + skb_shinfo(skb) : NULL); } #endif @@ -2263,9 +2267,11 @@ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst, if (md5) { tp->af_specific->calc_md5_hash(md5_hash_location, md5, - NULL, dst, req, - tcp_hdr(skb), sk->sk_protocol, - skb->len); + NULL, dst, req, sk->sk_protocol, + th, th->doff << 2, + skb_headlen(skb), + skb_is_nonlinear(skb) ? + skb_shinfo(skb) : NULL); } #endif ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: MD5 SG fix 2008-07-01 21:49 ` Adam Langley @ 2008-07-01 22:38 ` Adam Langley 2008-07-01 22:46 ` Stephen Hemminger ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: Adam Langley @ 2008-07-01 22:38 UTC (permalink / raw) To: Stephen Hemminger; +Cc: netdev Cut three, * I should use sg_set_page, not sg_set_buf with page_address * Make the version that tcp_output etc use take an SKB * Replicate fix to IPv6 Cheers, AGL include/net/tcp.h | 6 +-- net/ipv4/tcp_ipv4.c | 112 +++++++++++++++++++++++++++++++++++++------------ net/ipv4/tcp_output.c | 11 ++--- net/ipv6/tcp_ipv6.c | 65 ++++++++++++++++++---------- 4 files changed, 133 insertions(+), 61 deletions(-) diff --git a/include/net/tcp.h b/include/net/tcp.h index 633147c..a9130a1 100644 --- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -1120,9 +1120,8 @@ extern int tcp_v4_calc_md5_hash(char *md5_hash, struct sock *sk, struct dst_entry *dst, struct request_sock *req, - struct tcphdr *th, int protocol, - unsigned int tcplen); + struct sk_buff *skb); extern struct tcp_md5sig_key *tcp_v4_md5_lookup(struct sock *sk, struct sock *addr_sk); @@ -1370,9 +1369,8 @@ struct tcp_sock_af_ops { struct sock *sk, struct dst_entry *dst, struct request_sock *req, - struct tcphdr *th, int protocol, - unsigned int len); + struct sk_buff *skb); int (*md5_add) (struct sock *sk, struct sock *addr_sk, u8 *newkey, diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index cd601a8..90a5f41 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -95,8 +95,9 @@ static struct tcp_md5sig_key *tcp_v4_md5_do_lookup(struct sock *sk, __be32 addr); static int tcp_v4_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, __be32 saddr, __be32 daddr, - struct tcphdr *th, int protocol, - unsigned int tcplen); + int protocol, + struct tcphdr *th, int data_off, int tcplen, + struct skb_shared_info *frags); #endif struct inet_hashinfo __cacheline_aligned tcp_hashinfo = { @@ -586,8 +587,9 @@ static void tcp_v4_send_reset(struct sock *sk, struct sk_buff *skb) key, ip_hdr(skb)->daddr, ip_hdr(skb)->saddr, - &rep.th, IPPROTO_TCP, - arg.iov[0].iov_len); + IPPROTO_TCP, &rep.th, + arg.iov[0].iov_len, arg.iov[0].iov_len, + NULL); } #endif arg.csum = csum_tcpudp_nofold(ip_hdr(skb)->daddr, @@ -680,8 +682,9 @@ static void tcp_v4_send_ack(struct tcp_timewait_sock *twsk, key, ip_hdr(skb)->daddr, ip_hdr(skb)->saddr, - &rep.th, IPPROTO_TCP, - arg.iov[0].iov_len); + IPPROTO_TCP, &rep.th, + arg.iov[0].iov_len, arg.iov[0].iov_len, + NULL); } #endif arg.csum = csum_tcpudp_nofold(ip_hdr(skb)->daddr, @@ -1004,20 +1007,56 @@ static int tcp_v4_parse_md5_keys(struct sock *sk, char __user *optval, newkey, cmd.tcpm_keylen); } +/* +static void +md5_dump(struct scatterlist *sglist, int nsg, unsigned nbytes, int data_off, int tcplen, char frags) { + struct scatterlist *sg; + unsigned e; + + printk(KERN_INFO "MD5: %d %d %d %d %d\n", nsg, nbytes, data_off, tcplen, frags); + for_each_sg(sglist, sg, nsg, e) { + u8 *buf = sg_virt(sg); + unsigned i; + + for (i = 0; i < sg->length; ++i) + printk(" %02x", buf[i]); + + printk("\n"); + } + + printk("\n"); +}*/ + +/** + * tcp_v4_do_calc_md5_hash - calculate an MD5 hash (RFC 2385) + * @md5_hash: (output) a 16 byte space into which the MD5 sig is written + * @key: the key that is appened to the hash input + * @saddr: source IP address for the packet + * @daddr: destination IP address for the packet + * @protocol: the protocol number in the IP header (see the RFC) + * @th: the TCP header, followed by options and (optional) data + * @data_off: the offset of the optional data (in bytes) from @th + * @tcplen: the length of the buffer (in bytes) pointed to by @th. If + * @tcplen == @data_off then there is no data following the header + * @frags: (maybe NULL) a list of additional fragments of data + * + * We don't always have the SKB when this function is called, thus the pointer + * to the TCP header and all the length arguments. + */ static int tcp_v4_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, - __be32 saddr, __be32 daddr, - struct tcphdr *th, int protocol, - unsigned int tcplen) + __be32 saddr, __be32 daddr, int protocol, + struct tcphdr *th, int data_off, int tcplen, + struct skb_shared_info *frags) { - struct scatterlist sg[4]; - __u16 data_len; + struct scatterlist sg[MAX_SKB_FRAGS + 3]; + const int head_data_len = tcplen - data_off; int block = 0; __sum16 old_checksum; struct tcp_md5sig_pool *hp; struct tcp4_pseudohdr *bp; struct hash_desc *desc; - int err; - unsigned int nbytes = 0; + int err, i; + unsigned int nbytes = tcplen; /* * Okay, so RFC2385 is turned on for this connection, @@ -1040,9 +1079,14 @@ static int tcp_v4_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, bp->daddr = daddr; bp->pad = 0; bp->protocol = protocol; - bp->len = htons(tcplen); + if (frags) + for (i = 0; i < frags->nr_frags; ++i) + nbytes += frags->frags[i].size; + bp->len = htons(nbytes); + nbytes = 0; - sg_init_table(sg, 4); + sg_init_table(sg, 3 + (head_data_len > 0) + + (frags ? frags->nr_frags : 0)); sg_set_buf(&sg[block++], bp, sizeof(*bp)); nbytes += sizeof(*bp); @@ -1056,11 +1100,19 @@ static int tcp_v4_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, nbytes += sizeof(struct tcphdr); /* 3. the TCP segment data (if any) */ - data_len = tcplen - (th->doff << 2); - if (data_len > 0) { - unsigned char *data = (unsigned char *)th + (th->doff << 2); - sg_set_buf(&sg[block++], data, data_len); - nbytes += data_len; + if (head_data_len > 0) { + unsigned char *data = (unsigned char *)th + data_off; + sg_set_buf(&sg[block++], data, head_data_len); + nbytes += head_data_len; + } + + if (frags) { + for (i = 0; i < frags->nr_frags; ++i) { + const struct skb_frag_struct *f = &frags->frags[i]; + sg_set_page(&sg[block++], f->page, f->size, + f->page_offset); + nbytes += f->size; + } } /* 4. an independently-specified key or password, known to both @@ -1069,7 +1121,7 @@ static int tcp_v4_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, sg_set_buf(&sg[block++], key->key, key->keylen); nbytes += key->keylen; - sg_mark_end(&sg[block - 1]); + /*md5_dump(sg, block, nbytes, data_off, tcplen, frags != NULL);*/ /* Now store the Hash into the packet */ err = crypto_hash_init(desc); @@ -1099,10 +1151,10 @@ int tcp_v4_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, struct sock *sk, struct dst_entry *dst, struct request_sock *req, - struct tcphdr *th, int protocol, - unsigned int tcplen) + int protocol, struct sk_buff *skb) { __be32 saddr, daddr; + struct tcphdr *th = tcp_hdr(skb); if (sk) { saddr = inet_sk(sk)->saddr; @@ -1113,9 +1165,10 @@ int tcp_v4_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, saddr = rt->rt_src; daddr = rt->rt_dst; } - return tcp_v4_do_calc_md5_hash(md5_hash, key, - saddr, daddr, - th, protocol, tcplen); + return tcp_v4_do_calc_md5_hash(md5_hash, key, saddr, daddr, protocol, + th, th->doff << 2, skb_headlen(skb), + skb_is_nonlinear(skb) ? + skb_shinfo(skb) : NULL); } EXPORT_SYMBOL(tcp_v4_calc_md5_hash); @@ -1206,8 +1259,10 @@ done_opts: genhash = tcp_v4_do_calc_md5_hash(newhash, hash_expected, iph->saddr, iph->daddr, - th, sk->sk_protocol, - skb->len); + sk->sk_protocol, th, th->doff << 2, + skb_headlen(skb), + skb_is_nonlinear(skb) ? + skb_shinfo(skb) : NULL); if (genhash || memcmp(hash_location, newhash, 16) != 0) { if (net_ratelimit()) { @@ -1456,6 +1511,7 @@ struct sock *tcp_v4_syn_recv_sock(struct sock *sk, struct sk_buff *skb, if (newkey != NULL) tcp_v4_md5_do_add(newsk, inet_sk(sk)->daddr, newkey, key->keylen); + newsk->sk_route_caps &= ~NETIF_F_GSO_MASK; } #endif diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index e399bde..07269e9 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -542,8 +542,10 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it, * room for it. */ md5 = tp->af_specific->md5_lookup(sk, sk); - if (md5) + if (md5) { tcp_header_size += TCPOLEN_MD5SIG_ALIGNED; + sk->sk_route_caps &= ~NETIF_F_GSO_MASK; + } #endif skb_push(skb, tcp_header_size); @@ -606,9 +608,7 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it, tp->af_specific->calc_md5_hash(md5_hash_location, md5, sk, NULL, NULL, - tcp_hdr(skb), - sk->sk_protocol, - skb->len); + sk->sk_protocol, skb); } #endif @@ -2264,8 +2264,7 @@ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst, tp->af_specific->calc_md5_hash(md5_hash_location, md5, NULL, dst, req, - tcp_hdr(skb), sk->sk_protocol, - skb->len); + sk->sk_protocol, skb); } #endif diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index 715965f..b547a08 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c @@ -736,18 +736,19 @@ static int tcp_v6_parse_md5_keys (struct sock *sk, char __user *optval, static int tcp_v6_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, struct in6_addr *saddr, struct in6_addr *daddr, - struct tcphdr *th, int protocol, - unsigned int tcplen) + int protocol, + struct tcphdr *th, int data_off, int tcplen, + struct skb_shared_info *frags) { - struct scatterlist sg[4]; - __u16 data_len; + struct scatterlist sg[MAX_SKB_FRAGS + 3]; + const int head_data_len = tcplen - data_off; int block = 0; __sum16 cksum; struct tcp_md5sig_pool *hp; struct tcp6_pseudohdr *bp; struct hash_desc *desc; - int err; - unsigned int nbytes = 0; + int err, i; + unsigned int nbytes = tcplen; hp = tcp_get_md5sig_pool(); if (!hp) { @@ -760,10 +761,15 @@ static int tcp_v6_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, /* 1. TCP pseudo-header (RFC2460) */ ipv6_addr_copy(&bp->saddr, saddr); ipv6_addr_copy(&bp->daddr, daddr); - bp->len = htonl(tcplen); bp->protocol = htonl(protocol); + if (frags) + for (i = 0; i < frags->nr_frags; ++i) + nbytes += frags->frags[i].size; + bp->len = htonl(nbytes); + nbytes = 0; - sg_init_table(sg, 4); + sg_init_table(sg, 3 + (head_data_len > 0) + + (frags ? frags->nr_frags : 0)); sg_set_buf(&sg[block++], bp, sizeof(*bp)); nbytes += sizeof(*bp); @@ -775,19 +781,25 @@ static int tcp_v6_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, nbytes += sizeof(*th); /* 3. TCP segment data (if any) */ - data_len = tcplen - (th->doff << 2); - if (data_len > 0) { - u8 *data = (u8 *)th + (th->doff << 2); - sg_set_buf(&sg[block++], data, data_len); - nbytes += data_len; + if (head_data_len > 0) { + u8 *data = (u8 *)th + data_off; + sg_set_buf(&sg[block++], data, head_data_len); + nbytes += head_data_len; + } + + if (frags) { + for (i = 0; i < frags->nr_frags; ++i) { + const struct skb_frag_struct *f = &frags->frags[i]; + sg_set_page(&sg[block++], f->page, f->size, + f->page_offset); + nbytes += f->size; + } } /* 4. shared key */ sg_set_buf(&sg[block++], key->key, key->keylen); nbytes += key->keylen; - sg_mark_end(&sg[block - 1]); - /* Now store the hash into the packet */ err = crypto_hash_init(desc); if (err) { @@ -821,10 +833,11 @@ static int tcp_v6_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, struct sock *sk, struct dst_entry *dst, struct request_sock *req, - struct tcphdr *th, int protocol, - unsigned int tcplen) + int protocol, + struct sk_buff *skb) { struct in6_addr *saddr, *daddr; + struct tcphdr *th = tcp_hdr(skb); if (sk) { saddr = &inet6_sk(sk)->saddr; @@ -834,8 +847,10 @@ static int tcp_v6_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, daddr = &inet6_rsk(req)->rmt_addr; } return tcp_v6_do_calc_md5_hash(md5_hash, key, - saddr, daddr, - th, protocol, tcplen); + saddr, daddr, protocol, + th, th->doff << 2, skb_headlen(skb), + skb_is_nonlinear(skb) ? + skb_shinfo(skb) : NULL); } static int tcp_v6_inbound_md5_hash (struct sock *sk, struct sk_buff *skb) @@ -910,8 +925,10 @@ done_opts: genhash = tcp_v6_do_calc_md5_hash(newhash, hash_expected, &ip6h->saddr, &ip6h->daddr, - th, sk->sk_protocol, - skb->len); + sk->sk_protocol, th, + th->doff << 2, skb_headlen(skb), + skb_is_nonlinear(skb) ? + skb_shinfo(skb) : NULL); if (genhash || memcmp(hash_location, newhash, 16) != 0) { if (net_ratelimit()) { printk(KERN_INFO "MD5 Hash %s for " @@ -1051,7 +1068,8 @@ static void tcp_v6_send_reset(struct sock *sk, struct sk_buff *skb) tcp_v6_do_calc_md5_hash((__u8 *)&opt[1], key, &ipv6_hdr(skb)->daddr, &ipv6_hdr(skb)->saddr, - t1, IPPROTO_TCP, tot_len); + IPPROTO_TCP, t1, tot_len, + tot_len, NULL); } #endif @@ -1157,7 +1175,8 @@ static void tcp_v6_send_ack(struct tcp_timewait_sock *tw, tcp_v6_do_calc_md5_hash((__u8 *)topt, key, &ipv6_hdr(skb)->daddr, &ipv6_hdr(skb)->saddr, - t1, IPPROTO_TCP, tot_len); + IPPROTO_TCP, t1, tot_len, + tot_len, NULL); } #endif ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: MD5 SG fix 2008-07-01 22:38 ` Adam Langley @ 2008-07-01 22:46 ` Stephen Hemminger 2008-07-01 22:49 ` Stephen Hemminger 2008-07-02 4:55 ` Evgeniy Polyakov 2 siblings, 0 replies; 12+ messages in thread From: Stephen Hemminger @ 2008-07-01 22:46 UTC (permalink / raw) To: Adam Langley; +Cc: netdev On Tue, 1 Jul 2008 15:38:14 -0700 "Adam Langley" <agl@imperialviolet.org> wrote: > Cut three, > * I should use sg_set_page, not sg_set_buf with page_address > * Make the version that tcp_output etc use take an SKB > * Replicate fix to IPv6 > > Cheers, > > > AGL > > include/net/tcp.h | 6 +-- > net/ipv4/tcp_ipv4.c | 112 +++++++++++++++++++++++++++++++++++++------------ > net/ipv4/tcp_output.c | 11 ++--- > net/ipv6/tcp_ipv6.c | 65 ++++++++++++++++++---------- > 4 files changed, 133 insertions(+), 61 deletions(-) > > diff --git a/include/net/tcp.h b/include/net/tcp.h > index 633147c..a9130a1 100644 > --- a/include/net/tcp.h > +++ b/include/net/tcp.h > @@ -1120,9 +1120,8 @@ extern int tcp_v4_calc_md5_hash(char *md5_hash, > struct sock *sk, > struct dst_entry *dst, > struct request_sock *req, > - struct tcphdr *th, > int protocol, > - unsigned int tcplen); > + struct sk_buff *skb); > extern struct tcp_md5sig_key *tcp_v4_md5_lookup(struct sock *sk, > struct sock *addr_sk); > > @@ -1370,9 +1369,8 @@ struct tcp_sock_af_ops { > struct sock *sk, > struct dst_entry *dst, > struct request_sock *req, > - struct tcphdr *th, > int protocol, > - unsigned int len); > + struct sk_buff *skb); > int (*md5_add) (struct sock *sk, > struct sock *addr_sk, > u8 *newkey, > diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c > index cd601a8..90a5f41 100644 > --- a/net/ipv4/tcp_ipv4.c > +++ b/net/ipv4/tcp_ipv4.c > @@ -95,8 +95,9 @@ static struct tcp_md5sig_key *tcp_v4_md5_do_lookup(struct sock *sk, > __be32 addr); > static int tcp_v4_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, > __be32 saddr, __be32 daddr, > - struct tcphdr *th, int protocol, > - unsigned int tcplen); > + int protocol, > + struct tcphdr *th, int data_off, int tcplen, > + struct skb_shared_info *frags); > #endif > > struct inet_hashinfo __cacheline_aligned tcp_hashinfo = { > @@ -586,8 +587,9 @@ static void tcp_v4_send_reset(struct sock *sk, struct sk_buff *skb) > key, > ip_hdr(skb)->daddr, > ip_hdr(skb)->saddr, > - &rep.th, IPPROTO_TCP, > - arg.iov[0].iov_len); > + IPPROTO_TCP, &rep.th, > + arg.iov[0].iov_len, arg.iov[0].iov_len, > + NULL); > } > #endif > arg.csum = csum_tcpudp_nofold(ip_hdr(skb)->daddr, > @@ -680,8 +682,9 @@ static void tcp_v4_send_ack(struct tcp_timewait_sock *twsk, > key, > ip_hdr(skb)->daddr, > ip_hdr(skb)->saddr, > - &rep.th, IPPROTO_TCP, > - arg.iov[0].iov_len); > + IPPROTO_TCP, &rep.th, > + arg.iov[0].iov_len, arg.iov[0].iov_len, > + NULL); > } > #endif > arg.csum = csum_tcpudp_nofold(ip_hdr(skb)->daddr, > @@ -1004,20 +1007,56 @@ static int tcp_v4_parse_md5_keys(struct sock *sk, char __user *optval, > newkey, cmd.tcpm_keylen); > } > > +/* > +static void > +md5_dump(struct scatterlist *sglist, int nsg, unsigned nbytes, int data_off, int tcplen, char frags) { > + struct scatterlist *sg; > + unsigned e; > + > + printk(KERN_INFO "MD5: %d %d %d %d %d\n", nsg, nbytes, data_off, tcplen, frags); > + for_each_sg(sglist, sg, nsg, e) { > + u8 *buf = sg_virt(sg); > + unsigned i; > + > + for (i = 0; i < sg->length; ++i) > + printk(" %02x", buf[i]); > + > + printk("\n"); > + } > + > + printk("\n"); > +}*/ > + > +/** > + * tcp_v4_do_calc_md5_hash - calculate an MD5 hash (RFC 2385) > + * @md5_hash: (output) a 16 byte space into which the MD5 sig is written > + * @key: the key that is appened to the hash input > + * @saddr: source IP address for the packet > + * @daddr: destination IP address for the packet > + * @protocol: the protocol number in the IP header (see the RFC) > + * @th: the TCP header, followed by options and (optional) data > + * @data_off: the offset of the optional data (in bytes) from @th > + * @tcplen: the length of the buffer (in bytes) pointed to by @th. If > + * @tcplen == @data_off then there is no data following the header > + * @frags: (maybe NULL) a list of additional fragments of data > + * > + * We don't always have the SKB when this function is called, thus the pointer > + * to the TCP header and all the length arguments. > + */ > static int tcp_v4_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, > - __be32 saddr, __be32 daddr, > - struct tcphdr *th, int protocol, > - unsigned int tcplen) > + __be32 saddr, __be32 daddr, int protocol, > + struct tcphdr *th, int data_off, int tcplen, > + struct skb_shared_info *frags) > { > - struct scatterlist sg[4]; > - __u16 data_len; > + struct scatterlist sg[MAX_SKB_FRAGS + 3]; Since this can be big, why not allocate with kmalloc()? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: MD5 SG fix 2008-07-01 22:38 ` Adam Langley 2008-07-01 22:46 ` Stephen Hemminger @ 2008-07-01 22:49 ` Stephen Hemminger 2008-07-01 22:52 ` Adam Langley 2008-07-02 4:55 ` Evgeniy Polyakov 2 siblings, 1 reply; 12+ messages in thread From: Stephen Hemminger @ 2008-07-01 22:49 UTC (permalink / raw) To: Adam Langley; +Cc: netdev On Tue, 1 Jul 2008 15:38:14 -0700 "Adam Langley" <agl@imperialviolet.org> wrote: > static int tcp_v6_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, > struct in6_addr *saddr, > struct in6_addr *daddr, > - struct tcphdr *th, int protocol, > - unsigned int tcplen) > + int protocol, > + struct tcphdr *th, int data_off, int tcplen, > + struct skb_shared_info *frags) > { > - struct scatterlist sg[4]; > - __u16 data_len; > + struct scatterlist sg[MAX_SKB_FRAGS + 3]; > + const int head_data_len = tcplen - data_off; > int block = 0; > __sum16 cksum; > struct tcp_md5sig_pool *hp; > struct tcp6_pseudohdr *bp; > struct hash_desc *desc; > - int err; > - unsigned int nbytes = 0; > + int err, i; > + unsigned int nbytes = tcplen; > > hp = tcp_get_md5sig_pool(); > if (!hp) { > @@ -760,10 +761,15 @@ static int tcp_v6_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, > /* 1. TCP pseudo-header (RFC2460) */ > ipv6_addr_copy(&bp->saddr, saddr); > ipv6_addr_copy(&bp->daddr, daddr); > - bp->len = htonl(tcplen); > bp->protocol = htonl(protocol); > + if (frags) > + for (i = 0; i < frags->nr_frags; ++i) > + nbytes += frags->frags[i].size; > + bp->len = htonl(nbytes); > + nbytes = 0; Just pass the skb in and not tcplen, frags, ... avoid all this nonsense. tcplen == skb->len, and you have the correct size info. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: MD5 SG fix 2008-07-01 22:49 ` Stephen Hemminger @ 2008-07-01 22:52 ` Adam Langley 2008-07-01 23:10 ` Stephen Hemminger 0 siblings, 1 reply; 12+ messages in thread From: Adam Langley @ 2008-07-01 22:52 UTC (permalink / raw) To: Stephen Hemminger; +Cc: netdev On Tue, Jul 1, 2008 at 3:49 PM, Stephen Hemminger <shemminger@vyatta.com> wrote: > Just pass the skb in and not tcplen, frags, ... > avoid all this nonsense. tcplen == skb->len, and you have the correct size info. But there are places where that function is called that we don't have an SKB to pass in (tcp_v6_send_reset and tcp_v6_send_ack, likewise in v4). I could duplicate the whole function, once for SKBs and once for those users, but that's a bunch of duplicated code. I'll defer to your experience on what the kernel's preferred practice is here. AGL -- Adam Langley agl@imperialviolet.org http://www.imperialviolet.org ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: MD5 SG fix 2008-07-01 22:52 ` Adam Langley @ 2008-07-01 23:10 ` Stephen Hemminger 0 siblings, 0 replies; 12+ messages in thread From: Stephen Hemminger @ 2008-07-01 23:10 UTC (permalink / raw) To: Adam Langley; +Cc: Stephen Hemminger, netdev On Tue, 1 Jul 2008 15:52:56 -0700 "Adam Langley" <agl@imperialviolet.org> wrote: > On Tue, Jul 1, 2008 at 3:49 PM, Stephen Hemminger <shemminger@vyatta.com> wrote: > > Just pass the skb in and not tcplen, frags, ... > > avoid all this nonsense. tcplen == skb->len, and you have the correct size info. > > But there are places where that function is called that we don't have > an SKB to pass in (tcp_v6_send_reset and tcp_v6_send_ack, likewise in > v4). I could duplicate the whole function, once for SKBs and once for > those users, but that's a bunch of duplicated code. I'll defer to your > experience on what the kernel's preferred practice is here. > Break the calc function into: setup header setup data calculate Then make two handles, one for data block, other for skb If you want, let's take this off list and I'll bounce you back what I mean. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: MD5 SG fix 2008-07-01 22:38 ` Adam Langley 2008-07-01 22:46 ` Stephen Hemminger 2008-07-01 22:49 ` Stephen Hemminger @ 2008-07-02 4:55 ` Evgeniy Polyakov 2008-07-04 0:07 ` Adam Langley 2 siblings, 1 reply; 12+ messages in thread From: Evgeniy Polyakov @ 2008-07-02 4:55 UTC (permalink / raw) To: Adam Langley; +Cc: Stephen Hemminger, netdev Hi. On Tue, Jul 01, 2008 at 03:38:14PM -0700, Adam Langley (agl@imperialviolet.org) wrote: > @@ -1040,9 +1079,14 @@ static int tcp_v4_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, > bp->daddr = daddr; > bp->pad = 0; > bp->protocol = protocol; > - bp->len = htons(tcplen); > + if (frags) > + for (i = 0; i < frags->nr_frags; ++i) > + nbytes += frags->frags[i].size; > + bp->len = htons(nbytes); > + nbytes = 0; > > - sg_init_table(sg, 4); > + sg_init_table(sg, 3 + (head_data_len > 0) + > + (frags ? frags->nr_frags : 0)); > > sg_set_buf(&sg[block++], bp, sizeof(*bp)); > nbytes += sizeof(*bp); > @@ -1056,11 +1100,19 @@ static int tcp_v4_do_calc_md5_hash(char *md5_hash, struct tcp_md5sig_key *key, > nbytes += sizeof(struct tcphdr); > > /* 3. the TCP segment data (if any) */ > - data_len = tcplen - (th->doff << 2); > - if (data_len > 0) { > - unsigned char *data = (unsigned char *)th + (th->doff << 2); > - sg_set_buf(&sg[block++], data, data_len); > - nbytes += data_len; > + if (head_data_len > 0) { > + unsigned char *data = (unsigned char *)th + data_off; > + sg_set_buf(&sg[block++], data, head_data_len); > + nbytes += head_data_len; > + } > + > + if (frags) { > + for (i = 0; i < frags->nr_frags; ++i) { > + const struct skb_frag_struct *f = &frags->frags[i]; > + sg_set_page(&sg[block++], f->page, f->size, > + f->page_offset); > + nbytes += f->size; > + } > } > > /* 4. an independently-specified key or password, known to both Why dont't you want to iterate over all provided data/page pointers with crypto_hash_update() and do not mess with huge scatterlist arrays? -- Evgeniy Polyakov ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: MD5 SG fix 2008-07-02 4:55 ` Evgeniy Polyakov @ 2008-07-04 0:07 ` Adam Langley 2008-07-04 4:02 ` Stephen Hemminger 0 siblings, 1 reply; 12+ messages in thread From: Adam Langley @ 2008-07-04 0:07 UTC (permalink / raw) To: Evgeniy Polyakov; +Cc: Stephen Hemminger, netdev On Tue, Jul 1, 2008 at 9:55 PM, Evgeniy Polyakov <johnpol@2ka.mipt.ru> wrote: > Why dont't you want to iterate over all provided data/page pointers with > crypto_hash_update() and do not mess with huge scatterlist arrays? You're right, it's easier that way. I was just stuck in the thinking of the original code. Stephen: I managed to test the SG stuff by setting up lo in jumbo frames mode, so no need to test it on your BGP stuff. Cheers, AGL -- Adam Langley agl@imperialviolet.org http://www.imperialviolet.org ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: MD5 SG fix 2008-07-04 0:07 ` Adam Langley @ 2008-07-04 4:02 ` Stephen Hemminger 0 siblings, 0 replies; 12+ messages in thread From: Stephen Hemminger @ 2008-07-04 4:02 UTC (permalink / raw) To: Adam Langley; +Cc: Evgeniy Polyakov, netdev On Thu, 3 Jul 2008 17:07:54 -0700 "Adam Langley" <agl@imperialviolet.org> wrote: > On Tue, Jul 1, 2008 at 9:55 PM, Evgeniy Polyakov <johnpol@2ka.mipt.ru> wrote: > > Why dont't you want to iterate over all provided data/page pointers with > > crypto_hash_update() and do not mess with huge scatterlist arrays? > > You're right, it's easier that way. I was just stuck in the thinking > of the original code. > > Stephen: I managed to test the SG stuff by setting up lo in jumbo > frames mode, so no need to test it on your BGP stuff. > > > Cheers, > > AGL Here is patch to netcat which is one of the easier ways to test this. Should also make sure sendfile() a.k.a splice still works. --- a/src/netcat.c 2008-06-25 16:35:25.000000000 -0700 +++ b/src/netcat.c 2008-06-25 16:35:25.000000000 -0700 @@ -59,7 +59,7 @@ int opt_wait = 0; /* wait time */ char *opt_outputfile = NULL; /* hexdump output file */ char *opt_exec = NULL; /* program to exec after connecting */ nc_proto_t opt_proto = NETCAT_PROTO_TCP; /* protocol to use for connections */ - +char *opt_md5 = NULL; /* signature to use on TCP connection */ /* signal handling */ @@ -192,6 +192,7 @@ int main(int argc, char *argv[]) { "help", no_argument, NULL, 'h' }, { "interval", required_argument, NULL, 'i' }, { "listen", no_argument, NULL, 'l' }, + { "md5", required_argument, NULL, 'M' }, { "tunnel", required_argument, NULL, 'L' }, { "dont-resolve", no_argument, NULL, 'n' }, { "output", required_argument, NULL, 'o' }, @@ -216,7 +217,7 @@ int main(int argc, char *argv[]) { 0, 0, 0, 0 } }; - c = getopt_long(argc, argv, "cde:g:G:hi:lL:no:p:P:rs:S:tTuvVxw:z", + c = getopt_long(argc, argv, "cde:g:G:hi:lM:L:no:p:P:rs:S:tTuvVxw:z", long_options, &option_index); if (c == -1) break; @@ -247,6 +248,15 @@ int main(int argc, char *argv[]) ncprint(NCPRINT_ERROR | NCPRINT_EXIT, _("Invalid interval time \"%s\""), optarg); break; + case 'M': /* TCP MD5 */ +#ifdef TCP_MD5SIG + opt_md5 = strdup(optarg); +#else + ncprint(NCPRINT_ERROR | NCPRINT_EXIT, + _("This platform does not support TCP MD5 option")); +#endif + break; + case 'l': /* mode flag: listen mode */ if (netcat_mode != NETCAT_UNSPEC) ncprint(NCPRINT_ERROR | NCPRINT_EXIT, @@ -390,6 +400,13 @@ int main(int argc, char *argv[]) debug_v(("Trying to parse non-args parameters (argc=%d, optind=%d)", argc, optind)); +#ifdef TCP_MD5SIG + if (opt_md5 && netcat_mode == NETCAT_LISTEN && optind == argc) { + ncprint(NCPRINT_ERROR | NCPRINT_EXIT, + _("Remote host required with MD5 option")); + } +#endif + /* try to get an hostname parameter */ if (optind < argc) { char *myhost = argv[optind++]; --- a/src/network.c 2008-06-25 16:35:25.000000000 -0700 +++ b/src/network.c 2008-06-26 11:47:04.000000000 -0700 @@ -375,6 +375,27 @@ int netcat_socket_new(int domain, int ty return sock; } +#ifdef TCP_MD5SIG +int netcat_tcp_md5(int sock, int family, const struct in_addr *addr, in_port_t port) +{ + struct sockaddr_in *remote; + struct tcp_md5sig tcpm; + + memset(&tcpm, 0, sizeof(tcpm)); + + remote = (struct sockaddr_in *) &tcpm.tcpm_addr; + remote->sin_family = family; + remote->sin_port = port; + memcpy(&remote->sin_addr, addr, sizeof(*addr)); + + tcpm.tcpm_keylen = strlen(opt_md5); + memcpy(tcpm.tcpm_key, opt_md5, tcpm.tcpm_keylen); + + return setsockopt(sock, IPPROTO_TCP, TCP_MD5SIG, + &tcpm, sizeof(tcpm)); +} +#endif + /* Creates a full outgoing async socket connection in the specified `domain' and `type' to the specified `addr' and `port'. The connection is originated using the optionally specified `local_addr' and `local_port'. @@ -432,6 +453,11 @@ int netcat_socket_new_connect(int domain } } +#ifdef TCP_MD5SIG + if (opt_md5) + netcat_tcp_md5(sock, my_family, &rem_addr.sin_addr, rem_addr.sin_port); +#endif + /* add the non-blocking flag to this socket */ if ((ret = fcntl(sock, F_GETFL, 0)) >= 0) ret = fcntl(sock, F_SETFL, ret | O_NONBLOCK); --- a/src/proto.h 2008-06-25 16:35:25.000000000 -0700 +++ b/src/proto.h 2008-06-25 16:35:25.000000000 -0700 @@ -62,6 +62,7 @@ extern nc_proto_t opt_proto; extern FILE *output_fp; extern bool use_stdin, signal_handler, got_sigterm, got_sigint, got_sigusr1, commandline_need_newline; +extern char *opt_md5; /* network.c */ bool netcat_resolvehost(nc_host_t *dst, const char *name); @@ -77,6 +78,8 @@ int netcat_socket_new_connect(int domain int netcat_socket_new_listen(int domain, const struct in_addr *addr, in_port_t port); int netcat_socket_accept(int fd, int timeout); +int netcat_tcp_md5(int fd, int family, + const struct in_addr *addr, in_port_t port); /* telnet.c */ void netcat_telnet_parse(nc_sock_t *ncsock); --- a/src/misc.c 2008-06-25 16:35:25.000000000 -0700 +++ b/src/misc.c 2008-06-25 16:35:25.000000000 -0700 @@ -317,6 +317,9 @@ void netcat_printhelp(char *argv0) " -G, --pointer=NUM source-routing pointer: 4, 8, 12, ...\n" " -h, --help display this help and exit\n" " -i, --interval=SECS delay interval for lines sent, ports scanned\n" +#ifdef TCP_MD5SIG +" -M, --md5=PASSWD use TCP MD5 option\n" +#endif " -l, --listen listen mode, for inbound connects\n")); printf(_("" " -L, --tunnel=ADDRESS:PORT forward local port to remote address\n" --- a/src/core.c 2008-06-25 16:35:25.000000000 -0700 +++ b/src/core.c 2008-06-26 11:45:49.000000000 -0700 @@ -420,6 +420,12 @@ static int core_tcp_listen(nc_sock_t *nc ncprint(NCPRINT_ERROR | NCPRINT_EXIT, _("Couldn't setup listening socket (err=%d)"), sock_listen); +#ifdef TCP_MD5SIG + if (opt_md5) + netcat_tcp_md5(sock_listen, AF_INET, + &ncsock->host.iaddrs[0], ncsock->port.netnum); +#endif + /* if the port was set to 0 this means that it is assigned randomly by the OS. Find out which port they assigned to us. */ if (ncsock->local_port.num == 0) { --- a/src/netcat.h 2008-06-25 16:35:25.000000000 -0700 +++ b/src/netcat.h 2008-06-25 16:35:25.000000000 -0700 @@ -42,6 +42,7 @@ #include <arpa/inet.h> /* inet_ntop(), inet_pton() */ /* other misc unchecked includes */ +#include <netinet/tcp.h> #if 0 #include <netinet/in_systm.h> /* misc crud that netinet/ip.h references */ #include <netinet/ip.h> /* IPOPT_LSRR, header stuff */ ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2008-07-04 4:02 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-07-01 21:16 MD5 SG fix Adam Langley 2008-07-01 21:39 ` Stephen Hemminger 2008-07-01 21:48 ` Adam Langley 2008-07-01 21:49 ` Adam Langley 2008-07-01 22:38 ` Adam Langley 2008-07-01 22:46 ` Stephen Hemminger 2008-07-01 22:49 ` Stephen Hemminger 2008-07-01 22:52 ` Adam Langley 2008-07-01 23:10 ` Stephen Hemminger 2008-07-02 4:55 ` Evgeniy Polyakov 2008-07-04 0:07 ` Adam Langley 2008-07-04 4:02 ` Stephen Hemminger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).