Subject: Re: [Ecrit] review of draft-ietf-ecrit-lost-sync-12

From: Hannes Tschofenig <[email-masked]>
Date: Fri, 13 Jan 2012 11:17:10 +0200

Hi Martin, 

thanks for your review. 

On Sep 22, 2011, at 2:48 PM, Martin J. Dürst wrote:

> I have been selected as the Applications Area Review Team reviewer for this draft (for background on apps-review, please see http://www.apps.ietf.org/content/applications-area-review-team).
> 
> Document: draft-ietf-ecrit-lost-sync-12
> Reviewer: Martin Dürst
> Review Date: September 22, 2011
> 
> Review Summary:
> 
> This draft should be clarified in various places before being published.
> There is also a deeper design issue that I will mention upfront and that hopefully will be taken to heart for future work.
> 
> Document Summary:
> 
> The document defines a 'protocol' (actually a number of XML messages, under a common Media type, carried over http(s)) for synchronizing Location-to-Service Translation (LoST) service boundaries and mapping elements.
> 
> 
> Design Issue:
> 
> It seems still in fashion to define a 'protocol' independent of underlying transport. This is preferable if there is a truely expected need for multiple transports, and if there is a large abstraction gap between the newly defined 'protocol' and the potential underlying transport mechanism. However, in the case at hand, only one transport (HTTP, including its secured equivalent HTTPS) is used, and the abstraction gap is extremely low.
> 
> Once one has understood REST, it's very easy to see a set of mappings as a resource, a <getMappingRequest> as a simple HTTP GET and a <pushMappingRequest> as a PUT. A <getMappingRequest> with <mapping-fingerprint> element would require some thought, although there are several possible solutions.
> 
> 
I certainly agree with you we could have designed the protocol differently. A design based on REST would have been one possible option. I don't think it would have made any difference to the result but it is certainly more fashionable these days. 

> Major Issues (necessary clarifications):
> 
> Independence of LoST: The Introduction says:
>   In any case, this document can also be
>   used without the LoST protocol even though the format of the
>   <mapping> element is re-used from the LoST specification.
> What does "can be used without the LoST protocol" mean? If it said something like "this specification is independent of the LoST protocol specification except in that it reuses the <mapping> element (and some error elements) from that specification", that would be easier to understand.

Good question. 

What I wanted to express in this rather complicated way is the following:

There is the LoST protocol, which the group had developed a few years ago. It runs between a LoST client and a LoST server. 
Then, there is an architecture document that has to be understood in the context of the IETF emergency services architecture. This architecture document in a nutshell says: Here is an architecture similar to DNS. Use LoST as a protocol between the nodes and a synchronization protocol between the top-level servers. This document is this synchronization protocol. 
The synchronization protocol is not LoST but a new protocol. There is, however, a component (namely these mappings) that is defined in LoST that can (and is) reused in this protocol.

For a definition: 

   mapping:  A mapping is a short-hand for 'mapping from a location
      object to either another mapping server or the desired service
      URLs'.

Any idea how I could phrase this a bit better to get my message across. 
(One option is obviously to avoid mentioning the fact that the mapping element is re-used from LoST; an implementer will figure it out anyway.)

> 
> Namespaces: It is overkill to define new namespaces for each spec. It's a well-known secret in the XML community that it's easily possible to add a number of new elements to an existing namespace. This would be very appropriate in this case, because the number of reused elements and attributes is quite large, and the number of new elements is low. This would simplify most of the examples.

I guess you refer to the namespace registration in the IANA consideration section, namely urn:ietf:params:xml:ns:lostsync1

In the RAI area we have (to my knowledge) always created new namespaces for new schemas. But I am pleased to hear that this is not needed. 

Could you explain what we should be doing instead? 

> 
> 4.2 Behavior of the LoST Sync Destination: This needs various clarifications.
> 
> The first paragraph uses 'replace', the next one after the conditions uses 'update'. Is this the same? Then use the same word. Also, it says:
>   If the received mapping M' does not update any existing mapping M
>   then it MUST be added to the local cache as an independent mapping.
> If I apply this to a mapping with conditions 1. and 2. being true, but condition 3. being false, then this means that older mappings of the same source and sourceID MUST be added to the local cache. Surely this must be wrong.

Good catch. I updated the text.

> Also, it says that an emply <mapping> element means that a "corresponding" mapping has to be determined based on source, sourceID, and lastUpdated. The "lastUpdated" here is ambiguous and could lead to differing implementations. Does lastUpdated have to match with equality? Does it have to be (the same or?) newer than the one being cached?

I have changed that sentence so that only source and sourceID have to match. It may indeed the possible that someone understands the sentence in a way that the indicated mapping is removed from the cache but then the previously received one is now active. That was, however, not my intention to implement a sort of "rollback". 

> 
> Also, all the examples (Figs. 10/12) also contain an 'expires' attribute. Is that necessary? Does it have to be the same as the lastUpdated attribute? Please specify.

A mapping element must contain an expires attributes and it contains the absolute time at which the mapping becomes invalid.

It has nothing to do with the lastUpdated attribute.

> 
> Further, we have: "The <notDeleted> element MAY carry a <message> element": What kind of message element? If this is from RFC 5222, then please say so. In Fig. 12, there is a 'message' attribute, is that what is meant? If yes, then please fix the prose (element-> attribute). If no, then point out the difference.

I added a note that this is a message that provides information for debugging purposes. This element is not from RFC 5222. 

The 'message' is an attribute. Corrected. 
 
> 
> Fig. 12: This uses an attribute value for human-readable text. This is considered bad practice, because it forecloses the use of further markup in case this is considered necessary (can often become the case for internationalization reasons).

While this is human readable text I now state that it is only used for debugging purposes.
What would you use instead? 

> 
> in Transport, it says:
>   The HTTP request MUST use the
>   Cache-Control response directive "no-cache" to (*) HTTP-level caching
> At (*), a verb such as 'suppress' or 'avoid' or some such seems to be missing. Please clarify.

I changed that section based on Julian Reschke's review. 

> 
> In Operational Considerations, the use of XML Digital Signatures is mentioned. With just the text that is there, it seems impossible to guess what actually should be done. For example, how is authorization verified? Also, why only sign the first time? Wouldn't it be easier/more straightforward to just sign every time?
> This looks like either an afterthought (in which case the MUST isn't appropriate, because it doesn't have anything to do with the protocol per se) or some missing context (e.g. from LoST itself) in which case there should be some reference or so to clarify the context.
> 
I added text to clarify the issues you raised. 

> Security Considerations: In particular the last two sentences read like security considerations not for synchronization, but for the underlying PSAP data and LoST itself. Probably similar considerations have already been given in RFC 5222 or elsewhere, in which case they should be included by reference.
> 
Changed the wording. 

> 9.1: "8-bit characters": This term is highly inappropriate, because it's actually 8-bit bytes, not characters. Please just point to RFC 3023.
> 
Changed it. 

> 9.1, security considerations: Replace the current text with a pointer to the security considerations in the spec (text similar to the "Published Specification" item)
> 
Done. 

> p. 25: The namespace document has the namespace as
> urn:ietf:params:xml:ns:lost1:sync, while in the examples it is
> urn:ietf:params:xml:ns:lostsync1. Please fix and check carefully.
> 
Thanks. 

> 
> Minor Issues (mainly editorial):
> 
> Abstract and Introduction: add a comma at end of clause
>   The main data structure, the <mapping> element, used for
>   encapsulating information about service boundaries*,* is defined...
> 
Done. 

> p.4: "we call them Austria and Finland": "we call them" makes sense with fictive country names, but in this case "for example Austria and Finland" makes more sense.

OK. 

> 
> p. 8: "Node B issuing the first request and plays"
>   -> "Node B is issuing the first request and plays"

Ok.
> 
> p. 12: There should be a short explanation of the <mapping-fingerprint> element in the prose.
> 
Added text. 

> 4.3: The first two example given with bullet points add a linebreak after the first attribute, it would be easier if they also added a linebreak after the second one.
> 
Done. 

> 6. Relax: Please add some introductory text.
> 
What text are you thinking about?

> 7. Operational Considerations:
>   When B obtained this new mapping it would find out that it has to
>   distribute it to its peer C. C would also want to distribute the
>   mapping to B (and vice versa).
> Either remove the 'vice versa' or the sentence before it.

Ok
> 
> "If the originally mapping" ->
> "If the original mapping"
> 
Thanks. 

> Security Considerations: Split the long paragraph into shorter pieces for easier readability.
> 
OK. 

> 9.1, Additional Information: Please add "None"
> 
Done. 

> 10. In earlier times, the shepherd was called the PROTO shepherd. But these days, PROTO isn't used anymore as far as I know. Please remove.

Done. 

Ciao
Hannes

> Regards,    Martin.