[Drip] Re: [Cbor] Request for CBOR/CDDL review

[Daniel Migault <[e-mail]>]

Thanks Christian for the feedback. This is very well appreciated.

Cheers!
Daniel

On Wed, Oct 23, 2024 at 5:36 PM Christian Amsüss <[e-mail]>
wrote:

> Hello Adam,
>
> thanks to bringing this to this group's attention; I think I can provide
> some feedback here:
>
> * A.1:
>   - The CDDL is serialized in an unconventional way -- .size is a
>     control, so the usual form to write would be
>
>     tstr .size 15
>
>     as opposed to the form with the parentheses. The form used might not
>     be wrong (I don't know the grammar well enough to say that), but if
>     nothing else it is unusual enough to trip up readers.
>
>     (Also applies to B.1)
>
>   - Just a remark: uint .size 2 just means any uint in from 0 to 65535,
>     inclusive. It has no impact on serialization (which will use 1, 2 or
>     3 octets depending on the value). That is probably what you want, as
>     it is the usual behavior of CBOR, let's just be sure you don't
>     expect it to always be encoded in 16 bits. Rephrasing the HHIT
>     Entity Type description of A.3 may help avoid that impression,
>     maybe "This field is a number with values defined in".
>
>     Taking that into further context, is there a reason this is limited
>     to 16 bits, as the type registry is? If those are only ever to be
>     used in CBOR, CBOR readily supports integers up to 64 bit, so this
>     might just as well be left open. Unless there is a mandate that the
>     CBOR is expressed in preferred encoding or even CDE (deterministic
>     encoding), receivers should be ready to receive values in encodings
>     up to 64 bit anyway.
>
>   - For the #6.TBD you mentioned to be waiting for a draft, I'm not sure
>     which draft you are referring to, but you can ask the authors and
>     chairs of that document to do an early allocation -- or you use the
>     mechanism proposed in draft-bormann-cbor-draft-numbers.
>
> * A.2:
>
>   - The text in figure 4 is not correct diagnostic notation: The base64
>     value should be b64'WQGC...uAo=' (b64 prefix, single quotes, three
>     dots for ellipsis, and the padding is not required)
>
>   - Appendix G of 8610 is not a formal definition; we have one in the
>     pipeline as draft-ietf-cbor-edn-literals (which is past WGLC,
>     although the outcome of IESG processing may be to be sent back to
>     the WG for confirmation of some resolution, we'll know in Dublin).
>     This also has an ABNF grammar.
>
>     Note that diagnostic notation (neither as specified nor in the
>     upcoming document) is not intended as an interchange format. To my
>     understanding, the presentation format of DNS records is merely a
>     convention shared between DNS servers, and not an interchange
>     format. If that is the case, the use of EDN is certainly fine -- I
>     just find the "MUST be in Extended Diagnostic Notation" odd in that
>     case.
>
>     (Also applies to B.2)
>
>   - Unless IPv6 addresses become valid certificates, there are no IPv6
>     addresses in hhit-rr, so the limitation on the base64 representation
>     is odd.
>
> * A.3:
>
>   - Is Abbreviation intended to be text for human consumption? (From it
>     being a text string, and the lack of specific defined content, I'd
>     guess so). If so, with ART-ART reviewer hat on, you may want to
>     consider whether language and direction information should be
>     embeddable here. For that, CBOR has you covered: You can put
>     `abbreviation: tstr / tag38` in there (where tag38 is defined in
>     RFC9290). How the length limitation is best applied would then
>     depend on where that is coming from (which I don't see from this
>     document).
>
>     If this is not intended for human consumption, or if there are some
>     concrete conventions to be upheld (say, only characters from some
>     internationally agreed-on set), it may make sense to express those
>     constraints if they are sufficiently stable.
>
> * B in general:
>
>   - I'm certainly biased working in the area of byte shaving, but I do
>     find it odd to see string keys in here. Have you considered just
>     using numeric values for the dictionary keys?
>
>     Using the EDN extension point of application literals for the e''
>     literal type (draft-ietf-cbor-edn-e-ref), CDDL such as
>
>     bcast-rr = {
>       uas_type => nibble-field,
>       uas_ids => [+ uas-id-grp],
>       ; abbreviated
>     }
>     uas_type = 0
>     uas_ids = 1
>
>     would read in diagnostic notation:
>
>     {
>       e'uas_type': 0,
>       e'uas_ids': [4, h'012001003FFE0001056A2621D4EF572EF5'],
>       ...: ...
>     }
>
>     and be more compact in its representation.
>
> * B.2:
>
>   - The example does not match the CDDL: The rule `[+ auth-grp]` creates
>     a single array with multiple repetitions of auth-grp, so auths
>     should be `[5, b64'QYDQ...dqo', 5, b64'AYDQ...Wgs', 5,
>     b64'AYDQ...7gA']`. This saves space, and the CDDL ensures that it is
>     understood without the deep structure how the fields are grouped.
>     Same goes for uas_ids: According to the CDDL, that should just be
>     one level of array.
>
>   - There is a mismatch between the label "auth" and "auths".
>
>
> Hope this helps
> Christian
> (who should *really* read up on the state of DRIP, given that properly
> done privacy preserving transponders would be an awesome thing in so
> many more areas than drones)
>
> --
> To use raw power is to make yourself infinitely vulnerable to greater
> powers.
>   -- Bene Gesserit axiom
> --
> Tm-rid mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]
>


-- 
Daniel Migault
Ericsson