Subject: Detailed review of draft-gont-6man-stable-privacy-addresses

From: Mark ZZZ Smith <[email-masked]>
Date: Tue, 17 Sep 2013 04:05:40 -0700 (PDT)

Hi,

As requested by Fernando and the WG chairs, I've performed a detailed review of draft-gont-6man-stable-privacy-addresses.

Suggested changes highlighted by pairs of '|'

Abstract
~~~~~~

* nit: Perhaps 

" This method is meant to be an
   alternative to generating Interface Identifiers based on hardware
   address (e.g., using IEEE identifiers),"

changed to

" This method is meant to be |a
   replacement for| generating Interface Identifiers based on hardware
   address (e.g., using IEEE identifiers),"

1.  Introduction
~~~~~~~~~~~

*nit: global change "IEEE identifiers" to "IEEE address"/"IEEE addresses" as that is the IEEE terminology for node identifiers

* nit: Perhaps

"... which typically results in hosts configuring one or
   more "stable" addresses composed of a network prefix advertised by a
   local router, and an Interface Identifier (IID) that typically embeds
   a hardware address (e.g., using IEEE identifiers)"

changed to

"... which typically results in hosts configuring one or
   more "stable" addresses composed of a network prefix advertised by a
   local router, and an Interface Identifier (IID) that typically embeds
   a hardware |identifier| (e.g., using an IEEE |address|)"

* nit: Perhaps

"Generally, the traditional SLAAC addresses are thought to simplify network management, since they simplify Access Control Lists (ACLs) and logging."

changed to

"Generally, || traditional SLAAC addresses are thought to simplify network management, since they simplify Access Control Lists (ACLs) and logging."

* nit: Perhaps

"...  they allow correlation of node activities within the same network,
      thus negatively affecting the privacy of users."

changed to

"... they allow correlation of node activities within the same network,
      thus |reducing| the privacy of users."

* nit: Perhaps

"... (e.g. track and correlate the activities of a typical client
      connecting to the public Internet from different locations), thus
      negatively affecting the privacy of users."

changed to

"... (e.g. track and correlate the activities of a typical client
      connecting to the public Internet from different locations), thus
      |reducing| the privacy of users.)"

* nit: Reference to RFC5157

"...  such patterns
      may be leveraged by attackers to reduce the search space when
      performing address scanning attacks. [RFC5157]"

* nit: Perhaps

"...  the IPv6
      addresses of all nodes manufactured by the same vendor (at a given
      time frame) will likely contain the same IEEE Organizationally ..."

changed to

"...  the IPv6
      addresses of all nodes manufactured by the same vendor (|within| a given
      time frame) will likely contain the same IEEE Organizationally ..."

* nit: addition of a comma

" ... (henceforth referred to as "temporary addresses")
   were introduced to complicate the task of eavesdroppers and other
   information collectors (e.g.|,|  IPv6 addresses in web server logs or
   email headers, etc.)"

* nit: Perhaps

" ... all a host
      is left with is the stable addresses that have been generated
      using e.g.  IEEE identifiers. "

changed to

" ... all a host
      is left with is the stable addresses that have |typically| been generated
      using |hardware identifiers|. "

*nit: Perhaps

"...  since "temporary addresses" [RFC4941] do not replace the
      traditional SLAAC addresses, an attacker can still leverage
      patterns in those addresses to greatly reduce the search space for
      "alive" nodes"

changed to

"since "temporary addresses" [RFC4941] do not replace the
      traditional SLAAC addresses, an attacker can still leverage
      patterns in |SLAAC| addresses to greatly reduce the search space for
      "alive" nodes"

*nit: Move/remove paragraph

   "Hence, there is a motivation to improve the properties of "stable"
   addresses regardless of whether temporary addresses are employed or
   not.

   We note that attackers can employ a plethora of probing techniques
   [I-D.ietf-opsec-ipv6-host-scanning] to exploit the aforementioned
   issues.  Some of them (such as the use of ICMPv6 Echo Request and
   ICMPv6 Echo Response packets) could mitigated by a personal firewall
   at the target host.  For other vectors, such listening to ICMPv6
   "Destination Unreachable, Address Unreachable" (Type 1, Code 3) error
   messages referring to the target addresses
   [I-D.ietf-opsec-ipv6-host-scanning], there is nothing a host can do
   (e.g., a personal firewall at the target host would not be able to
   mitigate this probing technique).

   This document specifies a method to generate Interface Identifiers
   that are stable/constant for each network interface within each
   subnet, but that change as hosts move from one network to another,
   thus keeping the "stability" properties of the Interface Identifiers
   specified in [RFC4291], while still mitigating address-scanning
   attacks and preventing correlation of the activities of a node as it
   moves from one network to another."

I think the middle paragraph is either not really necessary, or should be moved somewhere else. The first paragraph reads like it is ending the justification for stable and opaque IIDs, and the last paragraph reads to me like it is the start of the description of the method. I think it would read better if the first and last paragraphs of the three were adjacent.

*nit: In above middle paragraph, if preserved in the document, perhaps:

"For other vectors, such |as| listening to ICMPv6
   "Destination Unreachable, Address Unreachable""

*nit: Addition of a comma (and IEEE identifier change as mentioned above)

" employed, implementation
   of the mechanism described in this document (in replacement of stable
   addresses based on e.g.|,|  IEEE |addresses|)

*nit: Perhaps

" While the method specified in this document is meant to be used with
   SLAAC, this does not preclude the same algorithm from being used with
   other address configuration mechanisms, ..."

changed to

"While the method specified in this document is meant to be used with
   SLAAC, this does not preclude |this algorithm| from being used with
   other address configuration mechanisms, ..."

2.  Design goals
~~~~~~~~~~~~

*nit: Perhaps

"This document specifies a method for selecting Interface Identifiers
   to be used with IPv6 SLAAC, ..."

changed to

"This document specifies a method for |generating| Interface Identifiers
   to be used with IPv6 SLAAC, ..."

*nit: Perhaps

"Depending on the specific implementation approach (see Section 3
      and Appendix A), the resulting Interface Identifiers may be
      independent of the underlying hardware (e.g. link-layer address)."

changed to

"Depending on the specific implementation approach (see Section 3
      and Appendix A), the resulting Interface Identifiers may be
      independent of the underlying hardware |identifiers| (e.g.|,| link-layer address)."

*nit: Perhaps include reference to more than just Ethernet/IPv6 RFC.

"The method specified in this document is meant to be an
      alternative to producing IPv6 addresses based on e.g.  IEEE
      identifiers (as specified in [RFC2464])."

changed to 

"The method specified in this document is meant to be an
      alternative to producing IPv6 addresses based on |hardware identifiers, such as specified in [RFC2464] and [RFC2470]|."

*nit: The following paragraph seems to be repeating quite a lot of topics of previous text. I suggest removing it, and finding somewhere else for the I-D.cooper and IAB-PRIVACY references.

" We note that of use of the algorithm specified in this document is
   (to a large extent) orthogonal to the use of "temporary addresses"
   [RFC4941].  When employed along with "temporary addresses", the
   method specified in this document will mitigate address-scanning
   attacks and correlation of node activities across networks (see
   [I-D.cooper-6man-ipv6-address-generation-privacy] and [IAB-PRIVACY]).
   On the other hand, hosts that do not implement/use "temporary
   addresses" but employ the method specified in this document would, at
   the very least, mitigate the host-tracking and address scanning
   issues discussed in the previous section."

3.  Algorithm specification
~~~~~~~~~~~~~~~~~~~

*nit: The first sentence of the first paragraph,

"IPv6 implementations conforming to this specification MUST generate
   Interface Identifiers using the algorithm specified in this section
   in replacement of any other algorithms used for generating "stable"
   addresses with SLAAC (such as those specified in [RFC2464])."

seems to be pretty much repeated again as the last sentence of the paragraph:

"The method specified in this document MUST be
   employed for generating the Interface Identifiers with SLAAC for all
   the stable addresses of a given interface, including IPv6 global,
   link-local, and unique-local addresses."

so I suggest it be deleted.

* nit: The last sentence of this note also seems to be repeating the "MUST" statement, so I suggest deleting it:

"This means that this document does not formally obsolete or
      deprecate any of the existing algorithms to generate Interface
      Identifiers (e.g. such as that specified in [RFC2464]).  However,
      those IPv6 implementations that employ this specification MUST
      generate all of their "stable" addresses as specified in this
      document."

changed to

"This means that this document does not formally obsolete or
      deprecate any of the existing algorithms to generate Interface
      Identifiers (e.g. such as that specified in [RFC2464]).||"

*nit: The SHOULD in the following sentence seems to be contradicting the MUSTs in the previous sentences. If this method MUST be used, then I don't think it SHOULD be able to be switched on or off.

" Implementations conforming to this specification SHOULD provide the
   means for a system administrator to enable or disable the use of this
   algorithm for generating Interface Identifiers."

*nit: I think 'tentative' would be better in the following sentence, because the generated IID hasn't passed a DAD check yet:

"1.  Compute a random (but stable) identifier with the expression:"

changed to

"1.  Compute a random |tentative| identifier with the expression:"

*nit: I think "Random Identifier" would be better, as it isn't an Interface Identifier yet:

"RID:
          Random (but stable) Interface Identifier"

changed to

"RID:
          Random || Identifier"

*nit: Perhaps

"The prefix to be used for SLAAC, as learned from an ICMPv6
          Router Advertisement message, ..."

changed to

"The prefix to be used for SLAAC, as learned from an ICMPv6
          Router Advertisement message |Prefix Information Option|, ..."

*nit: Remove lonely bracket :

"MUST initialize
          DAD_Counter to the recorded value if such an entry exists in
          non-volatile memory)."

changed to

"MUST initialize
          DAD_Counter to the recorded value if such an entry exists in
          non-volatile memory||."

*nit: Perhaps

"The secret
          key MUST be initialized at operating system installation time
          to a pseudo-random number ..."

changed to

"The secret
          key MUST be initialized |to a pseudo-random number| at operating system installation time or when the IPv6 protocol stack is first initialized."

*nit: Perhaps

"However, the method discussed in
          this document could be employed for generating Interface IDs
          of any arbitrary length, albeit at the expense of reduced
          entropy (when employing Interface IDs smaller than 64 bits)."

changed to

""However, the method discussed in
          this document could be employed for generating Interface IDs
          of any arbitrary length, |with entropy reducing for smaller Interface IDs.|"

*nit: I think the following text should be a MUST, and should not be a note, but part of the method specification, somewhere else in the document:

"The resulting Interface Identifier should be compared against the
       Subnet-Router Anycast [RFC4291] and the Reserved Subnet Anycast
       Addresses [RFC2526], and against those Interface Identifiers
       already employed in an address of the same network interface and
       the same network prefix."

changed to:

"The resulting Interface Identifier |MUST| be compared against the
       Subnet-Router Anycast [RFC4291] and the Reserved Subnet Anycast
       Addresses [RFC2526], and against those Interface Identifiers
       already employed in an address of the same network interface and
       the same network prefix."

*nit: Perhaps

"The DAD_Counter parameter provides the means to intentionally cause
   this algorithm produce a different IPv6 addresses (all other
   parameters being the same)."

changed to

"The DAD_Counter parameter provides the means to intentionally cause
   this algorithm |to| produce a different IPv6 address |, with all other
   parameters being the same|."

4.  Resolving Duplicate Address Detection (DAD) conflicts
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

*nit: Perhaps

 "We also
   note that hosts MUST limit the number of tentative addresses that are
   tried (rather than indefinitely try a new tentative address until the
   conflict is resolved)."

changed to

" We also
   note that hosts MUST limit the number of tentative addresses that are
   tried |, rather than indefinitely try a new tentative address until the
   conflict is resolved|."

*nit: Perhaps

"In those (unlikely) scenarios in which duplicate addresses are
   detected and in which the order in which the conflicting nodes
   configure their addresses may vary ..."

changed to

"In those |unlikely| scenarios in which duplicate addresses are
   detected and in which the order in which the conflicting nodes
   configure their addresses may vary ..."

(A good rule about bracketed text is that it should add to the text, but be optional. In other words, if you remove the bracketed text, the text shouldn't lose or change meaning. In the above, removing "(unlikely)" would suggest duplicate addresses is quite common, which isn't the case, so "unlikely" shouldn't be in (). )

7.  Security Considerations
~~~~~~~~~~~~~~~~~~~~~~~~~~~

*nit: Delete 'of'

"... for each configured address,
      knowledge/leakage of the Interface Identifier employed for one
      stable address of will not negatively affect the security/privacy ..."

changed to

"... for each configured address,
      knowledge/leakage of the Interface Identifier employed for one
      stable address || will not negatively affect the security/privacy ..."

Regards,
Mark.