Re: [PATCH] extensions: add libxt_bpf extension

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Wed, Jan 09, 2013 at 07:12:07PM -0500, Willem de Bruijn wrote:
> Changes:
> - v2
>   - update to match latest kernel module (fixed size match struct)
>   - add manual page
>   - fix save/restore
>   - fix matching whole program on delete
> 
> Support filtering using Linux Socket Filters
> ---
>  extensions/libxt_bpf.c   |  199 ++++++++++++++++++++++++++++++++++++++++++++++
>  extensions/libxt_bpf.man |   17 ++++
>  2 files changed, 216 insertions(+), 0 deletions(-)
>  create mode 100644 extensions/libxt_bpf.c
>  create mode 100644 extensions/libxt_bpf.man
> 
> diff --git a/extensions/libxt_bpf.c b/extensions/libxt_bpf.c
> new file mode 100644
> index 0000000..8ad540f
> --- /dev/null
> +++ b/extensions/libxt_bpf.c
> @@ -0,0 +1,199 @@
> +/*
> + * Xtables BPF extension
> + *
> + * Written by Willem de Bruijn (willemb@google.com)
> + * Copyright Google, Inc. 2013
> + * Licensed under the GNU General Public License version 2 (GPLv2)
> +*/
> +
> +#include <linux/netfilter/xt_bpf.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <xtables.h>
> +
> +#define BCODE_FILE_MAX_LEN_B	1024

This has to be updated now that this sticks to 64 instructions per
filter.

> +
> +enum { O_BCODE_STDIN = 0,
> +       O_BCODE_FILE};

Better:

enum {
        O_...
        O_...
};

> +
> +static void bpf_help(void)
> +{
> +	printf(
> +"bpf match options:\n"
> +"--bytecode-file <file>	: a bpf program as generated by\n"
> +"  `tcpdump -i any -ddd <expr> > file\n"
> +"--bytecode <program>	: a bpf program as generated by\n"
> +"  `tcpdump -i any -ddd <expr> | tr '\\n' ','`\n");
> +}
> +
> +static const struct xt_option_entry bpf_opts[] = {
> +	{.name = "bytecode", .id = O_BCODE_STDIN, .type = XTTYPE_STRING},
> +	{.name = "bytecode-file", .id = O_BCODE_FILE, .type = XTTYPE_STRING},
> +	XTOPT_TABLEEND,
> +};
> +
> +static void bpf_parse_string(struct xt_option_call *cb, const char *bpf_program,
> +			     const char separator)
> +{
> +	struct xt_bpf_info *bi = (void *) cb->data;
> +	const char *token;
> +	char sp;
> +	int i;
> +
> +	/* parse head: length. */
> +	if (sscanf(bpf_program, "%hu%c", &bi->bpf_program_num_elem, &sp) != 2 ||
> +		   sp != separator)
> +		xtables_error(PARAMETER_PROBLEM,
> +			      "bpf: error parsing program length");
> +	if (!bi->bpf_program_num_elem)
> +		xtables_error(PARAMETER_PROBLEM,
> +			      "bpf: illegal zero length program");
> +	if (bi->bpf_program_num_elem > XT_BPF_MAX_NUM_INSTR)
> +		xtables_error(PARAMETER_PROBLEM,
> +			      "bpf: number of instructions exceeds maximum");
> +
> +	/* parse instructions. */
> +	i = 0;
> +	token = bpf_program;
> +	while ((token = strchr(token, separator)) && (++token)[0]) {
> +		if (i >= bi->bpf_program_num_elem) {
> +			xtables_error(PARAMETER_PROBLEM,
> +				      "bpf: program length: parameter");

better error reporting?

> +			return;

That return is superfluous. xtables_error calls exit.

> +		}
> +		if (sscanf(token, "%hu %hhu %hhu %u,",
> +			   &bi->bpf_program[i].code,
> +			   &bi->bpf_program[i].jt,
> +			   &bi->bpf_program[i].jf,
> +			   &bi->bpf_program[i].k) != 4) {
> +			xtables_error(PARAMETER_PROBLEM,
> +				      "bpf: error at instr %d", i);
> +			return;

Same thing.

> +		}
> +		i++;
> +	}
> +
> +	if (i != bi->bpf_program_num_elem) {
> +		xtables_error(PARAMETER_PROBLEM,
> +			      "bpf: program length: parsing");

Better error reporting?

> +		return;

Same thing.

> +	}
> +}
> +
> +static void bpf_parse_file(struct xt_option_call *cb, const char *filepath)
> +{
> +	struct stat stats;
> +	char *contents;
> +	int fd, len = 0, ret;
> +
> +	if (access(filepath, F_OK))
> +		xtables_error(PARAMETER_PROBLEM, "bpf: no such file");
> +	if (access(filepath, R_OK))
> +		xtables_error(PARAMETER_PROBLEM, "bpf: no read permissions");

Remove these above so...

> +
> +	fd = open(filepath, O_RDONLY);
> +	if (fd == -1)
> +		xtables_error(PARAMETER_PROBLEM, "bpf: error opening file");

you can use strerror here.

> +	if (fstat(fd, &stats))
> +		xtables_error(PARAMETER_PROBLEM, "bpf: error reading metadata");
> +	if (stats.st_size >= BCODE_FILE_MAX_LEN_B)
> +		xtables_error(PARAMETER_PROBLEM, "bpf: file too large");
> +
> +	contents = malloc(stats.st_size + 1);

Better a buffer allocated in the stack now that we have 64
instructions per filter?

Or use fread to get each line and put it into the filter? so reading
the filter and parsing it happens all at once.

> +	if (!contents)
> +		xtables_error(PARAMETER_PROBLEM, "bpf: allocation failed");
> +	contents[stats.st_size] = 0;
> +
> +	while (len < stats.st_size) {
> +		ret = read(fd, contents + len, stats.st_size - len);
> +		if (ret == -1) {
> +			if (errno == EAGAIN)
> +				continue;
> +			xtables_error(PARAMETER_PROBLEM, "bpf: read failed");
> +		}
> +		if (!ret)
> +			xtables_error(PARAMETER_PROBLEM, "bpf: unexpected EOF");
> +		len += ret;
> +	}
> +
> +	bpf_parse_string(cb, contents, '\n');
> +
> +	free(contents);
> +	close(fd);
> +}
> +
> +static void bpf_parse(struct xt_option_call *cb)
> +{
> +	xtables_option_parse(cb);
> +	switch (cb->entry->id) {
> +	case O_BCODE_STDIN:
> +		bpf_parse_string(cb, cb->arg, ',');
> +		break;
> +	case O_BCODE_FILE:
> +		bpf_parse_file(cb, cb->arg);
> +		break;
> +	default:
> +		xtables_error(PARAMETER_PROBLEM, "bpf: unknown option");
> +	}
> +}
> +
> +static void bpf_print_code(const void *ip, const struct xt_entry_match *match)
> +{
> +	const struct xt_bpf_info *info = (void *) match->data;
> +	int i;
> +
> +	for (i = 0; i < info->bpf_program_num_elem; i++)
> +		printf("%hu %hhu %hhu %u,", info->bpf_program[i].code,
> +					    info->bpf_program[i].jt,
> +					    info->bpf_program[i].jf,
> +					    info->bpf_program[i].k);
> +}
> +
> +static void bpf_save(const void *ip, const struct xt_entry_match *match)
> +{
> +	const struct xt_bpf_info *info = (void *) match->data;
> +
> +	printf(" --bytecode \"%hu,", info->bpf_program_num_elem);
> +	bpf_print_code(ip, match);
> +	printf("\"");
> +}
> +
> +static void bpf_fcheck(struct xt_fcheck_call *cb)
> +{
> +	if (((!!(cb->xflags & (1 << O_BCODE_STDIN))) ^
> +	     (!!(cb->xflags & (1 << O_BCODE_FILE)))) == 0)
> +		xtables_error(PARAMETER_PROBLEM,
> +			      "bpf: pass bytecode or bytecode-file, not both");
> +}
> +
> +static void bpf_print(const void *ip, const struct xt_entry_match *match,
> +		      int numeric)
> +{
> +	printf("match bpf ");
> +	return bpf_print_code(ip, match);
> +}
> +
> +static struct xtables_match bpf_match = {
> +	.family		= NFPROTO_UNSPEC,
> +	.name		= "bpf",
> +	.version	= XTABLES_VERSION,
> +	.size		= XT_ALIGN(sizeof(struct xt_bpf_info)),
> +	.userspacesize	= XT_ALIGN(sizeof(struct xt_bpf_info) - sizeof(void *)),

Use offsetof instead, please.

> +	.help		= bpf_help,
> +	.print		= bpf_print,
> +	.save		= bpf_save,
> +	.x6_parse	= bpf_parse,
> +	.x6_fcheck	= bpf_fcheck,
> +	.x6_options	= bpf_opts,
> +};
> +
> +void _init(void)
> +{
> +	xtables_register_match(&bpf_match);
> +}
> diff --git a/extensions/libxt_bpf.man b/extensions/libxt_bpf.man
> new file mode 100644
> index 0000000..359684c
> --- /dev/null
> +++ b/extensions/libxt_bpf.man
> @@ -0,0 +1,17 @@
> +.TP
> +Match using Linux Socket Filter. Expects a BPF program in decimal format for a device without link layer. This is the format generated by \fBtcpdump \-ddd \-i $DEV\fP, if $DEV is of type DLT_RAW.
> +.TP
> +\fB\-\-bytecode\-file\fP \fIfilename\fP
> +Pass the program stored in a file.
> +For example:
> +.IP
> +iptables \-A OUTPUT \-m bpf \-\-bytecode-file program.bpf \-j ACCEPT

It's a good idea to describe the format of the file here.

> +.TP
> +\fB\-\-bytecode\fP \fIcode\fP
> +Pass the program on the command line. In this case all end-lines
> +must have been converted to commas (for instance using `tr '\\n' ','`).
> +For example:
> +.IP
> +iptables \-A OUTPUT \-m bpf \-\-bytecode
> +         '6,40 0 0 14, 21 0 3 2048,48 0 0 25,21 0 1 20,6 0 0 96,6 0 0 0'

Please, a filter that actually works ;-)

> +         \-j ACCEPT
> -- 
> 1.7.7.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html