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

murmur/Cert: remove auto-generation of Diffie-Hellman parameters. #3184

Merged
merged 1 commit into from Aug 7, 2017

Conversation

@mkrautz
Copy link
Member

commented Jul 19, 2017

Remove automatic generation of Diffie-Hellman parameters for vservers.

There were various problems with this approach.

  • It was somewhat slow.
  • To avoid blocking the main thread while generating the DH parameters,
    the code would enter an event loop. However, this meant that other code
    could still execute (for example, clients could connect to the server
    before the DH params were generated).

As a replacement, we're going to be shipping the Diffie-Hellman parameters
from RFC 7919 bundled into Murmur, and we'll default to one of them.

murmur/Cert: remove auto-generation of Diffie-Hellman parameters.
Remove automatic generation of Diffie-Hellman parameters for vservers.

There were various problems with this approach.

 - It was somewhat slow.
 - To avoid blocking the main thread while generating the DH parameters,
   the code would enter an event loop. However, this meant that other code
   could still execute (for example, clients could connect to the server
   before the DH params were generated).

As a replacement, we're going to be shipping the Diffie-Hellman parameters
from RFC 7919 bundled into Murmur, and we'll default to one of them.

@mkrautz mkrautz requested review from Kissaki, hacst and davidebeatrici Jul 19, 2017

@Kissaki

This comment has been minimized.

Copy link
Member

commented Jul 20, 2017

As a replacement, we're going to be shipping the Diffie-Hellman parameters
from RFC 7919 bundled into Murmur, and we'll default to one of them.

This diff does not add anything. Do we already ship that?

@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Jul 20, 2017

That's available in #3183
and implemented in Murmur via #3185

@hacst
Copy link
Member

left a comment

Basically LGTM. But considering the fallback for an empty parameterset is a 1024-bit MODP from RFC 2409 maybe we should block this one until #3185 is complete and has landed?

@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Jul 29, 2017

Basically LGTM. But considering the fallback for an empty parameterset is a 1024-bit MODP from RFC 2409 maybe we should block this one until #3185 is complete and has landed?

Agreed, PR #3185 is now reviewable.

PR #3185 is now reviewable

@hacst
hacst approved these changes Aug 6, 2017
Copy link
Member

left a comment

LGTM to land with #3185

@mkrautz mkrautz merged commit 923d649 into mumble-voip:master Aug 7, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.