Module Name: src
Committed By: rmind
Date: Thu Aug 8 21:29:16 UTC 2019
Modified Files:
src/usr.sbin/npf/npfctl: npf_bpf_comp.c npf_build.c npfctl.h
Log Message:
NPF: fix BPF byte-code generation for a port-range used in a group.
Resolved PR/52609 and PR/54169.
To generate a diff of this commit:
cvs rdiff -u -r1.13 -r1.14 src/usr.sbin/npf/npfctl/npf_bpf_comp.c
cvs rdiff -u -r1.50 -r1.51 src/usr.sbin/npf/npfctl/npf_build.c
cvs rdiff -u -r1.48 -r1.49 src/usr.sbin/npf/npfctl/npfctl.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/usr.sbin/npf/npfctl/npf_bpf_comp.c
diff -u src/usr.sbin/npf/npfctl/npf_bpf_comp.c:1.13 src/usr.sbin/npf/npfctl/npf_bpf_comp.c:1.14
--- src/usr.sbin/npf/npfctl/npf_bpf_comp.c:1.13 Tue Jul 23 00:52:02 2019
+++ src/usr.sbin/npf/npfctl/npf_bpf_comp.c Thu Aug 8 21:29:15 2019
@@ -1,5 +1,5 @@
/*-
- * Copyright (c) 2010-2014 The NetBSD Foundation, Inc.
+ * Copyright (c) 2010-2019 The NetBSD Foundation, Inc.
* All rights reserved.
*
* This material is based upon work partially supported by The
@@ -29,10 +29,60 @@
/*
* BPF byte-code generation for NPF rules.
+ *
+ * Overview
+ *
+ * Each NPF rule is compiled into BPF micro-program. There is a
+ * BPF byte-code fragment for each higher-level filtering logic,
+ * e.g. to match L4 protocol, IP/mask, etc. The generation process
+ * combines multiple BPF-byte code fragments into one program.
+ *
+ * Basic case
+ *
+ * Consider a basic case, where all filters should match. They
+ * are expressed as logical conjunction, e.g.:
+ *
+ * A and B and C and D
+ *
+ * Each test (filter) criterion can be evaluated to true (match) or
+ * false (no match) and the logic is as follows:
+ *
+ * - If the value is true, then jump to the "next" test (offset 0).
+ *
+ * - If the value is false, then jump to the JUMP_MAGIC value (0xff).
+ * This "magic" value is used to indicate that it will have to be
+ * patched at a later stage.
+ *
+ * Once all byte-code fragments are combined into one, then there
+ * are two additional steps:
+ *
+ * - Two instructions are appended at the end of the program: return
+ * "success" followed by return "failure".
+ *
+ * - All jumps with the JUMP_MAGIC value are patched to point to the
+ * "return failure" instruction.
+ *
+ * Therefore, if all filter criteria will match, then the first
+ * instruction will be reached, indicating a successful match of the
+ * rule. Otherwise, if any of the criteria will not match, it will
+ * take the failure path and the rule will not matching.
+ *
+ * Grouping
+ *
+ * Filters can have groups, which are have a meaning of logical
+ * disjunction, e.g.:
+ *
+ * A and B and (C or D)
+ *
+ * In such case, the logic inside the group has to be inverted i.e.
+ * the jump values swapped. If the test value is true, then jump
+ * out of the group; if false, then jump "next". At the end of the
+ * group, an addition failure path is appended and the JUMP_MAGIC
+ * uses within the group are patched to jump past the said path.
*/
#include <sys/cdefs.h>
-__RCSID("$NetBSD: npf_bpf_comp.c,v 1.13 2019/07/23 00:52:02 rmind Exp $");
+__RCSID("$NetBSD: npf_bpf_comp.c,v 1.14 2019/08/08 21:29:15 rmind Exp $");
#include <stdlib.h>
#include <stdbool.h>
@@ -75,7 +125,10 @@ struct npf_bpf {
sa_family_t af;
uint32_t flags;
- /* The current group offset and block number. */
+ /*
+ * The current group offset (counted in BPF instructions)
+ * and block number at the start of the group.
+ */
bool ingroup;
u_int goff;
u_int gblock;
@@ -120,6 +173,7 @@ fixup_jumps(npf_bpf_t *ctx, u_int start,
for (u_int i = start; i < end; i++) {
struct bpf_insn *insn = &bp->bf_insns[i];
const u_int fail_off = end - i;
+ bool seen_magic = false;
if (fail_off >= JUMP_MAGIC) {
errx(EXIT_FAILURE, "BPF generation error: "
@@ -128,15 +182,37 @@ fixup_jumps(npf_bpf_t *ctx, u_int start,
if (BPF_CLASS(insn->code) != BPF_JMP) {
continue;
}
- if (swap) {
+ if (BPF_OP(insn->code) == BPF_JA) {
+ /*
+ * BPF_JA can be used to jump to the failure path.
+ * If we are swapping i.e. inside the group, then
+ * jump "next"; groups have a failure path appended
+ * at their end.
+ */
+ if (insn->k == JUMP_MAGIC) {
+ insn->k = swap ? 0 : fail_off;
+ }
+ continue;
+ }
+
+ /*
+ * Fixup the "magic" value. Swap only the "magic" jumps.
+ */
+
+ if (insn->jt == JUMP_MAGIC) {
+ insn->jt = fail_off;
+ seen_magic = true;
+ }
+ if (insn->jf == JUMP_MAGIC) {
+ insn->jf = fail_off;
+ seen_magic = true;
+ }
+
+ if (seen_magic && swap) {
uint8_t jt = insn->jt;
insn->jt = insn->jf;
insn->jf = jt;
}
- if (insn->jt == JUMP_MAGIC)
- insn->jt = fail_off;
- if (insn->jf == JUMP_MAGIC)
- insn->jf = fail_off;
}
}
@@ -225,11 +301,11 @@ npfctl_bpf_destroy(npf_bpf_t *ctx)
}
/*
- * npfctl_bpf_group: begin a logical group. It merely uses logical
+ * npfctl_bpf_group_enter: begin a logical group. It merely uses logical
* disjunction (OR) for compares within the group.
*/
void
-npfctl_bpf_group(npf_bpf_t *ctx)
+npfctl_bpf_group_enter(npf_bpf_t *ctx)
{
struct bpf_program *bp = &ctx->prog;
@@ -242,7 +318,7 @@ npfctl_bpf_group(npf_bpf_t *ctx)
}
void
-npfctl_bpf_endgroup(npf_bpf_t *ctx, bool invert)
+npfctl_bpf_group_exit(npf_bpf_t *ctx, bool invert)
{
struct bpf_program *bp = &ctx->prog;
const size_t curoff = bp->bf_len;
@@ -255,7 +331,7 @@ npfctl_bpf_endgroup(npf_bpf_t *ctx, bool
/*
* If inverting, then prepend a jump over the statement below.
- * If matching, jump will jump below and the fail will happen.
+ * On match, it will skip-through and the fail path will be taken.
*/
if (invert) {
struct bpf_insn insns_ret[] = {
@@ -318,7 +394,7 @@ fetch_l3(npf_bpf_t *ctx, sa_family_t af,
*/
if (ingroup) {
assert(ctx->nblocks == ctx->gblock);
- npfctl_bpf_endgroup(ctx, false);
+ npfctl_bpf_group_exit(ctx, false);
}
/*
@@ -338,7 +414,7 @@ fetch_l3(npf_bpf_t *ctx, sa_family_t af,
done_raw_block(ctx, mwords, sizeof(mwords));
}
if (ingroup) {
- npfctl_bpf_group(ctx);
+ npfctl_bpf_group_enter(ctx);
}
} else if (af && af != ctx->af) {
@@ -508,8 +584,9 @@ npfctl_bpf_ports(npf_bpf_t *ctx, u_int o
} else {
/* Port range case. */
struct bpf_insn insns_range[] = {
- BPF_JUMP(BPF_JMP+BPF_JGE+BPF_K, from, 0, JUMP_MAGIC),
- BPF_JUMP(BPF_JMP+BPF_JGT+BPF_K, to, JUMP_MAGIC, 0),
+ BPF_JUMP(BPF_JMP+BPF_JGE+BPF_K, from, 0, 1),
+ BPF_JUMP(BPF_JMP+BPF_JGT+BPF_K, to, 0, 1),
+ BPF_STMT(BPF_JMP+BPF_JA, JUMP_MAGIC),
};
add_insns(ctx, insns_range, __arraycount(insns_range));
}
Index: src/usr.sbin/npf/npfctl/npf_build.c
diff -u src/usr.sbin/npf/npfctl/npf_build.c:1.50 src/usr.sbin/npf/npfctl/npf_build.c:1.51
--- src/usr.sbin/npf/npfctl/npf_build.c:1.50 Thu Jul 25 00:48:55 2019
+++ src/usr.sbin/npf/npfctl/npf_build.c Thu Aug 8 21:29:15 2019
@@ -32,7 +32,7 @@
*/
#include <sys/cdefs.h>
-__RCSID("$NetBSD: npf_build.c,v 1.50 2019/07/25 00:48:55 rmind Exp $");
+__RCSID("$NetBSD: npf_build.c,v 1.51 2019/08/08 21:29:15 rmind Exp $");
#include <sys/types.h>
#define __FAVOR_BSD
@@ -290,7 +290,7 @@ npfctl_build_vars(npf_bpf_t *ctx, sa_fam
const int type = npfvar_get_type(vars, 0);
size_t i;
- npfctl_bpf_group(ctx);
+ npfctl_bpf_group_enter(ctx);
for (i = 0; i < npfvar_get_count(vars); i++) {
void *data = npfvar_get_data(vars, type, i);
assert(data != NULL);
@@ -316,7 +316,7 @@ npfctl_build_vars(npf_bpf_t *ctx, sa_fam
assert(false);
}
}
- npfctl_bpf_endgroup(ctx, (opts & MATCH_INVERT) != 0);
+ npfctl_bpf_group_exit(ctx, (opts & MATCH_INVERT) != 0);
}
static void
@@ -423,10 +423,10 @@ npfctl_build_code(nl_rule_t *rl, sa_fami
/* Build port-range blocks. */
if (need_tcpudp) {
/* TCP/UDP check for the ports. */
- npfctl_bpf_group(bc);
+ npfctl_bpf_group_enter(bc);
npfctl_bpf_proto(bc, AF_UNSPEC, IPPROTO_TCP);
npfctl_bpf_proto(bc, AF_UNSPEC, IPPROTO_UDP);
- npfctl_bpf_endgroup(bc, false);
+ npfctl_bpf_group_exit(bc, false);
}
npfctl_build_vars(bc, family, apfrom->ap_portrange, MATCH_SRC);
npfctl_build_vars(bc, family, apto->ap_portrange, MATCH_DST);
Index: src/usr.sbin/npf/npfctl/npfctl.h
diff -u src/usr.sbin/npf/npfctl/npfctl.h:1.48 src/usr.sbin/npf/npfctl/npfctl.h:1.49
--- src/usr.sbin/npf/npfctl/npfctl.h:1.48 Tue Jul 23 00:52:02 2019
+++ src/usr.sbin/npf/npfctl/npfctl.h Thu Aug 8 21:29:15 2019
@@ -167,8 +167,8 @@ struct bpf_program *npfctl_bpf_complete(
const void * npfctl_bpf_bmarks(npf_bpf_t *, size_t *);
void npfctl_bpf_destroy(npf_bpf_t *);
-void npfctl_bpf_group(npf_bpf_t *);
-void npfctl_bpf_endgroup(npf_bpf_t *, bool);
+void npfctl_bpf_group_enter(npf_bpf_t *);
+void npfctl_bpf_group_exit(npf_bpf_t *, bool);
void npfctl_bpf_proto(npf_bpf_t *, sa_family_t, int);
void npfctl_bpf_cidr(npf_bpf_t *, u_int, sa_family_t,
