Subject: [Gen-art] Gen-art review of draft-ietf-xmpp-3920bis-19 (updated)

From: Elwyn Davies <[email-masked]>
Date: Tue, 30 Nov 2010 10:12:00 +0000

I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at
< http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Please wait for direction from your document shepherd
or AD before posting a new version of the draft.

[Apologies to the author:  This document slipped through the gen-art
last call review net and so I only got to volunteer on this one 24 hours
ago.  Later... Having slept on the comments I see there are a couple that were 
items that I missed and should be modified or removed.  In particular 
about whitespace (where I missed the definition in terminology).
Further I have now read (very quickly) the address draft which clarifies
some matters on what are possible JIDs but doesn't totally solve some of the issues.]

Document: draft-ietf-xmpp-3920bis-19
Reviewer: Elwyn Davies
Review Date: 29 November 2010
IETF LC End Date: 
IESG Telechat date: 2 December 2010

Summary:
This is a very well written document that is mostly easy to read and
follow.  However I believe it is not (quite) ready for the IESG for the
following reasons:

It has (IMO) one major issue as an EXTENSIBLE system - the three level-1
stanzas are hard coded and it is not immediately possible to add new
types of level-1 stanza should this be thought useful in future.  This is, 
I guess a philosophical issue, and as a -bis document it probably isn't 
going to get changed at this stage, so this need not concern us deeply.  
However I think that it might be useful to provide an explanation of
the general semantics of the three types of stanza early on in the document -
not having participated in XMPP, it wasn't until I stepped back from
the details of the document (as well as finding out what IQ stood for
when I reached section 8) that I realized that the three existing 
categories of stanza are  effectively semantic types that are appropriate
for the three classes of message for which they are named, but could 
be used for other sorts of function - this might avoid people thinking that
they need extra stanza types and complaining that it isn't extensible :-( . 

There are several more minor issues with these three being important:

There are a largish number of SHOULD requirements where the
alternative is not discussed or specified.

There seems to be a lack of clarity or definiteness about the effects of
support of SASL being mandatory.  There are a number of places in the
document where the text is written as if SASL were not mandatory, and
others where the reverse is true but the surrounding text doesn't seem
to think it is mandatory..  All these places need to be looked at
critically to ensure that the wording is appropriate for SASL being
mandatory and that the whole is consistent.

The separation of the address format into a separate document means
that there should be some information about the terms that are 
imported in the terminology section.  The early sections on stream setup 
talking about 'from' and 'to' addresses have some difficulties with the 
use of hostname versus the term used in the JID/addressing draft of 
domainname.  Almost the whole of this document implies that a JID *necessarily*
has a localpart, whereas the definition in the addressing draft allows
it to be just a bare (or mere!) domainname.  The couple of places where 
JIDs may or in some cases, must not have a localpart confuse matters because
they don't describe the hostname only forms as JIDs in the main text, but they 
are suddenly transformed into JIDs in summaries (in particular the summary table in s4.6.6
where it turns out that, yes, all the 'to' and 'from' addresses are actually
JIDs but (1) only restricted forms are allowed for some interactions
and (2) these restricted forms were carefully *not* described as JIDs earlier in s4.6).
Again, the term 'bare JID' is almost everywhere shown as <localpart@domainname>
except for in s8.1.2.1.  This term is not actually defined in the addressing draft
and appears only in one place as a term for <localpart@domainname>.   

Apart from these there are a number of other minor issues and various
nits/editorial issues.

Caveat: As usual I haven't tried to verify the XML in Appendix A, and my
scan of the examples in s9 was very cursory.

Major issues:
s4.1, Definition of XML Stanza: Given that xmpp is supposed to be
extensible, is there a really good reason why the first-level
stanza-defining elements (message, presence, iq) are hard-coded into the
specification rather than being possibly extensible? There is already a
stream features mechanism where additional first-level items could be
signaled.  This might be alleviated by describing the semantic types of
message that are covered by the available stanzas - the naming covers up
that they are quite general types of message that could be used for other 
purposes.

Minor issues:
s2.5, first bullet describing server respnsibilities:  The term 'local
clients' is slightly confusing.  My immediate understanding of 'local'
would be something attached to the server from the local network or
machine.  In this case it is presumably intended to imply clients that
have a session connected to this server rather than via some other
server but may of course be very remote as regards physical and network
location.  Maybe 'attached clients' or 'associated clients' (this term
is used elsewhere in the document) might be clearer, or maybe this just
needs a terminology definition for how 'local' is used - there is the
use of 'local entities' previously in s2.5 but this didn't jar so much.

s2.5, last para: Does the use of 'local services' imply anything about
the disposition of the code implementing the service? This use of
'local' may or may not conflict with the implications of 'local
mentioned in the last comment.  Terminology needs to be made a little
more rigorous.

s4.2.2: Towards the end of this section, there are two statements
stating 'the initiating entity is cleared to send XML stanzas'. Should
this be qualified with 'unless a stream restart is required as a result
of the feature negotiation' (at least at the first appearance)?  This
probably depends on whether voluntary-to-negotiate features can require
stream restarts if agreed but not otherwise.

s4.2.3 and s4.4: In s4.2.3, if a stream has to be restarted, the parties
are supposed to consider the the stream 'replaced'.  This presumably
means that the old stream is closed.  It is not absolutely clear whether
this requires that a <stream/> is exchanged or definitely not exchanged
in this case. [OK: This is cleared up for the specific TLS case at item 
3 in s5.4.3.3 and for SASL in s6.4.6 - a general note at s4.2.3 would be
appropriate since these are not necessarily the only restart cases.]

s4.2.6, paras 2 and 3: The first parts of these paragraphs are written
as if SASL autrhentication was mandatory (which it is) and is the only
possibility (which it might not be).  Para 2 (the client-to-server case)
does not appear to allow for any other options.  Para 3, OTOH, goes on
to deal with Server Dialback but the first part still reads as if SASL
was the only option.  Both paras need to be rewritten to cater for other
options (and the version 0.9 case?). 

s4.6.1, para 6: 
  'For initial stream headers in server-to-server communication, a
   server MUST include the 'from' attribute and MUST set the value to
   the domainpart of the 'from' attribute of the stanza that caused the
   stream to be established...':  Is it absolutely always the case that 
a server-to-server connection would be established because some stanza 
came in (presumably from an attached client)?  Could this be indirectly
through some local service requiring a connection for any other reason?
It is clear why the server needs to specify 'from' but the constraint
on the origin seems overly restrictive.

s5.3.5: 'Because it is relatively inexpensive to establish streams in XMPP,
   for the first two cases it is preferable to use an XMPP stream reset
   (as described under Section 4.8.3.16) instead of performing TLS
   renegotiation.':
Is this always true?  Probably so for simple instant messaging but if 
the client is using some other services provided by the server, this may 
an unwarranted generalization.  I am not sufficiently expert in either
XMPP or TLS renegotiation to know what the trade-offs would be.

s5.3.5: 'However, the third case is sufficiently rare that XMPP entities
   SHOULD NOT blindly attempt TLS renegotiation.' (the third case is 
separating client authentication from server authentication):
I don't understand what is being said here.  Is this an opinion that the
risk of client credential leakage is sufficiently low that the expense of 
two stage negotiation is not really worth it?  Or is this something that 
should be controlled by a configurable client policy?  Or is this an
implementation choice?  Some clarification would help.

s5.4.3.1, rule 3: Why should the receiving entity not request a certificate?
Why would the initiating entity not send its certificate if it had one? (Is 
this related to the renegotiation issue from the last comment?)

s5.4.3.1, rule 4: How should the receiving entity make the choice given the 
'to' attribute (presumably some field in the certificate)? What happens if
it decides not to follow this should?

s5.4.3.2, para 2: The comment about not sending </stream> relates to the 
comment on s4.3.2 above. However, the reasoning for not sending </stream>
when the TCP connection is closed due to TLS failure differs from reasoning
given elsewhere (e.g., s5.3.5, para 8 - here the error is said to be at the
TLS layer and not the XMPP layer so that it is not appropriate to send </stream>).
A consistemt story is needed here.  

s5.4.3.3, item 5:  Given that SASL support is mandatory, why is offering the SASL 
feature not a MUST?  Is this to do with possible future alternatives?

s6.3.8, para 1: I suspect the two SHOULDs in this para ought to be MUSTs.  What
other options does initiating entity have if it does/does not want to act on
behalf of another entity and SASL is being used?  However maybe all this should be 
qualified by 'If SASL is being used...'

s6.4.1, para 5: 'If the receiving entity supports SASL, the stream features
   SHOULD include an advertisement for support of SASL negotiation,...':
s6.2 says that XMPP entities MUST support SASL. I think that this sentence 
probably needs to be turned round making it something like 'Unless support for SASL
negotiation is only allowed once STARTLS negotiation has been completed, the 
stream features MUST include an advertisement for the support of SASL.'  Aside from
the STARTLS case (or equivalent cases with future secure stream mechanisms), what 
happens if the SASL advertisement is not made?  

s6.4.2, last para: '... the server SHOULD discard the ongoing handshake...':
What happens if the server (receiving entity) chooses not to
comply with the new proposed SASL mechanism choice?

s6.4.5, paras 4 and 6:  'SHOULD close the stream':  What would the entity do instead?

s6.4.6, para 1: The two SHOULDs in this paragraph ought to be MUSTs.  (if there is 
a 'from' address then the receiving entity MUST do the correlation - what else 
could it do?  If it does the match and they don't match, what would do other than
terminate the attempt?)

s8.1.2.1, item 2; also s10.5.1 and s10.5.2:  s8.1.2.1, item 2 appears to introduce 
the idea that the <domainname> is the 'bare JID' of a server.  This concept does 
not appear anywhere else in this document (or from a brief Google search elsewhere).  
It was not used in RFC 3920.  It appears that the term is used once but not explicitly 
defined in the addressing draft - there it is used for the <localpart@domainname> form.
It also doesn't match the 'definition' of Bare JID in
s10.5.3.2.  Previously in this document this was known as the 'hostname'.  However 
s10.5.1 refers to a 'Mere Domain' as a JID and s10.5.2 to a <domain/resourcepart> as a 
JID.  This seems rather inconsistent although technically it is covered by the addressing 
draft.  This all probably be helped by summarizing the terminology used in the addressing
draft in the terminology section, and defining the term 'bare JID' properly (in the addressing
draft). 

s8.1.2.1, items 2 and 4:  As written, both items refer to stanzas 'generated by the
server'.  I take it that the distinction is that item 2 refers to messages from the 
server that are not generated by the server as a proxy for the client account  but 
are some sort of generic control message or the like, whereas item 4 refers to server
generated messages that are generated by the server as such a proxy for the client 
account.  The wording is somewhat confusing at present.  Presumably the lack of a 
'from' attribute is intended to distinguish item 1 messages from item 4 messages -
this could be spelt out more positively if this is correct.

s13.7.1.1. Both instances of item 4: 'The signatureAlgorithm SHOULD be the 
PKCS #1 version 1.5 signature algorithm with SHA-256 as defined by [PKIX-ALGO].':
What happens if it isn't? 

s13.7.1.1: First item 5: 'The certificate SHOULD include an Authority 
Information Access (AIA) extension that specifies the address of an Online
Certificate Status Protocol [OCSP] responder.'
What happens if it doesn't?

s13.9.5 and s15: The requirement for support of TLS-NEG if TLS Renegotiation is to be allowed 
should be mentioned here.  This is also a conformance requirement.

s14: I seem to remember that giving a working group as a registrant contact
for an URI is deprecated as it is likely to go out of existence fairly shortly.

Nits/editorial comments:

s3.2.1, bullet labeled '5': 'The initiating entity uses the IP
address(es) from the first successfully resolved hostname...': 'first'
implies that it depends on how fast the answers come back.  Presumably
what is wanted is the IP address of the highest priority SRV record that
is successfully resolved, maybe subject to some local policies?

s3.2.1,bullets labelled '3' and '8':  Bullet 3 suggests that the s3.2.2
fallaback process 'SHOULD' be used whereas bullet 8 has an implicit MAY
(that might be better if was explicit).  Why the distinction?
[Presumably, having read on, there is little point in trying the first
possibility in the fallback if the initiating entity has already tried
the default port, but the alternatives might still work.] Section 3.2.2
could mention that there is no point in trying the default ports if they
were already tried under s3.2.1.

s3.2.2: A pointer to s15.7 where the default port numbers are registered
would be useful.

s3.2.4: Where is the appropriate XML stanza for defining add-on services
defined?

s4.1, Definition of XML Stanza: The term/title 'iq'/'IQ' is not expanded
until s8.2.3 in this document where it is expanded as info/query.  It
would be useful to mention this here (first occurrence) and indeed maybe
to put all three of message, presence and iq into the terminology
section.  The usage of IQ stanzas would then be clearer and their use
in examples would be more transparent - currently they just appear with
no introduction (e.g, in s4.1 and then in s4.8.3.11).

s4.1, final set of bullets: Adding section references to the five topics
would be helpful.

s4.2.2, first para: A pointer to s4.6.5 which is very prescriptive about
how to do the version comparison would be useful.

Figure 3: It would help if the figure could be made one or two lines
shorter so that the title appears on the same page.  (e.g., move the
bottom line 'yes' and 'no' above the transition lines that they apply
to, remove one line below the 'process complete' box.)

s4.6.1, para 8 (also s4.6.2):  Although it may seem a bit pedantic, it
is probably desirable to clarify that by 'hostname' the document means a
fully qualified DNS name.  The term is used ambiguously by many
operating systems. (The document could also stick to the 'domainname' term
used in the addressing draft.)

s4.6.6: The entries for to/initiating to receiving and from/receiving to
initiating are hostnames which are rstricted forms of JID.  The other two to and from
entries are hostnames rather than arbitrary JIDs for the server-to-server case.
[But the comments on s8.1.2.1 item 2 and ss10.5.1/2 under minor issues
may have some bearing here.]

[Comment withdrawn:  I missed the definition of whitespace in the terminology
section:
s5.3.3 and s6.3.5: Does 'whitespace' mean all the 'white space' items
referred to in Section 2.10 of the current version of the XML spec
(i.e., including line separators).  The term whitespace is used earlier
whan referring just to spaces used in the context of keep alives.  [It
turns out much later that s11.7 clarifies this but maybe doesn't totally
resolve s2.10.] Note that the XML specification uses 'white space'
rather than 'whitespace'.]

s6.4., last para:  For consistency and avoiding confusion between XMPP
server and SASL server: s/server/receiving entity/

s8.2.1: A reference to s10.3.1, where the meaning of a message with 'to'
attribute is explained, would clarify the first SHOULD.  Adding 'if
possible' to route or deliver it to the intended recipient' would
explain the second SHOULD which shoudl probably be owercase 'should' as this
isn't a conformance issue but a matter of whether the connections are
available.

s8.2.2: The second and third SHOULDs probably need 'if possible' to
explain them and they should probably be lowercase 'should' as this
isn't a conformance issue but a matter of whether the connections are
available.

s8.3.2/s8.3.3:  All the error specifications in s8.3.3 contain a SHOULD
specification of the error type recommended.  A note in s8.3.2 or s8.3.3
that the values of the error type given in s8.3.3 are the usual expected
error type, but that there are conceivably circumstances when some other
value would be more appropriate. (the only one I copuld think of so far,
apart from the remote-server-timeout case already in the draft, was
associated with payment-required where a modify or cancel error type
might be appropriate if the reason was not that the requestor wasn't 
actually authorized but that it didn't have enough credit.)

s11.4: 'A client SHOULD NOT rely on the ability to send data that does
not conform to the schemas':  This is not controllable by the protocol.
s/SHOULD NOT/should not/

s11.4: What should the client do if it doesn't ignore non-conformant
elements?

s8.3.3.15, Security Note: s/communicated/communicate/ 
 
s14. The IANA Considerations mostly duplicate those in RFC 3920.  Is the
intention that IANA should update the references to RFC3920 to point at
this document?  If so some statement should be made about this. 

s14: Presumably 'XXXX' is to be replaced by the RFC nnnn identifier for
the eventual RFC.  This isn't stated.  Should the various Namespace
names refer to document sections?

---

From: Gonzalo Camarillo <[email-masked]>
Date: Tue, 30 Nov 2010 12:16:54 +0200

Hi Elwyn,

thanks a lot for taking the time for reviewing this (long) document!
Peter will respond to your comments as soon as he has a chance.

By the way, I intend to get the IESG to review another (long) XMPP
document in the near future:

https://datatracker.ietf.org/doc/draft-ietf-xmpp-3921bis/?include_text=1

Now that you are familiar with XMPP, you may want to ask Mary to assign
that one to you as well ;o)

Cheers,

Gonzalo

---

From: Elwyn Davies <[email-masked]>
Date: Tue, 30 Nov 2010 23:01:49 +0000

Hi.

Ah!  My reply to Russ' email and this just crossed in the aether.

Yes these seem to be an excellent solution, especially combined with the
move from hostname to FQDN etc noted previously.

There are a couple of points later where I complained about some SHOULDS
and realized afterwards that they are not actually protocol
specifications but hopes about communications:

> explain the second SHOULD which should probably be lowercase 'should'
> as this

Full reeponses tomorrow.. last night was late enough :-)

Regards,
Elwyn

On Tue, 2010-11-30 at 15:45 -0700, Peter Saint-Andre wrote:
> Hi Elwyn, unfortunately I replied to your original message, not the updated
> one. I've run a diff between the two and here reply only to the points
> raised in your updated text.
> 
> On 11/30/10 3:12 AM, "Elwyn Davies" <[email-masked]> wrote:
> 
> How about if we add the following paragraph right after the definition of
> "XML Stanza" in Section 4.1?
> 
>    There are three kinds of stanzas: message, presence, and IQ (short
>    for "Info/Query").  These stanza types provide three different
>    communication primitives: a "push" mechanism for generalized
>    messaging, a specialized "publish-subscribe" mechanism for
>    broadcasting information about network availability, and a "request-
>    response" mechanism for more structured exchanges of data (similar to
>    [HTTP]).  Further explanation is provided under Section 8.2.1,
>    Section 8.2.2, and Section 8.2.3, respectively.
> 
> <snip/>
> 
> I suggest adding the following definitions to the Terminology section:
> 
>    The terms "localpart", "domainpart", and "resourcepart" are defined
>    in [XMPP-ADDR].
> 
>    The term "bare JID" refers to an XMPP address of the form
>    <localpart@domainpart> (for an account at a server) or of the form
>    <domainpart> (for a server).
> 
>    The term "full JID" refers to an XMPP address of the form
>    <localpart@domainpart/resourcepart> (for a particular authorized
>    client or device associated with an account) or of the form
>    <domainpart/resourcepart> (for a particular resource or script
>    associated with a server).
> 
> Let me know if I've missed anything from your updated review.
> 
> Thanks again.
> 
> Peter
>

---

From: Peter Saint-Andre <[email-masked]>
Date: Tue, 30 Nov 2010 15:45:26 -0700

<snip/>

Thanks again.

Peter