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.] 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. There are several more minor issues with these two being important: There are also 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. Apart form 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 signlaed. 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 enity 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 termionate 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 also doesn't match the definition of Bare JID in s10.5.3.2. Previously in this document this was known as the 'hostname'. I think this is probably an error. 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. 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 cotrrect. 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 defaults 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. s4.6.6: The entries for to/initiating to receiving and from/receiving to initiating are hostnames and not JIDs always. The other two to and from entries are hostnames rather than 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.] 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 be clarify the first SHOULD. Adding 'if possible' to route or deliver it to the intended recipient' would explain the second SHOULD. s8.2.2: The second and third SHOULDs probably need 'if possible' to explain them. 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? _______________________________________________ Gen-art mailing list [email protected] https://www.ietf.org/mailman/listinfo/gen-art
