pf: implement start/stop calls via netlink

Implement equivalents to DIOCSTART and DIOCSTOP in netlink. Provide a
libpfctl implementation and add a basic test case, mostly to verify that
we still return the same errors as before the conversion

Sponsored by:	Rubicon Communications, LLC ("Netgate")
Differential Revision:	https://reviews.freebsd.org/D42145

8 files changed, 151 insertions, 30 deletions

diff --git a/lib/libpfctl/libpfctl.c b/lib/libpfctl/libpfctl.c
index ca29645f9c34..51276d8bb343 100644
--- a/lib/libpfctl/libpfctl.c
+++ b/lib/libpfctl/libpfctl.c
@@ -177,6 +177,34 @@ pf_nvuint_64_array(const nvlist_t *nvl, const char *name, size_t maxelems,
 		*nelems = elems;
 }
 
+int
+pfctl_startstop(int start)
+{
+	struct snl_state ss = {};
+	struct snl_errmsg_data e = {};
+	struct snl_writer nw;
+	struct nlmsghdr *hdr;
+	uint32_t seq_id;
+	int family_id;
+
+	snl_init(&ss, NETLINK_GENERIC);
+	family_id = snl_get_genl_family(&ss, PFNL_FAMILY_NAME);
+
+	snl_init_writer(&ss, &nw);
+	hdr = snl_create_genl_msg_request(&nw, family_id,
+	    start ? PFNL_CMD_START : PFNL_CMD_STOP);
+
+	snl_finalize_msg(&nw);
+	seq_id = hdr->nlmsg_seq;
+
+	snl_send_message(&ss, hdr);
+
+	while ((hdr = snl_read_reply_multi(&ss, seq_id, &e)) != NULL) {
+	}
+
+	return (e.error);
+}
+
 static void
 _pfctl_get_status_counters(const nvlist_t *nvl,
     struct pfctl_status_counters *counters)
diff --git a/lib/libpfctl/libpfctl.h b/lib/libpfctl/libpfctl.h
index e75f93d8775e..06cd25e82c08 100644
--- a/lib/libpfctl/libpfctl.h
+++ b/lib/libpfctl/libpfctl.h
@@ -384,6 +384,7 @@ struct pfctl_syncookies {
 	uint8_t				lowwater;	/* Percent */
 };
 
+int	pfctl_startstop(int start);
 struct pfctl_status* pfctl_get_status(int dev);
 uint64_t pfctl_status_counter(struct pfctl_status *status, int id);
 uint64_t pfctl_status_fcounter(struct pfctl_status *status, int id);
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index 759b36d9cebe..56b1d28c6fd6 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -310,10 +310,12 @@ pfctl_proto2name(int proto)
 int
 pfctl_enable(int dev, int opts)
 {
-	if (ioctl(dev, DIOCSTART)) {
-		if (errno == EEXIST)
+	int ret;
+
+	if ((ret = pfctl_startstop(1)) != 0) {
+		if (ret == EEXIST)
 			errx(1, "pf already enabled");
-		else if (errno == ESRCH)
+		else if (ret == ESRCH)
 			errx(1, "pfil registeration failed");
 		else
 			err(1, "DIOCSTART");
@@ -331,8 +333,10 @@ pfctl_enable(int dev, int opts)
 int
 pfctl_disable(int dev, int opts)
 {
-	if (ioctl(dev, DIOCSTOP)) {
-		if (errno == ENOENT)
+	int ret;
+
+	if ((ret = pfctl_startstop(0)) != 0) {
+		if (ret == ENOENT)
 			errx(1, "pf not enabled");
 		else
 			err(1, "DIOCSTOP");
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index f5ea9bf71f0c..59579a0849d9 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -2166,6 +2166,8 @@ VNET_DECLARE(struct pf_krule *, pf_rulemarker);
 #define V_pf_rulemarker     VNET(pf_rulemarker)
 #endif
 
+int				 pf_start(void);
+int				 pf_stop(void);
 void				 pf_initialize(void);
 void				 pf_mtag_initialize(void);
 void				 pf_mtag_cleanup(void);
diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c
index 5a441c9723e3..38c09303a543 100644
--- a/sys/netpfil/pf/pf_ioctl.c
+++ b/sys/netpfil/pf/pf_ioctl.c
@@ -2337,6 +2337,49 @@ relock_DIOCKILLSTATES:
 	return (killed);
 }
 
+int
+pf_start(void)
+{
+	int error = 0;
+
+	sx_xlock(&V_pf_ioctl_lock);
+	if (V_pf_status.running)
+		error = EEXIST;
+	else {
+		hook_pf();
+		if (! TAILQ_EMPTY(V_pf_keth->active.rules))
+			hook_pf_eth();
+		V_pf_status.running = 1;
+		V_pf_status.since = time_second;
+		new_unrhdr64(&V_pf_stateid, time_second);
+
+		DPFPRINTF(PF_DEBUG_MISC, ("pf: started\n"));
+	}
+	sx_xunlock(&V_pf_ioctl_lock);
+
+	return (error);
+}
+
+int
+pf_stop(void)
+{
+	int error = 0;
+
+	sx_xlock(&V_pf_ioctl_lock);
+	if (!V_pf_status.running)
+		error = ENOENT;
+	else {
+		V_pf_status.running = 0;
+		dehook_pf();
+		dehook_pf_eth();
+		V_pf_status.since = time_second;
+		DPFPRINTF(PF_DEBUG_MISC, ("pf: stopped\n"));
+	}
+	sx_xunlock(&V_pf_ioctl_lock);
+
+	return (error);
+}
+
 static int
 pfioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread *td)
 {
@@ -2479,34 +2522,15 @@ pfioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread *td
 	CURVNET_SET(TD_TO_VNET(td));
 
 	switch (cmd) {
+#ifdef COMPAT_FREEBSD14
 	case DIOCSTART:
-		sx_xlock(&V_pf_ioctl_lock);
-		if (V_pf_status.running)
-			error = EEXIST;
-		else {
-			hook_pf();
-			if (! TAILQ_EMPTY(V_pf_keth->active.rules))
-				hook_pf_eth();
-			V_pf_status.running = 1;
-			V_pf_status.since = time_second;
-			new_unrhdr64(&V_pf_stateid, time_second);
-
-			DPFPRINTF(PF_DEBUG_MISC, ("pf: started\n"));
-		}
+		error = pf_start();
 		break;
 
 	case DIOCSTOP:
-		sx_xlock(&V_pf_ioctl_lock);
-		if (!V_pf_status.running)
-			error = ENOENT;
-		else {
-			V_pf_status.running = 0;
-			dehook_pf();
-			dehook_pf_eth();
-			V_pf_status.since = time_second;
-			DPFPRINTF(PF_DEBUG_MISC, ("pf: stopped\n"));
-		}
+		error = pf_stop();
 		break;
+#endif
 
 	case DIOCGETETHRULES: {
 		struct pfioc_nv		*nv = (struct pfioc_nv *)addr;
@@ -5416,8 +5440,6 @@ DIOCCHANGEADDR_error:
 		break;
 	}
 fail:
-	if (sx_xlocked(&V_pf_ioctl_lock))
-		sx_xunlock(&V_pf_ioctl_lock);
 	CURVNET_RESTORE();
 
 #undef ERROUT_IOCTL
diff --git a/sys/netpfil/pf/pf_nl.c b/sys/netpfil/pf/pf_nl.c
index 459a5dc6507e..e079edcc166d 100644
--- a/sys/netpfil/pf/pf_nl.c
+++ b/sys/netpfil/pf/pf_nl.c
@@ -336,6 +336,18 @@ pf_handle_getcreators(struct nlmsghdr *hdr, struct nl_pstate *npt)
 	return (error);
 }
 
+static int
+pf_handle_start(struct nlmsghdr *hdr __unused, struct nl_pstate *npt __unused)
+{
+	return (pf_start());
+}
+
+static int
+pf_handle_stop(struct nlmsghdr *hdr __unused, struct nl_pstate *npt __unused)
+{
+	return (pf_stop());
+}
+
 static const struct nlhdr_parser *all_parsers[] = { &state_parser };
 
 static int family_id;
@@ -353,6 +365,18 @@ static const struct genl_cmd pf_cmds[] = {
 		.cmd_cb = pf_handle_getcreators,
 		.cmd_flags = GENL_CMD_CAP_DO | GENL_CMD_CAP_DUMP | GENL_CMD_CAP_HASPOL,
 	},
+	{
+		.cmd_num = PFNL_CMD_START,
+		.cmd_name = "START",
+		.cmd_cb = pf_handle_start,
+		.cmd_flags = GENL_CMD_CAP_DO | GENL_CMD_CAP_HASPOL,
+	},
+	{
+		.cmd_num = PFNL_CMD_STOP,
+		.cmd_name = "STOP",
+		.cmd_cb = pf_handle_stop,
+		.cmd_flags = GENL_CMD_CAP_DO | GENL_CMD_CAP_HASPOL,
+	},
 };
 
 void
diff --git a/sys/netpfil/pf/pf_nl.h b/sys/netpfil/pf/pf_nl.h
index 98525641b43d..3c8c6d3b8ed4 100644
--- a/sys/netpfil/pf/pf_nl.h
+++ b/sys/netpfil/pf/pf_nl.h
@@ -38,6 +38,8 @@ enum {
 	PFNL_CMD_UNSPEC = 0,
 	PFNL_CMD_GETSTATES = 1,
 	PFNL_CMD_GETCREATORS = 2,
+	PFNL_CMD_START = 3,
+	PFNL_CMD_STOP = 4,
 	__PFNL_CMD_MAX,
 };
 #define PFNL_CMD_MAX (__PFNL_CMD_MAX -1)
diff --git a/tests/sys/netpfil/pf/pass_block.sh b/tests/sys/netpfil/pf/pass_block.sh
index 792b73b4a0a5..faf5c2de4de2 100644
--- a/tests/sys/netpfil/pf/pass_block.sh
+++ b/tests/sys/netpfil/pf/pass_block.sh
@@ -28,6 +28,43 @@
 
 common_dir=$(atf_get_srcdir)/../common
 
+atf_test_case "enable_disable" "cleanup"
+enable_disable_head()
+{
+	atf_set descr 'Test enable/disable'
+	atf_set require.user root
+}
+
+enable_disable_body()
+{
+	pft_init
+
+	j="pass_block:enable_disable"
+
+	vnet_mkjail ${j}
+
+	# Disable when disabled fails
+	atf_check -s exit:1 -e ignore \
+	    jexec ${j} pfctl -d
+
+	# Enable succeeds
+	atf_check -s exit:0 -e ignore \
+	    jexec ${j} pfctl -e
+
+	# Enable when enabled fails
+	atf_check -s exit:1 -e ignore \
+	    jexec ${j} pfctl -e
+
+	# Disable succeeds
+	atf_check -s exit:0 -e ignore \
+	    jexec ${j} pfctl -d
+}
+
+enable_disable_cleanup()
+{
+	pft_cleanup
+}
+
 atf_test_case "v4" "cleanup"
 v4_head()
 {
@@ -257,6 +294,7 @@ urpf_cleanup()
 
 atf_init_test_cases()
 {
+	atf_add_test_case "enable_disable"
 	atf_add_test_case "v4"
 	atf_add_test_case "v6"
 	atf_add_test_case "noalias"