Subject: [xmpp] SASL ambiguities

From: Peter Saint-Andre <[email-masked]>
Date: Tue, 29 Jun 2010 13:34:29 -0600

Waqas Hussain pinged me via IM about a few things that are unspecified
regarding XMPP's use of SASL.

1. What should the server do if the client sends an <auth/> element in
the middle of an ongoing handshake? Existing servers seem to discard the
ongoing handshake and start a new handshake for the requested SASL
mechanism.

2. What should the server do if the client sends <abort/> or <response/>
before sending <auth/>? Existing servers seem to ignore.

I can think of some more:

3. The client sends <challenge/>, <failure/>, or <success/>.

4. The server sends <abort/>, <auth/>, or <response/>.

Do folks think that we need to specify handling of these cases? I'd say
ignore in all cases except #1, where starting a new handshake seems
appropriate.

Peter

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

---

From: Matthew Wild <[email-masked]>
Date: Tue, 29 Jun 2010 20:40:15 +0100

> ongoing handshake and start a new handshake for the requested SASL

Seems sensible.

> before sending <auth/>? Existing servers seem to ignore.

Just about ok to ignore in my book.

> 4. The server sends <abort/>, <auth/>, or <response/>.

These two are essentially unrecognised stanzas for the respective
entities, and would probably be best answered with
unsupported-stanza-type as described elsewhere in the spec.

Matthew

---

From: Peter Saint-Andre <[email-masked]>
Date: Tue, 29 Jun 2010 13:46:19 -0600

> unsupported-stanza-type as described elsewhere in the spec.

Well, they aren't stanzas, they are unexpected SASL commands. Ignoring
them seems fine.

Peter

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

---

From: Peter Saint-Andre <[email-masked]>
Date: Tue, 29 Jun 2010 14:12:25 -0600

> ignore in all cases except #1, where starting a new handshake seems

After some discussion in the [email-masked] room, here is
proposed text:

###

6.4.13.  unexpected-command

   During SASL negotiation, the initiating entity sent a command that is
   unexpected, as follows:

   o  Sending <challenge/>, <failure/>, or <success/>.
   o  Sending <abort/> or <response/> before sending <auth/>.
   o  Sending non-SASL data before the SASL negotiation completed.

   However, the receiving entity MAY ignore unexpected commands instead
   of returning an error.  In addition, the initiating entity SHOULD
   ignore unexpected commands (e.g., <abort/>, <auth/>, or <response/>)
   from the receiving entity.

   I: <failure xmlns='urn:ietf:params:xml:ns:xmpp-sasl'/>

   R: <failure xmlns='urn:ietf:params:xml:ns:xmpp-sasl'>
        <unexpected-command/>
      </failure>

###

---

From: Dave Cridland <[email-masked]>
Date: Tue, 29 Jun 2010 21:36:00 +0100

>    R: <failure xmlns='urn:ietf:params:xml:ns:xmpp-sasl'>

FWIW, we currently drop the stream with not-authorized (§4.6.3.13) in  
such cases - basically, any time we receive an element when it's  
recognized but outside its expected state.

I can't imagine when this is caused by anything but bad code, so I'm  
satisfied that there's no need for a recoverable error at this point.

Dave.
-- 
Dave Cridland - mailto:[email-masked] - xmpp:[email-masked]
  - acap://acap.dave.cridland.net/byowner/user/dwd/bookmarks/
  - http://dave.cridland.net/
Infotrope Polymer - ACAP, IMAP, ESMTP, and Lemonade

---

From: Peter Saint-Andre <[email-masked]>
Date: Tue, 29 Jun 2010 15:05:22 -0600

On 6/29/10 2:36 PM, Dave Cridland wrote:
> I can't imagine when this is caused by anything but bad code, 

Execrable code, I'd imagine.

> so I'm

Sure. It might make sense to say:

###

   However, the receiving entity MAY (1) ignore unexpected commands
   instead of returning a SASL error, or (2) return an appropriate
   stream error (e.g., <not-authorized/>).  In addition, the initiating
   entity SHOULD ignore unexpected commands (e.g., <abort/>, <auth/>, or
   <response/>) from the receiving entity.

###

---

From: Dave Cridland <[email-masked]>
Date: Tue, 29 Jun 2010 22:14:03 +0100

> Execrable code, I'd imagine.
Right, so given the code is so abhorrent, why would any sane  
implementor want to let the client sully their nice shiny server? If  
the client isn't even vaguely trying to follow the spec, it doesn't  
bode well for further interactions.

I'd note in passing that being "sloppy" with handling errors in  
protocol has been a plague of SMTP servers, and conversly, tightening  
up the accepted protocol has been found to reduce spam in many cases.

As long as our implementation is allowed to reject such clients  
outright, though, I'm basically happy.

Dave.

---

From: Peter Saint-Andre <[email-masked]>
Date: Tue, 29 Jun 2010 16:47:09 -0600

On 6/29/10 3:14 PM, Dave Cridland wrote:
> Right, so given the code is so abhorrent, why would any sane implementor
> want to let the client sully their nice shiny server? If the client
> isn't even vaguely trying to follow the spec, it doesn't bode well for
> further interactions.
> 
> I'd note in passing that being "sloppy" with handling errors in protocol
> has been a plague of SMTP servers, and conversly, tightening up the
> accepted protocol has been found to reduce spam in many cases.

Good point.

> outright, though, I'm basically happy.

Reject away!

Peter

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

---

From: Matthew Wild <[email-masked]>
Date: Wed, 30 Jun 2010 00:18:52 +0100

On 29 June 2010 21:12, Peter Saint-Andre <[email-masked]> wrote:

>   During SASL negotiation, the initiating entity sent a command that is

Mmm, I really don't see why we ought to special-case SASL for this.

An example of similar bad behaviour is a client sending <proceed
xmlns="urn:ietf:params:xml:ns:xmpp-tls"/>, or similarly for extensions
such as <compressed xmlns="..."/> (sent by a server to indicate
successful stream compression setup), etc..

These are all elements under namespaces the server understands
(usually... it may not understand extensions such as compression) and
they should all result in termination of the stream I believe. We have
stream features to agree on what the client can and can't send at any
point in the stream.

Related to this are unknown elements under unknown namespaces. I don't
see that these should be handled any differently - also with a stream
error.

A server receiving <challenge
xmlns='urn:ietf:params:xml:ns:xmpp-sasl'/> is in my opinion no
different to a server receiving <blahblahblah
xmlns='urn:ietf:params:xml:ns:xmpp-sasl'/>. Both are elements it isn't
prepared to handle, regardless of the namespace, and unable to perform
whatever was expected by the sender, it needs to close the stream (we
don't have an equivalent of stanza errors for top-level elements).

Prosody currently uses unsupported-stanza-type for these top-level
elements with tags or namespaces it doesn't understand. I think Dave
said that M-Link uses not-authorized, so there's precedent for this
(though I don't like not-authorized because this is not really an
issue related to auth).

Thoughts?

Matthew

---

From: Peter Saint-Andre <[email-masked]>
Date: Tue, 29 Jun 2010 20:23:36 -0600

> (though I don't like not-authorized because this is not really an

Where <unsupported-stanza-type/> means "you've sent me a first-level
child of the stream that I don't understand because:

1. the namespace is unknown to me, e.g.:

   <foobar xmlns='http://example.com/foobar'/>

2. the namespace is known to me but the element name isn't, e.g.:

   <foobar xmlns='jabber:client'/>

(in XML terms, because I don't understand the qualified name)"

I'd be fine with that -- it simplifies SASL error processing and makes
processing of other features more generic.

+1 here.

Peter

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

---

From: Peter Saint-Andre <[email-masked]>
Date: Tue, 29 Jun 2010 20:41:48 -0600

> processing of other features more generic.

I've removed the <unexpected-command/> SASL error and tweaked the text
for the <unsupported-stanza-type/> stream error (Section 4.6.3.23) as
follows:

###

   The initiating entity has sent a first-level child of the stream that
   is not supported by the server, either because the receiving entity
   does not understand the namespace or because receiving entity does
   not understand the element name for the applicable namespace (which
   might be the default namespace).

   (In the following example, the client attempts to send a first-level
   child element of <pubsub/> qualified by the 'jabber:client'
   namespace, but the schema for that namespace defines no such
   element.)

###

---

From: Matthew Wild <[email-masked]>
Date: Wed, 30 Jun 2010 07:43:45 +0100

>   namespace, but the schema for that namespace defines no such

Exactly what I was thinking of, +1.

Thanks,
Matthew

>
>
>
> _______________________________________________
> xmpp mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/xmpp
>
>

---

From: Dave Cridland <[email-masked]>
Date: Wed, 30 Jun 2010 08:53:51 +0100

> (though I don't like not-authorized because this is not really an

We send unsupported-stanza-type for things not known, or not valid on  
that stream. We send not-authorized for known things, valid in the  
stream, but used at the wrong point.

This makes more sense if you consider that the expected common case  
is a peer sending <message/> before it's authenticated, for instance.

It also means that a server receiving <challenge/> emits  
unsupported-stanza-type, whereas a server receiving <auth/> on an  
authenticated - or authenticating - stream will get not-authorized.

However, this logic does require us to know the session type (S2S  
client, C2S server, etc) very early. We currently do this by checking  
what the default namespace in the stream is. If we're moving away  
from a hard requirement for default namespaces, then we'll need some  
alternative in the stream header to indicate the session type to  
maintain this behaviour.

Dave.

---

From: Matthew Wild <[email-masked]>
Date: Wed, 30 Jun 2010 10:41:20 +0100

On 30 June 2010 08:53, Dave Cridland <[email-masked]> wrote:
> We send unsupported-stanza-type for things not known, or not valid on that
> stream. We send not-authorized for known things, valid in the stream, but
> used at the wrong point.
>
> This makes more sense if you consider that the expected common case is a
> peer sending <message/> before it's authenticated, for instance.
>

Ok, that's expected, good.

> However, this logic does require us to know the session type (S2S client,
> C2S server, etc) very early. We currently do this by checking what the
> default namespace in the stream is. If we're moving away from a hard
> requirement for default namespaces, then we'll need some alternative in the
> stream header to indicate the session type to maintain this behaviour.
>

Yes indeed. Prosody detects this based on the port, but we were moving
away from that, and the latest release allows c2s/s2s on the same
port. Removing the requirement for a default namespace has unforeseen
consequences... I guess we need to decide how much we care about it.

Waqas and I have discussed it, but not reached any hard conclusions.
One option is to not distinguish at all... advertise dialback on every
stream for example.

Still thinking.

Matthew