Subject: Re: [xmpp] SecDir review of draft-ietf-xmpp-3920bis-17

From: Peter Saint-Andre <[email-masked]>
Date: Thu, 28 Oct 2010 22:01:15 -0600

Thanks for your careful and thorough review. To maintain forward
momentum, this is Part 1 of my reply, up through the end of Section 4. I
shall endeavor to reply regarding the remainder of your review in the
next 24-48 hours.

[Copying [email-masked] to keep the WG in the loop...]

On 10/28/10 4:28 AM, Yaron Sheffer wrote:
> I have reviewed this document as part of the security directorate's
> ongoing effort to review all IETF documents being processed by the IESG.
> These comments were written primarily for the benefit of the security
> area directors.  Document editors and WG chairs should treat these
> comments just like any other last call comments.
> 
> The document updates RFC 3920, which is the definition of the core XMPP
> real-time messaging protocol. It should be noted the instant messaging
> and presence are layered on top of this protocol, and specified separately.
> 
> General
> 
> The document is initially intimidating, because of its length. But it is
> extremely well written (for which I would like to thank the editor) and
> well organized. So overall, a good read.

Thanks.

> I have not found anything that I consider a glaring security hole. But
> this is a layered security architecture (application layer == XMPP core
> == SASL == TLS) which is not easy to do right. Hence the large number of
> comments and questions below.
> 
> I also appreciate the open discussion of the existing implementations
> and their security issues (e.g. "server dialback", shiver). I hope this
> document results in a security improvement in real deployments.

We hope so!

> Detailed Comments
> 
> Note: these comments are based on rev -17 of the draft. This only
> matters as far as section numbers, with the only security-relevant
> change in -18 being a useful note on end-to-end protection.
> 
> - 1.3: Implementation note: I suggest adding something like "Solutions
> specified in this document offer a significantly better level of security."

True. In my working I've added this sentence (including a forward
reference to the discussion of strong security):

   The solutions specified in this document offer a significantly
   stronger level of security (see also Section 13.6).

> - 3.3: why do we not recommend to use TLS (stateless) session resumption
> for reconnection?

Your suggestion is a good one.

The XMPP WG and broader XMPP developer community have had some
discussions about methods for "quick reconnect". These would involve the
use of TLS session resumption, pipelining of requests from the
initiating entity to the receiving entity, perhaps some SASL tricks (see
for example draft-cridland-sasl-tls-sessions-00), etc. Those discussions
are rather preliminary at this time, however I think we can safely
recommend the use TLS session resumption.

I propose that we add the following paragraph at the end of Section 3.3:

   It is RECOMMENDED to make use of TLS session resumption [TLS-RESUME]
   when reconnecting.  A future version of this document, or a separate
   specification, might provide more detailed guidelines regarding
   methods for speeding the reconnection process.

> - 4.2.5 the access control rule at the bottom of this section makes
> sense. Why why does it explicitly not apply to elements other than XML
> stanzas? Are there cases where you negotiate TLS with someone other than
> the server? You can even imagine weird tunneling attacks using this
> "feature".
> - 4.2.5: "other than itself" - am I really allowed to send "message"
> stanzas from one resource of a JID to another resource of the same JID,
> before feature negotiation and before TLS negotiation?

I now see that the following paragraph is poorly phrased:

   The initiating entity MUST NOT attempt to send XML stanzas
   (Section 8) to entities other than itself (i.e., the client's
   connected resource or any other authenticated resource of the
   client's account) or the server to which it is connected until stream
   negotiation has been completed.  Even if the initiating entity does
   attempt to do so, the receiving entity MUST NOT accept such stanzas
   and MUST return a <not-authorized/> stream error.  This rule applies
   to XML stanzas only (i.e., <message/>, <presence/>, and <iq/>
   elements qualified by the content namespace) and not to XML elements
   used for stream negotiation (e.g., elements used to complete TLS
   negotiation (Section 5) or SASL negotiation (Section 6)).

There are several points that could be clarified here:

1. You can't send non-stanzas to other entities because (as XMPP is
currently defined) only stanzas have 'to' addresses. The XML elements
that we use for things like TLS negotiation and SASL negotiation are not
addressable to other entities (e.g., a remote server or client) but only
to the server to which a client has connected, so we don't need to
mention them in this paragraph.

2. However, that's contingent on how XMPP is currently defined, i.e., we
assume that there are only stanzas and stream-negotiation elements. This
leaves a bit of a loophole for elements that don't fit in either of
those buckets, e.g., sending <foo:bar to='baz'/> from the client. Is the
server supposed to route or deliver that element to baz? I would say no.
Someone could write a server that allows the communication of arbitrary
XML elements over XML streams, but that's not what 3920bis defines. So I
think we need to tighten that up, although this paragraph is not the
place to do so (I think it belongs in Section 4.7.3 "Other Namespaces").

3. Yes, a server might allow a client to communicate with its bare JID
or other full JIDs after it has authenticated but before it has bound a
resource (e.g., to send a service discovery request so it can discover
which other resources are currently connected). This is not an important
"feature" by any means, but as far as I can see it does not open any
security holes as long as the server requires authentication first. Thus
the phrase "until stream negotiation has been completed" in the quoted
paragraph is misleading.

Taking these considerations together, I propose the following modified
paragraph at the end of Section 4.2.5:

   An initiating entity MUST NOT attempt to send data to entities
   other than itself (i.e., the bare JID of the user's account) or the
   server to which it has connected until it has authenticated with the
   receiving entity.  If the initiating entity attempts to do so, the
   receiving entity MUST NOT accept such data and MUST close the stream
   with a <not-authorized/> stream error.

Then I propose that we modify Section 4.7.4 as follows:

###

4.7.3.  Other Namespaces

   Either party to a stream MAY send data qualified by namespaces other
   than the content namespace and the streams namespace.  For example,
   this is how data related to TLS negotiation and SASL negotiation are
   exchanged, as well as XMPP extensions such as Stream Management
   [XEP-0198] and Server Dialback [XEP-0220].  (For historical reasons,
   some server implementations expect a declaration of the 'jabber:
   server:dialback' namespace on server-to-server streams, as explained
   in [XEP-0220].)

   However, an XMPP server MUST NOT route or deliver data received over
   an input stream if that data is (a) qualified by another namespace
   and (b) addressed to an entity other than the server, unless the
   other party to the output stream over which the server would send the
   data has explicitly negotiated or advertised support for receiving
   arbitrary data from the server.  This rule is included because XMPP
   is designed for the exchange of XML stanzas (not arbitrary XML data),
   and because allowing an entity to send arbitrary data to other
   entities could significantly increase the potential for exchanging
   malicious information.  As an example of this rule, the server
   hosting the example.net domain would not route the following first-
   level XML element from <[email-masked]> to <[email-masked]>:

     <ns1:foo xmlns:ns1='http://example.org/ns1'
              from=[email-masked]/resource1'
              to=[email-masked]'>
       <ns1:bar/>
     </ns1:foo>

   This rule also applies to first-level elements that look like stanzas
   but that are improperly namespaced and therefore really are not
   stanzas at all (see also Section 4.7.4), for example:

     <ns2:message xmlns:pre='http://example.org/ns2'
                  from=[email-masked]/resource1'
                  to=[email-masked]'>
       <body>hi</body>
     </ns2:message>

   Upon receiving arbitrary first-level XML elements over an input
   stream, a server MUST either ignore the data or return a stream
   error, which SHOULD be <unsupported-stanza-type/>.

###

> - 4.2.6: why do we even allow non-SASL protected server-to-server
> communication?

Because, in practice, most existing services end up using Server
Dialback, not TLS + SASL EXTERNAL with PKIX certificates.

> - 4.3: How is TLS negotiated for the additional streams? 

Each stream is separately secured.

> How is it bound
> to the SASL negotiation that (apparently) only takes place once?

It isn't, because each stream is separately secured (preferably by means
of TLS + SASL).

I propose that we clarify these matters by modifying the following
paragraphs from Section 4.3 ("Directionality"):

###

4.3.  Directionality

   An XML stream is always unidirectional, by which is meant that XML
   stanzas can be sent in only one direction over the stream (either
   from the initiating entity to the receiving entity or from the
   receiving entity to the initiating entity).

   Depending on the type of session that has been negotiated and the
   nature of the entities involved, the entities might use:

   o  Two streams over a single TCP connection, where the security
      context negotiated for the first stream is applied to the second
      stream.  This is typical for client-to-server sessions, and a
      server MUST allow a client to use the same TCP connection for both
      streams.

   o  Two streams over two TCP connections, where each stream is
      separately secured.  In this approach, one TCP connection is used
      for the stream in which stanzas are sent from the initiating
      entity to the receiving entity, and the other TCP connection is
      used for the stream in which stanzas are sent from the receiving
      entity to the initiating entity.  This is typical for server-to-
      server sessions.

   o  Multiple streams over two or more TCP connections, where each
      stream is separately secured.  This approach is sometimes used for
      server-to-server communication between two large XMPP service
      providers; however, this can make it difficult to maintain
      coherence of data received over multiple streams in situations
      described under Section 10.1, which is why a server MAY return a
      <conflict> stream error to a remote server that attempts to
      negotiate more than one stream (as described under
      Section 4.8.3.3).

   This concept of directionality applies only to stanzas and explicitly
   does not apply to first-level children of the stream root that are
   used to bootstrap or manage the stream (e.g., first-level elements
   used for TLS negotiation, SASL negotiation, Server Dialback
   [XEP-0220], and Stream Management [XEP-0198]).

   The foregoing considerations imply that while completing STARTTLS
   negotiation (Section 5) and SASL negotiation (Section 6) two servers
   would use one TCP connection, but after the stream negotiation
   process is done that original TCP connection would be used only for
   the initiating server to send XML stanzas to the receiving server.
   In order for the receiving server to send XML stanzas to the
   initiating server, the receiving server would need to reverse the
   roles and negotiate an XML stream from the receiving server to the
   initiating server over a separate TCP connection.

###

> - 4.4: this section appears to tie the two streams in the opposite
> directions together - when you close one you expect the other guy to
> close the other ASAP. But what is the behavior for "multiple streams
> over multiple connections" (Sec. 4.3)?

That is an excellent question. Section 4.4 currently assumes that there
is a defined pairing of streams because that is what happens in well
over 99% of the cases (I'd venture to guess that using multiple streams
over multiple TCP connections is extremely rare and happens only for
some high-volume server-to-server links). IMHO we don't have enough
experience with this case to recommend server behavior, although it
would be good to gather feedback from any service providers who are
using multiple streams over multiple TCP connections. Therefore I
propose that we modify Section 4.4 as follows:

###

   If the parties are using either two streams over a single TCP
   connection or two streams over two TCP connections, the entity that
   sends the closing stream tag SHOULD behave as follows:

   1.  Wait for the other party to also close its stream before
       terminating the underlying TCP connection(s); this gives the
       other party an opportunity to finish transmitting any data in the
       opposite direction before the TCP connection(s) is terminated.

   2.  Refrain from initiating the sending of further data over that
       stream but continue to process data sent by the other entity
       (and, if necessary, react to such data).

   3.  Consider both streams to be void if the other party does not send
       its closing stream tag within a reasonable amount of time (where
       the definition of "reasonable" is a matter of implementation or
       deployment).

   4.  After receiving a reciprocal closing stream tag from the other
       party or waiting a reasonable amount of time with no response,
       terminate the underlying TCP connection(s).

   If the parties are using multiple streams over multiple TCP
   connections, there is no defined pairing of streams and therefore the
   behavior is a matter for implementation.

###

> - 4.4: what about orderly tear-down of the TLS association ("closure
> alert")?

Sending of the closure alert is mandated by RFC 4346. Do you think it is
necessary (or would be helpful) to call it out in the specification of
an application protocol that re-use TLS?

I note that Section 7.2.1 ("Closure Alerts") of RFC 4346 states in part:

   If the application protocol using TLS provides that any data may be
   carried over the underlying transport after the TLS connection is
   closed, the TLS implementation must receive the responding
   close_notify alert before indicating to the application layer that
   the TLS connection has ended.  If the application protocol will not
   transfer any additional data, but will only close the underlying
   transport connection, then the implementation MAY choose to close the
   transport without waiting for the responding close_notify.  No part
   of this standard should be taken to dictate the manner in which a
   usage profile for TLS manages its data transport, including when
   connections are opened or closed.

I note also that rule #2 in Section 4.4 of 3920bis states:

   2.  Refrain from initiating the sending of further data over that
       stream but continue to process data sent by the other entity
       (and, if necessary, react to such data).

This implies that the entity that is closing the stream might send more
data and certaily might receive data (on the other stream). Thus I think
we do need to clarify use of closure alerts. I propose adding the
following security note to Section 4.4:

      Security Note: In accordance with Section 7.2.1 of [TLS], to help
      prevent a truncation attack the party that is closing the stream
      MUST send a TLS close_notify alert and MUST receive a responding
      close_notify alert from the other party before closing the
      underlying TCP connection(s).

> - 4.8.3.12: does not-authorized only refer to stream-level, rather than
> stanza-level errors? Are there cases when I am authorized to send some
> stanza types but not others?

There are three <not-authorized/> error conditions, each qualified by a
different namespace:

1. The stream error condition is triggered by trying to send data before
completing authentication.

2. The SASL error condition is triggered by providing a bad username or
incorrect credentials.

3. The stream error condition is triggered by attempting to complete an
application-layer action that requires authentication (e.g., attempting
to join a password-protected chatroom without providing the password).

A server could restrict a user's ability to send different types of
stanzas (e.g., a presence-only XMPP service could return a stanza error
if a user attempts to send a <message/> stanza, but in that case it's a
service-wide policy and the <not-allowed/> stanza error condition is
more appropriate). In general, the <not-authorized/> stanza error
condition implies that the sender could do *something* to modify the
data it sent in order to prove that it is in fact authorized to perform
the action requested in the stanza (e.g., include a password in the
chatroom join request).

> - 4.8.3.14 remote-connection-failed has dubious security benefit (why
> tell the world that your RADIUS server is down), compared to reusing
> internal-error.

It's not an internal error because the source of the failure is a remote
entity. As far as I know, this error condition has been used only in the
context of server dialback (in the case where a stream error occurs
between the Receiving Server and the Authoritative Server), but we've
tried to scrub the-protocol-that-shall-not-be-named from 3920bis
wherever possible; doing so in here has led to confusion on your part so
I propose that we might want to modify the description, as so:

   The server is unable to properly connect to a remote entity that is
   needed for authentication or authorization (e.g., in certain
   scenarios related to Server Dialback [XEP-0220]); this condition is
   not to be used when the cause of the error is within the
   administrative domain of the XMPP service provider, in which case the
   <internal-server-error/> condition is more appropriate.

> - 4.8.3.16: shouldn't we say that "reset" (when the stream is encrypted)
> also applies to the higher layers, i.e. encryption and authentication
> should be performed again?

Good point. I propose that we modify the description as follows:

   The server is closing the stream because it has new (typically
   security-critical) features to offer, because the keys or
   certificates used to establish a secure context for the stream have
   expired or have been revoked during the life of the stream
   (Section 13.7.2.3), because the TLS sequence number has wrapped
   (Section 5.3.5), etc.  The reset applies to the stream and to any
   security context established for that stream (e.g., via TLS and
   SASL), which means that encryption and authentication need to be
   negotiated again for the new stream (e.g., TLS session resumption
   cannot be used).

> - 4.8.3.18: what are the security implications of a "redirect"? 

Of <see-other-host/>? Seemingly underspecified. :(

> Should
> the client apply the same policy, e.g. for using TLS, as for the
> original server? 

Yes.

> Which "to" identity to use? 

The 'to' address of the initial stream header would still be the DNS
domain name of the XMPP service to which the initiating entity is trying
to connect (see also draft-saintandre-tls-server-id-check).

> Can redirection occur
> before the recipient is even authenticated?

Yes.

I propose that we modify the description as follows:

   The server will not provide service to the initiating entity but is
   redirecting traffic to another host under the administrative control
   of the same service provider.  The XML character data of the <see-
   other-host/> element returned by the server MUST specify the
   alternate hostname or IP address at which to connect, which MUST be a
   valid domainpart or a domainpart plus port number (separated by the
   ':' character in the form "domainpart:port").  If the domainpart is
   the same as the source domain, derived domain, or resolved IP address
   to which the initiating entity originally connected (differing only
   by the port number), then the initiating entity SHOULD simply attempt
   to reconnect at that address.  Otherwise, the initiating entity MUST
   resolve the hostname specified in the <see-other-host/> element as
   described under Section 3.2.

I also propose that we add the following paragraph to the end of the
section:

   When negotiating a stream with the host to which it has been
   redirected, the initiating entity MUST apply the same policies it
   would have applied to the original connection attempt (e.g., a policy
   requiring TLS), MUST specify the same 'to' address on the initial
   stream header, and MUST verify the identity of the new host using the
   same reference identifier(s) it would have used for the original
   connection attempt (in accordance with [TLS-CERTS].  Even if
   receiving entity returns a <see-other-host/> error before the
   confidentiality and integrity of the stream have been established
   (thus introducing the possibility of a denial of service attack), the
   fact that the initiating entity needs to verify the identity of the
   XMPP service based on the same reference identifiers implies that the
   initiating entity will not connect to a malicious entity; however, to
   reduce the possibility of a denial of service attack, the receiving
   entity SHOULD NOT return a <see-other-host/> error until after the
   stream has been protected (e.g., via TLS).

> - 4.9: Don't we resend the "stream" header again after completing the
> TLS negotiation (Sec. 4.2.3).

For sure. I propose that we change this:

   [ ... channel encryption ... ]

   [ ... authentication ... ]

   [ ... resource binding ... ]

to:

   [ ... stream negotiation ... ]

### END OF PART 1 ###

---

From: Peter Saint-Andre <[email-masked]>
Date: Fri, 29 Oct 2010 08:20:00 -0600

Hi Yaron, a few additional comments and text proposals inline.

On 10/29/10 1:46 AM, Yaron Sheffer wrote:
> Hi Peter,
> 
> On 10/29/2010 06:01 AM, Peter Saint-Andre wrote:
> Thanks a lot for using my comments and misunderstandings to improve the
> document.
> 
> I have removed pieces of this mail where I fully accept your
> response/changes.
> 
> Perhaps add a final sentence: "This separate TCP connection is then
> secured using a new round of TLS and/or SASL negotiation."

Yes, that's good. Paragraph updated with your text.

>> Of<see-other-host/>? Seemingly underspecified. :(
>>
>>> Should
>>     of the same service provider.  The XML character data of the<see-
>>     resolve the hostname specified in the<see-other-host/>  element as
>>     receiving entity returns a<see-other-host/>  error before the
>>     entity SHOULD NOT return a<see-other-host/>  error until after the
> ... and the sending entity SHOULD maintain a running redirect counter,
> and give up after a certain number of successive redirects.
> 
> Also, the mitigation you propose in the last sentence only helps if "An
> entity MAY have a policy where it only accepts <see-other-host> elements
> from authenticated peers.

Thanks. I propose that we change this paragraph to:

   When negotiating a stream with the host to which it has been
   redirected, the initiating entity MUST apply the same policies it
   would have applied to the original connection attempt (e.g., a policy
   requiring TLS), MUST specify the same 'to' address on the initial
   stream header, and MUST verify the identity of the new host using the
   same reference identifier(s) it would have used for the original
   connection attempt (in accordance with [TLS-CERTS].  Even if
   receiving entity returns a <see-other-host/> error before the
   confidentiality and integrity of the stream have been established
   (thus introducing the possibility of a denial of service attack), the
   fact that the initiating entity needs to verify the identity of the
   XMPP service based on the same reference identifiers implies that the
   initiating entity will not connect to a malicious entity; however, to
   reduce the possibility of a denial of service attack, the receiving
   entity SHOULD NOT return a <see-other-host/> error until after the
   stream has been protected (e.g., via TLS) and the receiving entity
   MAY have a policy of following redirects only if it has authenticated
   the receiving entity.  In addition, the initiating entity SHOULD give
   up after a certain number of successive redirects (e.g., at least 2
   but no more than 5).

> I'm not sure. Since we start the example with two <stream> elements, it
> would make sense to show the additional (4?) <stream> elements further
> down. Although the "simple" examples would no longer fit a page. Maybe
> add a "really simple" example with no security?

These examples are included to provide a high-level illustration of what
a session would look like, so I think eliding the whole stream
negotiation process is fine. However, I think it would be good to add a
forward reference to Section 9 so that the reader knows where to go for
detailed examples. Thus propose modifying the first paragraph of Section
4.9 as follows:

   This section contains two highly simplified examples of a stream-
   based connection between a client and a server; these examples are
   included for the purpose of illustrating the concepts introduced thus
   far, but the reader needs to be aware that these examples elide many
   details (see Section 9 for more complete examples).

Peter

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

---

From: Peter Saint-Andre <[email-masked]>
Date: Fri, 29 Oct 2010 08:41:59 -0600

Excellent. I'll try to finish Part 2 today.

On 10/29/10 8:26 AM, Yaron Sheffer wrote:
> Hi Peter,
> 
> I'm fine with your proposals.
> 
> Thanks,
>     Yaron
> 
> On 10/29/2010 04:20 PM, Peter Saint-Andre wrote:
>>> entity MAY have a policy where it only accepts<see-other-host>  elements
>>> I'm not sure. Since we start the example with two<stream>  elements, it
>>> would make sense to show the additional (4?)<stream>  elements further

---

From: Peter Saint-Andre <[email-masked]>
Date: Fri, 29 Oct 2010 12:32:20 -0600

This is Part 2 of my reply, from the start of Section 5 through the end
of Section 9. I should be able to address the remainder (Section 10
through Section 15) in Part 3.

On 10/28/10 4:28 AM, Yaron Sheffer wrote:

> - 5.3.5: the text mentions that client certificates are "sufficiently
> rare", which is a pity because they do make sense for server-to-server
> interaction. So I suggest to promote renegotiation from OPTIONAL to
> RECOMMENDED.

A bit of background: by client certificates we mean end-user
certificates (XMPP client, not TLS client). PKIX certificates are more
common for XMPP servers, but unfortunately they are still far from
ubiquitous. The XMPP Standards Foundation even ran an intermediate CA
for a few years (under the StartCom root) to help seed the network,
which helped us work out bugs in many codebases but still didn't result
in wide adoption. Many service admins still think that PKI is "too hard"
or "too expensive" (despite the fact that our ICA gave out free
certificates, and StartCom continues to do so).

The XMPP WG had a discussion about TLS renegotiation, starting here:

http://www.ietf.org/mail-archive/web/xmpp/current/msg01146.html

At one point we banned TLS renegotiation entirely, then we weighed the
benefits and the costs (including code complexity) and concluded that it
was appropriate to make support strictly optional in XMPP, given how
rarely it would be needed. Simon Josefsson suggested during the WG
discussion that we document our reasons, which we've done in Section
5.3.5. If you do not find those reasons compelling, the WG needs to
either explain itself more clearly or realize that it is wrong. :)

> - 5.3.6: extensions may be out of scope. But I think we need to include
> a few words re: the TLS version, at least to prohibit SSL 3.0.

In fact, discussions within the XMPP WG are what led to this:

https://datatracker.ietf.org/doc/draft-ietf-tls-ssl2-must-not/

However, it seems there is not yet consensus to prohibit SSL 3.0 in the
TLS WG. Are you suggesting that the XMPP WG go where the TLS WG has not
gone?

> - 5.4.1: don't you have to declare version >= 1.0 if you simply support
> this draft? Same question in 6.4.1.

Yes. The mentions in 5.4.1 and 6.4.1 are artifacts of RFC 3920, when the
'version' attribute was new. I've removed those clauses in both sections.

> - 5.4.3.1: why is the initiator required to present a certificate "So
> that mutual authentication will be possible"? There are many other ways
> of ensuring mutual auth. I suggest to reword as "mutual certificate
> authentication".

Done.

> - Global replace TLS "cipher" -> "ciphersuite".

Done.

> - 5.4.3.1, bullet 5: actually, based on the "from" attribute that the
> receiving entity has just sent.

You refer to this text:

   5.  The receiving entity SHOULD choose which certificate to present
       based on the 'to' attribute of the initial stream header.

I'm not quite sure about your recommendation. Practically speaking I
think it doesn't make a difference, because the two will be the same
unless the receiving entity does not service the domainpart specified by
the initiating entity (in which case the parties won't end up
negotiating TLS anyway, since the receiving entity will close the
stream). However, the 'to' address of the initial stream header seems
like a "purer" indication of the initiating entity's "reference
identifier" (in the language of [TLS-CERTS]).

> - 5.4.3.3: where do we say that both sides must validate the "to" and
> "from" identities in view of the identities presented at the TLS layer
> (if any)?

Section 13.7.2 ("Certificate Validation"). There is a forward reference
in bullet 4 of Section 5.4.3.3:

   4.  The initiating entity MUST validate the certificate to determine
       if the TLS negotiation will succeed; see Section 13.7.2 regarding
       certificate validation procedures.

Perhaps it would be easier to read if we switched bullets 4 and 5 in
that section?

Also, the bullet about validation probably also needs to say that if the
initiating entity presents a certificate then the receiving entity needs
to also perform validation. Do you agree?

> - 6.3.2: do you restart the stream twice, once after TLS and once after
> SASL??

Yes. We defined it that way in RFC 3920 because it was our understanding
of TLS and SASL that we needed to flush the previous security context in
both cases because either one of those technologies might result in
negotiation of a security layer. If we were defining things anew at this
point, we would probably remove the stream restarts, but I think it's
probably too late to do that now. (Perhaps that will be part of our work
on "quick reconnect" or streamlined stream negotiation.)

> - 6.3.3: shouldn't we simply "MUST NOT" the PLAIN mechanism?

Yes. Done.

> - 6.3.7: and what is the identity used for server-to-server auth? Also,
> it is very uncommon to consider the password as part of the identity.

I think we acquired this notion from a misreading of RFC 4422. Looking
at that specification again, I see that it refers to "credentials" as
consisting (in some SASL mechanisms) of a simple username / password,
whereas the authentication identity would be the simple username. I
propose modifying the offending paragraph as follows:

   Some SASL mechanisms (e.g., CRAM-MD5, DIGEST-MD5, and SCRAM) specify
   that the authentication identity used in the context of such
   mechanisms is a "simple user name" (see Section 2 of [SASL] as well
   as [SASLPREP]).  The exact form of the simple user name in any
   particular mechanism or deployment thereof is a local matter, and a
   simple user name does not necessarily map to an application
   identifier such as a JID or JID component (e.g., a localpart).
   However, in the absence of local information provided by the server,
   an XMPP client SHOULD assume that the authentication identity for
   such a SASL mechanism is a simple user name equal to the localpart of
   the user's JID.

> - 6.4.4: isn't it inconsistent that SASL aborts are converted into XMPP
> failures, but TLS failures are not?

I don't think so. TLS is a layer unto itself, and the STARTTLS feature
in XMPP is merely a way of triggering the underlying TLS negotiation (we
don't represent all the TLS bits in XMPP syntax). By contrast, there is
no special SASL layer to invoke, so all of the protocol bits to
negotiate SASL happen using XMPP syntax.

> - 7.7.2.2: the preferred option #1, while reasonable in itself, does not
> allow the client to determine its own policy (whether it wants multiple
> sessions from multiple devices or not).

If the client wants multiple sessions from multiple devices, it can
generate a unique resourcepart for each device or ask the server to do
so on its behalf.

> - 8.1.1.2: any validated domain -> any validated subdomain.

We've gone through some effort to scrub the term "subdomain" from the
spec (you'll notice it was used more frequently in RFC 3920) because of
ambiguities surrounding that term. This is connected to the common XMPP
practice of server-side components that offer services in addition to or
on top of the core XMPP router. Consider an XMPP service whose canonical
DNS domain name is "im.example.com"; that service might offer add-on
services such as groupchat rooms (say, "rooms.example.com") and
publish-subscribe (say, "pubsub.example.com"). Nothing says that such
add-on services need to be subdomains of the canonical DNS domain name
for the service, but RFC 3920 implied such a necessity, leading to
confusion among server developers and service administrators.

> - 8.3.1, rule #2: if the error is a result of something gone bad with
> the addresses, then simply swapping "to" and "from" may not be
> appropriate, and may even be a security issue.

I suppose that is possible. I propose modifying the rule as follows:

   2.  The error stanza SHOULD simply swap the 'from' and 'to' addresses
       from the generated stanza, unless doing so would result in an
       information leak (see under Section 13.10) or other breach of
       security.

> - Same, rule #6, the recipient MUST NOT include the original XML if it's
> not well formed, right?

True. Or if it is otherwise harmful. I propose modifying the rule as
follows:

   6.  The entity that returns an error stanza MAY include the original
       XML sent so that the sender can inspect and, if necessary,
       correct the XML before attempting to resend (however, this is a
       courtesy only and the originating entity MUST NOT depend on
       receiving the original payload); naturally, the entity MUST NOT
       include the original data if it not well-formed XML, violates the
       XML restrictions of XMPP (see under Section 11.1, or is otherwise
       harmful (e.g, exceeds a size limit).

> - 8.3.2: MUST NOT... be presented to the human user - this is impossible
> to enforce, and most likely will not be followed. Moreover, there's a
> reason why we include a language tag. I suggest to tone it down a bit.

I suppose SHOULD NOT is fine. The error message shown to the user is
supposed to be based on the defined condition, not some random text
placed in the <text/> element (even if language-tagged).

> - 8.3.3.11: what are "improper credentials"? It sounds like we are not
> differentiating the conditions of authentication failure vs.
> authorization failure.

As an example, if a client attempts to join a password-protected
chatroom without providing a password, the chatroom service will return
a <not-authorized/> stanza error. It will return the same error if the
client attempts to join a password-protected chatroom but provides the
wrong password. Is there a need to differentiate between these two cases?

> - 8.3.3.14: the "security note" doesn't makes sense to me - no matter
> what error code is returned, the sender has gained information that we
> don't want to provide. The note should say something along the lines of:
> such services must not be available to entities that cannot be trusted
> with knowing the status of an arbitrary recipient. See also 8.3.3.20.

It's not necessarily true that the sender has gained information that we
don't want to provide. In RFC 3920 (and 3920bis), we have the concept of
presence subscriptions. If the sender is authorized to know the
recipient's network availability (via presence subscription), then it is
perfectly legitimate for the recipient's server to tell the sender that
the recipient is now offline. Perhaps a reference to 3920bis would clear
this up. I propose modifying the note as follows:

      Security Note: An application MUST NOT return this error if doing
      so would provide information about the intended recipient's
      network availability to an entity that is not authorized to know
      such information (for details, refer to the discussion of presence
      subscriptions in [XMPP-IM]); instead it MUST return a <service-
      unavailable/> stanza error.

> - 8.3.3.15: Are there no security implications to redirection at the
> stanza level?

Good point, there certainly might be. If the sender is attempting to
join a chatroom at rooms.example.com and the service redirects the
request to conference.example.net, the client might want to ask the
sender if the redirect is acceptable before proceeding (e.g., because
the redirected service might have different security policies). I
propose adding the following security note:

      Security Note: An application receiving a stanza-level redirect
      SHOULD warn a human user of the redirection attempt and request
      approval before proceeding to communicated 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.

> - 8.3.3.18: The "wait" error type might not be appropriate for some
> situations, e.g. authentication issues.

True. I propose changing it to:

   A remote server or service specified as part or all of the JID of the
   intended recipient (or needed to fulfill a request) was resolved but
   communications could not be established within a reasonable amount of
   time (e.g., an XML stream cannot be established at the resolved IP
   address and port, or an XML stream can be established but stream
   negotiation fails because of problems with TLS, SASL, Server
   Dialback, etc.); the associated error type SHOULD be "wait" (unless
   the error is of a more permanent nature, e.g., the remote server is
   found but it cannot be authenticated or it violates security
   policies).

> - 8.3.3.21: do you explain anywhere what is the difference between
> "prior registration" and "prior subscription"?

That's really an issue for various XMPP applications. One such
application is IM and presence (3920bis), where we have the concept of
subscriptions. Another application in which we have subscriptions is
publish-subscribe <http://xmpp.org/extensions/xep-0060.html>. An example
of registration can be found in the multi-user chat extension
<http://xmpp.org/extensions/xep-0045.html> or in the older "gateway"
that we used to maintain for connecting to legacy IM services. I don't
see much reason to describe all those differences in 3920bis, but we can
add a sentence or two about it if that would be helpful.

> - 9.1.1, step 7: this is utterly trivial, but please mention that the
> new "stream" element is sent over TLS.

Changed to:

   Step 7: If TLS negotiation is successful, client initiates a new
   stream to server over the TLS-protected TCP connection:

(also changed in 9.2.1)

> - 9.1.5: as I noted in Sec. 4.4, the TLS connection needs to be closed
> as well.

Changed in my working copy to:

   The client now sends a TLS close_notify alert, receives a responding
   close_notify alert from the server, and then terminates the
   underlying TCP connection.

(also in 9.2.5 with "Server1" and "Server2")

### END OF PART 2 ###

---

From: Yaron Sheffer <[email-masked]>
Date: Fri, 29 Oct 2010 09:46:53 +0200

Hi Peter,

---

From: Yaron Sheffer <[email-masked]>
Date: Fri, 29 Oct 2010 16:26:22 +0200

Hi Peter,

Thanks,
	Yaron

---

From: Peter Saint-Andre <[email-masked]>
Date: Fri, 29 Oct 2010 23:05:45 -0600

This is Part 3 of my reply, from the start of Section 10 through the end
of Section 15. This completes my reply to Yaron's security review.

On 10/28/10 4:28 AM, Yaron Sheffer wrote:

> - 10.2: "try many different resources", I suppose it is typically fewer
> than 10 per JID, which does not make the attack uninteresting.

In my experience, few entities connect from more than three or four
devices at a time, although that might increase in the future. The use
of randomized resourceparts helps to mitigate the risk here (because an
attacker can't simply guess if the victim is at the "home" or "work"
resource or whatever) -- perhaps it would be good to mention that?

> - 10.4.2: if the protocol supports routing, shouldn't we mention that
> there are cases where the IQ stanza will be forwarded to another server,
> not the one mentioned in the "To" header? And what about loop prevention?

Could you clarify what you mean by "forward"? We don't have multi-hop
routing in XMPP, just direct server-to-server connections, so it's not
as if the stanza would be forwarded through multiple servers in order to
reach the destination domain.

> - 10.5.3.3: this is a strange rule. Does it mean that if I'm using a
> desktop (/desktop) and a mobile (/mobile), and I'm connected to the
> desktop, I cannot have messages targeted at my mobile be deferred (and
> stored somewhere) until I connect from that device?

That feature is not part of XMPP.

> - 13.4: "for the ensuring" -> "for ensuring" (or: to ensure).

Fixed.

> - 13.4: the security note is confusing, because it is unclear whether it
> has any normative status. Otherwise, it is almost trivial: of course you
> can provide these guarantees by different means.

This security note provides truth in advertising: we don't want to
misrepresent the extent to which the use of TLS can protect stanzas,
since they can be routed or delivered outside the context of their
original stream. It does not use any normative keywords because it is
purely informative.

> - 13.4: the cipher suites are not just "nun-null". They MUST provide
> both confidentiality and integrity.

Is this more accurate?

   The use of Transport Layer Security (TLS) with appropriate
   ciphersuites provides a reliable mechanism to ensure the
   confidentiality and integrity of data exchanged between a client and
   a server or between two servers.

> - 13.6: the out-of-band trust chain rule may be practical for
> server-to-server connections, but probably not for large client
> deployments and when certificates are sometimes rolled over.

Do you think that we need to modify the text of that paragraph?

> - 13.7: terminology: "mutual authentication" describes authentication
> with client certs, just as much as it describes the server presenting a
> cert and the client authenticating with SASL. This also applies to 5.4.3.

In this context we have always used the term "authentication" only with
regard to SASL. The use of SASL EXTERNAL with PKIX certificates enables
an entity to point back to a certificate it provided during TLS
negotiation, but in general we don't refer to TLS as an authentication
mechanism. I don't see an easy way to disambiguate the term here, but
suggestions are welcome.

> - 13.7.1.1: The word "issuer" is ambiguous. It usually refers to a
> person/organization, but here you use it to refer to the certificate
> itself. How about replacing the heading of the second list by: "the
> following rules apply to issuer certificates, used to sign XMPP
> end-entity certificates"?

I think the first and second headings are best phrased like this:

   The following rules apply to end entity public key certificates that
   are issued to XMPP servers or clients:

   [...]

   The following rules apply to certification authority (CA)
   certificates that are used by issuers of XMPP end entity
   certificates:

That way we are using the appropriate terminology from RFC 5280.

> - 13.7.1.1: it's strange to specify the hash algorithm, but not the
> signature algorithm (RSA-512 anyone?)

Ah, I got that text from Sean Turner (IIRC, we copied it from a
never-released version of draft-hildebrand-dna), and I'm pretty sure
that by "the hash algorithm for the signature" he meant the
signatureAlgorithm field from RFC 5280, as in this text from
draft-hildebrand-dna:

   The issuer MUST support signing attribute certificate with the PKCS
   #1 version 1.5 signature algorithm with SHA-256, as specified in
   [RFC4055].

Clearly something has been lost in translation. I'll check with Sean on
this point.

> - 13.7.1.1: what are "access certificates"? Neither 5280 nor Google can
> help... 

s/access/attribute/

> And how can an issuer cert NOT be marked with the CA bit?

The idea was that attribute certificates would be issued by non-CA
entities, such as organizations that wish to delegate an XMPP service to
a hosting provider. However, I think we should delete any mention of
attribute certificates because that was something we envisioned for use
in "domain name assertions" but the feature is far from well defined.
See here for more details:

http://tools.ietf.org/id/draft-hildebrand-dna-00.txt

http://tools.ietf.org/id/draft-ietf-xmpp-dna-00.txt

http://www.ietf.org/id/draft-barnes-xmpp-dna-00.txt

Therefore I propose that we remove these lines (where "access" really
should be "attribute", not that it matters if we remove the bullet):

   6.  For issuers of access certificates, the issuer's certificate MUST
       NOT contain a basicConstraints extension with the cA boolean set
       to TRUE.

> - 13.7.1.1: and most important, is the "relying party" (e.g. the client)
> required to check all these rules and fail validation if any of them is
> not met?

As I understand it from my conversations with Sean when we were adding
this text, these rules are not set in stone within RFC 5280 and
therefore need to be specified by any technology that reuses PKIX. Our
intent was that CAs would conform to these rules, not necessarily that
replying parties would necessarily fail on validation if the rules were
violated. Here again I'll check with Sean.

> - 13.7.1.2.2: enabling entity -> enabling entities.
> - 13.7.1.4: "conjuction" misspelled.

Typos fixed. Maybe it would be helpful for me to finally run this
document through a spell-checker...

> - 13.7.2: "An implementation MUST enable a human user to view
> information about the certification path." I'm afraid this is security
> theater, because 99.5% of your target population cannot understand this
> information.

Do you recommend changing SHOULD to MUST, removing this text entirely,
or something else?

I suppose that 0.5% could always view the path via a dedicated piece of
security software...

> - 13.7.2.2.1: "MUST either validate" - incomplete sentence (and
> actually, at a sensitive point in the text).

Here "either" is extraneous because it is an artifact of an earlier
version of the spec. Much earlier:

http://tools.ietf.org/id/draft-saintandre-rfc3920bis-05.txt

> - 13.7.2.2.1, subcase #3: please mention that some servers will simply
> fail validation at this point, subject to their policy. I.e., some might
> insist on correct client certs.

True. Does the following proposed text capture that point?

   Sub-Case #3:  The server finds no XmppAddrs, or finds at least one
      XmppAddr but the domainpart of the represented JID does not match
      one of the configured hostnames of the server; the server MUST NOT
      use the represented JID (if any) as the validated identity of the
      client but instead MUST validate the identity of the client using
      other means.  If the identity cannot be so validated, depending on
      local service policy the server MAY abort the validation process
      and terminate the TLS negotiation.

> - 13.7.2.3: "periodically query OCSP" - is there any guidance about the
> period? Is a recommended period specified in the cert or communicated by
> the OCSP responder?

I don't see a way to specify this in the certificate, but it seems
reasonable to query at or near the time communicated via the nextUpdate
field received in the OCSP response or, if the nextUpdate field is not
set, to query every 24 hours. This is for long-lived streams

> - 13.7.2: a silly question, but anyway: where do you say that the
> client's cert is correlated with the client's JID, as it appears in the
> From line (when setting up the original stream and/or when setting the
> stream anew after TLS negotiation)? 

Few clients include the 'from' address on the stream header and we're
not actively working to make sure they do, because what really matters
is that the client authenticates using the credentials of a registered
account.

> And vice versa, for the server cert
> and the client's To header.

For s2s communication, in essence (and using the terms of [TLS-CERTS])
the initiating entity sets its reference identifier to the 'to' address
it communicates in the initial stream header, and the receiving entity
sets its reference identifier to the 'from' address communicated by the
initiating entity in the initial stream header (i.e., the 'from' address
is the identity that the initiating entity is trying to assert). Per
recent list discussion Jeff Hodges and I have added a note about that to
our working copy of draft-saintandre-tls-server-id-check.

However, I agree that it would be helpful to mention this in 3920bis, so
I propose that we add some text to Sections 13.7.2.1 ("Server
Certificates") and 13.7.2.2 ("Client Certificates")...

###

13.7.2.1.  Server Certificates

   For server certificates, the rules and guidelines defined in
   [TLS-CERTS] apply, with the proviso that the XmppAddr identifier
   specified under Section 13.7.1.4 is allowed as a reference
   identifier.

   The identities to be checked are set as follows:

   o  The initiating entity sets its reference identifier to the 'to'
      address it communicates in the initial stream header; i.e., this
      is the identity it expects the receiving entity to provide in a
      PKIX certificate.

   o  The receiving entity sets its reference identifier to the 'from'
      address communicated by the initiating entity in the initial
      stream header; i.e., this is the identity that the initiating
      entity is trying to assert.

13.7.2.2.  Client Certificates

   When an XMPP server validates a certificate presented by a client,
   there are three possible cases, as discussed in the following
   sections.

   The identities to be checked are set as follows:

   o  The client sets its reference identifier to the 'to' address it
      communicates in the initial stream header; i.e., this is the
      identity it expects the server to provide in a PKIX certificate.

   o  The server sets its reference identifier to the 'from' address
      communicated by the initiating entity in the initial stream
      header; i.e., this is the identity that the client is trying to
      assert.

###

> - 13.8: I suggest to add (here or elsewhere): "All password-based
> mechanisms are susceptible to password guessing attacks, and therefore
> the authenticator MUST implement common rate-limiting mitigations."

I think the right place is Section 13.9.4 ("Use of SASL").

Added.

> - 13.8: "For both confidentiality and authentication with passwords" -
> here you don't specify a TLS ciphersuite.

RFC 5246 has TLS_RSA_WITH_AES_128_CBC_SHA as the mandatory-to-implement
ciphersuite, so that seems appropriate here. Would you recommend
something stronger, for example TLS_RSA_WITH_AES_256_CBC_SHA?

> - 13.8: the note kind of implies that PLAIN is preferable to DIGEST-MD5,
> which is clearly not the case.

Clearly not, and that was unintentional.

I think it is better to limit this note strictly to the interoperability
issue here (see also the next section)...

      Interoperability Note: The use of the SCRAM-SHA-1 or SASL-SCRAM-
      SHA-1-PLUS mechanism replaces the SASL DIGEST-MD5 mechanism as
      XMPP's mandatory-to-implement password-based method for
      authentication only, and the use of TLS plus either of those SCRAM
      variants replaces TLS plus DIGEST-MD5.  For backward-compatibility
      with existing deployed infrastructure, implementations are
      encouraged to continue supporting the DIGEST-MD5 mechanism as
      specified in [DIGEST-MD5], however there are known
      interoperability issues with DIGEST-MD5 that make it impractical
      in the long term.

> - 13.9.4: why is SASL-PLAIN singled out here? Other mechanisms are
> susceptible to off-line password guessing when used without TLS
> confidentiality, which is not a trivial attack but still a significant
> risk.

You're right.

I propose that we split up the text a bit here (e.g., the text about
PLAIN in fact talks mostly about TLS verification)...

So, add the following modified paragraph to Section 13.9.4 ("Use of SASL"):

   The use of the SCRAM-SHA-1 and SCRAM-SHA-1-PLUS mechanisms is
   strongly preferred over the SASL PLAIN mechanism because of their
   superior security properties, and TLS plus SASL PLAIN is intended to
   be a fallback only for implementations that do not yet support SCRAM.
   Implementations MUST NOT use the SASL PLAIN without confidentiality
   and integrity protection via TLS.  For additional security
   considerations related to these SASL mechanisms, see [SCRAM] and
   [PLAIN].

Then add the following modified paragraph to Section 13.9.5 ("Use of TLS"):

   To prevent man-in-the-middle attacks, the TLS client (which might be
   an XMPP client or an XMPP server) MUST verify the certificate of the
   TLS server and MUST check its understanding of the server hostname
   against the server's identity as presented in the TLS Certificate
   message (for details, see [TLS-CERTS].

> - 13.11: all three examples of "service unavailable" can be ruled out on
> an operational server. Are there no better examples?

I don't see how they can be ruled out.

> - 15: The sasl-whitespace "feature" is not really a feature, because
> you'd fail any interoperability if you send whitespace during the SASL
> phase, right? Similarly tls-whitespace.

I take it you mean they are "negative features" and interoperability
testing should focus on positive features. But isn't appropriate to
ensure compliance with a MUST NOT statement in the spec?

> - 15: if we insist on PLAIN, I would expect a security-mti-auth-order
> feature, where the proposals are ordered right (i.e. with PLAIN at the
> end).

I think it's better to remove TLS plus SASL PLAIN from the list of
mandatory-to-implement technologies.

> - 15: why is confidentiality-only a MUST? Is it widely deployed? Is it
> required for backward compatibility? 

It was required in RFC 3920 (with a different ciphersuite) and in
practice it is used in the case of TLS plus Server Dialback for s2s
communications.

> By the way, please expand the
> acronym MTI somewhere.

It's not used as an acronym, just as a string in some feature names.

> - 15: and hey, there's no feature defined for TLS+SCRAM+? Isn't this the
> one we want/expect to be deployed?

There is a feature for security-mti-auth-scram, which includes both
variants. I propose that we change the feature definition to:

   Feature:  security-mti-auth-scram
   Description:  Support the SASL Salted Challenge Response mechanism
      for authentication only or confidentiality and authentication
      (this implies support for both the SCRAM-SHA-1 and SCRAM-SHA-1-
      PLUS variants).
   Section:  Section 13.8
   Roles:  Client MUST, Server MUST.

> - 15: stanza-* features - if you have an XML schema, can't you just say
> that conformance to the schema is REQUIRED and eliminate all this stuff?
> Given that these are MUSTs, not SHOULDs, a formal specification is so
> much easier to validate. This does *not* contradict the fact that
> runtime validation is optional.

I suppose, but it seems useful for interoperability testing purposes
(which we do plan to pursue more rigorously) to define the more specific
features specified throughout the spec with regard to stanza syntax and
semantics.

> - 15: I would expect one or a few features around validation of
> identities at the various layers, since we're spending much of the
> document on this issue. "tls-certs" is an important piece of that, but
> not the whole thing.

I agree with you that the layering topics are important here. Do you
have specific suggestions?

Thanks again for your review.

Peter

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