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

Share an SSL context object between SSL connections #5417

Merged
merged 8 commits into from Jun 10, 2019

Conversation

Projects
None yet
2 participants
@richvdh
Copy link
Member

commented Jun 10, 2019

Hopefully fixes massive memory use with federation_verify_certificates: true,
by sharing a couple of openssl context objects between all connections.

This involves changing how the info callbacks work and is generally a bit of a
rewrite of the context_factory stuff.

Also fixes a bug where connections to IP literal homeservers would always fail.

richvdh added some commits Jun 9, 2019

Share an SSL context object between SSL connections
This involves changing how the info callbacks work.
Fix federation connections to literal IP addresses
turns out we need a shiny version of service_identity to enforce this
correctly.
Tests for SSL certs for federation connections
Add some tests for bad certificates for federation and .well-known connections

@richvdh richvdh requested a review from matrix-org/synapse-core Jun 10, 2019

@erikjohnston
Copy link
Member

left a comment

I think this looks sane?

# ... and we also gut-wrench a 'tls_verifier' attribute into the
# tls_protocol so that the SSL context's info callback has something to
# call to do the cert verification.
setattr(tls_protocol, "tls_verifier", self._verifier)

This comment has been minimized.

Copy link
@erikjohnston

erikjohnston Jun 10, 2019

Member

Looking at the Twisted code it looks somewhat like nothing else actually uses the app data, so I wonder if we can avoid the gut wrenching here.

But since we are gut wrenching can we name space this so that we don't accidentally collide with a param in twisted, e.g. _synapse_tls_verifier?

This comment has been minimized.

Copy link
@richvdh

richvdh Jun 10, 2019

Author Member

Looking at the Twisted code it looks somewhat like nothing else actually uses the app data, so I wonder if we can avoid the gut wrenching here.

that didn't seem like an assumption I particularly wanted to make.

But since we are gut wrenching can we name space this so that we don't accidentally collide with a param in twisted, e.g. _synapse_tls_verifier?

fair

richvdh added some commits Jun 10, 2019

@erikjohnston erikjohnston merged commit a6b1817 into release-v1.0.0 Jun 10, 2019

22 checks passed

buildkite/synapse Build #2030 passed (17 minutes, 36 seconds)
Details
buildkite/synapse/check-sample-config Passed (1 minute, 28 seconds)
Details
buildkite/synapse/isort Passed (27 seconds)
Details
buildkite/synapse/newspaper-newsfile Passed (23 seconds)
Details
buildkite/synapse/packaging Passed (31 seconds)
Details
buildkite/synapse/pep-8 Passed (53 seconds)
Details
buildkite/synapse/pipeline Passed (2 seconds)
Details
buildkite/synapse/python-2-dot-7-slash-postgres-9-dot-4 Passed (15 minutes, 31 seconds)
Details
buildkite/synapse/python-2-dot-7-slash-postgres-9-dot-5 Passed (15 minutes, 28 seconds)
Details
buildkite/synapse/python-2-dot-7-slash-sqlite Passed (5 minutes, 6 seconds)
Details
buildkite/synapse/python-2-dot-7-slash-sqlite-slash-old-deps Passed (6 minutes, 16 seconds)
Details
buildkite/synapse/python-3-dot-5-slash-postgres-9-dot-4 Passed (15 minutes, 48 seconds)
Details
buildkite/synapse/python-3-dot-5-slash-postgres-9-dot-5 Passed (15 minutes, 57 seconds)
Details
buildkite/synapse/python-3-dot-5-slash-sqlite Passed (5 minutes, 39 seconds)
Details
buildkite/synapse/python-3-dot-6-slash-sqlite Passed (5 minutes, 41 seconds)
Details
buildkite/synapse/python-3-dot-7-slash-postgres-11 Passed (15 minutes, 40 seconds)
Details
buildkite/synapse/python-3-dot-7-slash-postgres-9-dot-5 Passed (15 minutes, 38 seconds)
Details
buildkite/synapse/python-3-dot-7-slash-sqlite Passed (5 minutes, 27 seconds)
Details
ci/circleci: sytestpy2merged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy2postgresmerged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy3merged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy3postgresmerged Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.