[apps-discuss] Fwd: review of draft-ietf-ecrit-lost-sync-12

[S Moonesamy <[e-mail]>]

I am forwarding the review performed by Martin Durst.  Please direct 
the follow-up to the apps-discuss mailing list.

Thanks,
S. Moonesamy

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 Durst
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.


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.

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.

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.

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?

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.

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.

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).

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.

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.

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.

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.

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)

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.


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...

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.

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

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

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.

6. Relax: Please add some introductory text.

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.

"If the originally mapping" ->
"If the original mapping"

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

9.1, Additional Information: Please add "None"

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.


Regards,    Martin.