Re: [xmpp] Robert Sparks' Discuss on draft-ietf-xmpp-3921bis-19: (with DISCUSS and COMMENT)

[Robert Sparks <[e-mail]>]

On Jan 20, 2011, at 12:46 PM, Peter Saint-Andre wrote:

> [ cc'ing [e-mail] to keep the WG in the loop ]
> 
> On 1/19/11 2:02 PM, Robert Sparks wrote:
>> Robert Sparks has entered the following ballot position for 
>> draft-ietf-xmpp-3921bis-19: Discuss
>> 
>> 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:
>> ----------------------------------------------------------------------
>> 
>> DISCUSS In section 3.1.3 item 4, a server is required to keep a
>> complete presence stanza for an unbounded amount of time (potentially
>> forever).  This looks like a vector for a state-exhaustion attack.
>> The restriction for keeping only one such stanza per sender helps,
>> but a malicious attacking server could send stanzas from an arbitrary
>> number of identites. > I suggest providing implementation guidance on
>> when it's ok to drop information that's truly stale, and call out the
>> potential for this attack in the security considerations.
> 
> You're right that we had not considered this attack. Possibly also the
> attack is not limited to a rogue server -- e.g., a botnet spread across
> multiple accounts on one or more servers could flood a victim server or
> victim account with presence subscription requests.
> 
> I propose the following text:
> 
>      Security Warning: The mandate for the contact's server to store
>      the complete stanza of the presence subscription request
>      introduces the possibility of an application resource exhaustion
>      attack (see Section 2.1.2 of [DOS]), for example by a rogue server
>      or a coordinated group of users (e.g., a botnet) against the
>      contact's server or particular contact.  Server implementors are
>      advised to consider the possibility of such attacks and provide
>      tools for counteracting it, such as enabling service
>      administrators to set limits on the number or size of inbound
>      presence subscription requests that the server will store in
>      aggregate or for any given contact.
wfm
> 
>> Section 5.2.4 calls out "for the purpose of providing alternate
>> versions of the same subject" when allowing multiple subjects to
>> appear with different xml:lang attributes. Section 5.2.3 does not
>> have analogous text constraining multiple bodies. Should it?
> 
> Yes, it should. Fixed in my working copy.
> 
>> Section 5.3 - Was the intent to constrain the contents of a message
>> stanza to things intended to be rendered to a user? 
> 
> No.
> 
>> As written, this
>> section would allow <message/> to be used as part of the framing of
>> an arbitrary transport protocol.
> 
> Yep. Here is an example:
> 
> http://xmpp.org/extensions/xep-0047.html
> 
> And that works quite well, too! However, we've been working to get folks
> to use <iq/> stanzas for in-band bytestreams for better flow control.

Noted. 

> 
>> Section 5.3 - The document should say what an implementation
>> receiving a child element in a namespace it doesn't understand should
>> do with that child.
> 
> That's covered in Section 8.4 of draft-ietf-xmpp-3920bis, and inherited
> thereby.
> 
> I propose modifying the paragraph as follows:
> 
>   As described in [XMPP-CORE], an XML stanza MAY contain any child
>   element that is qualified by a namespace other than the default
>   namespace; this applies to the message stanza as well.  Guidelines
>   for handling extended content on the part of both routing servers and
>   end recipients are provided in Section 8.4 of [XMPP-CORE].
wfm
> 
>> ----------------------------------------------------------------------
>> 
>> 
> COMMENT:
>> ----------------------------------------------------------------------
>> 
>> COMMENT
>> 
>> In section 2.3.3, is the intent that error conditions listed earlier
>> in the section take precedence over error conditions listed later in
>> the section? What's the appropriate response if a message qualifies
>> for both a <forbidden/> and a <bad-request/> as described in this
>> section?
> 
> That up to the implementation. In general, implementations seem to take
> security-related transgressions (e.g., <forbidden/>) more seriously than
> protocol-related peccadilloes (e.g., <bad-request/>).

Right now it's implicitly up to the implementation - consider making it explicit?
I can see interop arguments over this point.

> 
>> When would an element choose not to follow the SHOULD in 3.1.2 item
>> 1, and what are the consequences?
> 
> There are two points covered in this paragraph:
> 
> 1. Whether the JID is a bare JID or a full JID. In some deployments it
> might be fine to allow full JIDs to subscribe, however we tend to
> discourage that, thus the SHOULD. The consequences mostly relate to the
> subscriber, not the subscribee, so as far as I know there is no strong
> agreement in the developer community about making this a MUST.
> 
> 2. Whether the subscriber JID adheres to the JID format. In general, it
> is the responsibility of the sending server to authenticate entities
> correctly and enforce the JID format, not the responsibility of a server
> or end recipient. This is related somewhat to the address spec (and
> rfc3920[bis]), which have never been completely clear about which
> entities are responsible for enforcing the JID format. However, if
> anything that would be the responsibility of the sending server, and
> this will be clarified in revisions to the address spec. Probably that
> will be a topic at the interim meeting a few weeks from now.
> 
> Given my reading of the community, SHOULD is as strong as we can get
> here for now.

My question was about "SHOULD check the syntax" not "SHOULD  treat it
as if". Apologies for not being more clear.
As you talk about the clarifications you indicate in 2), consider writing
your normative text more strongly and noting that there are existing implementations
that may not yet comply.

> 
>> The end of section 4.3 has an implementation note containing
>> normative requirements. Can these be moved into the main prose? >
> 
> Done in my working copy.
> 
>> This
>> same implementation note may be easy to misinterpret. I don't believe
>> it is meant to prevent an implementation from returning an error to a
>> malformed presence probe, but it could be read that way.
> 
> I propose the following modified paragraph:
> 
>   Presence probes SHOULD NOT be sent by a client, because in general a
>   client will not need to send them since the task of gathering
>   presence from a user's contacts is managed by the user's server.
>   However, if a user's client generates an outbound presence probe then
>   the user's server SHOULD route the probe (if the contact is at
>   another server) or process the probe (if the contact is at the same
>   server) and MUST NOT use its receipt of the presence probe from a
>   connected client as a cause for returning a stanza or stream error to
>   the client.

That text is more clear, but it still has the potential for the misread I'm pointing to. 
A change along the lines of "as a cause" to "as the sole reason" would close the 
hole I think I see.


> 
> The last clause speaks to a bug that existed in at least one XMPP
> server: if a connected client sent a presence probe then the server
> would return a presence error or even terminate the client's session.
> Hopefully the revised text is a bit clearer.
> 
>> Section 5.2.5 does not prevent a thread from claiming it is its own
>> parent (should that be allowed?), or warn implementations about the
>> possibility of thread A indicating a parent of B, and thread B
>> indicating a parent of A (generalize this to any kind of cycle). I
>> don't think XEP-0201 calls this out either. Should it be mentioned as
>> an implementation consideration? (A naive implementation would very
>> likely crash or do something surprising with its UI.)
> 
> You're right that we had not considered that scenario.
> 
> Does the following text address your concern?
> 
> ###
> 
>   The <thread/> element MAY possess a 'parent' attribute that
>   identifies another thread of which the current thread is an offshoot
>   or child.  The 'parent' attribute MUST conform to the syntax of the
>   <thread/> element itself and its value MUST be different from the XML
>   character data of the <thread/> element on which the 'parent'
>   attribute is included.
> 
>      Implementation Note: The ability to specify both a parent thread
>      and a child thread introduces the possibility of conflicts between
>      thread identifiers for overlapping threads.  For example, one
>      <thread/> element might contain XML character data of "foo" and a
>      'parent' attribute whose value is "bar", a second <thread/>
>      element might contain XML character data of "bar" and a 'parent'
>      attribute whose value is "baz", and a third <thread/> element
>      might contain XML character data of "baz" and a 'parent' attribute
>      whose value is once again "foo".  It is up to the implementation
>      how it will treat conflicts between such overlapping thread
>      identifiers (e.g., whether it will "chain together" thread
>      identifiers by showing "foo" as both a parent and grandchild of
>      "baz" in a multi-level user interface, or whether it will show
>      only one level of dependency at a time).
> 
> ###
That's certainly enough to get the implementer to start thinking about
the edge cases. I'm not sure it would cause them to recognize they need to deal with 
     <thread parent='foo'>
       foo
     </thread>
reasonably, and still wonder if the spec should say something about whether that should
be allowed, but leave that to your judgement.

> 
>> In section 8.5.2.1, I encourage adding text explaining why delivering
>> to all non-negative resources is SHOULD and not MUST. I understand
>> from out-of-band communication that local policy may determine that
>> some particular resource not receive the message, but the document
>> does not say this. As written, I can see implementations that adopt a
>> policy of delivering to the first non-negative resource and claim
>> they've met the requirement.
> 
> I assume you're referring to this paragraph in the subsection about
> message stanzas (8.5.2.1.1):
> 
>   o  If there is more than one resource with a non-negative presence
>      priority then the server SHOULD either (a) deliver the message to
>      the "most available" resource or resources (according to the
>      server's implementation-specific algorithm, e.g., treating the
>      resource or resources with the highest presence priority as "most
>      available") or (b) deliver the message to all of the non-negative
>      resources.

For all of the similar paragraphs for "normal", "chat", and "headline"

> 
> The traditional behavior was (a). Some implementations and deployments
> follow (b) -- the first major example was the Google Talk service. In
> the XMPP WG we had some discussions about this and decided to allow (b)
> if the implementation or deployment deems that necessary or desirable.
> However, making (b) a MUST and disallowing (a) was too large a change
> from existing practice.

We might be talking past each other a bit (since I made a similar comment on 3920).
I'm not asking for discarding a and making b a MUST. I'm suggesting it would be
better to say MUST (a or b) instead of SHOULD (a or b).

That said, you could argue that the implementation choice I call about above could be
justified as an instance of a) with a local policy of "the first non-negative resource I can 
find is the 'most available'", so the difference may not be great enough to bother.

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