Wireshark · Wireshark-dev: Re: [Wireshark-dev] dissect_per_constrained_integer() with

[Pascal Quantin <pascal.quantin@xxxxxxxxx>]

Hi Pavel,

2016-12-22 17:20 GMT+01:00 Pavel Strnad <strnadp@xxxxxxxxxx>:

Hi Pascal,
Thank You for fast response and clear comments.

Would it make sense to implement extra dissect_per_semi_constrained_integer?
I can try to implement dissect_per_semi_constrained_integer (tvbuff_t *tvb,
guint32 offset, asn1_ctx_t *actx, proto_tree *tree, int hf_index, guint32
min, guint32 max, guint32 *value, gboolean has_extension, gboolean
lb_infinite).

Extra parameter will allow us to distinguish both cases where either
min(lb_infinite=TRUE) or max(lb_infinite=FALSE) is set to NO_BOUND?
I can do the same for 64b version.
Afterwards I will need to update asn2wrs accordingly to have this issue
fixed.

This will not sort out the case of direct callers to
dissect_per_constrained_integer.

That's my main concern. I have no reviewed carefully callers to see if min or max could be unbounded or not, so I'm not sure what is the easiest: change the existing function signature with 2 extra booleans, or review all code paths to put where needed the new one.

Worse, I realize that I forgot to talk about ASN.1 BER encoding (as asn2wrs also supports it). I know nothing about this encoding format but it also uses the NO_BOUND magic define... We must ensure that we do not break it while modifying asn2wrs (or maybe fix BER decoder if needs be ? :) ).

Pascal.

What do You think?

Best Regards,
Pavel

-----------------------------------------------------------------------
Date: Wed, 21 Dec 2016 16:07:02 +0100
From: Pascal Quantin <pascal.quantin@xxxxxxxxx>
To: Developer support list for Wireshark <wireshark-dev@xxxxxxxxxxxxx>
Subject: Re: [Wireshark-dev] dissect_per_constrained_integer() with
        no_bound (MAX in ASN.1)
Message-ID:
        <CAGka-82MWxYv2aj0WJRL_p9ooEmBRpo7_sAH7KTpHc6C0vCFYA@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"

Hi Pavel,

2016-12-21 15:37 GMT+01:00 Pavel Strnad <strnadp@xxxxxxxxxx>:

> Hello,
>
> I am trying to understand the difference in usage of NO_BOUND or
> UINT_MAX in the place of max parameter in
> dissect_per_constrained_integer() function. In my case aligned PER
variant.
>
>
>
> From packet-per.h:
>
> #define NO_BOUND -1
>
> guint32 dissect_per_constrained_integer(tvbuff_t *tvb, guint32 offset,
> asn1_ctx_t *actx, proto_tree *tree, int hf_index, guint32 min, guint32
> max,
> guint32 *value, gboolean has_extension);
>
>
>
> Based on that it looks like that there is no different dissection of
> following two asn.1 definitions?
>

Correct, semi-constrained integer does not seem to be managed properly.
This matches the comment found in line 1283.

> 1)      seconds     INTEGER (0..4294967295)
>
> 2)      seconds     INTEGER (0..MAX) where MAX translates to
> NO_BOUND=4294967295 using asn2wrs
>
>
>
> Reading X.691 (aligned PER) wireshark seems to dissect well the 1st
> case that is using size constraint but not the 2nd case
>
> where semi-constraint size is used and the length determinant should
> include padding bits in the case of aligned PER.
>
>
>
> I would like to try to fix it myself but will need some hint how to
> differentiate these two cases and keep API unchanged?
>

IMHO you can't without changing the API. Maybe we would need to extra
booleans indicating whether the min and / or max values are bounded or not,
and adapt asn2wrs generator accordingly. This way we could drop the NO_BOUND
define

Note that this sounds like a non trivial project because:
- as indicated in the comments, dissect_per_constrained_integer only handle
32 bits integers and dissect_per_constrained_integer_64b only handles 64
bits integers, so both needs to be adapted
- MIN or MAX parameters do not seem properly handled in other types also,
and any change done in dissect_per_constrained_integer(_64b) needs to be
reflected in the caller functions and any other making use of NO_BOUND

Regards,
Pascal.

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request@wireshark.org?subject=unsubscribe