[PATCH xtables-addons v2 00/13] pknlusr improvements

[PATCH xtables-addons v2 00/13] pknlusr improvements

[Jeremy Sowden]

Since pknlusr is now installed, here are some improvements.  There's one
automake change related to the new man-page, a number of new patches
tidying up the source of pknlusr.c, before a new version of the patch to
remove the hard-coded group ID.  The last four patches do a bit of
tiding of the pknock kernel module.

Jeremy Sowden (13):
  pknock: pknlusr: ensure man-page is included by `make dist`.
  pknock: pknlusr: remove dest_addr and rename src_addr.
  pknock: pknlusr: tighten up variable scopes.
  pknock: pknlusr: tidy up initialization of local address.
  pknock: pknlusr: use NLMSG macros and proper types, rather than
    arithmetic on char pointers.
  pknock: pknlusr: use macro to define inet_ntop buffer size.
  pknock: pknlusr: don't treat recv return value of zero as an error.
  pknock: pknlusr: always close socket.
  pknock: pknlusr: fix hard-coded netlink multicast group ID.
  pknock: xt_pknock: use IS_ENABLED.
  pknock: xt_pknock: use kzalloc.
  pknock: xt_pknock: use pr_err.
  pknock: xt_pknock: remove DEBUG definition.

 extensions/pknock/Makefile.am |   2 +-
 extensions/pknock/pknlusr.c   | 120 ++++++++++++++++++++++------------
 extensions/pknock/xt_pknock.c |  27 +++-----
 extensions/pknock/xt_pknock.h |   2 -
 4 files changed, 88 insertions(+), 63 deletions(-)

-- 
2.28.0

[PATCH xtables-addons v2 00/13] pknlusr improvements

[Jeremy Sowden]

Since pknlusr is now installed, here are some improvements.  There's one
automake change related to the new man-page, a number of new patches
tidying up the source of pknlusr.c, before a new version of the patch to
remove the hard-coded group ID.  The last four patches do a bit of
tidying of the pknock kernel module.

Jeremy Sowden (13):
  pknock: pknlusr: ensure man-page is included by `make dist`.
  pknock: pknlusr: remove dest_addr and rename src_addr.
  pknock: pknlusr: tighten up variable scopes.
  pknock: pknlusr: tidy up initialization of local address.
  pknock: pknlusr: use NLMSG macros and proper types, rather than
    arithmetic on char pointers.
  pknock: pknlusr: use macro to define inet_ntop buffer size.
  pknock: pknlusr: don't treat recv return value of zero as an error.
  pknock: pknlusr: always close socket.
  pknock: pknlusr: fix hard-coded netlink multicast group ID.
  pknock: xt_pknock: use IS_ENABLED.
  pknock: xt_pknock: use kzalloc.
  pknock: xt_pknock: use `pr_err`.
  pknock: xt_pknock: remove DEBUG definition and disable debug output.

 extensions/pknock/Makefile.am    |   2 +-
 extensions/pknock/libxt_pknock.c |   4 +-
 extensions/pknock/pknlusr.c      | 113 +++++++++++++++++++------------
 extensions/pknock/xt_pknock.c    |  27 +++-----
 extensions/pknock/xt_pknock.h    |   2 -
 5 files changed, 83 insertions(+), 65 deletions(-)

-- 
2.28.0

[PATCH xtables-addons v2 01/13] pknock: pknlusr: ensure man-page is included by `make dist`.

[Jeremy Sowden]

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 extensions/pknock/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/extensions/pknock/Makefile.am b/extensions/pknock/Makefile.am
index e62c10884048..35528709aa15 100644
--- a/extensions/pknock/Makefile.am
+++ b/extensions/pknock/Makefile.am
@@ -6,4 +6,4 @@ AM_CFLAGS   = ${regular_CFLAGS} ${libxtables_CFLAGS}
 include ../../Makefile.extra
 
 sbin_PROGRAMS = pknlusr
-man_MANS = pknlusr.8
+dist_man_MANS = pknlusr.8
-- 
2.28.0

[PATCH xtables-addons v2 02/13] pknock: pknlusr: remove dest_addr and rename src_addr.

[Jeremy Sowden]

We only need to specify the address at our end, and given that we are
receiving messages, not sending them, calling it `src_addr` is
misleading.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 extensions/pknock/pknlusr.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/extensions/pknock/pknlusr.c b/extensions/pknock/pknlusr.c
index 14bc44a13887..4e3e02a0b9f0 100644
--- a/extensions/pknock/pknlusr.c
+++ b/extensions/pknock/pknlusr.c
@@ -12,7 +12,7 @@
 
 #define GROUP 1
 
-static struct sockaddr_nl src_addr, dest_addr;
+static struct sockaddr_nl local_addr;
 static int sock_fd;
 
 static unsigned char *buf;
@@ -21,7 +21,6 @@ static struct xt_pknock_nl_msg *nlmsg;
 
 int main(void)
 {
-	socklen_t addrlen;
 	int status;
 	int group = GROUP;
 
@@ -37,12 +36,12 @@ int main(void)
 		return 1;
 	}
 
-	memset(&src_addr, 0, sizeof(src_addr));
-	src_addr.nl_family = AF_NETLINK;
-	src_addr.nl_pid = getpid();
-	src_addr.nl_groups = group;
+	memset(&local_addr, 0, sizeof(local_addr));
+	local_addr.nl_family = AF_NETLINK;
+	local_addr.nl_pid = getpid();
+	local_addr.nl_groups = group;
 
-	status = bind(sock_fd, (struct sockaddr*)&src_addr, sizeof(src_addr));
+	status = bind(sock_fd, (struct sockaddr*)&local_addr, sizeof(local_addr));
 
 	if (status == -1) {
 		close(sock_fd);
@@ -50,11 +49,6 @@ int main(void)
 		return 1;
 	}
 
-	memset(&dest_addr, 0, sizeof(dest_addr));
-	dest_addr.nl_family = AF_NETLINK;
-	dest_addr.nl_pid = 0;
-	dest_addr.nl_groups = group;
-
 	buf_size = sizeof(struct xt_pknock_nl_msg) + sizeof(struct cn_msg) + sizeof(struct nlmsghdr);
 	buf = malloc(buf_size);
 
@@ -63,16 +57,14 @@ int main(void)
 		return 1;
 	}
 
-	addrlen = sizeof(dest_addr);
-
 	while(1) {
 
 		memset(buf, 0, buf_size);
 
-		status = recvfrom(sock_fd, buf, buf_size, 0, (struct sockaddr *)&dest_addr, &addrlen);
+		status = recv(sock_fd, buf, buf_size, 0);
 
 		if (status <= 0) {
-			perror("recvfrom()");
+			perror("recv()");
 			return 1;
 		}
 		nlmsg = (struct xt_pknock_nl_msg *)(buf + sizeof(struct cn_msg) + sizeof(struct nlmsghdr));
-- 
2.28.0

[PATCH xtables-addons v2 03/13] pknock: pknlusr: tighten up variable scopes.

[Jeremy Sowden]

Make global variables local, and move variables local to while-loop into
the loop.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 extensions/pknock/pknlusr.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/extensions/pknock/pknlusr.c b/extensions/pknock/pknlusr.c
index 4e3e02a0b9f0..808b737f1db2 100644
--- a/extensions/pknock/pknlusr.c
+++ b/extensions/pknock/pknlusr.c
@@ -12,22 +12,16 @@
 
 #define GROUP 1
 
-static struct sockaddr_nl local_addr;
-static int sock_fd;
-
-static unsigned char *buf;
-
-static struct xt_pknock_nl_msg *nlmsg;
-
 int main(void)
 {
 	int status;
 	int group = GROUP;
 
-	int buf_size;
+	struct sockaddr_nl local_addr;
+	int sock_fd;
 
-	const char *ip;
-	char ipbuf[48];
+	int buf_size;
+	unsigned char *buf;
 
 	sock_fd = socket(PF_NETLINK, SOCK_DGRAM, NETLINK_CONNECTOR);
 
@@ -59,6 +53,11 @@ int main(void)
 
 	while(1) {
 
+		struct xt_pknock_nl_msg *nlmsg;
+
+		const char *ip;
+		char ipbuf[48];
+
 		memset(buf, 0, buf_size);
 
 		status = recv(sock_fd, buf, buf_size, 0);
-- 
2.28.0

[PATCH xtables-addons v2 04/13] pknock: pknlusr: tidy up initialization of local address.

[Jeremy Sowden]

Use struct initialization and drop memset.  We don't need to set the port
ID, since the kernel will do it for us.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 extensions/pknock/pknlusr.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/extensions/pknock/pknlusr.c b/extensions/pknock/pknlusr.c
index 808b737f1db2..ed741599558b 100644
--- a/extensions/pknock/pknlusr.c
+++ b/extensions/pknock/pknlusr.c
@@ -17,7 +17,7 @@ int main(void)
 	int status;
 	int group = GROUP;
 
-	struct sockaddr_nl local_addr;
+	struct sockaddr_nl local_addr = { .nl_family = AF_NETLINK };
 	int sock_fd;
 
 	int buf_size;
@@ -30,9 +30,6 @@ int main(void)
 		return 1;
 	}
 
-	memset(&local_addr, 0, sizeof(local_addr));
-	local_addr.nl_family = AF_NETLINK;
-	local_addr.nl_pid = getpid();
 	local_addr.nl_groups = group;
 
 	status = bind(sock_fd, (struct sockaddr*)&local_addr, sizeof(local_addr));
-- 
2.28.0

[PATCH xtables-addons v2 05/13] pknock: pknlusr: use NLMSG macros and proper types, rather than arithmetic on char pointers.

[Jeremy Sowden]

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 extensions/pknock/pknlusr.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/extensions/pknock/pknlusr.c b/extensions/pknock/pknlusr.c
index ed741599558b..252fd42ffecd 100644
--- a/extensions/pknock/pknlusr.c
+++ b/extensions/pknock/pknlusr.c
@@ -20,8 +20,10 @@ int main(void)
 	struct sockaddr_nl local_addr = { .nl_family = AF_NETLINK };
 	int sock_fd;
 
-	int buf_size;
-	unsigned char *buf;
+	size_t nlmsg_size;
+	struct nlmgrhdr *nlmsg;
+	struct cn_msg *cn_msg;
+	struct xt_pknock_nl_msg *pknock_msg;
 
 	sock_fd = socket(PF_NETLINK, SOCK_DGRAM, NETLINK_CONNECTOR);
 
@@ -40,38 +42,38 @@ int main(void)
 		return 1;
 	}
 
-	buf_size = sizeof(struct xt_pknock_nl_msg) + sizeof(struct cn_msg) + sizeof(struct nlmsghdr);
-	buf = malloc(buf_size);
+	nlmsg_size = NLMSG_SPACE(sizeof(*cn_msg) + sizeof(*pknock_msg));
+	nlmsg = malloc(nlmsg_size);
 
-	if (!buf) {
+	if (!nlmsg) {
 		perror("malloc()");
 		return 1;
 	}
 
 	while(1) {
 
-		struct xt_pknock_nl_msg *nlmsg;
-
 		const char *ip;
 		char ipbuf[48];
 
-		memset(buf, 0, buf_size);
+		memset(nlmsg, 0, nlmsg_size);
 
-		status = recv(sock_fd, buf, buf_size, 0);
+		status = recv(sock_fd, nlmsg, nlmsg_size, 0);
 
 		if (status <= 0) {
 			perror("recv()");
 			return 1;
 		}
-		nlmsg = (struct xt_pknock_nl_msg *)(buf + sizeof(struct cn_msg) + sizeof(struct nlmsghdr));
-		ip = inet_ntop(AF_INET, &nlmsg->peer_ip, ipbuf, sizeof(ipbuf));
-		printf("rule_name: %s - ip %s\n", nlmsg->rule_name, ip);
+
+		cn_msg = NLMSG_DATA(nlmsg);
+		pknock_msg = (struct xt_pknock_nl_msg *)(cn_msg->data);
+		ip = inet_ntop(AF_INET, &pknock_msg->peer_ip, ipbuf, sizeof(ipbuf));
+		printf("rule_name: %s - ip %s\n", pknock_msg->rule_name, ip);
 
 	}
 
 	close(sock_fd);
 
-	free(buf);
+	free(nlmsg);
 
 	return 0;
 }
-- 
2.28.0

[PATCH xtables-addons v2 06/13] pknock: pknlusr: use macro to define inet_ntop buffer size.

[Jeremy Sowden]

POSIX provides a macro to define the minimum length required, so let's
use it.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 extensions/pknock/pknlusr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/extensions/pknock/pknlusr.c b/extensions/pknock/pknlusr.c
index 252fd42ffecd..9f11250510a1 100644
--- a/extensions/pknock/pknlusr.c
+++ b/extensions/pknock/pknlusr.c
@@ -53,7 +53,7 @@ int main(void)
 	while(1) {
 
 		const char *ip;
-		char ipbuf[48];
+		char ipbuf[INET_ADDRSTRLEN];
 
 		memset(nlmsg, 0, nlmsg_size);
 
-- 
2.28.0

[PATCH xtables-addons v2 07/13] pknock: pknlusr: don't treat recv return value of zero as an error.

[Jeremy Sowden]

A return-value of zero is not an error, so there's no point calling
perror, but since we have not requested and don't expect a zero-length
datagram, we treat it as EOF and exit.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 extensions/pknock/pknlusr.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/extensions/pknock/pknlusr.c b/extensions/pknock/pknlusr.c
index 9f11250510a1..2dd9ab7b9705 100644
--- a/extensions/pknock/pknlusr.c
+++ b/extensions/pknock/pknlusr.c
@@ -59,11 +59,14 @@ int main(void)
 
 		status = recv(sock_fd, nlmsg, nlmsg_size, 0);
 
-		if (status <= 0) {
+		if (status < 0) {
 			perror("recv()");
 			return 1;
 		}
 
+		if (status == 0)
+			break;
+
 		cn_msg = NLMSG_DATA(nlmsg);
 		pknock_msg = (struct xt_pknock_nl_msg *)(cn_msg->data);
 		ip = inet_ntop(AF_INET, &pknock_msg->peer_ip, ipbuf, sizeof(ipbuf));
-- 
2.28.0

[PATCH xtables-addons v2 08/13] pknock: pknlusr: always close socket.

[Jeremy Sowden]

On some error paths the socket was not being closed before exit.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 extensions/pknock/pknlusr.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/extensions/pknock/pknlusr.c b/extensions/pknock/pknlusr.c
index 2dd9ab7b9705..fba628e1f466 100644
--- a/extensions/pknock/pknlusr.c
+++ b/extensions/pknock/pknlusr.c
@@ -29,7 +29,7 @@ int main(void)
 
 	if (sock_fd == -1) {
 		perror("socket()");
-		return 1;
+		exit (EXIT_FAILURE);
 	}
 
 	local_addr.nl_groups = group;
@@ -37,9 +37,8 @@ int main(void)
 	status = bind(sock_fd, (struct sockaddr*)&local_addr, sizeof(local_addr));
 
 	if (status == -1) {
-		close(sock_fd);
 		perror("bind()");
-		return 1;
+		goto err_close_sock;
 	}
 
 	nlmsg_size = NLMSG_SPACE(sizeof(*cn_msg) + sizeof(*pknock_msg));
@@ -47,7 +46,7 @@ int main(void)
 
 	if (!nlmsg) {
 		perror("malloc()");
-		return 1;
+		goto err_close_sock;
 	}
 
 	while(1) {
@@ -61,7 +60,7 @@ int main(void)
 
 		if (status < 0) {
 			perror("recv()");
-			return 1;
+			goto err_free_msg;
 		}
 
 		if (status == 0)
@@ -74,9 +73,11 @@ int main(void)
 
 	}
 
-	close(sock_fd);
-
+err_free_msg:
 	free(nlmsg);
 
-	return 0;
+err_close_sock:
+	close(sock_fd);
+
+	exit (status == -1 ? EXIT_FAILURE : EXIT_SUCCESS);
 }
-- 
2.28.0

[PATCH xtables-addons v2 09/13] pknock: pknlusr: fix hard-coded netlink multicast group ID.

[Jeremy Sowden]

The group ID used by xt_pknock is configurable, but pknlusr hard-codes
it.  Modify pknlusr to accept an optional ID from the command-line.
Group ID's range from 1 to 32 and each ID appears in the group bit-mask
at position `group_id - 1`.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 extensions/pknock/pknlusr.c | 41 +++++++++++++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/extensions/pknock/pknlusr.c b/extensions/pknock/pknlusr.c
index fba628e1f466..255649aefbb5 100644
--- a/extensions/pknock/pknlusr.c
+++ b/extensions/pknock/pknlusr.c
@@ -7,15 +7,22 @@
 #include <arpa/inet.h>
 #include <linux/netlink.h>
 #include <linux/connector.h>
+#include <errno.h>
+#include <libgen.h>
+#include <limits.h>
 
 #include "xt_pknock.h"
 
-#define GROUP 1
+#define DEFAULT_GROUP_ID 1
 
-int main(void)
+#define MIN_GROUP_ID DEFAULT_GROUP_ID
+#define MAX_GROUP_ID \
+	(sizeof ((struct sockaddr_nl) { 0 }.nl_groups) * CHAR_BIT)
+
+int main(int argc, char **argv)
 {
 	int status;
-	int group = GROUP;
+	unsigned int group_id = DEFAULT_GROUP_ID;
 
 	struct sockaddr_nl local_addr = { .nl_family = AF_NETLINK };
 	int sock_fd;
@@ -25,6 +32,32 @@ int main(void)
 	struct cn_msg *cn_msg;
 	struct xt_pknock_nl_msg *pknock_msg;
 
+	if (argc > 2) {
+		char *prog;
+		if (!(prog = strdup (argv[0]))) {
+			perror("strdup()");
+		} else {
+			fprintf(stderr, "%s [ group-id ]\n", basename(prog));
+			free(prog);
+		}
+		exit(EXIT_FAILURE);
+	}
+
+	if (argc == 2) {
+		long n;
+		char *end;
+
+		errno = 0;
+		n = strtol(argv[1], &end, 10);
+		if (*end || (errno && (n == LONG_MIN || n == LONG_MAX)) ||
+		    n < MIN_GROUP_ID || n > MAX_GROUP_ID) {
+			fputs("Group ID invalid.\n", stderr);
+			exit(EXIT_FAILURE);
+		}
+
+		group_id = n;
+	}
+
 	sock_fd = socket(PF_NETLINK, SOCK_DGRAM, NETLINK_CONNECTOR);
 
 	if (sock_fd == -1) {
@@ -32,7 +65,7 @@ int main(void)
 		exit (EXIT_FAILURE);
 	}
 
-	local_addr.nl_groups = group;
+	local_addr.nl_groups = 1U << (group_id - 1);
 
 	status = bind(sock_fd, (struct sockaddr*)&local_addr, sizeof(local_addr));
 
-- 
2.28.0

[PATCH xtables-addons v2 10/13] pknock: xt_pknock: use IS_ENABLED.

[Jeremy Sowden]

It's more succinct than checking whether CONFIG_BLAH or
CONFIG_BLAH_MODULE are defined.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 extensions/pknock/xt_pknock.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/extensions/pknock/xt_pknock.c b/extensions/pknock/xt_pknock.c
index a9df420cc75e..ba8161517d27 100644
--- a/extensions/pknock/xt_pknock.c
+++ b/extensions/pknock/xt_pknock.c
@@ -677,7 +677,7 @@ static bool
 msg_to_userspace_nl(const struct xt_pknock_mtinfo *info,
                 const struct peer *peer, int multicast_group)
 {
-#if defined(CONFIG_CONNECTOR) || defined(CONFIG_CONNECTOR_MODULE)
+#if IS_ENABLED(CONFIG_CONNECTOR)
 	struct cn_msg *m;
 	struct xt_pknock_nl_msg msg;
 
@@ -1101,7 +1101,7 @@ static struct xt_match xt_pknock_mt_reg __read_mostly = {
 
 static int __init xt_pknock_mt_init(void)
 {
-#if !defined(CONFIG_CONNECTOR) && !defined(CONFIG_CONNECTOR_MODULE)
+#if !IS_ENABLED(CONFIG_CONNECTOR)
 	if (nl_multicast_group != -1)
 		pr_info("CONFIG_CONNECTOR not present; "
 		        "netlink messages disabled\n");
-- 
2.28.0

[PATCH xtables-addons v2 11/13] pknock: xt_pknock: use kzalloc.

[Jeremy Sowden]

Replace some instances of kmalloc + memset.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 extensions/pknock/xt_pknock.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/extensions/pknock/xt_pknock.c b/extensions/pknock/xt_pknock.c
index ba8161517d27..ae3ab2445c3b 100644
--- a/extensions/pknock/xt_pknock.c
+++ b/extensions/pknock/xt_pknock.c
@@ -450,13 +450,12 @@ add_rule(struct xt_pknock_mtinfo *info)
 		return true;
 	}
 
-	rule = kmalloc(sizeof(*rule), GFP_KERNEL);
+	rule = kzalloc(sizeof(*rule), GFP_KERNEL);
 	if (rule == NULL)
 		return false;
 
 	INIT_LIST_HEAD(&rule->head);
 
-	memset(rule->rule_name, 0, sizeof(rule->rule_name));
 	strncpy(rule->rule_name, info->rule_name, info->rule_name_len);
 	rule->rule_name_len = info->rule_name_len;
 
@@ -681,12 +680,9 @@ msg_to_userspace_nl(const struct xt_pknock_mtinfo *info,
 	struct cn_msg *m;
 	struct xt_pknock_nl_msg msg;
 
-	m = kmalloc(sizeof(*m) + sizeof(msg), GFP_ATOMIC);
+	m = kzalloc(sizeof(*m) + sizeof(msg), GFP_ATOMIC);
 	if (m == NULL)
 		return false;
-
-	memset(m, 0, sizeof(*m) + sizeof(msg));
-	m->seq = 0;
 	m->len = sizeof(msg);
 
 	msg.peer_ip = peer->ip;
@@ -731,7 +727,7 @@ static bool
 has_secret(const unsigned char *secret, unsigned int secret_len, uint32_t ipsrc,
     const unsigned char *payload, unsigned int payload_len)
 {
-	char result[64]; // 64 bytes * 8 = 512 bits
+	char result[64] = ""; // 64 bytes * 8 = 512 bits
 	char *hexresult;
 	unsigned int hexa_size;
 	int ret;
@@ -752,13 +748,10 @@ has_secret(const unsigned char *secret, unsigned int secret_len, uint32_t ipsrc,
 	if (payload_len != hexa_size + 1)
 		return false;
 
-	hexresult = kmalloc(hexa_size, GFP_ATOMIC);
+	hexresult = kzalloc(hexa_size, GFP_ATOMIC);
 	if (hexresult == NULL)
 		return false;
 
-	memset(result, 0, sizeof(result));
-	memset(hexresult, 0, hexa_size);
-
 	epoch_min = get_seconds() / 60;
 
 	ret = crypto_shash_setkey(crypto.tfm, secret, secret_len);
-- 
2.28.0

[PATCH xtables-addons v2 12/13] pknock: xt_pknock: use `pr_err`.

[Jeremy Sowden]

Replace some instances of `printk(KERN_ERR PKNOCK ...)`.  We define
`pr_fmt`, so `pr_err` is equivalent.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 extensions/pknock/xt_pknock.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/extensions/pknock/xt_pknock.c b/extensions/pknock/xt_pknock.c
index ae3ab2445c3b..f2c79529e21b 100644
--- a/extensions/pknock/xt_pknock.c
+++ b/extensions/pknock/xt_pknock.c
@@ -1016,7 +1016,7 @@ out:
 	return ret;
 }
 
-#define RETURN_ERR(err) do { printk(KERN_ERR PKNOCK err); return -EINVAL; } while (false)
+#define RETURN_ERR(err) do { pr_err(err); return -EINVAL; } while (false)
 
 static int pknock_mt_check(const struct xt_mtchk_param *par)
 {
@@ -1103,14 +1103,14 @@ static int __init xt_pknock_mt_init(void)
 	if (gc_expir_time < DEFAULT_GC_EXPIRATION_TIME)
 		gc_expir_time = DEFAULT_GC_EXPIRATION_TIME;
 	if (request_module(crypto.algo) < 0) {
-		printk(KERN_ERR PKNOCK "request_module('%s') error.\n",
+		pr_err("request_module('%s') error.\n",
                         crypto.algo);
 		return -ENXIO;
 	}
 
 	crypto.tfm = crypto_alloc_shash(crypto.algo, 0, 0);
 	if (IS_ERR(crypto.tfm)) {
-		printk(KERN_ERR PKNOCK "failed to load transform for %s\n",
+		pr_err("failed to load transform for %s\n",
 						crypto.algo);
 		return PTR_ERR(crypto.tfm);
 	}
@@ -1120,7 +1120,7 @@ static int __init xt_pknock_mt_init(void)
 
 	pde = proc_mkdir("xt_pknock", init_net.proc_net);
 	if (pde == NULL) {
-		printk(KERN_ERR PKNOCK "proc_mkdir() error in _init().\n");
+		pr_err("proc_mkdir() error in _init().\n");
 		return -ENXIO;
 	}
 	return xt_register_match(&xt_pknock_mt_reg);
-- 
2.28.0

[PATCH xtables-addons v2 13/13] pknock: xt_pknock: remove DEBUG definition and disable debug output.

[Jeremy Sowden]

The DEBUG definition in xt_pknock.h causes a compiler warning if one
adds a DEBUG define to xt_pknock.c to enable pr_debug.  Since it only
controls some debugging output in libxt_pknock.c, it would make sense to
move the definition there, but let's just disable the debugging instead.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 extensions/pknock/libxt_pknock.c | 4 ++--
 extensions/pknock/xt_pknock.h    | 2 --
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/extensions/pknock/libxt_pknock.c b/extensions/pknock/libxt_pknock.c
index 4852e9f25a9e..1cd829333a1d 100644
--- a/extensions/pknock/libxt_pknock.c
+++ b/extensions/pknock/libxt_pknock.c
@@ -123,7 +123,7 @@ __pknock_parse(int c, char **argv, int invert, unsigned int *flags,
 		info->ports_count = parse_ports(optarg, info->port, proto);
 		info->option |= XT_PKNOCK_KNOCKPORT;
 		*flags |= XT_PKNOCK_KNOCKPORT;
-#if DEBUG
+#ifdef DEBUG
 		printf("ports_count: %d\n", info->ports_count);
 #endif
 		break;
@@ -162,7 +162,7 @@ __pknock_parse(int c, char **argv, int invert, unsigned int *flags,
 		info->rule_name_len = strlen(info->rule_name);
 		info->option |= XT_PKNOCK_NAME;
 		*flags |= XT_PKNOCK_NAME;
-#if DEBUG
+#ifdef DEBUG
 		printf("info->rule_name: %s\n", info->rule_name);
 #endif
 		break;
diff --git a/extensions/pknock/xt_pknock.h b/extensions/pknock/xt_pknock.h
index d44905b44e0d..fb201df49e82 100644
--- a/extensions/pknock/xt_pknock.h
+++ b/extensions/pknock/xt_pknock.h
@@ -29,8 +29,6 @@ enum {
 	XT_PKNOCK_MAX_PASSWD_LEN = 31,
 };
 
-#define DEBUG 1
-
 struct xt_pknock_mtinfo {
 	char rule_name[XT_PKNOCK_MAX_BUF_LEN+1];
 	uint32_t			rule_name_len;
-- 
2.28.0

Re: [PATCH xtables-addons v2 00/13] pknlusr improvements

[Jan Engelhardt]

On Sunday 2020-10-25 14:15, Jeremy Sowden wrote:

>Since pknlusr is now installed, here are some improvements.  There's one
>automake change related to the new man-page, a number of new patches
>tidying up the source of pknlusr.c, before a new version of the patch to
>remove the hard-coded group ID.  The last four patches do a bit of
>tiding of the pknock kernel module.

So applied. I removed the periods from the summaries, 
and then some whitespace.

>Jeremy Sowden (13):
>  pknock: pknlusr: ensure man-page is included by `make dist`.
>  pknock: pknlusr: remove dest_addr and rename src_addr.
>  pknock: pknlusr: tighten up variable scopes.
>  pknock: pknlusr: tidy up initialization of local address.
>  pknock: pknlusr: use NLMSG macros and proper types, rather than
>    arithmetic on char pointers.
>  pknock: pknlusr: use macro to define inet_ntop buffer size.
>  pknock: pknlusr: don't treat recv return value of zero as an error.
>  pknock: pknlusr: always close socket.
>  pknock: pknlusr: fix hard-coded netlink multicast group ID.
>  pknock: xt_pknock: use IS_ENABLED.
>  pknock: xt_pknock: use kzalloc.
>  pknock: xt_pknock: use pr_err.
>  pknock: xt_pknock: remove DEBUG definition.