Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove hardcoded defaults of matrix.org and vector.im in configuration #6087

Closed
ara4n opened this issue Sep 23, 2019 · 5 comments

Comments

@ara4n
Copy link
Member

commented Sep 23, 2019

Currently we have a few fields which have matrix.org or vector.im hardcoded as the configuration defaults in synapse. This was a deliberate choice a while back (i can't find the PR) where we moved configs from being in the homeserver.yaml into config/*.py so that by default all of homeserver.yaml is commented out... but it is problematic from a privacy perspective, given it privileges matrix.org & vector.im if the default config silently uses them. This is particularly true for minimal homeserver.yaml files where the comments have been removed, which then silently prefer matrix.org.

From a quick grep, this impacts:

./registration.py:            "trusted_third_party_id_servers", ["matrix.org", "vector.im"]
./metrics.py:            "report_stats_endpoint", "https://matrix.org/report-usage-stats/push"
./key.py:            key_servers = [{"server_name": "matrix.org"}]

We need to figure out how to remove the hardcoded configs.

My suggestion would be to

  • remove the hardcoded defaults outright
  • move them into explicit config options for homeserver.yaml (temporarily breaking homeservers which don't update their configs as needed)
  • warn users who haven't changed the defaults (unless they set a suppress_warning config option).

Alternatively, we could leave them where they are, but loudly warn the user on stderr about it (Which is the temporary solution we're adopting for the specific issue of warning about matrix.org being the default notary server)

@richvdh

This comment has been minimized.

Copy link
Member

commented Sep 23, 2019

This was a deliberate choice a while back (i can't find the PR) where we moved configs from being in the homeserver.yaml into config/*.py

To clarify: these options have always had defaults in the python (see d624e2a#diff-7e17dede94ec13fbd96119ef254b8aa4R65 for example: note that the settings used there were used as defaults as well as providing a basis for the sample config).

The difficulty with making them required options now is that it will break everybody's existing configurations. I guess we'll have to do something phased where we deprecate the default (and omit a warning about it) for now, and later we can remove the default altogether.

@richvdh

This comment has been minimized.

Copy link
Member

commented Sep 23, 2019

./registration.py: "trusted_third_party_id_servers", ["matrix.org", "vector.im"]

this one is deprecated as of the next release of synapse anyway.

@ara4n

This comment has been minimized.

Copy link
Member Author

commented Sep 23, 2019

@richvdh, @neilisfragile, given the only item then actually impacted by this is the notary server option, i suggest we wrap this together with #6088, and:

  • add a warning to stderr if you're running on the hardcoded default (and mention that the default will be removed in future)
  • (re)add matrix.org as an explicit un-commented default in homeserver.yaml
  • add a warning to stderr if the config ends up using matrix.org and say "if you don't want to use matrix.org for this, please pick another long-lived reliable server instead"
  • add the suppress_notary_server_warning config option to get rid the warnings if you consciously don't care.
@ara4n

This comment has been minimized.

Copy link
Member Author

commented Sep 23, 2019

(we'll want to increase the priority of #4641 and #1539 so the warnings don't get lost)

@richvdh

This comment has been minimized.

Copy link
Member

commented Sep 26, 2019

I think this and #6088 are resolved by #6090.

@richvdh richvdh closed this Sep 26, 2019
Homeserver Task Board automation moved this from Review to Done Sep 26, 2019
@ara4n ara4n added the phase:1 label Sep 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
3 participants
You can’t perform that action at this time.