Subject: Re: [xmpp] Robert Sparks' DISCUSS and COMMENT on draft-ietf-xmpp-3920bis-19

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

[ cc'ing [email-masked] ]

On 11/30/10 3:19 PM, Robert Sparks wrote:
> Robert Sparks has entered the following ballot position for 
> draft-ietf-xmpp-3920bis-19.
> 
> When responding, please keep the subject line intact and reply to
> all email addresses included in the To and CC lines. (Feel free to
> cut this introductory paragraph, however.)
> 
> Please refer to
> http://www.ietf.org/iesg/statement/discuss-criteria.html for more
> information about IESG DISCUSS and COMMENT positions.
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
>
>  This is a solid document. I have one major point (the first below)
> that I would like to discuss. I expect the remaining points to be
> easy to address (or be convinced to remove them after discussion).
> 
> 1) This draft requires that an intial stream be established before
> starting TLS negotiation, and requires (at SHOULD strength) that the
> stream element that is sent in the clear have a from attribute
> identifying the user if known. It doesn't feel right to require a 
> privacy leak like that. Would it make more sense to elide the from if
> the client knows it wants to try using TLS, or at least provide a way
> to use a from that couldn't be used by third parties to identify the
> user?

Yes, it would be better not to include the 'from' address on unprotected
initial stream headers, especially from clients (servers are typically,
but not always, more public). We could include a paragraph such as the
following at the beginning of Section 4.6.1:

      Security Note: Notwithstanding the recommendations in the
      remainder of this section, the initiating entity SHOULD NOT
      include a 'from' address on the initial stream header it sends
      before the confidentiality of the stream is protected via TLS or
      an equivalent security layer, since doing so would expose the
      initiating entity's identity to eavesdroppers.

> 2) Section 3.2.2's MAY seems to make [DNS-TXT] and [XEP-0156]
> normative.

I don't think we want to be encouraging folks to use TXT records. I'd
rather change "MAY" to "might" or something even weaker. I suggest:

   If connections via TCP are unsuccessful, the initiating entity might
   attempt to find and use alternative connection methods such as the
   HTTP binding (see [XEP-0124] and [XEP-0206]), which might be
   discovered using [DNS-TXT] records as described in [XEP-0156].

> 3) Section 4.4 - why is the specified behavior for the element that
> sends a closing stream tag defined as a SHOULD? When is it ok for the
> element to do something different, and what would it do?

I'd be fine with MUST here.

> 4) Section 8.3.1 - Does "generated stanza" mean "the stanza that
> caused an error to be generated"? 

That term is essentially defined in Section 8.1.3:

   The 'id' attribute is used by the entity that generates a stanza
   ("the originating entity") to track any response or error stanza that
   it might receive in relation to the generated stanza from another
   entity (such as an intermediate server or the intended recipient).

However, we could move the definition to the Terminology section:

   The term "originating entity" refers to the entity that first
   generates a stanza that is sent over an XMPP network (e.g., a
   connected client, an add-on service, or a server).  The term
   "generated stanza" refers to the stanza so generated.

> Can you provide more detail about
> why swapping the to and from are SHOULD (I can't figure out what in
> section 13.10 would have you not simply swap.)

There are two main considerations:

1. Privacy. An entity might not want to expose its full JID (or a server
might obscure them).

2. Malformed JIDs. If the originating entity or a routing server sends a
stanza with a 'to' address that is malformed, simply swapping the 'to'
and the 'from' would result in an error stanza that also contains a
malformed JID. But the entity that sends the error stanza might not want
to pollute the stream with such a malformed JID.

I suggest modifying rule #3 in Section 8.3.1 to reflect that:

   3.  If the generated stanza was <message/> or <presence/> and
       included an 'id' attribute then it is REQUIRED for the error
       stanza to also include an 'id' attribute.  If the generated
       stanza was <iq/> then the error stanza MUST include an 'id'
       attribute.  In all cases, the value of the 'id' attribute MUST
       match that of the generated stanza (or be empty if the generated
       stanza did not include an 'id' attribute).

> 5) page 137 (delivery related attack point 2) : this paragraph "How a
> server ... bare JID." does not parse.

Right, that should be:

   2.  How a server processes stanzas sent to the bare JID
       <localpart@domainpart> has implications for directory harvesting,
       because an attacker could determine whether an account exists if
       the server responds differently depending on whether there there
       is an account for a given bare JID.

> 6) Section 10.5.3.1 The second bullet seems to make [XMPP-IM] a
> normative reference. There were other places in the document where
> [XMPP-IM] seemed to be required reading in order to build a
> conformant implementation.

I'd be fine with making it a normative reference -- that's especially
relevant with regard to presence.

> 7) Section 10.5.3.2 : Why, for a message stanza, do you say SHOULD
> deliver to at least one matching resource? Why is this not a MUST,

There are XMPP extensions that can be used to block abusive senders,
filter messages based on various criteria, etc.

> and why do you allow something between one and all?

Many server implementations have advanced message delivery logic based
on things like presence priority (e.g., all resources with the same
priority might receive the message).

> 8) Section 13.10.1 : It's not clear what you really mean by "make the
> IP address or method of access public". How do you test this MUST
> NOT? Can you give examples of what you're worried a server
> implemenation  might do in error?

This text goes back to RFC 3920 or beyond -- the basic contrast was with
IRC, which exposes your IP address to the world when you join a channel.
Whether that MUST NOT is testable is another matter. Examples other than
groupchat would include service discovery (XEP-0030).

> 9) Section 13.12, item 4 : this 10000 is a magic number - please
> consider calling out that it was arbitrarily chosen. 

The WG had much discussion about the matter and settled on 10000.
Whether that counts as arbitrary is open to interpretation.

> More
> importantly, please add text disuading implementers from chosing to
> set a limit of 10000 bytes by default.

Implementations or deployments? Typically this is a deployment matter.
For example, the jabber.org service traditionally has had a maximum
stanza size of 64k.

I propose:

   4.  A server implementation SHOULD enable a server administrator to
       limit the size of stanzas it will accept from a connected client
       or peer server (where "size" is inclusive of all XML markup as
       defined in Section 2.4 of [XML], from the opening "<" character
       of the stanza to the closing ">" character).  A deployed server's
       maximum stanza size MUST NOT be smaller than 10000 bytes, which
       reflects a reasonable compromise between the benefits of
       expressiveness for originating entities and the costs of stanza
       processing for servers.  A server implementation SHOULD NOT
       blindly set 10000 bytes as the default but instead SHOULD enable
       server administrators to set their own limits....

> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
>  Please consider clarifying that you are not requiring elements to
> reject streams that use a prefix with the literal value "stream",
> only that you are allowing them to do so to allow legacy
> implementations to be compliant. It might help to be even more
> explicit that clients that chose not to use the literal "stream" 
> prefix may have interoperability problems with those legacy
> implementations. 

I propose:

      Interoperability Note: For historical reasons, an implementation
      MAY accept only the prefix 'stream' for the stream namespace
      (resulting in prefixed names such as <stream:stream> and <stream:
      features>); this specification retains that allowance from
      [RFC3920] for the purpose of backward compatibility.
      Implementations are advised that using a prefix other than
      'stream' for the stream namespace might result in interoperability
      problems.  If an entity receives a stream header with a stream
      namespace prefix it does not accept, it MUST return a stream error
      to the sending entity, which SHOULD be <bad-namespace-prefix/>
      (although some existing implementations send <bad-format/>
      instead).

> Also consider pointing to this exception to the
> general principal of allowing the sender to set the string it uses as
> a prefix label in the section on page 124 (where you note that
> "receiving entities MUST NOT depend on the prefix strings to have any
> particular value.")

   Routing entities (typically servers) SHOULD try to maintain prefixes
   when serializing XML stanzas for processing, but receiving entities
   MUST NOT depend on the prefix strings to have any particular value
   (the allowance for the 'stream' prefix, described under
   Section 4.7.4, is an exception to this rule).

> Some additional explanation of SRV's resolving to "." meaning "don't
> use SRV" would help. Why is this mechanism there?

RFC 2782 states:

        A Target of "." means that the service is decidedly not
        available at this domain.

Thus I propose:

   3.  If a response is received, it will contain one or more
       combinations of a port and FDQN, each of which is weighted and
       prioritized as described in [DNS-SRV].  However, if the result of
       the SRV lookup is a single resource record with a Target of ".",
       i.e. the root domain, then the initiating entity MUST abort SRV
       processing at this point because according to [DNS-SRV] such a
       Target "means that the service is decidedly not available at this
       domain".

> In 4.2.5 - Please consider a little more explanation on why an
> initiating element might send stanzas to itself?

There are no compelling use cases for this functionality, but discussion
in the WG led to the conclusion that it could do no harm since the
entity is just communicating with itself (not sending stanzas to other
entities). One possible use case is sending a service discovery query to
the account's bare JID in order to discover what other devices are
connected before binding a resource. But, as I say, such use cases are
not exactly compelling.

> Please consider adding forward pointers from the keep-alive sections
> (such as 4.5.1) to the requirement to not send whitespace during TLS
> and SASL negotiation. (5.3.3, 6.3.5)

Done:

   One common method for checking the TCP connection is to send a space
   character (U+0020) between XML stanzas, which is allowed for XML
   streams as described under Section 11.7; the sending of such a space
   character is properly called a "whitespace keepalive" (the term
   "whitespace ping" is often used, despite the fact that it is not a
   ping since no "pong" is possible).  However, this is not allowed
   during TLS negotiation or SASL negotiation, as described under
   Section 5.3.3 and Section 6.3.5.

> In 4.8.3.9 - "not a valid JID" may be ambiguous - is it intended to
> include "not well formed"?

No, that refers to 'from' attributes that violate the rules for XMPP
addresses. Suggested change:

   The data provided in a 'from' attribute either violates the rules for
   XMPP addresses [XMPP-ADDR] or does not match an authorized JID or
   validated domain as negotiated (1) between two servers using SASL or
   Server Dialback, or (2) between a client and a server via
   authentication and resource binding.

> In 5.3.5 - what does "blindly attempt" mean? why is the requirement
> around it SHOULD NOT? When is it OK to blindly attempt?

That issue was raised by Elwyn Davies in his gen-art review. As a
result, I proposed change it to:

   The third case has improved security characteristics when the TLS
   client (which might be an XMPP server) presents credentials to the
   TLS server.  If communicating such credentials to an unauthenticated
   TLS server might leak private information, it can be appropriate to
   complete TLS negotiation for the purpose of authenticating the TLS
   server to the TLS client and then attempt TLS renegotiation for the
   purpose of authenticating the TLS client to the TLS server.  However,
   this case is extremely rare because the credentials presented by an
   XMPP server or XMPP client acting as a TLS client are almost always
   public (i.e., a PKIX certificate) and therefore providing those
   credentials before authenticating the XMPP server acting as a TLS
   server would not in general leak private information.

   As a result, implementers are encouraged to carefully weigh the costs
   and benefits of TLS renegotiation before supporting it in their
   software, and XMPP entities that act as TLS clients are discouraged
   from attempting TLS renegotiation unless the certificate (or other
   credential information) sent during TLS negotiation is known to be
   private.

> In 5.4.3.3 - the "Implementation Note:" actually contains a normative
> requirement defining protocol behavior. Similarly, 8.3.3.14 has a
> Security Note with normative requirements. I strongly suggest moving
> actual protocol requirements out of those notes.

Based on the gen-art review by Elwyn Davies, the normative text was
copied to Section 4.2.3 on stream restarts:

   On successful negotiation of a feature that necessitates a stream
   restart, both parties MUST consider the previous stream to be
   replaced but MUST NOT send a closing </stream> tag and MUST NOT
   terminate the underlying TCP connection; instead, the parties MUST
   reuse the existing connection, which might be in a new state (e.g.,
   encrypted as a result of TLS negotiation)....

Thus I think the text in Section 5.4.3.3 and Section 6.4.6 can be
modified as follows:

   3.  The initiating entity MUST send a new initial stream header to
       the receiving entity over the encrypted connection (as specified
       under Section 4.2.3, the initiating entity MUST NOT send a
       closing </stream> tag before sending the new initial stream
       header, since the receiving entity and initiating entity MUST
       consider the original stream to be replaced upon success of the
       TLS negotiation).

and:

   Upon receiving the <success/> element, the initiating entity MUST
   initiate a new stream over the existing TCP connection by sending a
   new initial stream header to the receiving entity (as specified under
   Section 4.2.3, the initiating entity MUST NOT send a closing
   </stream> tag before sending the new initial stream header, since the
   receiving entity and initiating entity MUST consider the original
   stream to be replaced upon success of the SASL negotiation).

As to the general policy, I think it's fine to have normative text in a
security note (warning, it's about security!), but not in an
implementation note.

Would it help to change "Security Note" to "Security Warning" so readers
will pay attention? I rather like that...

> In 6.3.8 - Can you provide a reference to where is the behavior for
> acting on the behalf of another party is specified?

Section 2 of RFC 4422. This is not widely used in any XMPP software that
I know of, but I think that a few clients do or will soon support it for
administrative purposes. I suggest the following modified text
(including topics that arose in the gen-art review):

   An authorization identity is an optional identity included by the
   initiating entity to specify an identity to act as (see Section 2 of
   [SASL]).  In client-to-server streams it would most likely be used by
   an administrator to perform some management task on behalf of another
   user, whereas in server-to-server streams it would most likely be
   used to specify a particular add-on application at an XMPP service
   (e.g., a multi-user chat server at conference.example.com that is
   hosted by the example.com XMPP service).  If the initiating entity
   wishes to act on behalf of another entity and the selected SASL
   mechanism supports transmission of an authorization identity, the
   initiating entity MUST provide an authorization identity during SASL
   negotiation.  If the initiating entity does not wish to act on behalf
   of another entity, it MUST NOT provide an authorization identity.

> In 6.4.6  - Can you add some text justifying why the entity SHOULD
> terminate the connection attempt if the from identity and the SASL
> authentication identity do not match, and why this is a SHOULD not a
> MUST?

This also came up in the gen-art review. Traditionally we haven't placed
great confidence in the 'from' attribute of the stream header, because
most clients have not included it and, more importantly, it can be
tampered with if the stream is not protected. In addition, XMPP servers
have not particularly cared if the client was confused about its
identity when it opened the stream, as long as it is able to
authenticate -- e.g., the server is allowed to override the client's JID
for several reasons (the client did not correctly specify Unicode
characters in the localpart, the server simply generates the JID based
on information provided in an end-user certificate, etc.). Perhaps the
following text explains matters more clearly:

   Before considering the SASL handshake to be a success, if the
   initiating entity provided a 'from' attribute on an initial stream
   header that whose confidentiality and integrity was ensured via TLS
   or an equivalent security layer then the receiving entity SHOULD
   correlate the authentication identity resulting from the SASL
   negotiation with that 'from' address; if the two identities do not
   match then the receiving entity SHOULD terminate the connection
   attempt (however, the receiving entity might have legitimate reasons
   not to terminate the connection attempt, for example because it has
   overridden a connecting client's address to correct the JID format or
   assign a JID based on information presented in an end-user
   certificate).

> In 8.3.3.15 : This section doesn't provide a way to protect against
> redirect loops (such as see-other-host did). Should it?

Good idea. I propose:

      Security Note: An application receiving a stanza-level redirect
      SHOULD warn a human user of the redirection attempt and request
      approval before proceeding to communicate with the entity whose
      URI or IRI is contained in the XML character data of the
      <redirect/> element, because that entity might have a different
      identity or might enforce different security policies.  However,
      the end-to-end authentication or signing of XMPP stanzas could
      help to mitigate this risk, since it would enable the sender to
      determine if the entity to which it has been redirected has the
      same identity as the entity it originally attempted to contact.
      An application MAY have a policy of following redirects only if it
      has authenticated the receiving entity.  In addition, an
      application SHOULD abort the communication attempt after a certain
      number of successive redirects (e.g., at least 2 but no more than
      5).

> In 10.4.1 : should this "will" be a MUST?

I think it should be a SHOULD -- servers are allowed to use multiple
streams between themselves, and MUST would essentially forbid that.

> In 13.1 : has the group considered a "fail this stanze if it would go
> over a hop that isn't TLS protected" request extension?

The group is chartered to develop a solution for end-to-end encryption.
That should solve the problem pretty nicely. :)

But see also http://xmpp.org/extensions/xep-0219.html (not seriously
pursued, although there is occasional interest).

Thanks for the review.

Peter

-- 
Peter Saint-Andre
https://stpeter.im/