David Waite already wrote most of what I wanted to say. I'd just like to add a few more points.
I agree with you (Dave) that servers can function without constructing a complete DOM for every stanza. But I disagree with some of what you said. On Fri, Oct 24, 2008 at 4:33 AM, Dave Cridland <[EMAIL PROTECTED]> wrote: > Let's quickly remind ourselves of the key issues. I apologise for this > message being so damn long, but for people who read all the way, there's > some suggested RFC text as a prize. > > > 1) Protecting Implementations in the current environment: > > Whether or not we want this to happen, servers can currently, and do, handle > XML for forwarding purposes without performing full namespace checks on the > data. In particular, they do not, always, check for undeclared namespaces, > and this has a very variable effect on the receiver. > > In some cases, the receiver terminates the session - this has proven highly > undesirable. In other cases, the undeclared prefix is ignored. Finally, in > principle, the stanza could be rejected somehow, although as Jack pointed > out last night, this would require an error without including the original > stanza if the receiver doesn't want to retransmit an undeclared prefix. > > We need to decide on desirable behaviour when faced with XML which is > well-formed, but has undeclared namespaces present. More than one behaviour > may be acceptable. > > My personal view is that stream termination is not really acceptable, but > sometimes implementors have little choice, and that the next easiest to > implement is to essentially ignore undeclared prefixes, treating them as if > they were an unknown URI. > > I'd agree that stream termination is currently unacceptable for data coming from a server. I think it would be a good idea for servers to at least fully validate data coming from clients, and terminate the stream for clients sending bad data. This can be done while fully "Protecting Implementations in the current environment" > 2) Why we might want to let servers forward "Bad XMLNS": > > As stated above, the status quo is that some servers do forward XML without > checking that every prefix used is declared. > > There are advantages in doing so, indeed, since an XML stream can be split > into stanzas, and the outer element of the stanza examined, without doing > more than a relatively simple, non-destructive, lex of a buffer. > > This seems to give people some confusion, so forgive me while I explain this > in detail. > > In other words, a server starts off with a string: > > "<message to='[EMAIL PROTECTED]' type='headline'><!-- A comment containing a < > --><![CDATA[ A CDATA section, perhaps with < or > in ]]><foo/></message>" > > By iterating through the string, character by character (or, more likely, > octet by octet, since it's equivalent in UTF-8 for these purposes), and > maintaining a very small amount of fixed state irrespective of the "depth" > of the XML, we can find the end of the element. > A simple lexer simply wouldn't cut it. Show me a lexer which doesn't do what a parser would need to do anyway, and I'll show you invalid XML which it happily accepts as valid. (If by lexing you mean parsing which doesn't do the usual SAX callbacks, then I'd agree with you, but that would still be a full parse, not lexical analysis). > No allocations are involved, no new buffers, and the original buffer can be > left intact, for copying directly to the write buffer of the outgoing > stream. This is more or less exactly what Isode's implementation does, and > this, like Artur has said as I type this, is a key reason for good > performance. (And if anyone wants to measure it, please feel free to do so > and publish the results). > You still need a stack of element names and a table for attributes of the current element. But yes, once those buffers stabilize, you don't need to allocate more. But the same thing applies to namespace processing too. (And since servers use stanza size limits anyway, we can optimize things a lot). > The ability to pass buffers around for copying is obviously more efficient > than creating DOMs and reserializing. The number of allocations are > significant to a long running application for two key reasons - note that > this applies equally to both relatively low level languages like C, and > higher level languages such as Python or Java. > Agreed. > Firstly, memory allocations typically involve a lock, and do not sit well > with multithreaded applications. This is typically reduced by pools, and > per-thread allocators, but is easily disrupted simply by handing ownership > of memory between threads. > > Secondly, rapid allocation patterns cause memory fragmentation, which > increases memory load, causing a substantial decrease over time of > performance. (Bad memory fragmentation sometimes appears to be a memory > leak, even though checking every allocation and deallocation won't find > one). It's fair to say that modern allocators have reduced the effects > somewhat, but no allocations are still considerably better than lots. > I agree completely. > If servers were unconditionally mandated to check each and every prefix in > incoming streams, then servers would be forced to build an allocated lookup > table for prefixes. This lookup table needs to be adjusted potentially on > every element - with every opening tag, new prefixes can be defined - and > these affect the tag they're declared in, so we must handle cases such as: > > "<foo:element bar:attr='false' xmlns:foo='l' xmlns:bar='k'>" > > Which essentially nessecitates a three-phase parse, first finding the > possibly prefixed names and attributes, then gathering XML namespace > declarations, then finally resolving each prefix. At the closing tag, of > course, we need to locate every prefix declaration now leaving scope, and > remove it from our lookup. > Namespace processing does not require a three-phase parse. You are doing it 'incorrectly' in Gajim. I saw you are creating new namespace tables for every element. You do not need to (and should not) do this. This can (and should) be done using stream level stacks. All you really need here is stream level stacks. The simplest way is to use a map and a stack. For element start, push all new namespaces into the map, and keep an undo history on the stack. For end element, pop the stack, and undo the change caused for that element. This would be pretty fast (nearly as fast as matching start and closing tags), and would add almost zero overhead when namespace prefixes are not used. > And what for? > What David said. > Servers do not care about these for themselves, please note. If they did, > they're be doing this anyway - and for cases like roster manipulation, and > many other things, that's exactly what we'll do. Hence you can start off a > stream with "<s:stream xmlns:s='http://etherx.jabber.org/streams' > xmlns='jabber:client' xmlns:a='urn:ietf:params:xml:ns:xmpp-sasl' > to='example.com' id='asd'>", and the server will note that you're using "s" > as your stream prefix, and "a" as you SASL prefix, and be quite happy for > you to later to <a:auth/> in that same stream. > > So the sole reason for doing this extra work is to protect clients. But, go > back to issue 1, up there, for a second - some choices there can eliminate > the damage, too, leaving the clients perfectly well able to protect > themselves, and this is a desirable thing for a lcient to do. So is this > really needed? > > I'd expect, incidentally, that some servers would always check namespaces, > for security reasons, and be chosen because of it - it's a marketable > feature. I'd also expect that many servers might perform more stringent > checking on certain traffic - MUC springs to mind - to avoid being a DoS > amplifier (We might even recommend or mandate this in XEP-0045). > > > 3) And what's this RFC 2119 thing, anyway? > > Finally, a reminder, to folks that haven't read RFC 2119 - they're not > statements of opinion. > > "MUST" = "If you do not follow this, there is serious damage to > interoperability." > "SHOULD" = "If you do not follow this, then your implementation may have or > cause problems in some cases. Consider the consequences carefully." > "MAY" = "It doesn't matter whether or not you choose to do this, but be > warned that someone else might choose either way too." > "SHOULD NOT" = "Don't do this unless you know what you're doing and fully > understand the consequences." > "MUST NOT" = "Don't do this and expect things to work." > > So I'm not really convinced that, given the status quo, mandating full > namespace checking on servers is really a MUST anyway. Most clients cope > perfectly well, after all. On that basis, this feels like a SHOULD. > They cope because they are forced to. That is /not/ a good thing. > It's been discussed on the IETF lists before that "SHOULD" and "SHOULD NOT" > ought to give guidance, so here is some, in xml2rfc form. I've reworded and > adjusted PSA's original text, to tighten the text a bit, provide suggested > answers to the above (which I don't claim to be consensus), and to pander to > my phrasing preferences. > > <t>An XMPP entity MUST NOT generate data that is not XML-well-formed. An > XMPP which receives data that is not XML-well-formed SHALL reject it by > terminating the stream over which the data was received with an > <xml-not-well-formed/> stream error.</t> > > <!-- Rationale for change: Saying "MUST NOT accept data" implies it's > possible, to me, which it isn't. And SHALL is terribly underused, so maybe > it'll get people to read RFC 2119 and discover what these things mean. This > rewording doesn't change PSA's meaning, of course. --> > > <t>Any elements within the XML stream other than XML stanzas, such as TLS or > SASL elements, MUST be namespace-well-formed, and XMPP entities SHALL reject > XML streams which fail to comply by terminating the stream over which the > data was received with an <not-well-formed/> stream error. XMPP > entities MUST generate namespace-well-formed stanzas.</t> > > <!-- There's all MUSTs here, and I think I'm safe in saying we all agree so > far. Nobody thinks we ought to have undeclared namespaces in > <stream:features/> or something, right? --> > > <t>It is known that deployed servers do not always exhaustively check for > undeclared namespaces in particular, before forwarding a stanza. Therefore > XMPP entities SHOULD NOT terminate a stream over which a stanza has been > received that is not namespace-well-formed, as otherwise there is a > potential Denial of Service attack, see <xref target='sec-xmlns'/>. Servers > SHOULD check that forwarded stanzas are namespace-well-formed in order to > protect clients from such data, as many existing XML parsers generate > unrecoverable errors in this case and therefore some clients are forced into > terminating the stream.</t> > > <!-- This is saying, essentially, that clients ought to protect themselves, > and servers ought to protect them too. Note the cross-reference to an > unwritten chunk of the Security Considerations. Sorry, Peter. :-) --> > > <t>XMPP entities which receive stanzas which are not namespace-well-formed > SHOULD reject them with a stanza error <not-acceptable/>.</t> > > <!-- I got the impression, perhaps wrongly, that this is the preferred > behaviour - I actually prefer this as a MAY, though - I think existing > clients actually tend to deal with the stanza as best as they can, by > treating the prefix as if it were declared to an unknown namespace, which > seems reasonable behaviour. This isn't just Gajim, I copied the behaviour > from Pidgin and Psi, which appear to do the same. --> > I think you might agree that a server should in fact disconnect C2S connections when receiving data which is not namespace-well-formed. My opinion of data originating from a server is that it should be sanitized before being passed on or used. But I disagree that namespace-invalid data should be allowed by the RFC. > Dave. > -- > Dave Cridland - mailto:[EMAIL PROTECTED] - xmpp:[EMAIL PROTECTED] > - acap://acap.dave.cridland.net/byowner/user/dwd/bookmarks/ > - http://dave.cridland.net/ > Infotrope Polymer - ACAP, IMAP, ESMTP, and Lemonade > Waqas.
