On Mon, Jan 21, 2013 at 7:19 AM, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> 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@xxxxxxxxxx)
>> + * 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.
Oh.. rereading my comments after sending the revised patch I see that
I missed this. Will fix up in the next revision. If my little
arithmetic below is correct, it should be either 1603, or a little bit
larger (this is mainly a safety against enormous files).
>> +
>> +enum { O_BCODE_STDIN = 0,
>> + O_BCODE_FILE};
>
> Better:
>
> enum {
> O_...
> O_...
> };
Done
>> +
>> +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?
Done
>> + return;
Done
> 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.
Done
>> + }
>> + i++;
>> + }
>> +
>> + if (i != bi->bpf_program_num_elem) {
>> + xtables_error(PARAMETER_PROBLEM,
>> + "bpf: program length: parsing");
>
> Better error reporting?
Done
>> + return;
>
> Same thing.
Done
>> + }
>> +}
>> +
>> +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...
Done
>
>> +
>> + fd = open(filepath, O_RDONLY);
>> + if (fd == -1)
>> + xtables_error(PARAMETER_PROBLEM, "bpf: error opening file");
>
> you can use strerror here.
Done
>
>> + 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?
The decimal notation still allows variable size, causing us to have to
allocate an upper estimate. This is 3 characters for the first line +
64 * (5 + 1 + 3 + 1 + 3 + 1 + 10 + 1) == 1603 B if I didn't mess up
somewhere.
> Or use fread to get each line and put it into the filter? so reading
> the filter and parsing it happens all at once.
That would involve duplicating some of the parsing logic from
bpf_parse_string, and the same error reports (program length mismatch,
instruction errors, etcetera). I think that parsing in only one
location is preferable (but I'm happy to change the code if you
prefer).
>> + 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.
Done.
>> + .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.
Done. Maybe a bit too verbose now. Let me know, if so.
>> +.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 ;-)
Oops! Done (and tested).
>> + \-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@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks!
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
