mirleft / ocaml-asn1-combinators

Embed typed ASN.1 grammars in OCaml
ISC License
35 stars 20 forks source link

Unexpected behavior of bit_string_flags #25

Closed wiml closed 4 years ago

wiml commented 5 years ago

The bit_string_flags combinator has unexpected and inconvenient behavior when reading a bit string containing a bit that is not given a corresponding value in xs.

What it does right now looks like unintentional behavior: depending on the position of the unexpected bit, it might ignore it (not unreasonable), or it might insert a spurious copy of whatever the first value in the xs list was (definitely wrong).

As a user there are a few behaviors I might find useful, in different situations:

  1. Ignore unknown bits
  2. Fail the parse
  3. Return a special value, e.g. UnexpectedFlagBit 12

None of those is appropriate in all situations though. Perhaps bit_string_flags could gain an optional parameter, e.g., ?unknown:(int -> (a, Asn.error) result) which is called to handle unexpectedly set bits, with the default value if unspecified being to completely ignore them? Or perhaps ?unknown:(int -> a option) if the default behavior should be a parse failure.

pqwy commented 5 years ago

Yeah, this is totally wrong. I think it should fail -- more complex logic can be achieved with a custom combinator.

But I wonder how often people have encodings with dead bits? Maybe converting it into bit_string_flags: 'a list -> 'a list asn makes more sense, where the input is taken to map to consecutive bits starting at position 0..?

wiml commented 5 years ago

I noticed it because my application, processing Kerberos messages, has lots of dead bits — Kerberos' flag fields have lots of gaps, including bit 0, which seems to be Reserved in every flag field. Kerberos may be unusual though.

It doesn't seem unreasonable for bit_string_flags to require a gap-less list of bits but I don't think that doing so would simplify the API: it's always possible for the sender to send a bit that isn't contained in the list (since DER can represent an arbitrarily wide bitstring), so it still has to have some behavior when that happens.

pqwy commented 5 years ago

Ughhhh.

   KerberosFlags   ::= BIT STRING (SIZE (32..MAX))
                       -- minimum number of bits shall be sent,
                       -- but no fewer than 32

So there's at least one reasonable use-case where unconditional error does not apply...

pqwy commented 5 years ago

The Oracle has spoken.

X.680:

22.6 The presence of a "NamedBitList" has no effect on the set of abstract values of this type. Values containing 1 bits other than the named bits are permitted. 1 bits other than the named bits are permitted.

Named bits are to be understood simply as a shorthand for bit positions, and bit_string_flags is now a shorthand for plucking out the named positions, ignoring the ones that are not named.

hannesm commented 4 years ago

thanks, this has been fixed and is part of the 0.2.1 release. closing