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.ice: add updateCertificate() method. #2221

Merged
merged 2 commits into from Jun 6, 2016

Conversation

@mkrautz
Copy link
Member

commented Apr 26, 2016

This allows live certificate/private key reloading
for individual virtual servers.

This makes it possible to integrate Murmur with
an external Let's Encrypt script, without having
to fully restart the virtual server in the process.

@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Apr 26, 2016

@fwaggle reports that changing the private key causes all connected users to disconnect.

@mkrautz mkrautz force-pushed the mkrautz:reloadcertificate branch from 150348b to 1d91998 Apr 29, 2016

@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Apr 29, 2016

@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Apr 30, 2016

Per IRC discussion, this should instead be:

void updateCertificate(string certificate, string key, string passphrase) throws InvalidInputDataException;

(and other exceptions it already has)

The idea being that it fails if:

  1. Passphrase doesn't decode key.
  2. Key doesn't match any cert in certificate.
  3. Certificate doesn't decode.
@hacst

This comment has been minimized.

Copy link
Member

commented May 1, 2016

LGTM code wise. As you already noted the interface isn't that user-friendly if for some reason the reload doesn't work as intended so I agree the later suggestion would be preferable.

@mkrautz mkrautz force-pushed the mkrautz:reloadcertificate branch from 1d91998 to beb699b Jun 3, 2016

@mkrautz mkrautz changed the title Murmur.ice: add reloadCertificate() method. Murmur.ice: add updateCertificate() method. Jun 3, 2016

@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Jun 3, 2016

PTAL.

The method now does error checking, and throws an exception if the given input data are not usable.

This avoids us getting into a state where new clients cannot be accepted.

RSA *privRSA = reinterpret_cast<RSA *>(privKey.handle());
RSA *pubRSA = reinterpret_cast<RSA *>(pubKey.handle());

if (BN_cmp(pubRSA->n, privRSA->n) != 0) {

This comment has been minimized.

Copy link
@hacst

hacst Jun 4, 2016

Member

Should have a comment that n and e represent the public key so this ensures the private key is actually matches the public key. Not that obvious if you aren't that familiar with RSA terms.

@hacst

This comment has been minimized.

Copy link
Member

commented Jun 4, 2016

LGTM

@mkrautz mkrautz referenced this pull request Jun 6, 2016

@mkrautz mkrautz force-pushed the mkrautz:reloadcertificate branch 2 times, most recently from 34174e8 to ff234a2 Jun 6, 2016

Murmur.ice: add updateCertificate() method.
This allows live certificate/private key reloading
for individual virtual servers.

This makes it possible to integrate Murmur with
an external Let's Encrypt script, without having
to fully restart the virtual server in the process.

@mkrautz mkrautz force-pushed the mkrautz:reloadcertificate branch from ff234a2 to 6091625 Jun 6, 2016

@mkrautz mkrautz merged commit 6091625 into mumble-voip:master Jun 6, 2016

@tazjin tazjin referenced this pull request Aug 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.