~samwhited/xmpp

xmpp/mux d---------
all: add strict namespace checking to mux

Previously anything that had a local name that looked like a stanza
("message", "iq", or "presence") would be treated as such, opening us up
to potential vulnerabilities in servers that allow a
{jabber:server}message on a jabber:client stream but don't treat it as a
stanza, for example.
Being more strict about checking namespaces helps to avoid this issue,
but does require a breaking change to the mux API.

Signed-off-by: Sam Whited <sam@samwhited.com>
all: be more strict about stanza namespaces

Previously we were checking if something was a stanza based on faulty
assumptions (that there were only two valid stanza namespaces) and
without precision (we'd accept it if it were either namespace or none
instead of only accepting the actual namespace used by teh stream).
This PR fixes this to ensure that eg. a vulnerability that causes the
server to allow {jabber:server}message's sent over a jabber:client
stream without any of the normal security measures still won't affect
this library (hopefully).

This PR updates all locations to use the new APIs, but does not
necessarily perform the correct checks. A followup PR will be rquired to
upgrade some of the packages (eg. mux) to use the correct namespace.

Signed-off-by: Sam Whited <sam@samwhited.com>
all: fix lints in doc comments

Signed-off-by: Sam Whited <sam@samwhited.com>
internal/stream: remove s2s bool from stream send

Previously we set the namespace depending on whether a server-to-server
value was set. However, the namespace is set in the stream and may be
other values (such as the Jabber Component namespace) as well, so just
leave this alone and let the negotiator handle it.

Signed-off-by: Sam Whited <sam@samwhited.com>
404b735d — genofire a month ago
mux: ignore namespace when matching stanzas

Previously we checked that stanzas were actually stanzas (they had the
namespace jabber:client or jabber:server), however, this check is not
technically correct because there is also the component namespace.
For now, remove the check and treat all elements named "message", "iq",
or "presence" as stanzas. In the future we should improve this to allow
the negotiator to set the stanza namespace on the session and only check
for the specific correct namespace which would be more correct than this
solution or the previous way.

Signed-off-by: genofire <geno+dev@fireorbit.de>
all: implement disco identities and forms

Previously our service discovery implementation could only respond with
features. However, service discovery also supports "identities" and has
an extension mechanism that lets it respond with arbitrary data in the
form (pun intended) of a form result.

Adding implementations of these iterators to the mux and using them in
the disco handler brings us fully up to date on service discovery
features and should allow us to also implement entity capabilities in
the near future.

Signed-off-by: Sam Whited <sam@samwhited.com>
mux: support responding to disco#items requests

Signed-off-by: Sam Whited <sam@samwhited.com>
all: implement server side of disco#info

This finally closes the long standing need for service discovery by
implementing the new design doc from https://mellium.im/design/28_disco!

Fixes #28

Signed-off-by: Sam Whited <sam@samwhited.com>
all: add function stanza.Is()

Add exported function stanza.Is() which tests an xml.Name for its name
and space. If it is a proper stanza it returns true.
This function was repeatedly defined in several files and is now defined
one time and exported in stanza/stanza.go

Fixes #113
mux: make error names match convention in tests

Signed-off-by: Sam Whited <sam@samwhited.com>
mux: fix the signature of IQFunc

Previously IQFunc was useless because it was identical to the IQ option.
This patch makes it take an IQHandlerFunc instead of an IQHandler
similar to how Message/MessageFunc and Presence/PresenceFunc work.

Signed-off-by: Sam Whited <sam@samwhited.com>
mux: remove unnecessary argument names

Signed-off-by: Sam Whited <sam@samwhited.com>
mux: move options into their own file

This is a minor refactor that just oves options out into their own file.
While working on the disco package I decided to move them because its
integration with the mux package was going to add even more options and
they were getting a bit hard to find in the sam file as everything else.
Going ahead and moving them out keeps the diff smaller during that work
and makes it easier to see what has changed.

Signed-off-by: Sam Whited <sam@samwhited.com>
all: use xmlstream.Iter and remove internal/iter

The internal/iter package previously contained an API for iterating over
child elements and decoding them lazily.

This is needed by any package that exposes an iterator such as the
roster package, and is mentioned in the documentation
(https://mellium.im/docs/extensions) as the API to use when creating
your own extensions, but it was internal and not actually usable by this
package. Its own doc comment said that it would eventually be exported
when the API stabalized. It hasn't been necessary to change the API
since creating it so it became time to let it graduate to the
mellium.im/xmlstream module where it can be more broadly useful.

This patch bumps the version of xmlstream used and makes the minor
changes necessary to use the xmlstream version of the iterator. It also
removes the old internal/iter package and updates the documentation to
mention its new location as part of xmlstream.

They grow up so fast!

Signed-off-by: Sam Whited <sam@samwhited.com>
all: improve documentation about multiplexers

I noticed that the main package already contained documentation about
the Serve function and writing a handler, but did not mention the mux
package or its more granular handler types.
Similarly, the mux package itself didn't have much in the way of
description and one would be forgiven for thinking that you couldn't
write your own, compatible, muxer.

Improving this documentation will go a long way towards making it easier
to get started with this library.

Signed-off-by: Sam Whited <sam@samwhited.com>
mux: close mux iterator

An element iterator used in the muxer was previously unclosed. In this
case, the close method will likely always be a noop, but just in case
close it so that if we make changes later we don't introduce a lock
contention or resource freeing issue.

Signed-off-by: Sam Whited <sam@samwhited.com>
mux: fix a possible infinite loop

Previously a bug in EOF error handling could have lead to the last token
of a message stream being cut off. This could lead to Decode entering an
infinite loop of nil tokens and nil errors being returned. This is
likely a bug in the encoding/xml package which should be fixed as well,
but for now we can just make sure that we don't trigger it.

Signed-off-by: Sam Whited <sam@samwhited.com>
all: include close element in handler stream

Previously the close element was not included in the limited XML stream
passed to handlers. This made it possible to enter an infinite loop or
deadlock when trying to use the DecodeElement method of xml.Decoder's.
A regression test has also been added to make sure that we can't read
beyond the end of the element (which was previously the case but there
was no test for this, which was dangerous) and to make sure that the new
behavior (with the end element) is enforced.

Signed-off-by: Sam Whited <sam@samwhited.com>
mux: use xml.CopyToken instead of manual copying

Previously we were manually copying the token for token types that have
bufferes that get reused. Instead, use a function out of the
encoding/xml package that can copy any valid token type.

Signed-off-by: Sam Whited <sam@samwhited.com>
mux: use existing child element iterator

An iterator already exists in the mellium.im/xmpp/internal/iter package
that can be used to iterate over child elements. Reusing this keeps
things simple.

Signed-off-by: Sam Whited <sam@samwhited.com>
Next