Subject: Re: [core] Review of draft-ietf-core-echo-request-tag-07

From: Göran Selander <[email-masked]>
Date: Tue, 5 Nov 2019 13:48:57 +0000

Hi Francesca, 

Please find inline the remaining comments responding to your review.  (The other part is here: https://mailarchive.ietf.org/arch/msg/core/ccN-2OIAtrScW9u-ddhiBf4tmzc)

The changes are tracked on the CoRE WG github and proposed resolution is compiled in -08 which was submitted yesterday:
https://tools.ietf.org/html/draft-ietf-core-echo-request-tag-08

On 2019-09-24, 17:00, "Francesca Palombini" <[email-masked]> wrote:

    Finally, I have compiled a list of nits and will soon make a PR for it.

GS: Looking forward to that!

    # High level suggestions #
    
    * Move the Terminology section (current section 1.4) as section 1.1 (under Introduction).
    
    * Current sections 1.1, 1.2, 1.3 are very useful background sections about the problem this document sets out to solve. I do not see why they need to be in this order, for readability I would have: 1.1 as new section 2.1, moving the text in 2. to the current 2.1 (that would become 2.2); 1.2 as new section 3.1, moving the text in 3. to current 3.1 (new 3.2); 1.3 as new section 5.1, old 5. text becoming 5.2. This will require mainly copy-paste and minor introductory text in sections 2, 3, and 5.
    
    The goal with this restructuring would be to help keep the 3 parallel topics in focus (Echo options, Request Tag option, Token update), without having to switch from one to the other.
    
GS: These changes are in -08.

    * Move appendix A to the main body: there is already RECOMMENDED text, it would fit to have that as the last subsection of the Echo section.

GS: This we didn't do to avoid the details of this example in the main body.
    
    # Questions, Open points #
    
    * Section 1.4
    
    "   (Concurrent block-wise request operations are impossible with the
       options of [RFC7959] because the second operation's block overwrites
       any state of the first exchange.).
    "
    
    I believe this is true, but could you point to the section that states that? I didn't manage to find it while checking.

GS: Reference to section in RFC 7959 is now stated.

    * Section 2.1
    
    "   When receiving an Echo option in a request, the server MUST be able
       to verify when the Echo option value was generated.  This implies
       that the server MUST be able to verify that the Echo option value was
       generated by the server or some other party that the server trusts.
    "
    
    I don't necessarily agree with the "This implies", would just replace with "and". These are two parallel requirements.

GS: Agree. "This implies" is removed and sentence rephrased.
    
    * Section 2.1 : event-based freshness is mentioned, but no example of it is given of how that would work with the echo option, while the time based one is quite clear thanks to the example in Figure 2. Would it be possible to add one for an event based as well?

GS: Included.
    
    * Section 2.2
    
    "  The server MAY include the same Echo
       option value in several different responses and to different clients.
    "
    
    First, I was confused about this: I wondered if "different responses" referred to different response codes or actual responses. I think the answer is both is correct, but here you meant actual responses; this would need more discussion on why re-using the same option value is OK (i.e. highlight this is not a binding of response to next request, it just provides freshness). If that is already done elsewhere, I missed it.
    
GS: A following sentence is added to clarify this.

    * Section 2.2
    
    "...  the
       server MUST use the Echo option to verify that the request is fresh
       enough. 
    "
    
    A RECOMMENDED way of specifying "fresh enough" would be good, to help implementers. For time based, it would be enough to point to Figure 2. For event based, I don't know.
    
GS: Removed "enough".  Is that better?

    * Section 2.2
    
    "
    When used to serve freshness requirements (including client aliveness
       and state synchronizing), CoAP messages containing the Echo option
       MUST be integrity protected between the intended endpoints, e.g.
       using DTLS, TLS, or an OSCORE Inner option ([RFC8613]). 
    "
    
    Why do the both messages (request and response) need to be integrity protected? 

GS: Because we don’t have a mechanism for mixing integrity protected messages with non-integrity protected messages.

Also Inner option is not used for protecting CoAP messages, but options.

GS: Changed "CoAP messages containing the Echo option" to " the Echo option value"

 So I am a bit confused if here you meant option (which is a different discussion) or message. Also the following sentence ("when used to demonstrate...") belongs with the discussion about the option being Inner or Outer (Section 2.1 4th par), rather than here where we are talking about the message.
    
GS: Section 2.1 deals with OSCORE, but this statement is more general.  We think it is better to keep here: This paragraph describes when and when not to integrity protect.

    * Section 2.3
    
    "
    the authority of the property
    "
    
    can you explain this terminology?

GS: Yes, see if you agree.
    
----------------------------------------------------->8

    * Section 3.2
    
    "
      They can
       still be treated as independent messages by the server (e.g. when it
       sends 2.01/2.04 responses for every block), or initiate a new
       operation (overwriting kept context) when the later message carries
       Block1 number 0.
    "
    
    I have a hard time parsing/understanding this paragraph, particularly from "or initiate..."
    
    * Section 3.2
    
    I am not sure it is said anywhere what the server should do if it supports the Request-Tag option and it receives it in a non-blockwise message... I guess just discard, but it would be good to explicitely state.
    
    Also, could this option be used for something else? For example, I am thinking of Observe operations, re-registration... Does not need to be defined here, but if we MANDATE that it can only be used with blockwise, we are practically stopping any other use that might come up...
    
    * Section 3.3
    
    "
       Clients MUST NOT recycle a request tag unless the first operation has
       concluded. 
    "
    
    in the case where a client supports but does not use Request-Tag, this implies "concurrent block operations without Request-Tag are not allowed". If that is the case, I would like that to be stated explicitely.
    
    (also minor, I'd like to add "that support Request-Tag" after "Clients")
    
    * Section 3.3
    
    The last sentence gives some requirements about where the Request-Tag option can/musty be used per message. I felt the document was missing on usage requirements (e.g. if it is set, it MUST be set for all requests for a specific operation)
    
    * Section 3.4.2
    
    "
       When initializing a new block-wise operation, a client has to look at
       other active operations:
    
       o  If any of them is matchable to the new one, and the client neither
          wants to cancel the old one nor postpone the new one, it can pick
          a Request-Tag value that is not in use by the other matchable
          operations for the new operation.
    
       o  Otherwise, it can start the new operation without setting the
          Request-Tag option on it.
    "
    
    Does not cover the fact that Request-Tag can be omitted if that is a different from the other matchable operations. ("pick a new value" excludes it)
    
    * Section 3.4.3
    
    I believe this section is missing something on the lines of: 
    
    "Proxies that are aware of this specification can modify their implementation to simply forward block operations."
    
    Would you agree?
    
----------------------------------------------------->8

    * Section 6
    
    Missing references/terminology for monotonic and wall clock.

GS: Monotonic clock seems to be well defined. We removed the reference to wall clock.

    * Section 6.1
    
    cought a couple of normative text not being normative:
    
    "message must be discarded"
    
    "approach is recommended"
    
    "need to be blacklisted"
    
    Any particular reason why?

GS: The first and third are not normative -- we clarified that. The second one is so we made it upper case.

Thanks,
Göran