Subject: [secdir] secdir review of draft-ietf-mpls-residence-time-12

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 18 Jan 2017 00:00:26 -0600

I have reviewed this document as part of the security directorate's 
ongoing effort to review all IETF documents being processed by the 
IESG.  These comments were written primarily for the benefit of the 
security area directors.  Document editors and WG chairs should treat 
these comments just like any other last call comments.

This document is Almost Ready.

This document describes a mechanism for recording the residence time of
timing packets along a network path, so that time protocols (NTP, PTP) can obtain
more accurate estimates of transit time.  (Well, the document just claims
to record the residence time and I am inferring the bit about more accurate
estimates; maybe there should be an explicit mention.)  It is limited (at
present) to MPLS networks established using RSVP-TE, to the exclusion of
LSPs established via LDP and non-MPLS networks.  The residence time measurement
is performed in a Generic Associated Channel, and several new data structures
(and sub-data structures) and corresponding type values (and sub-type values)
are established to carry the needed information.

I almost marked this document as Ready with Issues (discussion below), but
ended up changing to Almost Ready, since I think the document structure
needs some more work in order to clearly describe what an interoperable
implementation would need and how the pieces fit together, as is required
for Standards-Track documents.

The security considerations section incorporates that section from
RFC 5586 by reference, but 5586's security considerations are basically
just pointers to those considerations in RFCs 4385 and 5085.
This document also mentions RFC 7384, whose entirety is security requirements
of time procotols, which probably contains more detail than this document would
need if discussion was inline.  However, the security considerations of
draft-ietf-mpls-residence-time-12 also contains discussion about how
PTP-aware nodes on the path are required to modify the messages, and the
needed trust model involves these nodes being trusted to perform those modifications.
That seems true and is probably fine for a protocol that is running on
"trusted infrastructure", but the claim is also made that the messages modified
by intermediate nodes "cannot be authenticated".  This is only somewhat
true, as one can create complex crypto schemes that involve giving key
material to intermediate nodes that can let them make authenticated
(but detectable) modifications.  Such schemes seem far too complex for the
topic at hand, though, as they are likely to increase the processing delay
for the time packets, and it seems fine to defer investigating them in the
same way that it is fine to defer investigating authenticating/encrypting
the RTM data that does not need to be modified by intermediate nodes, which
is explicitly noted in the security considerations.

I do think there are some relevant security considerations that are not
mentioned, though -- for the two-step flow, an RTM-capable node is
required to wait for the follow-up RTM message and make the corresponding
residence time update.  This requirement is unbounded and could lead to
a resource leak if that follow-up packet fails to arrive, for an implementation
that blindly follows the spec without resorting to practical engineering
knowledge.  I do not expect there to be any such implementations, but this
document should probably indicate that timing out is okay within
"reasonable" bounds, or whatever similar workaround is best practice in this
domain.

In terms of other security/privacy considerations that are new in this
document, there is some information exposure about nodes along the path
that could potentially be used for fingerprinting, but since the timing
packets carry destination addresses already, and the LSP setup appears
to involve declaring the path anyway, this doesn't seem to merit any concern.

The other main issue I have with this document is arguably not an issue
at all, but it relates to the plethora of TLVs and sub-TLVs and TLVs
in other registries, with an IANA considerations section that sometimes
does not clearly indicate what registry is to be updated.  As per the
checklist at https://www.ietf.org/iesg/template/doc-writeup-essay-style.html,
the IANA considerations shoudl refer to registries by their exact names,
which probably means the name of the sub-registry and the overarching
parent registry should be clearly written out.  It might also be nice
to have more descriptive names, so I do not have to keep track that there
are RTM G-ACh packets whose values are sometimes sub-TLVs in the PTP case;
RTM Capability (or is it Capabilities?) sub-TLVs that can be contained
in any of OSPFv2, IS-IS, and maybe OSPFv3 data structures; an RTM_SET
TLV whose presence is indicated as an Attribute Flag from some registry that does
not seem to be named; sub-TLVs within those RTM_SET messages; and also
the RTM sub-TLVs that get a registry created for them in section 8.3
and I apparently missed when paging through the document to create this list.
The names used to refer to these structures have me flipping back and
forth to figure out which is which, whereas a name like "RTM_SET
address identifier" (viz. section 8.6) would help the reader a lot.
In a similar vein, it would be nice to have some test vectors that show
the encoding of these structures and their encapsulation in the parent
data structures, to make sure that the implementor gets all the right
layers of T+L wrapping in place.  Alternately (or additionally!), more
clear text that the T+L in the figures here are included in the encoded
data that is the contents of a specific, named, parent data structure
would be useful.

Some other more nit-level things:

In the example in section 4.6, when F is updating the correction field
of the PTP message, I assume that F should also use its measurement of
the residence time on F in addition to the value received in the scratch
pad field, but the example seems to not indicate that.

A couple paragraphs later, "[a]n ingress node that is configured to
perform RTM [...] verifies that the selected egress [node supports RTM]";
should that be a MUST-level requirement that the verification is done?

On page 12, last paragraph, we have some text "If no RTM_SET TLV has been
found, then the LSP setup MUST fail [...]".  Is this only in the case
when the RTM_SET flag is set?  If so, that should probably be made more
clear in the text, as on my first reading I was surprised, since
the RTM_SET generally goes in the LSP_ATTRIBUTES and not the
LSP_REQUIRED_ATTRIBUTES, and as such would not be globally mandatory.

Section 5 makes an offhand note that a 4.6 nanosecond error would
probably be ignorable, which leads to the question: what is the
actual measurment precision that is needed for this scheme to be useful?
The scratch pad uses an IEEE double to count nanoseconds, so potentially
sub-nanosecond values are in-scope, but as someone not well-versed in
PTP I really have no idea how good things can/need to be.

The "A" and "B" subcases mentioned in section 7 get multiple paragraphs
each; it might be more clear to make them subsections instead.

I'm also left puzzled by the last paragraph of section 7; it seems to say
that the *last* RTM(-capable) node of the LSP will generate the follow-up
message, but I thought it was generally an earlier node that would be
setting the S bit and generating the follow-up message.

This document uses several abbreviations/acronyms without introduction
that do not appear on the RFC Editor's abbreviations list
(https://www.rfc-editor.org/materials/abbrev.expansion.txt) as not
needing expansion: G-ACh (also appears in the abstract; the RFC Editor
will likely want to not use the acronym at all in the abstract),
RSVP-TE, and PW are ones I noted.
(LDP is also used without expansion, but does appear on the list as
"well-known".)

There are also a lot of grammar nits (including very many missing
instances of the definite article), but it does not seem worth enumerating
them here.  I will try to send a diff to the authors later this week,
but time is a bit short at the moment.

-Ben

---

From: Greg Mirsky <[email-masked]>
Date: Fri, 20 Jan 2017 17:47:00 -0800

Hi Ben,
thank you for the careful review and the most helpful comments and
suggestions. We're working on the new version to address GEN-ART, OPS and
Security comments. I've attached the diff and current working version of
the draft. Please find my responses to your comments in-lined and tagged
GIM>>.

Regards,
Greg

> just pointers to those considerations in RFCs 4385 and 5085.
 GIM>> Will reference RFC 4385 and RFC 5085 directly.

> is explicitly noted in the security considerations.
GIM>> I agree with your suggestion. Would the following change address your
comment:

---

OLD TEXT:

   As a result, the content of the PTP-related data in RTM messages that

   will be modified by intermediate nodes cannot be authenticated, and

   the additional information that must be accessible for proper

   operation of PTP 1-step and 2-step modes MUST be accessible to

   intermediate nodes (i.e. - MUST NOT be encrypted in a manner that

   makes this data inaccessible).

...

   The ability for potentially authenticating and/or encrypting RTM and

   PTP data that is not needed by intermediate RTM/PTP-capable nodes is

   for further study.

NEW TEXT:

  That likely to require some complex crypto schemes that involve giving key

material to intermediate RTM/PTP-capable nodes that can let them make

authenticated (but detectable) modifications to the additional

information in RTM messages.

   The ability for potentially authenticating and/or encrypting RTM and

   PTP data for scenarios both with and without participation of

   intermediate RTM/PTP-capable nodes is for further study.

-------

> "reasonable" bounds, or whatever similar workaround is best practice in
GIM>> Indeed, we've implicitly relied on good engineering practice and left
out discussion of the timer associated with two-step RTM.

I agree with your observation and propose the following update to text

in section One-step Clock and two-step Clock Modes (added sentence
underlined):

If the S bit is already set, then the RTM capable node MUST wait for the
RTM message with the PTP type of follow-up and matching

originator and sequence number to make the corresponding residence time
update to the Scratch Pad field.

*The wait period MUST be reasonably bound.*

> not seem to be named;

GIM>> There is one registry for Attribute Fags in LSP_ATTRIBUTES  Class
Types or C-Types ‒ 197 LSP_ATTRIBUTES

> sub-TLVs within those RTM_SET messages; and also
GIM>> Yes, node F should add its residence time to correctionField in PTP
message. The proposed new text:

   o  F is the egress LER and the last RTM capable node.  It removes the
      RTM ACH encapsulation and processes the timing packet carried in
      the Value field using the value in the Scratch Pad field.  In
      particular, the value in the Scratch Pad field of the RTM ACH is
      used in updating the Correction field of the PTP message(s).  The
      LER should also include its own residence time before creating the
      outgoing PTP packets.  The details of this process depend on
      whether or not the node F is itself operating as one-step or two-
      step clock.

> should that be a MUST-level requirement that the verification is done?
GIM>> Agree. Will use s/verifies/MUST verify/

> LSP_REQUIRED_ATTRIBUTES, and as such would not be globally mandatory.
GIM>> Earlier, in the same paragraph, we've said

"If the RTM_SET flag set, the node MUST inspect the LSP_ATTRIBUTES object
for presence of RTM_SET TLV." ("Node" is used in place of "RTM-capable
node")
Thus nodes that are not RTM-capable would not act on RTM_SET Attribure
Flag, would not be chacking for presence of RTM_SET TLV.

> PTP I really have no idea how good things can/need to be.
GIM>> Adding informational reference to

   [ITU-T.G.8271]
              "Packet over Transport aspects - Synchronization, quality
              and availability targets", ITU-T Recomendation
              G.8271/Y.1366, July 2016.

 and the text to follow:

This may be acceptable for applications where the target accuracy is in the
order
of hundreds of ns. As an example several applications being
considered in the area of wireless applications are satisfied with an
accuracy of 1.5 microseconds [ITU-T.G.8271].

> setting the S bit and generating the follow-up message.
GIM>> Updated text as the following:

   The egress RTM-capable node of the LSP will be removing RTM
   encapsulation and, in case of two-step clock mode being indicated,
   will generate PTP messages as appropriate (according to the
   [IEEE.1588.2008]).  In this case, the common header of the PTP packet
   carrying the synchronization message would have to be modified in the
   twoStepFlag field indicating that there is now a follow up message
associated to that.

> (LDP is also used without expansion, but does appear on the list as
GIM>> Expanded all acronyms in the Abstract.

> but time is a bit short at the moment.
GIM>> Many thanks and greatly appreciate your kind help.

---

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 24 Jan 2017 23:57:31 -0600

> the draft. Please find my responses to your comments in-lined and tagged

Thanks!
Most of the changes are helpful; the only thing I would note about the
new text is in the security considerations, where the discussion of
"complex crypto schemes" seems like it should come after the mention
of "for further study", not before.

I as a new reader to the field would still benefit from some discussion
and/or examples of how the new data structures interact with each other
and the parent containers, but I should defer to the experts as to what
is actually needed.

I will trim the parts of the message that I sound good and I have no
further comment on.

>    intermediate RTM/PTP-capable nodes is for further study.

I think this should be reordered to be more useful, something like
(with a few more tweaks):

   In addition - particularly as applied to use related to PTP - there
   is a presumed trust model that depends on the existence of a trusted
   relationship of at least all PTP-aware nodes on the path traversed by
   PTP messages.  This is necessary as these nodes are expected to
   correctly modify specific content of the data in PTP messages and
   proper operation of the protocol depends on this ability.  In practice,
   this means that those portions of the messages cannot be covered by
   either confidentiality or integrity protection.  Though there are
   methods that make it possible in theory to provide either or both such
   protections and still allow for intermediate nodes to make
   detectable but authenticated modifications, such methods do not seem
   practical at present, particularly for timing protocols that are
   sensitive to latency.

   The ability for potentially authenticating and/or encrypting RTM and
   PTP data for scenarios both with and without participation of
   intermediate RTM/PTP-capable nodes is left for further study.

> *The wait period MUST be reasonably bound.*

Sounds good.

> Flag, would not be chacking for presence of RTM_SET TLV.

Okay.

>    twoStepFlag field indicating that there is now a follow up message

Ah, maybe I have un-confused myself.  This about the case where the
underlying PTP is a one-step clock, but the RTM path includes two-step
nodes, so the node that removes the RTM wrapper has to synthesize a
follow-up PTP message to contain the correction?

Making (A) and (B) fully fledged subsections would let them have
indicative tiles, like "Two-step RTM with two-step upstream" and
"Two-step RTM with one-step upstream".

In any case, I would suggest being more explicit than "the associated RTM
packet must be created" means, explicitly describing what type of RTM packet
is being created (i.e., the follow-up one?).

> GIM>> Many thanks and greatly appreciate your kind help.

I guess it is lucky that I did not have time last week, since there is
an updated version that I could be basing changes onto.  (Things are still
busy for me, so no guarantees of anything.)

-Ben

---

From: Greg Mirsky <[email-masked]>
Date: Wed, 25 Jan 2017 11:34:06 -0800

Hi Ben,
many thanks for taking time to review the proposed updates and share your
comments, much obliged.
My next steps:

   - Will use your text in the Security Considerations section, thank you.
   - Will place cases A and B as sub-sections, per your suggestion.
   - Will share the update by end of the week.

Kind regards,
Greg

On Tue, Jan 24, 2017 at 9:57 PM, Benjamin Kaduk <[email-masked]> wrote:

> > > of time procotols, which probably contains more detail than this
> document
> > > would