On 12. Juli 2014 14:10:12 GMT+01:00, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
>Hi Alvaro,
>
>On Fri, Jul 11, 2014 at 01:09:34PM +0200, Alvaro Neira Ayuso wrote:
>> From: Álvaro Neira Ayuso <alvaroneay@xxxxxxxxx>
>>
>> In all case that we have added a payload dependency, we have supposed
>> that the byteorder must to be BYTEORDER_HOST_ENDIAN, the problem is
>> when we want to add a dependency that the value has another
>byteorder.
>> For example, if we try to add a new payload dependency in a bridge
>table
>> and we use ether type, the byteorder is BYTEORDER_BIG_ENDIAN. The
>value
>> of the type ip is 0x0800 in ether but when we add the payload
>dependency
>> for this specific protocol, we will have a payload like this:
>>
>> [ payload load 2b @ link header + 12 => reg 1 ]
>> [ cmp eq reg 1 0x00000008 ]
>>
>> This patch allows to create payload dependency with the byteorder of
>the
>> template. For that I have updated the function for updating the
>context for
>> using the byteorder of the template too. With this changes we have a
>payload
>> with the correct format:
>>
>> [ payload load 2b @ link header + 12 => reg 1 ]
>> [ cmp eq reg 1 0x00000800 ]
>>
>> Signed-off-by: Alvaro Neira Ayuso <alvaroneay@xxxxxxxxx>
>> ---
>> [tested with the rules]
>> nft add rule filter input ip protocol tcp counter
>> nft add rule filter input ip protocol udp counter
>> nft add rule filter input tcp dport 22 counter
>>
>> src/payload.c | 9 +++++++--
>> src/proto.c | 8 ++++++++
>> 2 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/payload.c b/src/payload.c
>> index a1785a5..fb78ba5 100644
>> --- a/src/payload.c
>> +++ b/src/payload.c
>> @@ -69,13 +69,18 @@ static void payload_expr_pctx_update(struct
>proto_ctx *ctx,
>> {
>> const struct expr *left = expr->left, *right = expr->right;
>> const struct proto_desc *base, *desc;
>> + const struct proto_hdr_template *tmpl;
>> + uint32_t value = 0;
>>
>> if (!(left->flags & EXPR_F_PROTOCOL))
>> return;
>>
>> assert(expr->op == OP_EQ);
>> base = ctx->protocol[left->payload.base].desc;
>> - desc = proto_find_upper(base, mpz_get_uint32(right->value));
>> + tmpl = &base->templates[base->protocol_key];
>
>Note here, you are fetching the template based on the protocol key:
>
>struct proto_desc {
> const char *name;
> enum proto_bases base;
> unsigned int protocol_key;
> struct {
> unsigned int num;
> const struct proto_desc *desc;
> } protocols[PROTO_UPPER_MAX];
> struct proto_hdr_template templates[PROTO_HDRS_MAX]; <---
>};
>
>However, the length of the templates array is:
>
>#define PROTO_HDRS_MAX 20
>
>and now see below:
>
>> + mpz_export_data(&value, right->value, tmpl->dtype->byteorder,
>> + div_round_up(tmpl->len, BITS_PER_BYTE));
>> + desc = proto_find_upper(base, value);
>>
>> proto_ctx_update(ctx, left->payload.base + 1, &expr->location,
>desc);
>> }
>> @@ -208,7 +213,7 @@ int payload_gen_dependency(struct eval_ctx *ctx,
>const struct expr *expr,
>> left = payload_expr_alloc(&expr->location, desc,
>desc->protocol_key);
>>
>> right = constant_expr_alloc(&expr->location, tmpl->dtype,
>> - BYTEORDER_HOST_ENDIAN,
>> + tmpl->dtype->byteorder,
>> tmpl->len,
>> constant_data_ptr(protocol, tmpl->len));
>>
>> diff --git a/src/proto.c b/src/proto.c
>> index 546ef10..4192108 100644
>> --- a/src/proto.c
>> +++ b/src/proto.c
>> @@ -249,6 +249,7 @@ const struct proto_desc proto_ah = {
>> const struct proto_desc proto_esp = {
>> .name = "esp",
>> .base = PROTO_BASE_TRANSPORT_HDR,
>> + .protocol_key = IPPROTO_ESP,
>
>IPPROTO_ESP is 50, this won't fly since it's over the array boundary,
>so you will be accessing out of boundary memory area.
>
>I think the protocol_key needs to be an internal value defined in
>include/proto.h, for example:
The protocol *key* specifies the header field contaibing the next layer protocol value, not the protocol number, so using it this way is not correct.
The easy fix would be to always use big endian byteorder, as all other cases have 1 byte keys. I'm actually wondering though, the ethertype field is defined using big endian, so the constant should be converted to the correct byte order during evaluation. Unfortunately my notebook broke so I can't check the code right now, but my feeling is that this is where the bug is actually located.
>
>enum transport_protocol {
> L4PROTO_INVALID,
> L4PROTO_TCP,
> L4PROTO_UDP,
> ...
>};
>
>that you should use instead.
--
Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet.
--
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
