[PATCH net-next 0/4] ipv6: Update RFC references and implement ICMP errors for limits

[PATCH net-next 0/4] ipv6: Update RFC references and implement ICMP errors for limits

[Tom Herbert]

This patch set contains:

- Resolve comment for ipv6_skip_exthdr that the function is
  non conformant with RFC2460. I requested and received an
  on the interpretation of the requirements on 6man list.
- Update references, in comments, of RFC2460 to be RFC8200.
  RFC8200 (aka IPv6) is now a full Internet standard.
- Add references to RFC8504 (updated node requirements) for
  extension header limits and limits on padding in Destination
  and Hop-by-Hop options.
- Send ICMP errors for exceeding extension header limits. These
  are specified in Internet Draft draft-ietf-6man-icmp-limits-02
  (that draft is in working group last call in IETF)

Tom Herbert (4):
  ipv6: Resolve comment that EH processing order is being violated
  ipv6: Update references from RFC2460 to RFC8200
  ipv6: Reference RFC8504 for limits in padding and EH
  ipv6: Send ICMP errors for exceeding extension header limits

 .../net/ethernet/qualcomm/rmnet/rmnet_map_data.c   |  2 +-
 drivers/net/usb/smsc95xx.c                         |  2 +-
 include/net/ipv6.h                                 |  9 ++---
 include/uapi/linux/icmpv6.h                        |  6 ++++
 net/ipv6/exthdrs.c                                 | 42 +++++++++++++++++-----
 net/ipv6/exthdrs_core.c                            | 23 ++++--------
 net/ipv6/netfilter/nf_conntrack_reasm.c            |  2 +-
 net/ipv6/reassembly.c                              |  2 +-
 net/ipv6/syncookies.c                              |  2 +-
 net/ipv6/tcp_ipv6.c                                |  2 +-
 net/ipv6/udp.c                                     |  2 +-
 net/netfilter/xt_TCPMSS.c                          |  2 +-
 12 files changed, 60 insertions(+), 36 deletions(-)

-- 
2.7.4

[PATCH net-next 1/4] ipv6: Resolve comment that EH processing order is being violated

[Tom Herbert]

ipv6_skip_exthdr is preambled with a comment indicating that the
function necessarily violates the extension header processing order
requirements of RFC2460 in order to fulfill requirements of ICMPv6
processing. This patch revises the comment to indicate that the
function is conformant with RFC8200 (which obsoletes RFC2460) on
the basis that later headers are only being parsed and not actually
processed.

Signed-off-by: Tom Herbert <tom@quantonium.net>
---
 net/ipv6/exthdrs_core.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/net/ipv6/exthdrs_core.c b/net/ipv6/exthdrs_core.c
index 11a43ee..aa025e5 100644
--- a/net/ipv6/exthdrs_core.c
+++ b/net/ipv6/exthdrs_core.c
@@ -27,22 +27,13 @@ EXPORT_SYMBOL(ipv6_ext_hdr);
 /*
  * Skip any extension headers. This is used by the ICMP module.
  *
- * Note that strictly speaking this conflicts with RFC 2460 4.0:
- * ...The contents and semantics of each extension header determine whether
- * or not to proceed to the next header.  Therefore, extension headers must
- * be processed strictly in the order they appear in the packet; a
- * receiver must not, for example, scan through a packet looking for a
- * particular kind of extension header and process that header prior to
- * processing all preceding ones.
- *
- * We do exactly this. This is a protocol bug. We can't decide after a
- * seeing an unknown discard-with-error flavour TLV option if it's a
- * ICMP error message or not (errors should never be send in reply to
- * ICMP error messages).
- *
- * But I see no other way to do this. This might need to be reexamined
- * when Linux implements ESP (and maybe AUTH) headers.
- * --AK
+ * Note that Section 4, RFC8200 specifies "extension headers must be processed
+ * strictly in the order they appear in the packet". This function does skip
+ * over extension headers, however it is only for the purpose of extracting
+ * information about deeper header layers in the packet. Specified protocol
+ * processing is not being doing for scanned headers, hence extension headers
+ * are only being parsed but not processed out of order. Therefore, this
+ * function is conformant with RFC8200.
  *
  * This function parses (probably truncated) exthdr set "hdr".
  * "nexthdrp" initially points to some place,
-- 
2.7.4

[PATCH net-next 2/4] ipv6: Update references from RFC2460 to RFC8200

[Tom Herbert]

IPv6 is now a full Internet standard in RFC8200! Update references
in the code to reflect that.

Signed-off-by: Tom Herbert <tom@quantonium.net>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c | 2 +-
 drivers/net/usb/smsc95xx.c                           | 2 +-
 net/ipv6/exthdrs.c                                   | 4 ++--
 net/ipv6/netfilter/nf_conntrack_reasm.c              | 2 +-
 net/ipv6/reassembly.c                                | 2 +-
 net/ipv6/syncookies.c                                | 2 +-
 net/ipv6/tcp_ipv6.c                                  | 2 +-
 net/ipv6/udp.c                                       | 2 +-
 net/netfilter/xt_TCPMSS.c                            | 2 +-
 9 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 57a9c31..4711150 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -163,7 +163,7 @@ rmnet_map_ipv6_dl_csum_trailer(struct sk_buff *skb,
 	if (unlikely(csum_value_final == 0)) {
 		switch (ip6h->nexthdr) {
 		case IPPROTO_UDP:
-			/* RFC 2460 section 8.1
+			/* RFC 8200 section 8.1
 			 * DL6 One's complement rule for UDP checksum 0
 			 */
 			csum_value_final = ~csum_value_final;
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 355be77..496ed4d 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1269,7 +1269,7 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 	spin_lock_init(&pdata->mac_cr_lock);
 
 	/* LAN95xx devices do not alter the computed checksum of 0 to 0xffff.
-	 * RFC 2460, ipv6 UDP calculated checksum yields a result of zero must
+	 * RFC 8200, ipv6 UDP calculated checksum yields a result of zero must
 	 * be changed to 0xffff. RFC 768, ipv4 UDP computed checksum is zero,
 	 * it is transmitted as all ones. The zero transmitted checksum means
 	 * transmitter generated no checksum. Hence, enable csum offload only
diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
index 20291c2..fdb4a32 100644
--- a/net/ipv6/exthdrs.c
+++ b/net/ipv6/exthdrs.c
@@ -150,7 +150,7 @@ static bool ip6_parse_tlv(const struct tlvtype_proc *procs,
 			break;
 
 		case IPV6_TLV_PADN:
-			/* RFC 2460 states that the purpose of PadN is
+			/* RFC 8200 states that the purpose of PadN is
 			 * to align the containing header to multiples
 			 * of 8. 7 is therefore the highest valid value.
 			 * See also RFC 4942, Section 2.1.9.5.
@@ -561,7 +561,7 @@ static int ipv6_rthdr_rcv(struct sk_buff *skb)
 
 	/*
 	 *	This is the routing header forwarding algorithm from
-	 *	RFC 2460, page 16.
+	 *	Section 4.4, RFC 8200.
 	 */
 
 	n = hdr->hdrlen >> 1;
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index 3de0e9b..5fc9a5d 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -228,7 +228,7 @@ static int nf_ct_frag6_queue(struct frag_queue *fq, struct sk_buff *skb,
 		 * Required by the RFC.
 		 */
 		if (end & 0x7) {
-			/* RFC2460 says always send parameter problem in
+			/* RFC8200 says always send parameter problem in
 			 * this case. -DaveM
 			 */
 			pr_debug("end of fragment not rounded to 8 bytes.\n");
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index 1a832f5..bc0a361 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -158,7 +158,7 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
 		 * Required by the RFC.
 		 */
 		if (end & 0x7) {
-			/* RFC2460 says always send parameter problem in
+			/* RFC8200 says always send parameter problem in
 			 * this case. -DaveM
 			 */
 			*prob_offset = offsetof(struct ipv6hdr, payload_len);
diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
index e997141..48e07b2 100644
--- a/net/ipv6/syncookies.c
+++ b/net/ipv6/syncookies.c
@@ -27,7 +27,7 @@
 
 static siphash_key_t syncookie6_secret[2] __read_mostly;
 
-/* RFC 2460, Section 8.3:
+/* RFC 8200, Section 8.3:
  * [ipv6 tcp] MSS must be computed as the maximum packet size minus 60 [..]
  *
  * Due to IPV6_MIN_MTU=1280 the lowest possible MSS is 1220, which allows
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index beaf284..d74537f 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -602,7 +602,7 @@ static int tcp_v6_md5_hash_headers(struct tcp_md5sig_pool *hp,
 	struct tcphdr *_th;
 
 	bp = hp->scratch;
-	/* 1. TCP pseudo-header (RFC2460) */
+	/* 1. TCP pseudo-header (RFC8200) */
 	bp->saddr = *saddr;
 	bp->daddr = *daddr;
 	bp->protocol = cpu_to_be32(IPPROTO_TCP);
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 07fa579..f0e0f2a 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -739,7 +739,7 @@ static bool __udp_v6_is_mcast_sock(struct net *net, struct sock *sk,
 
 static void udp6_csum_zero_error(struct sk_buff *skb)
 {
-	/* RFC 2460 section 8.1 says that we SHOULD log
+	/* RFC 8200 section 8.1 says that we SHOULD log
 	 * this error. Well, it is reasonable.
 	 */
 	net_dbg_ratelimited("IPv6: udp checksum is 0 for [%pI6c]:%u->[%pI6c]:%u\n",
diff --git a/net/netfilter/xt_TCPMSS.c b/net/netfilter/xt_TCPMSS.c
index 98efb20..2fd6ffd 100644
--- a/net/netfilter/xt_TCPMSS.c
+++ b/net/netfilter/xt_TCPMSS.c
@@ -167,7 +167,7 @@ tcpmss_mangle_packet(struct sk_buff *skb,
 	/*
 	 * IPv4: RFC 1122 states "If an MSS option is not received at
 	 * connection setup, TCP MUST assume a default send MSS of 536".
-	 * IPv6: RFC 2460 states IPv6 has a minimum MTU of 1280 and a minimum
+	 * IPv6: RFC 8200 states IPv6 has a minimum MTU of 1280 and a minimum
 	 * length IPv6 header of 60, ergo the default MSS value is 1220
 	 * Since no MSS was provided, we must use the default values
 	 */
-- 
2.7.4

[PATCH net-next 3/4] ipv6: Reference RFC8504 for limits in padding and EH

[Tom Herbert]

RFC8504 specifies requirements for applying limits to Hop-by-Hop
and Destination Options extension headers and options (including
padding). Reference this RFC appropriately.

Signed-off-by: Tom Herbert <tom@quantonium.net>
---
 include/net/ipv6.h | 9 +++++----
 net/ipv6/exthdrs.c | 3 ++-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index daf8086..fd01823 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -54,10 +54,11 @@
 
 /* Limits on Hop-by-Hop and Destination options.
  *
- * Per RFC8200 there is no limit on the maximum number or lengths of options in
- * Hop-by-Hop or Destination options other then the packet must fit in an MTU.
- * We allow configurable limits in order to mitigate potential denial of
- * service attacks.
+ * Section 5.3 of RFC8504 describes limits that may be applied by an
+ * implementation to protect a node from excessive extension header options
+ * (for instance, to protect a node from denial of service attacks where
+ * the attacker sends packets filled with tiny options that will be skipped
+ * by a receiver).
  *
  * There are three limits that may be set:
  *   - Limit the number of options in a Hop-by-Hop or Destination options
diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
index fdb4a32..f0e0f7a 100644
--- a/net/ipv6/exthdrs.c
+++ b/net/ipv6/exthdrs.c
@@ -153,7 +153,8 @@ static bool ip6_parse_tlv(const struct tlvtype_proc *procs,
 			/* RFC 8200 states that the purpose of PadN is
 			 * to align the containing header to multiples
 			 * of 8. 7 is therefore the highest valid value.
-			 * See also RFC 4942, Section 2.1.9.5.
+			 * See also RFC 4942, Section 2.1.9.5, and
+			 * RFC 8504, Section 5.3.
 			 */
 			padlen += optlen;
 			if (padlen > 7)
-- 
2.7.4

[PATCH net-next 4/4] ipv6: Send ICMP errors for exceeding extension header limits

[Tom Herbert]

Define constants and add support to send ICMPv6 Parameter Problem
errors as defined in draft-ietf-6man-icmp-limits-02.

ICMPV6_TOOBIG_OPTION is sent if a packet exceeding the padding limit
is received (more than seven consecutive bytes of padding),
ICMPV6_TOOMANY_OPTIONS is sent if a packet is received and HBH option
count exceeds ipv6.sysctl.max_hbh_opts_cnt or DO option count exceeds
ipv6.sysctl.max_dst_opts_cnt. ICMPV6_EXTHDR_TOOBIG is sent if length
of HBH EH exceeds ipv6.sysctl.max_hbh_opts_len or length of DO EH
exceeds ipv6.sysctl.max_dst_opts_len.

Additionally, when packets are dropped in the above cases bump
IPSTATS_MIB_INHDRERRORS.

Signed-off-by: Tom Herbert <tom@quantonium.net>
---
 include/uapi/linux/icmpv6.h |  6 ++++++
 net/ipv6/exthdrs.c          | 35 ++++++++++++++++++++++++++++++-----
 2 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/include/uapi/linux/icmpv6.h b/include/uapi/linux/icmpv6.h
index 2622b5a..966279b 100644
--- a/include/uapi/linux/icmpv6.h
+++ b/include/uapi/linux/icmpv6.h
@@ -124,6 +124,7 @@ struct icmp6hdr {
 #define ICMPV6_PORT_UNREACH		4
 #define ICMPV6_POLICY_FAIL		5
 #define ICMPV6_REJECT_ROUTE		6
+#define ICMPV6_SRCRT_ERR		7
 
 /*
  *	Codes for Time Exceeded
@@ -137,6 +138,11 @@ struct icmp6hdr {
 #define ICMPV6_HDR_FIELD		0
 #define ICMPV6_UNK_NEXTHDR		1
 #define ICMPV6_UNK_OPTION		2
+#define ICMPV6_FIRST_FRAG_INCOMP	3
+#define ICMPV6_EXTHDR_TOOBIG		4
+#define ICMPV6_EXTHDR_CHAINLONG		5
+#define ICMPV6_TOOMANY_OPTIONS		6
+#define ICMPV6_TOOBIG_OPTION		7
 
 /*
  *	constants for (set|get)sockopt
diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
index f0e0f7a..fe6d9b2 100644
--- a/net/ipv6/exthdrs.c
+++ b/net/ipv6/exthdrs.c
@@ -145,8 +145,11 @@ static bool ip6_parse_tlv(const struct tlvtype_proc *procs,
 		case IPV6_TLV_PAD1:
 			optlen = 1;
 			padlen++;
-			if (padlen > 7)
+			if (padlen > 7) {
+				icmpv6_send(skb, ICMPV6_PARAMPROB,
+					    ICMPV6_TOOBIG_OPTION, off);
 				goto bad;
+			}
 			break;
 
 		case IPV6_TLV_PADN:
@@ -157,8 +160,11 @@ static bool ip6_parse_tlv(const struct tlvtype_proc *procs,
 			 * RFC 8504, Section 5.3.
 			 */
 			padlen += optlen;
-			if (padlen > 7)
+			if (padlen > 7) {
+				icmpv6_send(skb, ICMPV6_PARAMPROB,
+					    ICMPV6_TOOBIG_OPTION, off);
 				goto bad;
+			}
 			/* RFC 4942 recommends receiving hosts to
 			 * actively check PadN payload to contain
 			 * only zeroes.
@@ -174,8 +180,11 @@ static bool ip6_parse_tlv(const struct tlvtype_proc *procs,
 				goto bad;
 
 			tlv_count++;
-			if (tlv_count > max_count)
+			if (tlv_count > max_count) {
+				icmpv6_send(skb, ICMPV6_PARAMPROB,
+					    ICMPV6_TOOMANY_OPTIONS, off);
 				goto bad;
+			}
 
 			for (curr = procs; curr->type >= 0; curr++) {
 				if (curr->type == nh[off]) {
@@ -201,6 +210,8 @@ static bool ip6_parse_tlv(const struct tlvtype_proc *procs,
 	if (len == 0)
 		return true;
 bad:
+	__IP6_INC_STATS(dev_net(skb->dev), __in6_dev_get(skb->dev),
+			IPSTATS_MIB_INHDRERRORS);
 	kfree_skb(skb);
 	return false;
 }
@@ -301,8 +312,15 @@ static int ipv6_destopt_rcv(struct sk_buff *skb)
 	}
 
 	extlen = (skb_transport_header(skb)[1] + 1) << 3;
-	if (extlen > net->ipv6.sysctl.max_dst_opts_len)
+	if (extlen > net->ipv6.sysctl.max_dst_opts_len) {
+		icmpv6_send(skb, ICMPV6_PARAMPROB,
+			    ICMPV6_EXTHDR_TOOBIG,
+			    skb_network_header_len(skb) +
+				net->ipv6.sysctl.max_dst_opts_len);
+		__IP6_INC_STATS(dev_net(dst->dev), idev,
+				IPSTATS_MIB_INHDRERRORS);
 		goto fail_and_free;
+	}
 
 	opt->lastopt = opt->dst1 = skb_network_header_len(skb);
 #if IS_ENABLED(CONFIG_IPV6_MIP6)
@@ -844,8 +862,15 @@ int ipv6_parse_hopopts(struct sk_buff *skb)
 	}
 
 	extlen = (skb_transport_header(skb)[1] + 1) << 3;
-	if (extlen > net->ipv6.sysctl.max_hbh_opts_len)
+	if (extlen > net->ipv6.sysctl.max_hbh_opts_len) {
+		__IP6_INC_STATS(net, __in6_dev_get(skb->dev),
+				IPSTATS_MIB_INHDRERRORS);
+		icmpv6_send(skb, ICMPV6_PARAMPROB,
+			    ICMPV6_EXTHDR_TOOBIG,
+			skb_network_header_len(skb) +
+				net->ipv6.sysctl.max_hbh_opts_len);
 		goto fail_and_free;
+	}
 
 	opt->flags |= IP6SKB_HOPBYHOP;
 	if (ip6_parse_tlv(tlvprochopopt_lst, skb,
-- 
2.7.4

Re: [PATCH net-next 0/4] ipv6: Update RFC references and implement ICMP errors for limits

[David Miller]

Any time you say "Do A and B" it means that your patch set is doing
two different things and should be split up properly into two separate
submissions.