Subject: [Cbor] Benjamin Kaduk's No Objection on draft-ietf-cbor-network-addresses-09: (with COMMENT)

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Mon, 04 Oct 2021 11:38:22 -0700

Benjamin Kaduk has entered the following ballot position for
draft-ietf-cbor-network-addresses-09: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/blog/handling-iesg-ballot-positions/
for more information about how to handle DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-cbor-network-addresses/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

What should a recipient do if they encounter something in a form that is
expected to include a "natural length" byte string for the IP address
family indicated, but the byte string is a different length?  Do we just
need to say that it is "invalid" and thereby invoke the core CBOR rules
that require protocols to specify how their decoders handle invalid
data?  Or is even that implicit from the CDDL?

In light of the genart reviewer's comment, I think we should say
something like "this specification does not deal with Ethernet
addresses, and tag 260 remains available for that usage" to clarify that
we are not deprecating use of that tag for Ethernet addresses.

   This specification defines tags 54 and 52.  These new tags are
   intended to be used in preference to tags 260 and 261.  They provide
   formats for IPv6 and IPv4 addresses, prefixes, and addresses with
   prefixes, achieving an explicit indication of IPv6 or IPv4.  The

I'd suggest saying here that the tag number is the differentiator
between IPv4 and IPv6 -- that could be helpful to know before we go into
the discussion of address/prefix/interface format.  Perhaps "achieving
an explicit indication of IPv6 or IPv4 by the tag number"?

Section 3.1.3

   When applied to an array that starts with a byte string, which stands
   for an IP address, followed by an unsigned integer giving the bit
   length of a prefix built out of the first "length" bits of the
   address, they represent information that is commonly used to specify
   both the network prefix and the IP address of an interface.

We say "full-length" down in §3.2 and 3.3 (and in the CDDL), but should
we perhaps also say here that this byte string need to be the "natural
length" or "full length" for the IP address family in question?

Section 5

   unused_bits = (-prefix_length_in_bits) & 7;
   if (length_in_bytes > 0)
     address_bytes[length_in_bytes - 1] &= (0xFF << unused_bits);

This looks like it's trying to be C, and the XML agrees.  I think it
only has the desired effect when two's-complement representation for
signed integers is used, though (this is not guaranteed by C through at
least C11, though I have not been closely tracking whether the proposal
to make C and C++ two's-complement-only has been adopted), and suggest
reformulating to something like:

   unused_bits = (8 - (prefix_length_in_bits & 7)) % 8;

(This would be DISCUSS level if it was the only specification for the
behavior, but since it is "or simply", it seems more illustrative in
intent.)

NITS

Section 3.2

   54([h'20010db81234DEEDBEEFCAFEFACEFEED', 56])

DEADBEEF (rather than DEEDBEEF) is in somewhat common usage as a pattern
for indicating "tainted" memory in various sanitizer codes...

Section 5

   A particularly paranoid decoder could examine the lower non-relevant
   bits to determine if they are non-zero, and reject the prefix.  This

This would have to be done before the clearing done by the previous
snippet, of course...

---

From: Michael Richardson <[email-masked]>
Date: Tue, 05 Oct 2021 13:04:13 -0400

addressing low-hanging fruit.
Other issues are going to issues.

    > an explicit indication of IPv6 or IPv4 by the tag number"?

Suggested accepted.

    > length" or "full length" for the IP address family in question?

+ The length of the byte string is always 16 bytes (for IPv6) and 4 bytes (for IPv4).

    > unused_bits = (8 - (prefix_length_in_bits & 7)) % 8;

I'll use your version.

    > This would have to be done before the clearing done by the previous

Yes.

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

---

From: Michael Richardson <[email-masked]>
Date: Tue, 05 Oct 2021 13:05:57 -0400

Opened as issue:  https://github.com/cbor-wg/cbor-network-address/issues/10

---

From: Michael Richardson <[email-masked]>
Date: Tue, 05 Oct 2021 13:07:46 -0400

opened as issue:
https://github.com/cbor-wg/cbor-network-address/issues/11

    > for indicating "tainted" memory in various sanitizer codes...

Eric asks:

-- Section 3.2 --
Is there any reason why part of the IPv6 is in lowercase (as in RFC 5952) and
the other part in uppercase ? This is confusing to the reader (even if
hexadecimal numbers are obviously case insensitive).

---

From: Carsten Bormann <[email-masked]>
Date: Tue, 5 Oct 2021 22:38:18 +0200

>   unused_bits = (8 - (prefix_length_in_bits & 7)) % 8;

Well, I for one don’t like making the “simply” more complicated (*).

In reality, the C family languages have been used exclusively in two’s complement environments for a long time.
C++20 finally made that fact official (**).
(I wouldn’t react as strongly if we hadn't discussed this a while ago for the (pseudo-)code in RFC 8949 [1].)

C itself is lagging (making the same change did not make it into C17 and is scheduled only for C2X [2] [3] [4]).
We could simply say the pseudocode is in C++, maybe using the same text as in RFC 8949 (or, better, pointing to it).

Grüße, Carsten

(*) and, I would at least use % 8 *or* & 7, not one and then the other making the reader wonder if they are different here.
(**) simplifying the specification, but more importantly reducing the immense cost (including security vulnerabilities) from people trying to make their code signed number representation agnostic and in the process getting even the two’s complement case wrong.
[1]: https://www.rfc-editor.org/rfc/rfc8949.html#section-1.2-7
[2]: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2412.pdf
[3]: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2596.pdf
[4]: https://en.wikipedia.org/wiki/C2x

---

From: Michael Richardson <[email-masked]>
Date: Wed, 06 Oct 2021 16:41:40 -0400

> Well, I for one don’t like making the “simply” more complicated (*).

I have no strong preference.
If I didn't have to read/write C again in my life, that would be fine.

I'll go with whatever the working group can agree on, and put that in -11.

---

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 6 Oct 2021 14:19:24 -0700

> If I didn't have to read/write C again in my life, that would be fine.

I'm sure many share that sentiment :)

> I'll go with whatever the working group can agree on, and put that in -11.

Carsten's complaint about mixing & and % is reasonable.

My main stance here is that if we're using code in a standard to illustrate
the behavior, the code should be as easy as possible to understand and
confirm the behavior of.  If that means the expression is a little longer,
rather than requiring the reader to "know" the bit representation of signed
integers, that seems like a wonderful tradeoff to make, to me.

-Ben

---

From: Michael Richardson <[email-masked]>
Date: Thu, 07 Oct 2021 14:55:55 -0400

> I'm sure many share that sentiment :)

okay, so
  unused_bits = (8 - (prefix_length_in_bits % 8);

produces the wrong answer (8) when it's a multiple of 8 and the answer should
be 0.

  unused_bits = (8 - (prefix_length_in_bits % 8) % 8;

gives the right answer.  Can I use that?

---

From: Benjamin Kaduk <[email-masked]>
Date: Thu, 7 Oct 2021 12:57:01 -0700

> gives the right answer.  Can I use that?

In light of the discussion so far, that expression would be my
proposal/recommendation.

-Ben

---

From: Carsten Bormann <[email-masked]>
Date: Fri, 8 Oct 2021 09:31:43 +0200

>  unused_bits = (8 - (prefix_length_in_bits % 8) % 8;

Yes.  If you want to make this simpler, take note that adding 8 to a modulo doesn’t change anything, and use

 unused_bits = (- prefix_length_in_bits) % 8;

Oh, that’s the current text with & 7 (which is representation specific) replaced by % 8.

WFM.

Grüße, Carsten

---

From: Carsten Bormann <[email-masked]>
Date: Fri, 8 Oct 2021 12:03:43 +0200

Strike that; I forgot that C doesn’t actually have a modulo operator (that’s why I’m used to & 7).
Duuh.

Maybe 

unused_bits = (128 - prefix_length_in_bits) % 8;

But that arbitrary 128 causes cognitive dissonance.

Grüße, Carsten