Subject: [Cbor] Document shepherd review of draft-ietf-cbor-network-addresses

From: Barry Leiba <[email-masked]>
Date: Mon, 19 Jul 2021 10:18:21 -0400

Hi, all.  Here's my shepherd review of draft-ietf-cbor-network-addresses-05.

— Section 1 —

I would merge the one-sentence first paragraph into the second paragraph.

   Tag 260 and tag 261 was later defined through IANA.

“were”, plural.  I might also say “Tags 260 and 261” rather than
repeating the word “tag”, but keep this as you please.

   The present specification achieves an explicit indication of IPv4 or
   IPv6, and the possibility to omit trailing zeroes.

What is “the present specification”?  The one associated with the IANA
registration (I don’t think that’s what you mean, as there's no detail
there)?  Or this document?  Assuming the latter, it would be a lot
clearer if you just merged this sentence into the following paragraph,
where you already talk about what “this document” does.  If you really
mean something else, it could use a reference citation and rewording
of “the present specification”.

   Due to the complexity of testing the value of
   omitting trailing zeros for addresses was considered non-essential
   and support for that was removed in this specification.

This seems hard to parse.  As I read this, I see “testing the value”
as a unit, but I think you mean for there to be a comma after
“testing”?  Or maybe you do mean that “testing the value” is complex,
but then the word “of” seems wrong.  Can you reword this to clean that
up?

— Sections 3.1 and 3.2 —

There are examples of [prefix-length, addr] and [addr, prefix-length],
but no examples of just [addr], so it’s easy to miss that that’s also
valid.  It would be useful to add an example after this paragraph:

   An IPv6 address is to be encoded as a sixteen-byte byte string
   (Section 3.1 of [RFC8949], major type 2), enclosed in Tag number 54.

…and similarly for Section 3.2.

— Section 4 —

   An encoder may omit as many right-hand (trailing) bytes which are all
   zero as it wishes.

How does this fit with “Trailing zero bytes MUST be omitted.” in
Sections 3.1 and 3.2?

— Section 7 —

   Identifying which byte sequences in a protocol are addresses may
   allow an attacker or eavesdropper to better understand what parts of
   a packet to attack.

   Reading the relevant RFC may provide more information, so it would
   seem that any additional security that was provided by not being able
   to identify what are IP addresses falls into the security by
   obscurity category.

Does this mean the following (and if so, might this be a better way to say it?):

   Identifying which byte sequences in a protocol are addresses may
   allow an attacker or eavesdropper to better understand what parts of
   a packet to attack.  That information, however, is likely to be
   found in the relevant RFCs anyway, so this is not a significant
   exposure.

-- 
Barry

---

From: Carsten Bormann <[email-masked]>
Date: Mon, 19 Jul 2021 17:44:12 +0200

> Hi, all.  Here's my shepherd review of draft-ietf-cbor-network-addresses-05.

Thank you!

Some editorial work was already done based on Marco’s comments, which partially overlaps yours; so if the comment isn’t mentioned below, we already have done it or did it now.

> 
> What is “the present specification”?  

I love how English doesn’t seem to have a translation for
“Die vorliegende Spezifikation”.

https://www.deepl.com/translator#de/en/die%20vorliegende%20Spezifikation

At least, many people don’t seem to associate the meaning of that with “the present specification”.  “This specification” is always a problem because it may refer to a specification that was just talked about instead of “the present specification”, which is the specification that is present before your eyes.

Anyway, merging the paragraphs got rid of the problem in a natural way.

> but no examples of just [addr],

(Which would be wrong — we only have `addr`, not `[addr]`.)

Added examples for bare addrs now.
(And reduced the irregularity of the spacing in the examples.)

> How does this fit with “Trailing zero bytes MUST be omitted.” in

Not very well.
Fixed in Section 4, together with some editorial improvements.

> […]

Thanks, much better.

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

Grüße, Carsten

---

From: Barry Leiba <[email-masked]>
Date: Mon, 19 Jul 2021 11:58:01 -0400

Thanks for the quick reply, and I've put my approval n the PR.

b

---

From: Michael Richardson <[email-masked]>
Date: Mon, 19 Jul 2021 17:11:53 -0400

> (Which would be wrong — we only have `addr`, not `[addr]`.)

"future work" :-)

I guess, if I write code for this (I haven't done the interface spec), and I
saw [addr], I'm assume [addr,128].
I don't think that this is worth writing down, nor worth encouraging.

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

---

From: Barry Leiba <[email-masked]>
Date: Mon, 19 Jul 2021 21:18:17 -0400

No, I just wrote it as "[addr]", but didn't mean it as an actual
array.  Carsten did the right thing in the PR.

b

On Mon, Jul 19, 2021 at 5:12 PM Michael Richardson
<[email-masked]> wrote:
> _______________________________________________
> CBOR mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/cbor

---

From: Brian E Carpenter <[email-masked]>
Date: Tue, 20 Jul 2021 14:40:13 +1200

You got me. My demo code now supports all three forms and I switched to using the "prefix" form instead of "interface".

My code would treat [addr] as an error.

See the functions build5254() and detag5254() inside https://github.com/becarpenter/graspy/blob/master/pfxm4.py

Regards
   Brian