Re: [core] Benjamin Kaduk's No Objection on draft-ietf-core-hop-limit-06: (with COMMENT)

[<[e-mail]>]

Hi Ben, 

Thank you for the review. 

Please see inline. 

Cheers,
Med

> -----Message d'origine-----
> De : Benjamin Kaduk via Datatracker [mailto:[e-mail]]
> Envoyé : mardi 15 octobre 2019 18:32
> À : The IESG
> Cc : [e-mail]; Jaime Jimenez; core-
> [e-mail]; [e-mail]; [e-mail]
> Objet : Benjamin Kaduk's No Objection on draft-ietf-core-hop-limit-06:
> (with COMMENT)
> 
> Benjamin Kaduk has entered the following ballot position for
> draft-ietf-core-hop-limit-06: 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/iesg/statement/discuss-criteria.html
> for more information about IESG DISCUSS and COMMENT positions.
> 
> 
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-core-hop-limit/
> 
> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> Section 1.1
> 
> It's not entirely clear to me that the original ("specific") use case
> needs to be mentioned given that the document now has a generic intended
> usage.
> 
>    Note that this means that a server that receives requests both via
>    proxies and directly from clients may see otherwise identical
>    requests with and without the Hop-Limit option included; servers with
>    internal caching will therefore also want to implement this option.
> 
> We might want to have one more sentence here to expound on why (if they
> implement the option, then it's okay for them to ignore it when doing a
> cache lookup which would not otherwise be safe?)

[Med] Not sure to fully understand the comment. The key point though is that such servers should not ignore the option. 

> 
> Section 3
> 
> I'd suggest mentioning that C/U/N/R and the formatting of Table 1 also
> match up with Table 4 (Section 5.10) of RFC 7252, since without that
> reference for comparison it's a bit hard to know how to interpret the
> C/U/N/R columns in the table.
> 

[Med] OK
 

>    Nevertheless, the presence of such proxies will not prevent infinite
>    loop detection if at least one CoAP proxy which support the Hop-Limit
>    option is involved in the loop.
> 
> nit: s/support/supports/

[Med] Fixed. 

> 
>    A CoAP proxy which understands the Hop-Limit option MAY be
>    instructed, using a configuration parameter, to insert a Hop-Limit
>    option when relaying a request which do not include the Hop-Limit
>    option.
> 
> In light of the other discussion about how strongly to recommend
> Hop-Limit's usage, do we want to revisit MAY vs. SHOULD here?  (My
> apologies if we already did and I missed it.)

[Med] Makes sense.   

> 
>    The initial Hop-Limit value should be configurable.  If no initial
>    value is explicitly provided, the default initial Hop-Limit value of
>    16 MUST be used.  This value is chosen to be sufficiently large to
>    guarantee that a CoAP request would not be dropped in networks when
>    there were no loops, but not so large as to consume CoAP proxy
>    resources when a loop does occur.  [...]
> 
> It would have been nice to have an updated rev that includes feedback
> from, e.g., the secdir reviewer.  That is to say, I also think that
> "guarantee" is too strong here.

[Med] ACK. We shared (offline) with Radia the change to address here comment with a pointer to the github updated copy. Apologies for not sharing the link on the secdir list: https://github.com/boucadair/draft-hop-limit/blob/master/draft-ietf-core-hop-limit-07.txt.     

> 
>       Note: If a request with a given value of Hop-Limit failed to reach
>       a server because the hop limit is exhausted, then the same failure
>       will be observed if a less value of the Hop-Limit option is used
>       instead.
> 
> nit: s/less/smaller/

[Med] OK. 

> 
> Section 4
> 
>    To ease debugging and troubleshooting, the CoAP proxy which detects a
>    loop includes its information in the diagnostic payload under the
> 
> nit: given the subsequent discussion, maybe s/its information/an
> identifier for itself/?

[Med] OK

> 
>    conditions detailed in Section 5.5.2 of [RFC7252].  That information
>    MUST NOT include any space character.  The information inserted by a
> 
> Does "space character" just mean ASCII 0x20

[Med] Yes.  

, or any character with the
> WSpace property?
> 
>    Each intermediate proxy involved in relaying a TBA1 (Hop Limit
>    Reached) error message prepends its own information in the diagnostic
>    payload with a space character used as separator.  Only one
>    information per proxy should appear in the diagnostic payload.  Doing
> 
> Does this mean that a proxy is expected to inspect the diagnostic
> payload for its own identifier before prepending?

[Med] Yes, because otherwise there is a risk that TBA1 may be looping too. If the hop-limit is appropriately set, the loop will be detected early, which means that TBA1 can be forwarded to the CoAP endpoint. But, if the initial hop-limit is too high, the loop may not be detected earlier and the request may be crossing the same proxy several time till the hop-limit is exhausted. 

Will update the text to make this explicit. 

> 
> Section 5
> 
>    By default, the HTTP-to-CoAP Proxy inserts a Hop-Limit option
>    following the guidelines in Section 3.  The HTTP-to-CoAP Proxy MAY be
>    instructed by policy to insert a Hop-Limit option only if a Via
>    (Section 5.7.1 of [RFC7230]) or CDN-Loop header field [RFC8586] is
>    present in the HTTP request.
> 
> It seems like a descriptive "may" would work okay here, if desired.

[Med] OK.  

> 
>    By default, the CoAP-to-HTTP Proxy inserts a Via header field in the
>    HTTP request if the CoAP request includes a Hop-Limit option.  The
>    CoAP-to-HTTP Proxy MAY be instructed by policy to insert a CDN-Loop
>    header field instead of the Via header field.
> 
> (same here)
> 
>    When both HTTP-to-CoAP and CoAP-to-HTTP proxies are involved, the
>    loop detection may get broken if the proxy-forwarded traffic
>    repeatedly crosses the HTTP-to-CoAP and CoAP-to-HTTP proxies.
> 
> It feels like we could say more here -- if the hc and ch proxies use
> mapping rules such that proxy identifiers are preserved across a
> protocol-translation round-trip, wouldn't loop detection still work?

[Med] The issue is that proxy identifiers in the HTTP leg will be lost when entering the CoAP leg. 

It was raised in the WG whether we want to fix this or only record the issue. We went for the latter because this issue is also encountered when a CoAP request crosses multiple administrative domains; each boundary proxy resets the Hop-Limit. 

> 
> Section 7
> 
> It seems that any attacker in a position to add or modify a hop-limit
> option would be able to drop the request or forge a response anyway, so
> there's not much of a new DoS risk.
> 
> However, in addition to the topology leakage in the diagnostic payload,
> I think that an attacker could use small hop-limit values to probe the
> topology of a network into which it is making requests, finding the
> smallest hop-limit value that allows the request to succeed.

[Med] Agree. We didn't include this one in the security section because we thought this is nullified by the following: 

   Because forwarding errors may occur if inadequate Hop-Limit values
   are used, proxies at the boundaries of an administrative domain MAY
   be instructed to remove or rewrite the value of Hop-Limit carried in
   received messages (i.e., ignore the value of Hop-Limit received in a
   message).  

But now that you raised it, we can include the following: 

   A CoAP endpoint can probe the topology of a network into which it is
   making requests by tweaking the value of the Hop-Limit option.  Such
   probing is likely to fail if proxies at the boundaries of that
   network rewrite the value of Hop-Limit carried in received requests
   (see Section 3).