Concise Binary Object Representation (CBOR) Tags for IPv4 and IPv6 Addresses and Prefixes
draft-ietf-cbor-network-addresses-13

The updated version looks good, thanks! 

Text of former DISCUSS below for posterity.

--

Thanks for this document. In general I found it easy to read, blessedly concise, and useful. I do have one concern with how you treat the covert channel concern you raise, which I'm making a DISCUSS (which I think will be easily cleared).

Section 4 says:

   even though variations like:

   54([44, h'20010db81233'])
   54([45, h'20010db8123f'])

   would be parsed in the exact same way; they MUST be considered
   invalid.

You choose to use a RFC 2119 keyword here, and this is in the encoder section, so presumably you are insisting that the encoder MUST... what? You already said, in an earlier paragraph, that the encoder MUST set the trailing bits to zero, so I can't figure out what the quoted text is telling me to do. (Presumably any compliant encoder won't produce the depicted values, and an encoder that's noncompliant for the purpose of deliberately exfiltrating data using this covert channel won't be put off by this MUST.)

Then in Section 5 we have:

   A particularly paranoid decoder could examine the lower non-relevant
   bits to determine if they are non-zero, and reject the prefix.  This
   would detect non-compliant encoders, or a possible covert channel.

The fairly dismissive tone ("paranoid decoder could"), not to mention the preceding pseudocode, suggests that you have no real expectation the decoder will do anything to "consider invalid" values with nonzero low bits. So probably the MUST from Section 4 isn't meant to apply to the decoder.

In short I don't understand what that clause in Section 4 is telling me to do. One fix would simply be to weaken the text, as in

   would be parsed in the exact same way, they should not be 
   considered legitimate encodings.
   
P.S.: The semicolon in the quoted text is also either wrong, or I'm even more confused about what's being specified than I thought I was.

I concur with Lars' comment.

I'm not sure about the "WRONG" labels.  I suggest including that in prose instead; for instance, something like the following:

OLD:

   even though variations like:

   54([44, h'20010db81233'])  WRONG
   54([45, h'20010db8123f'])  WRONG

   would be parsed in the exact same way.

NEW:

   even though variations like:

   54([44, h'20010db81233'])
   54([45, h'20010db8123f'])

   would be parsed in the exact same way; they MUST be
   considered invalid.

** Section 7.  Recommend generalizing the text.

OLD
   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.

NEW
This document provides an CBOR encoding for IPv4 and IPv6 address information.  Any applications using these encodings will need to consider the security implications of this data in their specific context.  For example, 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.
 

** Section 8.3.  Recommend making the text clearer on what’s getting deprecated

OLD
   IANA is requested to add the note "DEPRECATED in favor of 52 and 54
   for IP addresses" to registrations 260 and 261

NEW
IANA is requested to add the note "DEPRECATED for use with IP addresses in favor of 52 and 54" to registrations 260 and 261

I am curious about Eric’s 2nd discuss point, hence supporting his discuss on that.

I also support Lars’s and Murray’s comments.


Thanks for working on this specification.

Thank you for the work put into this document and addressing my previous ballot points (DISCUSS & COMMENT); those points are kept below for archiving purposes only.

Special thanks to Barry Leiba for his concise shepherd's write-up but very clear about the WG consensus.

Thank you also to Donald Eastlake for this INT directorate review that I am vastly supporting:
https://mailarchive.ietf.org/arch/msg/int-dir/6Ox8iEBMqXkUoC2aUEF3wi4-c5g/

I hope that this helps to improve the document,

Regards,

-éric


== PREVIOUS DISCUSS (for archive) ==

Generic comment how are link-local address (LLA) with scope encoded ? I would expect CBOR to work also on LLA only networks... At the bare minimum, please state that link-local addresses cannot be encoded with their scope, hence, they cannot represent an interface.

-- Section 3.1.3 --
How can 2 valid link-local addresses (fe80::1%eth0, fe80::1%eth1) can be represented in order to identity two interfaces ?

== PREVIOUS COMMENTS (for archive) ==

I love how your start with IPv6 in section 3 and use the ASCII codes for '6' and '4' ;-)

-- 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).

== NITS ==

-- Abstract --
Should CBOR be expanded ?

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...

Section 1. , paragraph 3, comment:
>    This specification does not
>    deal with 6- or 8-byte Ethernet addresses.

I read this and kept wondering about what other Ethernet addresses there are
that I didn't know about... It might be clearer to say something like "does not
include support for Ethernet addresses".

-------------------------------------------------------------------------------
All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

These URLs in the document can probably be converted to HTTPS:
 * http://www.iana.org/assignments/cbor-tags

Thanks for defining these, they generally looks useful.

I note that this encoding doesn't cover RFC 4007 scoped IP addressed, e.g., that can be expressed by YANG's IPv4-address and IPv6-address (RFC 6991).  Is there any thought or plan to support scoped IP addresses, either by extending this tag format, or would these use separate tag numbers if support for them was ever required?  Perhaps the introduction should indicate that scoped IP addresses are out of scope ;-)

Section 3.1.2 and 3.1.3.  Is the array always of length 2 and if so is that worth explicitly stating here?

Section 4, is the last bad example right, i.e., with a prefix length of 45 rather than 44?

   54([45, h'20010db8123f'])  WRONG

Would this not be interpreted as the prefix: "2001:db8:1238::/45"?

I have to confess that I'm not super keen on the ordering of the array arguments defining the meaning of whether it is a prefix or interface definition.  This feels a bit too much like magic for my liking.

Rob