Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Config option to inhibit 3PID errors on /requestToken #7315

Merged
merged 1 commit into from
Apr 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/7315.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow `/requestToken` endpoints to hide the existence (or lack thereof) of 3PID associations on the homeserver.
10 changes: 10 additions & 0 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,16 @@ retention:
# longest_max_lifetime: 1y
# interval: 1d

# Inhibits the /requestToken endpoints from returning an error that might leak
# information about whether an e-mail address is in use or not on this
# homeserver.
# Note that for some endpoints the error situation is the e-mail already being
# used, and for others the error is entering the e-mail being unused.
# If this option is enabled, instead of returning an error, these endpoints will
# act as if no error happened and return a fake session ID ('sid') to clients.
#
#request_token_inhibit_3pid_errors: true


## TLS ##

Expand Down
21 changes: 21 additions & 0 deletions synapse/config/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,17 @@ class LimitRemoteRoomsConfig(object):

self.enable_ephemeral_messages = config.get("enable_ephemeral_messages", False)

# Inhibits the /requestToken endpoints from returning an error that might leak
# information about whether an e-mail address is in use or not on this
# homeserver, and instead return a 200 with a fake sid if this kind of error is
# met, without sending anything.
# This is a compromise between sending an email, which could be a spam vector,
# and letting the client know which email address is bound to an account and
# which one isn't.
self.request_token_inhibit_3pid_errors = config.get(
"request_token_inhibit_3pid_errors", False,
)

def has_tls_listener(self) -> bool:
return any(l["tls"] for l in self.listeners)

Expand Down Expand Up @@ -967,6 +978,16 @@ def generate_config_section(
# - shortest_max_lifetime: 3d
# longest_max_lifetime: 1y
# interval: 1d
# Inhibits the /requestToken endpoints from returning an error that might leak
# information about whether an e-mail address is in use or not on this
# homeserver.
# Note that for some endpoints the error situation is the e-mail already being
# used, and for others the error is entering the e-mail being unused.
# If this option is enabled, instead of returning an error, these endpoints will
# act as if no error happened and return a fake session ID ('sid') to clients.
#
#request_token_inhibit_3pid_errors: true
"""
% locals()
)
Expand Down
17 changes: 16 additions & 1 deletion synapse/rest/client/v2_alpha/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
)
from synapse.push.mailer import Mailer, load_jinja2_templates
from synapse.util.msisdn import phone_number_to_msisdn
from synapse.util.stringutils import assert_valid_client_secret
from synapse.util.stringutils import assert_valid_client_secret, random_string
from synapse.util.threepids import check_3pid_allowed

from ._base import client_patterns, interactive_auth_handler
Expand Down Expand Up @@ -100,6 +100,11 @@ async def on_POST(self, request):
)

if existing_user_id is None:
if self.config.request_token_inhibit_3pid_errors:
# Make the client think the operation succeeded. See the rationale in the
# comments for request_token_inhibit_3pid_errors.
babolivier marked this conversation as resolved.
Show resolved Hide resolved
babolivier marked this conversation as resolved.
Show resolved Hide resolved
return 200, {"sid": random_string(16)}

raise SynapseError(400, "Email not found", Codes.THREEPID_NOT_FOUND)

if self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
Expand Down Expand Up @@ -378,6 +383,11 @@ async def on_POST(self, request):
)

if existing_user_id is not None:
if self.config.request_token_inhibit_3pid_errors:
# Make the client think the operation succeeded. See the rationale in the
# comments for request_token_inhibit_3pid_errors.
return 200, {"sid": random_string(16)}

raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE)

if self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
Expand Down Expand Up @@ -441,6 +451,11 @@ async def on_POST(self, request):
existing_user_id = await self.store.get_user_id_by_threepid("msisdn", msisdn)

if existing_user_id is not None:
if self.hs.config.request_token_inhibit_3pid_errors:
# Make the client think the operation succeeded. See the rationale in the
# comments for request_token_inhibit_3pid_errors.
return 200, {"sid": random_string(16)}

raise SynapseError(400, "MSISDN is already in use", Codes.THREEPID_IN_USE)

if not self.hs.config.account_threepid_delegate_msisdn:
Expand Down
12 changes: 11 additions & 1 deletion synapse/rest/client/v2_alpha/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
from synapse.push.mailer import load_jinja2_templates
from synapse.util.msisdn import phone_number_to_msisdn
from synapse.util.ratelimitutils import FederationRateLimiter
from synapse.util.stringutils import assert_valid_client_secret
from synapse.util.stringutils import assert_valid_client_secret, random_string
from synapse.util.threepids import check_3pid_allowed

from ._base import client_patterns, interactive_auth_handler
Expand Down Expand Up @@ -135,6 +135,11 @@ async def on_POST(self, request):
)

if existing_user_id is not None:
if self.hs.config.request_token_inhibit_3pid_errors:
# Make the client think the operation succeeded. See the rationale in the
# comments for request_token_inhibit_3pid_errors.
return 200, {"sid": random_string(16)}

raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE)

if self.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
Expand Down Expand Up @@ -202,6 +207,11 @@ async def on_POST(self, request):
)

if existing_user_id is not None:
if self.hs.config.request_token_inhibit_3pid_errors:
# Make the client think the operation succeeded. See the rationale in the
# comments for request_token_inhibit_3pid_errors.
return 200, {"sid": random_string(16)}

raise SynapseError(
400, "Phone number is already in use", Codes.THREEPID_IN_USE
)
Expand Down
16 changes: 16 additions & 0 deletions tests/rest/client/v2_alpha/test_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,22 @@ def test_no_valid_token(self):
# Assert we can't log in with the new password
self.attempt_wrong_password_login("kermit", new_password)

@unittest.override_config({"request_token_inhibit_3pid_errors": True})
def test_password_reset_bad_email_inhibit_error(self):
"""Test that triggering a password reset with an email address that isn't bound
to an account doesn't leak the lack of binding for that address if configured
that way.
"""
self.register_user("kermit", "monkey")
self.login("kermit", "monkey")

email = "test@example.com"

client_secret = "foobar"
session_id = self._request_token(email, client_secret)

self.assertIsNotNone(session_id)

def _request_token(self, email, client_secret):
request, channel = self.make_request(
"POST",
Expand Down
47 changes: 46 additions & 1 deletion tests/rest/client/v2_alpha/test_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@

class RegisterRestServletTestCase(unittest.HomeserverTestCase):

servlets = [register.register_servlets]
servlets = [
login.register_servlets,
register.register_servlets,
synapse.rest.admin.register_servlets,
]
url = b"/_matrix/client/r0/register"

def default_config(self, name="test"):
Expand Down Expand Up @@ -260,6 +264,47 @@ def test_advertised_flows_no_msisdn_email_required(self):
[["m.login.email.identity"]], (f["stages"] for f in flows)
)

@unittest.override_config(
{
"request_token_inhibit_3pid_errors": True,
"public_baseurl": "https://test_server",
"email": {
"smtp_host": "mail_server",
"smtp_port": 2525,
"notif_from": "sender@host",
},
}
)
def test_request_token_existing_email_inhibit_error(self):
"""Test that requesting a token via this endpoint doesn't leak existing
associations if configured that way.
"""
user_id = self.register_user("kermit", "monkey")
self.login("kermit", "monkey")

email = "test@example.com"

# Add a threepid
self.get_success(
self.hs.get_datastore().user_add_threepid(
user_id=user_id,
medium="email",
address=email,
validated_at=0,
added_at=0,
)
)

request, channel = self.make_request(
"POST",
b"register/email/requestToken",
{"client_secret": "foobar", "email": email, "send_attempt": 1},
)
self.render(request)
self.assertEquals(200, channel.code, channel.result)

self.assertIsNotNone(channel.json_body.get("sid"))


class AccountValidityTestCase(unittest.HomeserverTestCase):

Expand Down