Dropping packets with IPv4/IPv6 options

[m00nbsd]

Description

It would be nice to add an option in NPF allowing to drop packets with IPv4/IPv6 options. This is an important yet missing feature, and IPv4/IPv6 options are a significant problem in network security.

The point is that IPv4/IPv6 options are big enablers when it comes to exploitation: they are hard to parse (that is the parser can easily be buggy), and they allow to push the actual payload farther in the mbuf, in a way that makes it a lot easier to exploit buffer overflows.

Two years ago I wrote a patch for that using BPF. Unfortunately I realized it wasn't correct, because when a connection already exists NPF does a pass-through of the packet without applying BPF rules, meaning that the subsequent TCP packets in the stream could have IPv4/IPv6 options and NPF would accept them.

Patch Suggestion

Probably the correct way to proceed is using npf-params, with ip4.drop_options and ip6.drop_options, along the lines of:

 		/* Retrieve the complete header. */
 		if ((u_int)(ip->ip_hl << 2) < sizeof(struct ip)) {
 			return NPC_FMTERR;
 		}
+		if (npf->ip4_drop_options && (ip->ip_hl != 5)) {
+			return NPC_FMTERR;
+		}
 		ip = nbuf_ensure_contig(nbuf, (u_int)(ip->ip_hl << 2));
 		if (ip == NULL) {
 			return NPC_FMTERR;
 		}

 			case IPPROTO_HOPOPTS:
 			case IPPROTO_DSTOPTS:
 			case IPPROTO_ROUTING:
+				if (npf->ip6_drop_options) {
+					return NPC_FMTERR;
+				}
 				hlen = (ip6e->ip6e_len + 1) << 3;
 				break;

By default I think that IPv4/IPv6 options should be dropped, like PF does.

[rmind]

@m00nbsd:

• Adding ip4.drop_options and ip6.drop_options parameters sounds good. The user should also be able to do rule-based filtering of options, but that is a separate patch.
• I'd swap the checks in the first patch fragment, i.e.: if (ip->ip_hl != 5 && npf->ip4_drop_options) ...
• Not sure about dropping by default; there are still couple useful options. I would say let's introduce the parameters with defaults being "off" and then initiate a discussion on NetBSD tech-net. Based on the feedback, we can then decide to flip the defaults from "off" to "on".

Would you like to make a pull request (PR)?

[rmind]

Fixed by #112