Fixes #3135 - Replace _OpenSSLECCurve with crypto.get_elliptic_curve #3157

Half-Shot · 2018-04-29T14:31:52Z

fixes #3135

Signed-off-by: Will Hunt [email protected]

matrixbot · 2018-04-29T14:31:54Z

Can one of the admins verify this patch?

matrixbot · 2018-04-29T14:31:54Z

Can one of the admins verify this patch?

richvdh

You'll also need to remove the pin to twisted<18.4 from python_dependencies.py.

richvdh · 2018-04-29T23:12:10Z

synapse/crypto/context_factory.py

-            _ecCurve.addECKeyToContext(context)
+            # This was removed in https://github.com/twisted/twisted/pull/928
+            # _ecCurve = _OpenSSLECCurve()
+            _evCurve = crypto.get_elliptic_curve(_defaultCurveName)


this appears to have been introduced in pyopenssl 0.15 - so at the very least we need to bump the requirement in python_dependencies.py.

However, there is a problem in that debian jessie has 0.14. There is a newer version in backports though - @erikjohnston is it ok to rely on things in backports?

Otherwise we might have to do some hackery dependent on the Twisted version.

However, there is a problem in that debian jessie has 0.14. There is a newer version in backports though - @erikjohnston is it ok to rely on things in backports?

It is, though I think we do some foo to host the backported packages we need on our repo. I've just had a look at it seems that we're already hosting v0.16, so this is fine.

richvdh · 2018-04-29T23:12:27Z

synapse/crypto/context_factory.py

-            _ecCurve = _OpenSSLECCurve(_defaultCurveName)
-            _ecCurve.addECKeyToContext(context)
+            # This was removed in https://github.com/twisted/twisted/pull/928
+            # _ecCurve = _OpenSSLECCurve()


not much point in leaving it here as a comment, then.

Left here mainly as a tombstone since it's a drop in replacement mostly, but to be fair git serves that purpose.

richvdh · 2018-04-29T23:12:47Z

synapse/crypto/context_factory.py

-from OpenSSL import SSL
-from twisted.internet._sslverify import _OpenSSLECCurve, _defaultCurveName
+from OpenSSL import SSL, crypto
+from twisted.internet._sslverify import _defaultCurveName


I wonder if there is any way we can fix this gut-wrenching at the same time.

richvdh · 2018-04-29T23:13:14Z

synapse/crypto/context_factory.py

-            _ecCurve.addECKeyToContext(context)
+            # This was removed in https://github.com/twisted/twisted/pull/928
+            # _ecCurve = _OpenSSLECCurve()
+            _evCurve = crypto.get_elliptic_curve(_defaultCurveName)


why is it called _evCurve now?

Brain corrected EC to EV (work product is called EV, my bad)

Half-Shot · 2018-04-29T23:36:26Z

You'll also need to remove the pin to twisted<18.4 from python_dependencies.py.

Are we sure this is the only incompatibility? This change won't break twisted<18.4 environments.

richvdh · 2018-04-29T23:47:52Z

Are we sure this is the only incompatibility?

I'm not sure, but I introduced that pin because of the import of _OpenSSLECCurve. It's the only incompatibility we know of.

richvdh

looks good, but potential problem with debian jessie remains

richvdh · 2018-04-30T09:26:06Z

ok so conclusion is that all we need to do is pin to pyopenssl>=0.15 (with a comment please as to why) and profit.

Half-Shot · 2018-04-30T12:40:49Z

@richvdh Happy with that?

richvdh · 2018-04-30T15:18:48Z

\o/

@kaiyou

Changes in synapse v0.29.1 (2018-05-17) ========================================== Changes: * Update docker documentation (PR #3222) Changes in synapse v0.29.0 (2018-05-16) =========================================== Not changes since v0.29.0-rc1 Changes in synapse v0.29.0-rc1 (2018-05-14) =========================================== Notable changes, a docker file for running Synapse (Thanks to @kaiyou!) and a closed spec bug in the Client Server API. Additionally further prep for Python 3 migration. Potentially breaking change: * Make Client-Server API return 401 for invalid token (PR #3161). This changes the Client-server spec to return a 401 error code instead of 403 when the access token is unrecognised. This is the behaviour required by the specification, but some clients may be relying on the old, incorrect behaviour. Thanks to @NotAFile for fixing this. Features: * Add a Dockerfile for synapse (PR #2846) Thanks to @kaiyou! Changes - General: * nuke-room-from-db.sh: added postgresql option and help (PR #2337) Thanks to @rubo77! * Part user from rooms on account deactivate (PR #3201) * Make 'unexpected logging context' into warnings (PR #3007) * Set Server header in SynapseRequest (PR #3208) * remove duplicates from groups tables (PR #3129) * Improve exception handling for background processes (PR #3138) * Add missing consumeErrors to improve exception handling (PR #3139) * reraise exceptions more carefully (PR #3142) * Remove redundant call to preserve_fn (PR #3143) * Trap exceptions thrown within run_in_background (PR #3144) Changes - Refactors: * Refactor /context to reuse pagination storage functions (PR #3193) * Refactor recent events func to use pagination func (PR #3195) * Refactor pagination DB API to return concrete type (PR #3196) * Refactor get_recent_events_for_room return type (PR #3198) * Refactor sync APIs to reuse pagination API (PR #3199) * Remove unused code path from member change DB func (PR #3200) * Refactor request handling wrappers (PR #3203) * transaction_id, destination defined twice (PR #3209) Thanks to @damir-manapov! * Refactor event storage to prepare for changes in state calculations (PR #3141) * Set Server header in SynapseRequest (PR #3208) * Use deferred.addTimeout instead of time_bound_deferred (PR #3127, #3178) * Use run_in_background in preference to preserve_fn (PR #3140) Changes - Python 3 migration: * Construct HMAC as bytes on py3 (PR #3156) Thanks to @NotAFile! * run config tests on py3 (PR #3159) Thanks to @NotAFile! * Open certificate files as bytes (PR #3084) Thanks to @NotAFile! * Open config file in non-bytes mode (PR #3085) Thanks to @NotAFile! * Make event properties raise AttributeError instead (PR #3102) Thanks to @NotAFile! * Use six.moves.urlparse (PR #3108) Thanks to @NotAFile! * Add py3 tests to tox with folders that work (PR #3145) Thanks to @NotAFile! * Don't yield in list comprehensions (PR #3150) Thanks to @NotAFile! * Move more xrange to six (PR #3151) Thanks to @NotAFile! * make imports local (PR #3152) Thanks to @NotAFile! * move httplib import to six (PR #3153) Thanks to @NotAFile! * Replace stringIO imports with six (PR #3154, #3168) Thanks to @NotAFile! * more bytes strings (PR #3155) Thanks to @NotAFile! Bug Fixes: * synapse fails to start under Twisted >= 18.4 (PR #3157) * Fix a class of logcontext leaks (PR #3170) * Fix a couple of logcontext leaks in unit tests (PR #3172) * Fix logcontext leak in media repo (PR #3174) * Escape label values in prometheus metrics (PR #3175, #3186) * Fix 'Unhandled Error' logs with Twisted 18.4 (PR #3182) Thanks to @Half-Shot! * Fix logcontext leaks in rate limiter (PR #3183) * notifications: Convert next_token to string according to the spec (PR #3190) Thanks to @mujx! * nuke-room-from-db.sh: fix deletion from search table (PR #3194) Thanks to @rubo77! * add guard for None on purge_history api (PR #3160) Thanks to @krombel!

as per matrix-org/synapse#3157

Half-Shot force-pushed the hs/update-crypto-factory branch from 5995ad8 to 91df9dc Compare April 29, 2018 14:32

Replace _OpenSSLECCurve with crypto.get_elliptic_curve

f692538

Half-Shot force-pushed the hs/update-crypto-factory branch from 91df9dc to f692538 Compare April 29, 2018 14:40

richvdh reviewed Apr 29, 2018

View reviewed changes

Half-Shot added 2 commits April 30, 2018 00:33

_evCurve > _ecCurve

7ae89bb

Remove tombstone comment

24bfcaf

Remove pin on Twisted 18.4

55bd158

richvdh reviewed Apr 30, 2018

View reviewed changes

Half-Shot added 2 commits April 30, 2018 11:24

pyopenssl>=0.15

fff7c33

Remove whitespace in python_dep

952d0ba

richvdh merged commit 2ad3fc3 into matrix-org:develop Apr 30, 2018

Half-Shot deleted the hs/update-crypto-factory branch April 30, 2018 15:46

richvdh mentioned this pull request Jun 1, 2018

Support HS unbinding 3pids matrix-org/sydent#67

Merged

dbkr added a commit to matrix-org/sydent that referenced this pull request Jun 4, 2018

Fix elliptic curve setup

acc0d14

as per matrix-org/synapse#3157

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #3135 - Replace _OpenSSLECCurve with crypto.get_elliptic_curve #3157

Fixes #3135 - Replace _OpenSSLECCurve with crypto.get_elliptic_curve #3157

Half-Shot commented Apr 29, 2018 •

edited by richvdh

Loading

matrixbot commented Apr 29, 2018

matrixbot commented Apr 29, 2018

richvdh left a comment

richvdh Apr 29, 2018

erikjohnston Apr 30, 2018

richvdh Apr 29, 2018

Half-Shot Apr 29, 2018

richvdh Apr 29, 2018

richvdh Apr 29, 2018

Half-Shot Apr 29, 2018

Half-Shot commented Apr 29, 2018

richvdh commented Apr 29, 2018

richvdh left a comment

richvdh commented Apr 30, 2018

Half-Shot commented Apr 30, 2018

richvdh commented Apr 30, 2018

Fixes #3135 - Replace _OpenSSLECCurve with crypto.get_elliptic_curve #3157

Fixes #3135 - Replace _OpenSSLECCurve with crypto.get_elliptic_curve #3157

Conversation

Half-Shot commented Apr 29, 2018 • edited by richvdh Loading

matrixbot commented Apr 29, 2018

matrixbot commented Apr 29, 2018

richvdh left a comment

Choose a reason for hiding this comment

richvdh Apr 29, 2018

Choose a reason for hiding this comment

erikjohnston Apr 30, 2018

Choose a reason for hiding this comment

richvdh Apr 29, 2018

Choose a reason for hiding this comment

Half-Shot Apr 29, 2018

Choose a reason for hiding this comment

richvdh Apr 29, 2018

Choose a reason for hiding this comment

richvdh Apr 29, 2018

Choose a reason for hiding this comment

Half-Shot Apr 29, 2018

Choose a reason for hiding this comment

Half-Shot commented Apr 29, 2018

richvdh commented Apr 29, 2018

richvdh left a comment

Choose a reason for hiding this comment

richvdh commented Apr 30, 2018

Half-Shot commented Apr 30, 2018

richvdh commented Apr 30, 2018

Half-Shot commented Apr 29, 2018 •

edited by richvdh

Loading