Re: [PATCH 3/3 nft v2] src: osf: import nfnl_osf.c to load osf fingerprints

Fernando Fernandez Mancera <ffmancera@xxxxxxxxxx> · Wed, 15 Aug 2018 18:22:14 +0200

On 8/14/18 4:10 PM, Pablo Neira Ayuso wrote:
On Mon, Aug 13, 2018 at 06:57:08PM +0200, Fernando Fernandez Mancera wrote:
[...]
+static void uloga(const char *f, struct netlink_ctx *ctx, ...)
+{
+	if (!(ctx->debug_mask & NFT_DEBUG_NETLINK))
+		return;
+
+	nft_print(ctx->octx, "%s", f);
+}
+ >> +static char *nf_osf_strchr(char *ptr, char c)
+{
+	char *tmp;
+
+	tmp = strchr(ptr, c);
+	if (tmp)
+		*tmp = '\0';
+
+	while (tmp && tmp + 1 && isspace(*(tmp + 1)))
+		tmp++;
+
+	return tmp;
+}
[...]
+static int osf_load_line(char *buffer, int len, int del, struct mnl_socket *nl,
+			 struct netlink_ctx *ctx)
+{
+	int i, cnt = 0;
+	char obuf[MAXOPTSTRLEN];
+	struct nf_osf_user_finger f;
+	char *pbeg, *pend;
+	struct nlmsghdr *nlh;
+	struct nfgenmsg *nfg;
+	char buf[MNL_SOCKET_BUFFER_SIZE];
+
+	memset(&f, 0, sizeof(struct nf_osf_user_finger));
+
+	uloga("Loading '%s'.\n", ctx, buffer);

I think we should remove uloga() and replace every "uloga(..)" with:

		if (ctx->debug_mask & NFT_DEBUG_NETLINK)
			nft_print(ctx->octx, "%s", f);

because using uloga and nft_print at the same time we cannot use strings 
with format specifiers i.e: in this case nft prints "Loading '%s'." so 
it is not using the argument given as a "%s".

I am going to replace it all if no objections.

+
+	for (i = 0; i < len && buffer[i] != '\0'; ++i) {
+		if (buffer[i] == ':')
+			cnt++;
+	}
+
+	if (cnt != 8) {
+		uloga("Wrong input line '%s': cnt: %d, must be 8, i: %d, must be %d.\n", ctx, buffer, cnt, i, len);
+		return -EINVAL;
+	}
+
+	memset(obuf, 0, sizeof(obuf));
+
+	pbeg = buffer;
+	pend = nf_osf_strchr(pbeg, OSFPDEL);
+	if (pend) {
+		*pend = '\0';
+		if (pbeg[0] == 'S') {
+			f.wss.wc = OSF_WSS_MSS;
+			if (pbeg[1] == '%')
+				f.wss.val = strtoul(&pbeg[2], NULL, 10);
+			else if (pbeg[1] == '*')
+				f.wss.val = 0;
+			else
+				f.wss.val = strtoul(&pbeg[1], NULL, 10);
+		} else if (pbeg[0] == 'T') {
+			f.wss.wc = OSF_WSS_MTU;
+			if (pbeg[1] == '%')
+				f.wss.val = strtoul(&pbeg[2], NULL, 10);
+			else if (pbeg[1] == '*')
+				f.wss.val = 0;
+			else
+				f.wss.val = strtoul(&pbeg[1], NULL, 10);
+		} else if (pbeg[0] == '%') {
+			f.wss.wc = OSF_WSS_MODULO;
+			f.wss.val = strtoul(&pbeg[1], NULL, 10);
+		} else if (isdigit(pbeg[0])) {
+			f.wss.wc = OSF_WSS_PLAIN;
+			f.wss.val = strtoul(&pbeg[0], NULL, 10);
+		}
+
+		pbeg = pend + 1;
+	}
+	pend = nf_osf_strchr(pbeg, OSFPDEL);
+	if (pend) {
+		*pend = '\0';
+		f.ttl = strtoul(pbeg, NULL, 10);
+		pbeg = pend + 1;
+	}
+	pend = nf_osf_strchr(pbeg, OSFPDEL);
+	if (pend) {
+		*pend = '\0';
+		f.df = strtoul(pbeg, NULL, 10);
+		pbeg = pend + 1;
+	}
+	pend = nf_osf_strchr(pbeg, OSFPDEL);
+	if (pend) {
+		*pend = '\0';
+		f.ss = strtoul(pbeg, NULL, 10);
+		pbeg = pend + 1;
+	}
+
+	pend = nf_osf_strchr(pbeg, OSFPDEL);
+	if (pend) {
+		*pend = '\0';
+		cnt = snprintf(obuf, sizeof(obuf), "%s,", pbeg);
+		pbeg = pend + 1;
+	}
+
+	pend = nf_osf_strchr(pbeg, OSFPDEL);
+	if (pend) {
+		*pend = '\0';
+		if (pbeg[0] == '@' || pbeg[0] == '*')
+			cnt = snprintf(f.genre, sizeof(f.genre), "%s", pbeg + 1);
+		else
+			cnt = snprintf(f.genre, sizeof(f.genre), "%s", pbeg);
+		pbeg = pend + 1;
+	}
+
+	pend = nf_osf_strchr(pbeg, OSFPDEL);
+	if (pend) {
+		*pend = '\0';
+		cnt = snprintf(f.version, sizeof(f.version), "%s", pbeg);
+		pbeg = pend + 1;
+	}
+
+	pend = nf_osf_strchr(pbeg, OSFPDEL);
+	if (pend) {
+		*pend = '\0';
+		cnt =
+		    snprintf(f.subtype, sizeof(f.subtype), "%s", pbeg);
+		pbeg = pend + 1;
+	}
+
+	nf_osf_parse_opt(f.opt, &f.opt_num, obuf, sizeof(obuf));
+
+	memset(buf, 0, sizeof(buf));
+
+	if (del) {
+		nlh = nftnl_nlmsg_build_hdr(buf, (NFNL_SUBSYS_OSF << 8) |
+					    OSF_MSG_REMOVE, AF_UNSPEC,
+					    NLM_F_REQUEST | NLM_F_ACK,
+					    ctx->seqnum);
+
+		nfg = mnl_nlmsg_put_extra_header(nlh, sizeof(*nfg));
+		nfg->nfgen_family = AF_UNSPEC;
+		nfg->version = NFNETLINK_V0;
+		nfg->res_id = 0;
+	} else {
+		nlh = nftnl_nlmsg_build_hdr(buf, (NFNL_SUBSYS_OSF << 8) |
+					    OSF_MSG_ADD, AF_UNSPEC,
+					    NLM_F_REQUEST | NLM_F_CREATE |
+					    NLM_F_ACK, ctx->seqnum);
+
+		nfg = mnl_nlmsg_put_extra_header(nlh, sizeof(*nfg));
+		nfg->nfgen_family = AF_UNSPEC;
+		nfg->version = NFNETLINK_V0;
+		nfg->res_id = 0;
+
+		mnl_attr_put(nlh, OSF_ATTR_FINGER, sizeof(struct nf_osf_user_finger), &f);
+	}
+
+	return nft_mnl_talk(ctx, nlh, nlh->nlmsg_len, 0, NULL);
+}
+
+static int osf_load_entries(int del, struct mnl_socket *nl,
+			    struct netlink_ctx *ctx)
+{
+	FILE *inf;
+	int err = 0;
+	char buf[1024];
+
+	inf = fopen(OS_SIGNATURES, "r");
+	if (!inf) {
+		uloga("Failed to open file '%s'", ctx, OS_SIGNATURES);
+		return -1;
+	}
+
+	while(fgets(buf, sizeof(buf), inf)) {
+		int len;
+
+		if (buf[0] == '#' || buf[0] == '\n' || buf[0] == '\r')
+			continue;
+
+		len = strlen(buf) - 1;
+
+		if (len <= 0)
+			continue;
+
+		buf[len] = '\0';
+
+		err = osf_load_line(buf, len, del, nl, ctx);
+		if (err)
+			break;
+
+		memset(buf, 0, sizeof(buf));
+	}
+
+	fclose(inf);
+	return err;
+}
+
+int nfnl_osf_load_fingerprints(struct netlink_ctx *ctx, int del)
+{
+	int err;
+	struct mnl_socket *nl;
+
+	nl = mnl_socket_open(NETLINK_NETFILTER);
+	if (nl == NULL) {
+		err = -EINVAL;
+		uloga("Failed to open mnl socket", ctx);
+		goto err_out_exit;
+	}
+
+	if (mnl_socket_bind(nl, 0, MNL_SOCKET_AUTOPID) < 0) {
+		err = -EINVAL;
+		uloga("Failed to bind mnl socket", ctx);
+		goto err_out_exit;
+	}
+
+#ifndef NFNL_SUBSYS_OSF
+#define NFNL_SUBSYS_OSF	5
+#endif

Remove this #ifdef, not needed. NFNL_SUBSYS_OSF is already defined in
nfnetlink_osf.h

+	err = osf_load_entries(del, nl, ctx);
+	if (err < 0)
+		goto err_out_close;
+
+	return 0;
+
+err_out_close:
+	mnl_socket_close(nl);
+err_out_exit:
+	return err;
+}

diff --git a/src/osf.c b/src/osf.c
index 131d54e..210bfbe 100644
--- a/src/osf.c
+++ b/src/osf.c
@@ -3,6 +3,7 @@
  #include <utils.h>
  #include <string.h>
  #include <osf.h>
+#include <nfnl_osf.h>
  
  static void osf_expr_print(const struct expr *expr, struct output_ctx *octx)
  {
@@ -26,6 +27,7 @@ struct expr *osf_expr_alloc(const struct location *loc)
  	const struct datatype *type = &string_type;
  	struct expr *expr;
  
+	osf_init = true;
  	expr = expr_alloc(loc, &osf_expr_ops, type,
  			  BYTEORDER_HOST_ENDIAN, len);
  
diff --git a/src/rule.c b/src/rule.c
index 7a7ac73..7421ffc 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -22,6 +22,7 @@
  #include <netdb.h>
  #include <netlink.h>
  #include <json.h>
+#include <nfnl_osf.h>
  
  #include <libnftnl/common.h>
  #include <libnftnl/ruleset.h>
@@ -2135,6 +2136,9 @@ int do_command(struct netlink_ctx *ctx, struct cmd *cmd)
  	default:
  		BUG("invalid command object type %u\n", cmd->obj);
  	}
+
+	if (osf_init)
+		nfnl_osf_load_fingerprints(ctx, 0);

You have to update do_command() to replace all:

         case blah:
                 return ...;

to
         case blah:
                 err = ...;
                 break;

Otherwise this code is never exercised.

When testing this, make sure nfnl_osf_load_fingerprints() is called.

You can use:

         nft --debug=netlink add rule x y osf name "linux"

to see if you trigger uloga() calls for debugging so you make sure you
properly test your patches.

Thanks.







