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.

Reply via email to