Subject: Re: [xmpp] host-unknown stream error example in rfc 6120

From: Philipp Hancke <[email-masked]>
Date: Tue, 10 Apr 2012 18:01:03 +0200

Am 02.03.2012 09:11, schrieb Philipp Hancke:
> the example for the host-unknown stream error is quite odd
> http://tools.ietf.org/html/rfc6120#section-4.9.1.3

Additionally, it is setting a version=1.0 on the response stream without 
sending <stream:features/> (which is a MUST from 4.3.2)

All examples (*) showing a stream error during setup should not set a 
version=1.0 on the response stream. Accordingly, there should be some 
text in 4.9.1.2 saying that the receiving entity should not set the 
version on the response stream if an error occured.

(*) examples 4.9.1.2, 4.9.1.3, 4.9.3.2, 4.9.3.3, 4.9.3.5, 4.9.3.6,
4.9.3.10, 4.9.3.15, 4.9.3.17, 4.9.3.19, 4.9.3.22 and 4.9.3.25.

---

From: Dave Cridland <[email-masked]>
Date: Tue, 10 Apr 2012 17:12:20 +0100

On Tue Apr 10 17:01:03 2012, Philipp Hancke wrote:
> Additionally, it is setting a version=1.0 on the response stream  
> without sending <stream:features/> (which is a MUST from 4.3.2)
> 
> All examples (*) showing a stream error during setup should not set  
> a version=1.0 on the response stream. Accordingly, there should be  
> some text in 4.9.1.2 saying that the receiving entity should not  
> set the version on the response stream if an error occured.

Philipp and I had quite a chat about this.

The examples are wrong in and of themselves because they're not  
sending stream features - but it turned out that we have both  
implenented this case to not send a version attribute (and,  
correspondingly, not send features) in order to avoid confusion - in  
general, my thinking is that existing features are worthless in this  
case.

Therefore I agree entirely with Philipp's suggestion here, which is  
to fix the examples by removing the version attribute, (rather than  
by adding features) and additionally add text recommending this  
practise in §4.9.1.2.

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: Justin Karneges <[email-masked]>
Date: Tue, 10 Apr 2012 09:34:44 -0700

> by adding features) and additionally add text recommending this

Does the spec ever generally discuss this kind of "XMPP 0.9" behavior? If not, 
I'd think this would be quite confusing to new readers.

Justin

---

From: Peter Saint-Andre <[email-masked]>
Date: Tue, 10 Apr 2012 11:09:24 -0600

> I'd think this would be quite confusing to new readers.

Agreed.

I'd rather finesse the issue by changing Section 4.3.2 as follows.

OLD
   If the initiating entity includes in the initial stream header the
   'version' attribute set to a value of at least "1.0" (see
   Section 4.7.5), after sending the response stream header the
   receiving entity MUST send a <features/> child element

NEW
   If the initiating entity includes in the initial stream header the
   'version' attribute set to a value of at least "1.0" (see
   Section 4.7.5), after sending the response stream header the
   receiving entity MUST, if it intends to maintain the stream, send a
   <features/> child element

This gives us some wiggle room about not sending features in the
situation where the receiving entity immediately closes the stream.

Peter

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

---

From: Florian Zeitz <[email-masked]>
Date: Tue, 10 Apr 2012 19:52:51 +0200

> situation where the receiving entity immediately closes the stream.
+∞
Just because we are sending a stream error, the server does not suddenly
not speak "XMPP 1.0" any more.
Changing the problematic text seems much nicer than "hacking around it"
everywhere else to me too.

Regards,
Florian Zeitz

---

From: Dave Cridland <[email-masked]>
Date: Tue, 10 Apr 2012 20:44:15 +0100

> everywhere else to me too.

We're not just considering changing problematic text; we're working  
around existing implementations.

I'll admit this is somewhat optimizing for the failure case, and as  
such it's possibly not the greatest hill to choose to die upon, but I  
do feel that the correct fix (from an engineering, not a purist  
design) standpoint is to drop the version attribute if the intent is  
not to send stream features.

This *is* compliant - it's not hacking around it - and it's compliant  
with the existing specification text, not the examples. As such, it's  
merely adding a clarification to the normative text and correcting  
the (non-normative) examples, and not changing the specification at  
all.

Finally, it'll work with all existing implementations.

Changing the protocol, on the other hand - whilst we may well be  
changing it to how we might have designed it in the first place with  
hindsight - is still changing it. The effects are unknown, and may  
well be detrimental. There's clearly a way to proceed without  
introducing this risk.

Dave.

---

From: Peter Saint-Andre <[email-masked]>
Date: Tue, 10 Apr 2012 13:52:32 -0600

On 4/10/12 1:44 PM, Dave Cridland wrote:
> I'll admit this is somewhat optimizing for the failure case, and as such
> it's possibly not the greatest hill to choose to die upon, but I do feel
> that the correct fix (from an engineering, not a purist design)
> standpoint is to drop the version attribute if the intent is not to send
> stream features.

Section 4.7.5 says:

   The inclusion of the version attribute set to a value of at least
   "1.0" signals support for the stream-related protocols defined in
   this specification, including TLS negotiation (Section 5), SASL
   negotiation (Section 6), stream features (Section 4.3.2), and stream
   errors (Section 4.9).

So not including the version attribute indicates that you don't support
stream errors; immediately sending a stream error after that seems odd.

> merely adding a clarification to the normative text and correcting the
> (non-normative) examples, and not changing the specification at all.
> 
> hindsight - is still changing it. The effects are unknown, and may well
> be detrimental. There's clearly a way to proceed without introducing
> this risk.

"Clearly" might be a bit strong.

Peter

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

---

From: Dave Cridland <[email-masked]>
Date: Tue, 10 Apr 2012 21:37:15 +0100

> stream errors; immediately sending a stream error after that seems  

Hmmm...

Trouble is, I suspect too many things expect a features block after  
the header, and I've certainly seen clients crash when faced with  
empty features - and sending features beyond empty seems wasteful.

So I'll back down from "clearly", yes - but I think I'll stand by my  
position for now at least - partly because I'm pretty sure that there  
are XMPP entities capable of understanding stream errors but not  
features...

If we need to have a version, I'd really rather send features.

Dave.

---

From: Justin Karneges <[email-masked]>
Date: Tue, 10 Apr 2012 13:51:06 -0700

On Tuesday, April 10, 2012 01:37:15 PM Dave Cridland wrote:
> >    negotiation (Section 6), stream features (Section 4.3.2), and
> > 
> > stream
> > 
> >    errors (Section 4.9).
> > 

The issue here seems to be the possibility of implementations unable to 
process a stream error while waiting for stream features.

As a worst case, I would think even if there exist implementations incapable 
of handling this properly, they might handle this in some acceptably improper 
way, such as considering it to be invalid protocol or to be a random 
disconnect. The true reason for error is lost in this case, which could cause 
some problems, but in practice I wonder they'd be. Maybe mishandling of "host-
unknown" or "conflict" ? Any others?

Justin

---

From: Peter Saint-Andre <[email-masked]>
Date: Tue, 10 Apr 2012 14:51:32 -0600

On 4/10/12 2:37 PM, Dave Cridland wrote:
> Trouble is, I suspect too many things expect a features block after the
> header, and I've certainly seen clients crash when faced with empty
> features 

Submit a bug report?

> - and sending features beyond empty seems wasteful.
> 

OK, so: what about about sending empty features in these cases of
immediate stream close? The receiving entity isn't offering any features
then anyway, because it's immediately closing the stream with an error.

Peter

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

---

From: Justin Karneges <[email-masked]>
Date: Tue, 10 Apr 2012 13:57:33 -0700

On Tuesday, April 10, 2012 01:51:06 PM Justin Karneges wrote:
> As a worst case, I would think even if there exist implementations
> incapable of handling this properly, they might handle this in some
> acceptably improper way, such as considering it to be invalid protocol or
> to be a random disconnect. The true reason for error is lost in this case,
> which could cause some problems, but in practice I wonder they'd be. Maybe
> mishandling of "host- unknown" or "conflict" ? Any others?

Also, it seems Prosody puts version 1.0 on stream errors even if there are no 
stream features. I tested this just now with Psi by trying to connect to an 
unknown domain against a manually specified server. Also, Psi correctly picks 
this up as a host-unknown error in the UI.

I suspect that clients have had to deal with this behavior in the wild now for 
awhile, and don't rely on a stream features arriving.

Justin

---

From: Dave Cridland <[email-masked]>
Date: Tue, 10 Apr 2012 21:57:55 +0100

> then anyway, because it's immediately closing the stream with an  

I could go along with that, yes, but I'd like to look at the extent  
of the "empty features" issue. If it's just one client I'm happy.

Dave.

---

From: Matt Miller <[email-masked]>
Date: Tue, 10 Apr 2012 15:34:43 -0600

> awhile, and don't rely on a stream features arriving.

Basically this.  All of the clients I've worked on have had to deal with an opened stream, no features, and a stream:error.

- m&m

Matt Miller - <[email-masked]>
Cisco Systems, Inc.

---

From: Florian Zeitz <[email-masked]>
Date: Wed, 11 Apr 2012 02:33:27 +0200

Am 10.04.2012 22:57, schrieb Dave Cridland:
> I could go along with that, yes, but I'd like to look at the extent of
> the "empty features" issue. If it's just one client I'm happy.
> 
I think I need some clarification on what we're actually trying to
accomplish here.

As I have not seen a real world example yet, I'm going to assume we're
designing for a hypothetical client (that may or may not exist in reality).

That hypothetical client would expect a features block if it sees a
version attribute in the stream header.
The implication being it does not actually look at the name of the first
element it receives.

What I'm not clear on is what this hypothetical client does when it
doesn't receive a version attribute.
Does it act more intelligently and check whether the element's name is
"error" or "feature"? Does it blindly assume it is an "error" element?

In either case I'm under the impression that just looking at the
element's name instead of relying on the existence of the version
attribute is a far more intuitive implementation. As such I'd be very
surprised to see a client that does not choose this way to implement it.

Also we have text that mandates the inclusion of the version attribute:
«The receiving entity MUST set the value of the 'version' attribute in
the response stream header to either the value supplied by the
initiating entity or the highest version number supported by the
receiving entity, whichever is lower»

As such I consider both, not including features, as well as, not
including a version attribute in violation of the RFC.
That, from a protocol standpoint, would imply sending a version and
empty features.
However, accommodating current implementations, I too have seen
implementations that will not play nicely with empty features, I have
however not yet witnessed an implementation that would not accept an
error when the version attribute was specified.

Regards,
Florian Zeitz

---

From: Justin Karneges <[email-masked]>
Date: Tue, 10 Apr 2012 18:13:36 -0700

> error when the version attribute was specified.

Good point. Sending an empty features list to a client seems sketchy, and 
there's a good chance the client would throw an error about encountering 
invalid protocol before it would have a chance to read the stream:error that 
follows.

Looking at the code for Psi, it would appear that if an empty features element 
is receved then it will switch to legacy auth mode and immediately send an iq 
for jabber:iq:auth. This works because starttls is optional, and bind doesn't 
matter unless SASL is used and completed. A client not supporting legacy auth 
would almost surely complain about the lack of SASL mechanisms and fail early 
though.

Justin

---

From: Dave Cridland <[email-masked]>
Date: Wed, 11 Apr 2012 08:41:53 +0100

> designing for a hypothetical client (that may or may not exist in  
I'm mostly concerned with servers. Diagnosing S2S failure is much,  
much harder than diagnosing C2S failure as it is.

> The implication being it does not actually look at the name of the  
No, that's not the implication. The implication is that its code  
paths rely on features being the first element, and it fails to  
handle any other case. You do indeed describe one possible case.

This is reasonable behaviour according to the specification, I'd  
remind you - we have made a guarantee that an initiator will see a  
features element directly after the stream header. Changing that  
effectively means pulling the rug out from under those  
implementations.

> "error" or "feature"? Does it blindly assume it is an "error"  
It won't be expecting an error. What'll likely happen is it'll check  
for the dialback namespace declaration, and probably start trying  
dialback regardless of whether it finds one - and find the error "in  
response".

Since in practise most servers understand stream errors even if they  
only do 0.9 on S2S, because they already do 1.0 on C2S (think Tigase,  
Openfire, Google...) then they'll handle the case fine.

Sorry, did you want to stick with hypothetical implementations?

> surprised to see a client that does not choose this way to  
Be surprised about servers, then.

I'm not claiming that servers which fall foul of the "got version,  
expect features, ignore error" are very good implementations, or  
indeed that you (or I) would write one this way.

But the reality is that such implementations are within the  
specification as it is currently written, and if there's a way of  
addressing the issue *without* changing the behaviour this seems the  
best path.

> receiving entity, whichever is lower»
Only if you're intending speaking XMPP.

> error when the version attribute was specified.

I can believe that of clients.

In any case, I think if you do want to go this route, the correct fix  
is to state that a responding stream header MUST be followed by  
either a features or an error element.

Dave.

---

From: Ralph Meijer <[email-masked]>
Date: Wed, 11 Apr 2012 12:09:27 +0200

On 2012-04-11 09:41, Dave Cridland wrote:
> [..]
> In any case, I think if you do want to go this route, the correct fix is
> to state that a responding stream header MUST be followed by either a
> features or an error element.

… or nothing, closing the connection with or without a "</stream:stream>".

That seems good to me. I don't see any value in sending an empty 
features element at all. As it is now, this is really sending the wrong 
message, too. From RFC 6120, section 4.3.2:

     The <features/> element can contain one child, contain multiple
     children, or be empty.

and:

   An empty <features/> element indicates that the stream negotiation is
   complete and that the initiating entity is cleared to send XML
   stanzas.

I think that we can agree that entities that cannot cope with an empty 
features element are broken. I'm ok with them treating the combination 
with an error element and a connection close as a 'random' lost connection.

-- 
ralphm

---

From: Matthew Wild <[email-masked]>
Date: Wed, 11 Apr 2012 15:16:04 +0100

> this up as a host-unknown error in the UI.

Indeed. In my view a stream:error can happen anywhere, including
before stream:features. I don't really understand what the fuss is
about here.

Regards,
Matthew

---

From: Matt Miller <[email-masked]>
Date: Wed, 11 Apr 2012 08:30:09 -0600

> before stream:features. I don't really understand what the fuss is

+1

- m&m

---

From: Peter Saint-Andre <[email-masked]>
Date: Wed, 11 Apr 2012 08:38:30 -0600

On 4/11/12 8:30 AM, Matt Miller wrote:
>> before stream:features. I don't really understand what the fuss
>> is about here.
> 
> +1

Right, that's why I proposed adding a bit of wiggle room in the text.

Peter

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

---

From: Matt Miller <[email-masked]>
Date: Wed, 11 Apr 2012 08:51:28 -0600

On Apr 11, 2012, at 08:38, Peter Saint-Andre wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 

Consider this a transitive +1 to the proposal (-:

- m&m