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