Skip to content

Commit

Permalink
Validate hostname in more places
Browse files Browse the repository at this point in the history
  • Loading branch information
Denis Kasak committed Mar 31, 2021
1 parent 9ad5c3d commit 8936925
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 5 deletions.
11 changes: 11 additions & 0 deletions sydent/hs_federation/verifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from signedjson.sign import SignatureVerifyException

from sydent.http.httpclient import FederationHttpClient
from sydent.util.stringutils import is_valid_hostname


logger = logging.getLogger(__name__)
Expand All @@ -37,6 +38,13 @@ class NoAuthenticationError(Exception):
pass


class InvalidServerName(Exception):
"""
Raised when the provided origin parameter is not a valid hostname (plus optional port).
"""
pass


class Verifier(object):
"""
Verifies signed json blobs from Matrix Homeservers by finding the
Expand Down Expand Up @@ -197,6 +205,9 @@ def strip_quotes(value):
if not json_request["signatures"]:
raise NoAuthenticationError("Missing X-Matrix Authorization header")

if not is_valid_hostname(json_request["origin"]):
raise InvalidServerName("X-Matrix header's origin parameter must be a valid hostname")

yield self.verifyServerSignedJson(json_request, [origin])

logger.info("Verified request from HS %s", origin)
Expand Down
13 changes: 9 additions & 4 deletions sydent/http/servlets/threepidunbindservlet.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import json
import logging

from sydent.hs_federation.verifier import NoAuthenticationError
from sydent.hs_federation.verifier import NoAuthenticationError, InvalidServerName
from signedjson.sign import SignatureVerifyException

from sydent.http.servlets import dict_to_json_bytes
Expand Down Expand Up @@ -81,7 +81,7 @@ def _async_render_POST(self, request):
# and "client_secret" fields, they are trying to prove that they
# were the original author of the bind. We then check that what
# they supply matches and if it does, allow the unbind.
#
#
# However if these fields are not supplied, we instead check
# whether the request originated from a homeserver, and if so the
# same homeserver that originally created the bind. We do this by
Expand Down Expand Up @@ -121,7 +121,7 @@ def _async_render_POST(self, request):
'error': "This validation session has not yet been completed"
}))
return

if s.medium != threepid['medium'] or s.address != threepid['address']:
request.setResponseCode(403)
request.write(dict_to_json_bytes({
Expand All @@ -143,7 +143,12 @@ def _async_render_POST(self, request):
request.write(dict_to_json_bytes({'errcode': 'M_FORBIDDEN', 'error': str(ex)}))
request.finish()
return
except:
except InvalidServerName as ex:
request.setResponseCode(400)
request.write(dict_to_json_bytes({'errcode': 'M_INVALID_PARAM', 'error': str(ex)}))
request.finish()
return
except Exception:
logger.exception("Exception whilst authenticating unbind request")
request.setResponseCode(500)
request.write(dict_to_json_bytes({'errcode': 'M_UNKNOWN', 'error': 'Internal Server Error'}))
Expand Down
14 changes: 13 additions & 1 deletion sydent/threepid/bind.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@

from sydent.threepid import ThreepidAssociation

from sydent.util.stringutils import is_valid_hostname

from twisted.internet import defer

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -131,15 +133,25 @@ def _notify(self, assoc, attempt):
"""
mxid = assoc["mxid"]
mxid_parts = mxid.split(":", 1)

if len(mxid_parts) != 2:
logger.error(
"Can't notify on bind for unparseable mxid %s. Not retrying.",
assoc["mxid"],
)
return

matrix_server = mxid_parts[1]

if not is_valid_hostname(matrix_server):
logger.error(
"MXID server part '%s' not a valid hostname. Not retrying.",
matrix_server,
)
return

post_url = "matrix://%s/_matrix/federation/v1/3pid/onbind" % (
mxid_parts[1],
matrix_server,
)

logger.info("Making bind callback to: %s", post_url)
Expand Down

0 comments on commit 8936925

Please sign in to comment.