Subject: [Cbor] draft-ietf-cbor-network-addresses: tag validity; deterministic encoding

From: Carsten Bormann <[email-masked]>
Date: Fri, 8 Oct 2021 13:52:37 +0200

There are various admonitions scattered in draft-ietf-cbor-network-addresses about doing the right thing when encoding and decoding the tags.
We do have a concept in CBOR to handle this properly:
Tag validity (Section 5.3.2 of RFC8949).

In response to various observations about those admonitions during IESG processing, I went ahead and consolidated the actual content to a proper section about Tag Validity.

https://github.com/cbor-wg/cbor-network-address/pull/15

I also wrote:

> The tag validity rules, combined with the rules in {{Section 4.2.1 of
> -cbor}}, lead to deterministic encoding for tags 54 and 52 and require
> no further Additional Deterministic Encoding Considerations as per
> {{Section 4.2.2 of -cbor}}.

… which might come in handy when deterministic encoding is actually needed.

Comments welcome before we merge.

Grüße, Carsten

---

From: Michael Richardson <[email-masked]>
Date: Fri, 08 Oct 2021 18:38:44 -0400

Carsten Bormann <[email-masked]> wrote:
    > There are various admonitions scattered in
    > draft-ietf-cbor-network-addresses about doing the right thing when
    > encoding and decoding the tags.

    > https://github.com/cbor-wg/cbor-network-address/pull/15

I reviewed it, and merged it.

I have posted -11.

--
Michael Richardson <[email-masked]>   . o O ( IPv6 IøT consulting )
           Sandelman Software Works Inc, Ottawa and Worldwide

---

From: Brian E Carpenter <[email-masked]>
Date: Sat, 9 Oct 2021 11:40:36 +1300

On 09-Oct-21 00:52, Carsten Bormann wrote:
> In response to various observations about those admonitions during IESG 
processing, I went ahead and consolidated the actual content to a proper section about Tag Validity.

I glanced at my Python code while checking the PR and noticed this:

>> A recommendation for prefix decoder implementations is to first create
>> an array of 16 (or 4) zero bytes.

That's exactly what I don't do in my code, although I'm sure it's exactly
what I would do in C. I think 'recommendation' is a bit strong.

>> check that any unused
>> bits in the byte string are zero:

I came up with this:

    if prefix[length//8] & bytemasks[length%8][0] != prefix[length//8]:
        #extra bits in prefix, signal an error

I also came up with a corner use case - suppose all I want to convey is
a prefix length but no prefix? At the moment, I simply send an all-zeroes
prefix, but those are wasted bits. Probably, this isn't worth fixing.

   Brian

> 
> Grüße, Carsten
> 
> _______________________________________________
> CBOR mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/cbor
>

---

From: Carsten Bormann <[email-masked]>
Date: Sat, 9 Oct 2021 00:57:45 +0200

Hi Brian,

The weird thing about the current definition of the prefix is that trailing zero bytes are not sent, even if they are part of the prefix.

   As an example, ::/128 is encoded as	
 		
   54([128, h’’])

(::/128 is 16 zero bytes).

So don’t forget to zero-expand the prefix you get (which is another way to do the “recommendation”).

I’m not fluent in Python after midnight, so here’s Ruby:

   fail if prefix[-1] == “\0” # may need to check for prefix != ‘’ in your language
   l = prefix.bytesize
   fail if l > 16
   prefix << “\0”.b * (16 - l)
   [now do the trailing bit check in the document[

Grüße, Carsten

> On 9. Oct 2021, at 00:40, Brian E Carpenter <[email-masked]> wrote:
>

---

From: Carsten Bormann <[email-masked]>
Date: Sat, 9 Oct 2021 01:07:05 +0200

On 9. Oct 2021, at 00:40, Brian E Carpenter <[email-masked]> wrote:
> 
> I think 'recommendation' is a bit strong.

It’s just “A recommendation” :-)

Better text that achieves “do this if you don’t know what you are doing” without insulting the reader gladly appreciated.

Grüße, Carsten

---

From: Brian E Carpenter <[email-masked]>
Date: Sat, 9 Oct 2021 15:55:26 +1300

> Better text that achieves “do this if you don’t know what you are doing” without insulting the reader gladly appreciated.

Oh, we can certainly let this slide. Indeed, my code does expand the bytes object out to 4 or 16 bytes. The spec is fine and entirely implementable in Python3, which is what I wanted to check.

Not midnight here, just Saturday afternoon so maybe this was not the best 
use of my time, but I had to re-debug my code. It took me a while to realise that with one module using cbor.py calling another one using cbor2.py, the tag handling is slightly different. It's not good if one module thinks the type is cbor.cbor.Tag and the other thinks it's cbor.CBORTag.

   Brian

---

From: Brian E Carpenter <[email-masked]>
Date: Fri, 15 Oct 2021 17:35:02 +1300

>    [now do the trailing bit check in the document[

I was tidying up my Python code so that it would work with either the cbor or cbor2 library, and was distracted by a mysterious segfault in the Python interpreter, but only on Linux. It was a nightmare to find the offending statement because the GRASP code is threaded and gdb has no way to tell you which thread crashed.

Anyway, there was of course a user error *and* an oversight in the cbor2 library (now fixed: it wasn't reporting my user error as an exception).

So, my ugly code is in the functions build5254() and detag5254() in https://github.com/becarpenter/graspy/blob/master/pfxm4.py

   Brian

---

From: Carsten Bormann <[email-masked]>
Date: Fri, 15 Oct 2021 06:50:58 +0200

> So, my ugly code is in the functions build5254() and detag5254() in https://github.com/becarpenter/graspy/blob/master/pfxm4.py

Nice.

After line 122, there needs to be the Python equivalent of a

prefix.sub!(/\x00*\z/, ‘’)

(Why not mix floating point calculations and a regex in the same five lines of code :-)

Before line 146, you’d need to check whether the size really is `asize`.

I think the while loop in 154 needs to be before 151.

If v[0] == 128 and prefix is 16 bytes, what does prefix[v[0]//8] yield in Python?

Grüße, Carsten

---

From: Brian E Carpenter <[email-masked]>
Date: Sat, 16 Oct 2021 08:41:42 +1300

Thanks. I have to do some other things more urgently, but I will work on your comments in due course. In the context of my code, anything that gets passed to detag() was manufactured by build(), so the verification is always going to succeed. I didn't build a test harness and throw invalid tags at detag(), but of course I should have done that.

I'd actually prefer if these tags were supported directly in the underlying libraries, so that the code could be optimized and the validation could be complete. Or possibly there should be a separate library providing build and detag/verify functions in a generic way.

Regards
   Brian

---

From: Michael Richardson <[email-masked]>
Date: Sat, 16 Oct 2021 17:06:23 -0400

Brian E Carpenter <[email-masked]> wrote:
    > I'd actually prefer if these tags were supported directly in the
    > underlying libraries, so that the code could be optimized and the
    > validation could be complete. Or possibly there should be a separate
    > library providing build and detag/verify functions in a generic way.

yeah, so...
      cbor2sockaddr()

But, we didn't do port numbers or protocols, so sockaddr is wrong.
I actually found this annoying in my Rust code, because Sockaddr is the best
abstraction for v4/v6 addresses with scopes around, but without a service and
port number, you can't make one.