stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kamal Mostafa <kamal@canonical.com>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	kernel-team@lists.ubuntu.com
Cc: Daniel Borkmann <dborkman@redhat.com>,
	Steffen Klassert <steffen.klassert@secunet.com>,
	Hannes Frederic Sowa <hannes@stressinduktion.org>,
	"David S. Miller" <davem@davemloft.net>,
	Kamal Mostafa <kamal@canonical.com>
Subject: [PATCH 3.8 37/81] net: sctp: fix ipv6 ipsec encryption bug in sctp_v6_xmit
Date: Tue, 29 Oct 2013 11:03:58 -0700	[thread overview]
Message-ID: <1383069882-11437-38-git-send-email-kamal@canonical.com> (raw)
In-Reply-To: <1383069882-11437-1-git-send-email-kamal@canonical.com>

3.8.13.12 -stable review patch.  If anyone has any objections, please let me know.

------------------

From: Daniel Borkmann <dborkman@redhat.com>

[ Upstream commit 95ee62083cb6453e056562d91f597552021e6ae7 ]

Alan Chester reported an issue with IPv6 on SCTP that IPsec traffic is not
being encrypted, whereas on IPv4 it is. Setting up an AH + ESP transport
does not seem to have the desired effect:

SCTP + IPv4:

  22:14:20.809645 IP (tos 0x2,ECT(0), ttl 64, id 0, offset 0, flags [DF], proto AH (51), length 116)
    192.168.0.2 > 192.168.0.5: AH(spi=0x00000042,sumlen=16,seq=0x1): ESP(spi=0x00000044,seq=0x1), length 72
  22:14:20.813270 IP (tos 0x2,ECT(0), ttl 64, id 0, offset 0, flags [DF], proto AH (51), length 340)
    192.168.0.5 > 192.168.0.2: AH(spi=0x00000043,sumlen=16,seq=0x1):

SCTP + IPv6:

  22:31:19.215029 IP6 (class 0x02, hlim 64, next-header SCTP (132) payload length: 364)
    fe80::222:15ff:fe87:7fc.3333 > fe80::92e6:baff:fe0d:5a54.36767: sctp
    1) [INIT ACK] [init tag: 747759530] [rwnd: 62464] [OS: 10] [MIS: 10]

Moreover, Alan says:

  This problem was seen with both Racoon and Racoon2. Other people have seen
  this with OpenSwan. When IPsec is configured to encrypt all upper layer
  protocols the SCTP connection does not initialize. After using Wireshark to
  follow packets, this is because the SCTP packet leaves Box A unencrypted and
  Box B believes all upper layer protocols are to be encrypted so it drops
  this packet, causing the SCTP connection to fail to initialize. When IPsec
  is configured to encrypt just SCTP, the SCTP packets are observed unencrypted.

In fact, using `socat sctp6-listen:3333 -` on one end and transferring "plaintext"
string on the other end, results in cleartext on the wire where SCTP eventually
does not report any errors, thus in the latter case that Alan reports, the
non-paranoid user might think he's communicating over an encrypted transport on
SCTP although he's not (tcpdump ... -X):

  ...
  0x0030: 5d70 8e1a 0003 001a 177d eb6c 0000 0000  ]p.......}.l....
  0x0040: 0000 0000 706c 6169 6e74 6578 740a 0000  ....plaintext...

Only in /proc/net/xfrm_stat we can see XfrmInTmplMismatch increasing on the
receiver side. Initial follow-up analysis from Alan's bug report was done by
Alexey Dobriyan. Also thanks to Vlad Yasevich for feedback on this.

SCTP has its own implementation of sctp_v6_xmit() not calling inet6_csk_xmit().
This has the implication that it probably never really got updated along with
changes in inet6_csk_xmit() and therefore does not seem to invoke xfrm handlers.

SCTP's IPv4 xmit however, properly calls ip_queue_xmit() to do the work. Since
a call to inet6_csk_xmit() would solve this problem, but result in unecessary
route lookups, let us just use the cached flowi6 instead that we got through
sctp_v6_get_dst(). Since all SCTP packets are being sent through sctp_packet_transmit(),
we do the route lookup / flow caching in sctp_transport_route(), hold it in
tp->dst and skb_dst_set() right after that. If we would alter fl6->daddr in
sctp_v6_xmit() to np->opt->srcrt, we possibly could run into the same effect
of not having xfrm layer pick it up, hence, use fl6_update_dst() in sctp_v6_get_dst()
instead to get the correct source routed dst entry, which we assign to the skb.

Also source address routing example from 625034113 ("sctp: fix sctp to work with
ipv6 source address routing") still works with this patch! Nevertheless, in RFC5095
it is actually 'recommended' to not use that anyway due to traffic amplification [1].
So it seems we're not supposed to do that anyway in sctp_v6_xmit(). Moreover, if
we overwrite the flow destination here, the lower IPv6 layer will be unable to
put the correct destination address into IP header, as routing header is added in
ipv6_push_nfrag_opts() but then probably with wrong final destination. Things aside,
result of this patch is that we do not have any XfrmInTmplMismatch increase plus on
the wire with this patch it now looks like:

SCTP + IPv6:

  08:17:47.074080 IP6 2620:52:0:102f:7a2b:cbff:fe27:1b0a > 2620:52:0:102f:213:72ff:fe32:7eba:
    AH(spi=0x00005fb4,seq=0x1): ESP(spi=0x00005fb5,seq=0x1), length 72
  08:17:47.074264 IP6 2620:52:0:102f:213:72ff:fe32:7eba > 2620:52:0:102f:7a2b:cbff:fe27:1b0a:
    AH(spi=0x00003d54,seq=0x1): ESP(spi=0x00003d55,seq=0x1), length 296

This fixes Kernel Bugzilla 24412. This security issue seems to be present since
2.6.18 kernels. Lets just hope some big passive adversary in the wild didn't have
its fun with that. lksctp-tools IPv6 regression test suite passes as well with
this patch.

 [1] http://www.secdev.org/conf/IPv6_RH_security-csw07.pdf

Reported-by: Alan Chester <alan.chester@tekelec.com>
Reported-by: Alexey Dobriyan <adobriyan@gmail.com>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
---
 net/sctp/ipv6.c | 42 +++++++++++++-----------------------------
 1 file changed, 13 insertions(+), 29 deletions(-)

diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index 391a245..d70cabb 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -210,45 +210,24 @@ out:
 		in6_dev_put(idev);
 }
 
-/* Based on tcp_v6_xmit() in tcp_ipv6.c. */
 static int sctp_v6_xmit(struct sk_buff *skb, struct sctp_transport *transport)
 {
 	struct sock *sk = skb->sk;
 	struct ipv6_pinfo *np = inet6_sk(sk);
-	struct flowi6 fl6;
-
-	memset(&fl6, 0, sizeof(fl6));
-
-	fl6.flowi6_proto = sk->sk_protocol;
-
-	/* Fill in the dest address from the route entry passed with the skb
-	 * and the source address from the transport.
-	 */
-	fl6.daddr = transport->ipaddr.v6.sin6_addr;
-	fl6.saddr = transport->saddr.v6.sin6_addr;
-
-	fl6.flowlabel = np->flow_label;
-	IP6_ECN_flow_xmit(sk, fl6.flowlabel);
-	if (ipv6_addr_type(&fl6.saddr) & IPV6_ADDR_LINKLOCAL)
-		fl6.flowi6_oif = transport->saddr.v6.sin6_scope_id;
-	else
-		fl6.flowi6_oif = sk->sk_bound_dev_if;
-
-	if (np->opt && np->opt->srcrt) {
-		struct rt0_hdr *rt0 = (struct rt0_hdr *) np->opt->srcrt;
-		fl6.daddr = *rt0->addr;
-	}
+	struct flowi6 *fl6 = &transport->fl.u.ip6;
 
 	SCTP_DEBUG_PRINTK("%s: skb:%p, len:%d, src:%pI6 dst:%pI6\n",
 			  __func__, skb, skb->len,
-			  &fl6.saddr, &fl6.daddr);
+			  &fl6->saddr, &fl6->daddr);
 
-	SCTP_INC_STATS(sock_net(sk), SCTP_MIB_OUTSCTPPACKS);
+	IP6_ECN_flow_xmit(sk, fl6->flowlabel);
 
 	if (!(transport->param_flags & SPP_PMTUD_ENABLE))
 		skb->local_df = 1;
 
-	return ip6_xmit(sk, skb, &fl6, np->opt, np->tclass);
+	SCTP_INC_STATS(sock_net(sk), SCTP_MIB_OUTSCTPPACKS);
+
+	return ip6_xmit(sk, skb, fl6, np->opt, np->tclass);
 }
 
 /* Returns the dst cache entry for the given source and destination ip
@@ -261,10 +240,12 @@ static void sctp_v6_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
 	struct dst_entry *dst = NULL;
 	struct flowi6 *fl6 = &fl->u.ip6;
 	struct sctp_bind_addr *bp;
+	struct ipv6_pinfo *np = inet6_sk(sk);
 	struct sctp_sockaddr_entry *laddr;
 	union sctp_addr *baddr = NULL;
 	union sctp_addr *daddr = &t->ipaddr;
 	union sctp_addr dst_saddr;
+	struct in6_addr *final_p, final;
 	__u8 matchlen = 0;
 	__u8 bmatchlen;
 	sctp_scope_t scope;
@@ -287,7 +268,8 @@ static void sctp_v6_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
 		SCTP_DEBUG_PRINTK("SRC=%pI6 - ", &fl6->saddr);
 	}
 
-	dst = ip6_dst_lookup_flow(sk, fl6, NULL, false);
+	final_p = fl6_update_dst(fl6, np->opt, &final);
+	dst = ip6_dst_lookup_flow(sk, fl6, final_p, false);
 	if (!asoc || saddr)
 		goto out;
 
@@ -339,10 +321,12 @@ static void sctp_v6_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
 		}
 	}
 	rcu_read_unlock();
+
 	if (baddr) {
 		fl6->saddr = baddr->v6.sin6_addr;
 		fl6->fl6_sport = baddr->v6.sin6_port;
-		dst = ip6_dst_lookup_flow(sk, fl6, NULL, false);
+		final_p = fl6_update_dst(fl6, np->opt, &final);
+		dst = ip6_dst_lookup_flow(sk, fl6, final_p, false);
 	}
 
 out:
-- 
1.8.1.2


  parent reply	other threads:[~2013-10-29 18:03 UTC|newest]

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-29 18:03 [3.8.y.z extended stable] Linux 3.8.13.12 stable review Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 01/81] ipvs: add backup_only flag to avoid loops Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 02/81] random: run random_int_secret_init() run after all late_initcalls Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 03/81] tile: use a more conservative __my_cpu_offset in CONFIG_PREEMPT Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 04/81] ALSA: snd-usb-usx2y: remove bogus frame checks Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 05/81] drm/i915/hsw: Disable L3 caching of atomic memory operations Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 06/81] drm/i915: Only apply DPMS to the encoder if enabled Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 07/81] ALSA: hda - hdmi: Fix channel map switch not taking effect Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 08/81] ALSA: hda - Add fixup for ASUS N56VZ Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 09/81] hwmon: (applesmc) Always read until end of data Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 10/81] drm/radeon: fix typo in CP DMA register headers Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 11/81] drm/radeon: fix hw contexts for SUMO2 asics Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 12/81] drm/radeon: forever loop on error in radeon_do_test_moves() Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 13/81] i2c: omap: Clear ARDY bit twice Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 14/81] KVM: PPC: Book3S HV: Fix typo in saving DSCR Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 15/81] random: allow architectures to optionally define random_get_entropy() Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 16/81] Btrfs: use right root when checking for hash collision Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 17/81] compiler-gcc4.h: Reorder macros based upon gcc ver Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 18/81] compiler-gcc.h: Add gcc-recommended GCC_VERSION macro Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 19/81] compiler/gcc4: Add quirk for 'asm goto' miscompilation bug Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 20/81] ALSA: hda - Fix microphone for Sony VAIO Pro 13 (Haswell model) Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 21/81] ext4: fix memory leak in xattr Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 22/81] vfs: allow O_PATH file descriptors for fstatfs() Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 23/81] parisc: fix interruption handler to respect pagefault_disable() Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 24/81] tuntap: correctly handle error in tun_set_iff() Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 25/81] xen-blkfront: use a different scatterlist for each request Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 26/81] can: flexcan: fix flexcan_chip_start() on imx6 Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 27/81] can: flexcan: flexcan_chip_start: fix regression, mark one MB for TX and abort pending TX Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 28/81] caif: Add missing braces to multiline if in cfctrl_linkup_request Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 29/81] tcp: Add missing braces to do_tcp_setsockopt Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 30/81] ipv6/exthdrs: accept tlv which includes only padding Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 31/81] net: fib: fib6_add: fix potential NULL pointer dereference Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 32/81] net: sctp: fix smatch warning in sctp_send_asconf_del_ip Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 33/81] net: flow_dissector: fix thoff for IPPROTO_AH Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 34/81] net_sched: htb: fix a typo in htb_change_class() Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 35/81] r8169: enforce RX_MULTI_EN for the 8168f Kamal Mostafa
2013-10-29 18:03 ` [PATCH 3.8 36/81] netpoll: fix NULL pointer dereference in netpoll_cleanup Kamal Mostafa
2013-10-29 18:03 ` Kamal Mostafa [this message]
2013-10-29 18:03 ` [PATCH 3.8 38/81] xen-netback: count number required slots for an skb more carefully Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 39/81] resubmit bridge: fix message_age_timer calculation Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 40/81] bridge: Clamp forward_delay when enabling STP Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 41/81] bridge: use br_port_get_rtnl within rtnl lock Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 42/81] bridge: fix NULL pointer deref of br_port_get_rcu Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 43/81] ip6_tunnels: raddr and laddr are inverted in nl msg Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 44/81] net: sctp: rfc4443: do not report ICMP redirects to user space Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 45/81] net:dccp: " Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 46/81] ip: use ip_hdr() in __ip_make_skb() to retrieve IP header Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 47/81] ip: generate unique IP identificator if local fragmentation is allowed Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 48/81] ipv6: udp packets following an UFO enqueued packet need also be handled by UFO Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 49/81] via-rhine: fix VLAN priority field (PCP, IEEE 802.1p) Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 50/81] dm9601: fix IFF_ALLMULTI handling Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 51/81] bonding: Fix broken promiscuity reference counting issue Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 52/81] ipv6: gre: correct calculation of max_headroom Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 53/81] ipv4 igmp: use in_dev_put in timer handlers instead of __in_dev_put Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 54/81] ipv6 mcast: use in6_dev_put in timer handlers instead of __in6_dev_put Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 55/81] ll_temac: Reset dma descriptors indexes on ndo_open Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 56/81] sit: allow to use rtnl ops on fb tunnel Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 57/81] ip6tnl: " Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 58/81] esp_scsi: Fix tag state corruption when autosensing Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 59/81] sparc64: Fix ITLB handler of null page Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 60/81] sparc64: Remove RWSEM export leftovers Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 61/81] sparc64: Fix off by one in trampoline TLB mapping installation loop Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 62/81] sparc64: Fix not SRA'ed %o5 in 32-bit traced syscall Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 63/81] sparc32: Fix exit flag passed from traced sys_sigreturn Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 64/81] cifs: Fix inability to write files >2GB to SMB2/3 shares Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 65/81] xhci: quirk for extra long delay for S4 Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 66/81] xhci: Fix spurious wakeups after S5 on Haswell Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 67/81] USB: support new huawei devices in option.c Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 68/81] USB: serial: ti_usb_3410_5052: add Abbott strip port ID to combined table as well Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 69/81] USB: serial: option: add support for Inovia SEW858 device Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 70/81] ARM: 7851/1: check for number of arguments in syscall_get/set_arguments() Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 71/81] USB: quirks.c: add one device that cannot deal with suspension Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 72/81] ALSA: us122l: Fix pcm_usb_stream mmapping regression Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 73/81] dm snapshot: fix data corruption Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 74/81] USB: quirks: add touchscreen that is dazzeled by remote wakeup Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 75/81] serial: vt8500: Fix range-checking on vt8500_uart_ports Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 76/81] serial: vt8500: add missing braces Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 77/81] usb: serial: option: blacklist Olivetti Olicard200 Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 78/81] usb-storage: add quirk for mandatory READ_CAPACITY_16 Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 79/81] fs: buffer: move allocation failure loop into the allocator Kamal Mostafa
2013-10-31 14:01   ` Johannes Weiner
2013-10-31 16:40     ` Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 80/81] writeback: fix negative bdi max pause Kamal Mostafa
2013-10-29 18:04 ` [PATCH 3.8 81/81] mm: fix BUG in __split_huge_page_pmd Kamal Mostafa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1383069882-11437-38-git-send-email-kamal@canonical.com \
    --to=kamal@canonical.com \
    --cc=davem@davemloft.net \
    --cc=dborkman@redhat.com \
    --cc=hannes@stressinduktion.org \
    --cc=kernel-team@lists.ubuntu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=steffen.klassert@secunet.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).