OpenCloud
/
MatrixSynapse
mirror of https://github.com/matrix-org/synapse
1
0
Fork 0
Code Issues Releases Wiki Activity

Merge branch 'develop' into mv/add-mxid-validation-log

mv/add-mxid-validation-log
Mathieu Velten 2023-08-04 12:46:08 +02:00 committed by GitHub
parent dc9957dbba 0a5f4f7665
commit 1fe7be0be1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 208 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
changelog.d/15754.misc Normal file
View File

@ -0,0 +1 @@
Allow for the configuration of the backoff algorithm for federation destinations.

1
changelog.d/16017.removal Normal file
View File

@ -0,0 +1 @@
Move support for application service query parameter authorization behind a configuration option.

16
docs/upgrade.md
View File

@ -88,6 +88,21 @@ process, for example:
dpkg -i matrix-synapse-py3_1.3.0+stretch1_amd64.deb
```
# Upgrading to v1.90.0
## App service query parameter authorization is now a configuration option
Synapse v1.81.0 deprecated application service authorization via query parameters as this is
considered insecure - and from Synapse v1.71.0 forwards the application service token has also been sent via
[the `Authorization` header](https://spec.matrix.org/v1.6/application-service-api/#authorization)], making the insecure
query parameter authorization redundant. Since removing the ability to continue to use query parameters could break
backwards compatibility it has now been put behind a configuration option, `use_appservice_legacy_authorization`.
This option defaults to false, but can be activated by adding
```yaml
use_appservice_legacy_authorization: true
```
to your configuration.
# Upgrading to v1.89.0
## Removal of unspecced `user` property for `/register`
@ -97,7 +112,6 @@ The standard `username` property should be used instead. See the
[Application Service specification](https://spec.matrix.org/v1.7/application-service-api/#server-admin-style-permissions)
for more information.
# Upgrading to v1.88.0
## Minimum supported Python version

25
docs/usage/configuration/config_documentation.md
View File

@ -1242,6 +1242,14 @@ like sending a federation transaction.
* `max_short_retries`: maximum number of retries for the short retry algo. Default to 3 attempts.
* `max_long_retries`: maximum number of retries for the long retry algo. Default to 10 attempts.
The following options control the retry logic when communicating with a specific homeserver destination.
Unlike the previous configuration options, these values apply across all requests
for a given destination and the state of the backoff is stored in the database.
* `destination_min_retry_interval`: the initial backoff, after the first request fails. Defaults to 10m.
* `destination_retry_multiplier`: how much we multiply the backoff by after each subsequent fail. Defaults to 2.
* `destination_max_retry_interval`: a cap on the backoff. Defaults to a week.
Example configuration:
```yaml
federation:
@ -1250,6 +1258,9 @@ federation:
max_long_retry_delay: 100s
max_short_retries: 5
max_long_retries: 20
destination_min_retry_interval: 30s
destination_retry_multiplier: 5
destination_max_retry_interval: 12h
```
---
## Caching
@ -2837,6 +2848,20 @@ Example configuration:
```yaml
track_appservice_user_ips: true
```
---
### `use_appservice_legacy_authorization`
Whether to send the application service access tokens via the `access_token` query parameter
per older versions of the Matrix specification. Defaults to false. Set to true to enable sending
access tokens via a query parameter.
**Enabling this option is considered insecure and is not recommended. **
Example configuration:
```yaml
use_appservice_legacy_authorization: true
```
---
### `macaroon_secret_key`

34
synapse/appservice/api.py
View File

@ -16,7 +16,6 @@ import logging
import urllib.parse
from typing import (
TYPE_CHECKING,
Any,
Dict,
Iterable,
List,
@ -25,6 +24,7 @@ from typing import (
Sequence,
Tuple,
TypeVar,
Union,
)
from prometheus_client import Counter
@ -119,6 +119,7 @@ class ApplicationServiceApi(SimpleHttpClient):
def __init__(self, hs: "HomeServer"):
super().__init__(hs)
self.clock = hs.get_clock()
self.config = hs.config.appservice
self.protocol_meta_cache: ResponseCache[Tuple[str, str]] = ResponseCache(
hs.get_clock(), "as_protocol_meta", timeout_ms=HOUR_IN_MS
@ -132,9 +133,12 @@ class ApplicationServiceApi(SimpleHttpClient):
assert service.hs_token is not None
try:
args = None
if self.config.use_appservice_legacy_authorization:
args = {"access_token": service.hs_token}
response = await self.get_json(
f"{service.url}{APP_SERVICE_PREFIX}/users/{urllib.parse.quote(user_id)}",
{"access_token": service.hs_token},
args,
headers={"Authorization": [f"Bearer {service.hs_token}"]},
)
if response is not None: # just an empty json object
@ -155,9 +159,12 @@ class ApplicationServiceApi(SimpleHttpClient):
assert service.hs_token is not None
try:
args = None
if self.config.use_appservice_legacy_authorization:
args = {"access_token": service.hs_token}
response = await self.get_json(
f"{service.url}{APP_SERVICE_PREFIX}/rooms/{urllib.parse.quote(alias)}",
{"access_token": service.hs_token},
args,
headers={"Authorization": [f"Bearer {service.hs_token}"]},
)
if response is not None: # just an empty json object
@ -190,10 +197,12 @@ class ApplicationServiceApi(SimpleHttpClient):
assert service.hs_token is not None
try:
args: Mapping[Any, Any] = {
**fields,
b"access_token": service.hs_token,
}
args: Mapping[bytes, Union[List[bytes], str]] = fields
if self.config.use_appservice_legacy_authorization:
args = {
**fields,
b"access_token": service.hs_token,
}
response = await self.get_json(
f"{service.url}{APP_SERVICE_PREFIX}/thirdparty/{kind}/{urllib.parse.quote(protocol)}",
args=args,
@ -231,9 +240,12 @@ class ApplicationServiceApi(SimpleHttpClient):
# This is required by the configuration.
assert service.hs_token is not None
try:
args = None
if self.config.use_appservice_legacy_authorization:
args = {"access_token": service.hs_token}
info = await self.get_json(
f"{service.url}{APP_SERVICE_PREFIX}/thirdparty/protocol/{urllib.parse.quote(protocol)}",
{"access_token": service.hs_token},
args,
headers={"Authorization": [f"Bearer {service.hs_token}"]},
)
@ -344,10 +356,14 @@ class ApplicationServiceApi(SimpleHttpClient):
}
try:
args = None
if self.config.use_appservice_legacy_authorization:
args = {"access_token": service.hs_token}
await self.put_json(
f"{service.url}{APP_SERVICE_PREFIX}/transactions/{urllib.parse.quote(str(txn_id))}",
json_body=body,
args={"access_token": service.hs_token},
args=args,
headers={"Authorization": [f"Bearer {service.hs_token}"]},
)
if logger.isEnabledFor(logging.DEBUG):

8
synapse/config/appservice.py
View File

@ -43,6 +43,14 @@ class AppServiceConfig(Config):
)
self.track_appservice_user_ips = config.get("track_appservice_user_ips", False)
self.use_appservice_legacy_authorization = config.get(
"use_appservice_legacy_authorization", False
)
if self.use_appservice_legacy_authorization:
logger.warning(
"The use of appservice legacy authorization via query params is deprecated"
" and should be considered insecure."
)
def load_appservices(

18
synapse/config/federation.py
View File

@ -65,5 +65,23 @@ class FederationConfig(Config):
self.max_long_retries = federation_config.get("max_long_retries", 10)
self.max_short_retries = federation_config.get("max_short_retries", 3)
# Allow for the configuration of the backoff algorithm used
# when trying to reach an unavailable destination.
# Unlike previous configuration those values applies across
# multiple requests and the state of the backoff is stored on DB.
self.destination_min_retry_interval_ms = Config.parse_duration(
federation_config.get("destination_min_retry_interval", "10m")
)
self.destination_retry_multiplier = federation_config.get(
"destination_retry_multiplier", 2
)
self.destination_max_retry_interval_ms = min(
Config.parse_duration(
federation_config.get("destination_max_retry_interval", "7d")
),
# Set a hard-limit to not overflow the database column.
2**62,
)
_METRICS_FOR_DOMAINS_SCHEMA = {"type": "array", "items": {"type": "string"}}

29
synapse/util/retryutils.py
View File

@ -27,15 +27,6 @@ if TYPE_CHECKING:
logger = logging.getLogger(__name__)
# the initial backoff, after the first transaction fails
MIN_RETRY_INTERVAL = 10 * 60 * 1000
# how much we multiply the backoff by after each subsequent fail
RETRY_MULTIPLIER = 5
# a cap on the backoff. (Essentially none)
MAX_RETRY_INTERVAL = 2**62
class NotRetryingDestination(Exception):
def __init__(self, retry_last_ts: int, retry_interval: int, destination: str):
@ -169,6 +160,16 @@ class RetryDestinationLimiter:
self.notifier = notifier
self.replication_client = replication_client
self.destination_min_retry_interval_ms = (
self.store.hs.config.federation.destination_min_retry_interval_ms
)
self.destination_retry_multiplier = (
self.store.hs.config.federation.destination_retry_multiplier
)
self.destination_max_retry_interval_ms = (
self.store.hs.config.federation.destination_max_retry_interval_ms
)
def __enter__(self) -> None:
pass
@ -220,13 +221,15 @@ class RetryDestinationLimiter:
# We couldn't connect.
if self.retry_interval:
self.retry_interval = int(
self.retry_interval * RETRY_MULTIPLIER * random.uniform(0.8, 1.4)
self.retry_interval
* self.destination_retry_multiplier
* random.uniform(0.8, 1.4)
)
if self.retry_interval >= MAX_RETRY_INTERVAL:
self.retry_interval = MAX_RETRY_INTERVAL
if self.retry_interval >= self.destination_max_retry_interval_ms:
self.retry_interval = self.destination_max_retry_interval_ms
else:
self.retry_interval = MIN_RETRY_INTERVAL
self.retry_interval = self.destination_min_retry_interval_ms
logger.info(
"Connection to %s was unsuccessful (%s(%s)); backoff now %i",

85
tests/appservice/test_api.py
View File

@ -11,7 +11,7 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
from typing import Any, List, Mapping, Sequence, Union
from typing import Any, List, Mapping, Optional, Sequence, Union
from unittest.mock import Mock
from twisted.test.proto_helpers import MemoryReactor
@ -22,6 +22,7 @@ from synapse.types import JsonDict
from synapse.util import Clock
from tests import unittest
from tests.unittest import override_config
PROTOCOL = "myproto"
TOKEN = "myastoken"
@ -39,7 +40,7 @@ class ApplicationServiceApiTestCase(unittest.HomeserverTestCase):
hs_token=TOKEN,
)
def test_query_3pe_authenticates_token(self) -> None:
def test_query_3pe_authenticates_token_via_header(self) -> None:
"""
Tests that 3pe queries to the appservice are authenticated
with the appservice's token.
@ -74,12 +75,88 @@ class ApplicationServiceApiTestCase(unittest.HomeserverTestCase):
args: Mapping[Any, Any],
headers: Mapping[Union[str, bytes], Sequence[Union[str, bytes]]],
) -> List[JsonDict]:
# Ensure the access token is passed as both a header and query arg.
if not headers.get("Authorization") or not args.get(b"access_token"):
# Ensure the access token is passed as a header.
if not headers or not headers.get("Authorization"):
raise RuntimeError("Access token not provided")
# ... and not as a query param
if b"access_token" in args:
raise RuntimeError(
"Access token should not be passed as a query param."
)
self.assertEqual(headers.get("Authorization"), [f"Bearer {TOKEN}"])
self.request_url = url
if url == URL_USER:
return SUCCESS_RESULT_USER
elif url == URL_LOCATION:
return SUCCESS_RESULT_LOCATION
else:
raise RuntimeError(
"URL provided was invalid. This should never be seen."
)
# We assign to a method, which mypy doesn't like.
self.api.get_json = Mock(side_effect=get_json) # type: ignore[assignment]
result = self.get_success(
self.api.query_3pe(self.service, "user", PROTOCOL, {b"some": [b"field"]})
)
self.assertEqual(self.request_url, URL_USER)
self.assertEqual(result, SUCCESS_RESULT_USER)
result = self.get_success(
self.api.query_3pe(
self.service, "location", PROTOCOL, {b"some": [b"field"]}
)
)
self.assertEqual(self.request_url, URL_LOCATION)
self.assertEqual(result, SUCCESS_RESULT_LOCATION)
@override_config({"use_appservice_legacy_authorization": True})
def test_query_3pe_authenticates_token_via_param(self) -> None:
"""
Tests that 3pe queries to the appservice are authenticated
with the appservice's token.
"""
SUCCESS_RESULT_USER = [
{
"protocol": PROTOCOL,
"userid": "@a:user",
"fields": {
"more": "fields",
},
}
]
SUCCESS_RESULT_LOCATION = [
{
"protocol": PROTOCOL,
"alias": "#a:room",
"fields": {
"more": "fields",
},
}
]
URL_USER = f"{URL}/_matrix/app/v1/thirdparty/user/{PROTOCOL}"
URL_LOCATION = f"{URL}/_matrix/app/v1/thirdparty/location/{PROTOCOL}"
self.request_url = None
async def get_json(
url: str,
args: Mapping[Any, Any],
headers: Optional[
Mapping[Union[str, bytes], Sequence[Union[str, bytes]]]
] = None,
) -> List[JsonDict]:
# Ensure the access token is passed as a both a query param and in the headers.
if not args.get(b"access_token"):
raise RuntimeError("Access token should be provided in query params.")
if not headers or not headers.get("Authorization"):
raise RuntimeError("Access token should be provided in auth headers.")
self.assertEqual(args.get(b"access_token"), TOKEN)
self.assertEqual(headers.get("Authorization"), [f"Bearer {TOKEN}"])
self.request_url = url
if url == URL_USER:
return SUCCESS_RESULT_USER

9
tests/storage/test_transactions.py
View File

@ -17,7 +17,6 @@ from twisted.test.proto_helpers import MemoryReactor
from synapse.server import HomeServer
from synapse.storage.databases.main.transactions import DestinationRetryTimings
from synapse.util import Clock
from synapse.util.retryutils import MAX_RETRY_INTERVAL
from tests.unittest import HomeserverTestCase
@ -57,8 +56,14 @@ class TransactionStoreTestCase(HomeserverTestCase):
self.get_success(d)
def test_large_destination_retry(self) -> None:
max_retry_interval_ms = (
self.hs.config.federation.destination_max_retry_interval_ms
)
d = self.store.set_destination_retry_timings(
"example.com", MAX_RETRY_INTERVAL, MAX_RETRY_INTERVAL, MAX_RETRY_INTERVAL
"example.com",
max_retry_interval_ms,
max_retry_interval_ms,
max_retry_interval_ms,
)
self.get_success(d)

22
tests/util/test_retryutils.py
View File

@ -11,12 +11,7 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
from synapse.util.retryutils import (
MIN_RETRY_INTERVAL,
RETRY_MULTIPLIER,
NotRetryingDestination,
get_retry_limiter,
)
from synapse.util.retryutils import NotRetryingDestination, get_retry_limiter
from tests.unittest import HomeserverTestCase
@ -42,6 +37,11 @@ class RetryLimiterTestCase(HomeserverTestCase):
limiter = self.get_success(get_retry_limiter("test_dest", self.clock, store))
min_retry_interval_ms = (
self.hs.config.federation.destination_min_retry_interval_ms
)
retry_multiplier = self.hs.config.federation.destination_retry_multiplier
self.pump(1)
try:
with limiter:
@ -57,7 +57,7 @@ class RetryLimiterTestCase(HomeserverTestCase):
assert new_timings is not None
self.assertEqual(new_timings.failure_ts, failure_ts)
self.assertEqual(new_timings.retry_last_ts, failure_ts)
self.assertEqual(new_timings.retry_interval, MIN_RETRY_INTERVAL)
self.assertEqual(new_timings.retry_interval, min_retry_interval_ms)
# now if we try again we should get a failure
self.get_failure(
@ -68,7 +68,7 @@ class RetryLimiterTestCase(HomeserverTestCase):
# advance the clock and try again
#
self.pump(MIN_RETRY_INTERVAL)
self.pump(min_retry_interval_ms)
limiter = self.get_success(get_retry_limiter("test_dest", self.clock, store))
self.pump(1)
@ -87,16 +87,16 @@ class RetryLimiterTestCase(HomeserverTestCase):
self.assertEqual(new_timings.failure_ts, failure_ts)
self.assertEqual(new_timings.retry_last_ts, retry_ts)
self.assertGreaterEqual(
new_timings.retry_interval, MIN_RETRY_INTERVAL * RETRY_MULTIPLIER * 0.5
new_timings.retry_interval, min_retry_interval_ms * retry_multiplier * 0.5
)
self.assertLessEqual(
new_timings.retry_interval, MIN_RETRY_INTERVAL * RETRY_MULTIPLIER * 2.0
new_timings.retry_interval, min_retry_interval_ms * retry_multiplier * 2.0
)
#
# one more go, with success
#
self.reactor.advance(MIN_RETRY_INTERVAL * RETRY_MULTIPLIER * 2.0)
self.reactor.advance(min_retry_interval_ms * retry_multiplier * 2.0)
limiter = self.get_success(get_retry_limiter("test_dest", self.clock, store))
self.pump(1)