PPTP: several issues in the code

[m00nbsd]

A few remarks on the latest PPTP additions, CC @alexk99 who may be interested:

• npfa_pptp_gre_conns_init(): KM_SLEEP does not fail, so either the flag should be KM_NOSLEEP, or the NULL check should be removed.

• npf_pptp_gre_cache(): there should be a NULL check on the nbuf_advance(), because it will fail if the packet is too small.

• npfa_pptp_tcp_translate():

  1. The access to tcp may not be safe after the first call to nbuf_advance(), because this function can reorder the underlying mbuf. If this happens, then tcp will point to the previous area, which is now freed.
  2. There should be a NULL check on pptp_call_reply because nbuf_ensure_contig() could fail if the packet is too small.
  3. Now that th_off has a real meaning within NPF, we may want to add a sanity check in npf_cache_all() to make sure that th_off >= 5, and leave with NPC_FMTERR if the check fails. This is to prevent possible surprises later in the NPF stack, and such packets are invalid anyway.

[rmind]

@m00nbsd: I fixed bugs you reported (apart from the last point) in #61.

Do you want to come up with a patch for the last point, if you have something specific in mind? At least some of the sanitization checks (triggering NPC_FMTERR) should be configurable, though, so the user could switch them off/on.

[m00nbsd]

Something along those lines (not tested): npf-tcp.txt.

We may want to be able to configure whether to drop with NPC_FMTERR or just ignore. However, the default should be to drop, as we already do on IP packets.

[rmind]

@m00nbsd: I can merge that; do you want to make a pull request (PR)?

[m00nbsd]

Please proceed, I cannot test right now

[rmind]

Resolved.