From: Ben Pfaff Date: Thu, 5 Aug 2010 16:24:00 +0000 (-0700) Subject: stream-ssl: Make changing keys and certificate at runtime reliable. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6f1e91b1d7c058d701145080c344bbc531b394ed;p=openvswitch stream-ssl: Make changing keys and certificate at runtime reliable. OpenSSL is picky about the order in which keys and certificates are changed: you have to change the certificate first, then the key. It doesn't document this, but deep in the source code, in a function that sets a new certificate, it has this comment: /* don't fail for a cert/key mismatch, just free * current private key (when switching to a different * cert & key, first this function should be used, * then ssl_set_pkey */ Brilliant, guys, thanks a lot. Bug #2921. --- diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c index 851e7a76..34dcc1f9 100644 --- a/lib/stream-ssl.c +++ b/lib/stream-ssl.c @@ -975,32 +975,75 @@ update_ssl_config(struct ssl_config_file *config, const char *file_name) return true; } +static void +stream_ssl_set_private_key_file__(const char *file_name) +{ + if (SSL_CTX_use_PrivateKey_file(ctx, file_name, SSL_FILETYPE_PEM) == 1) { + private_key.read = true; + } else { + VLOG_ERR("SSL_use_PrivateKey_file: %s", + ERR_error_string(ERR_get_error(), NULL)); + } +} + void stream_ssl_set_private_key_file(const char *file_name) { - if (!update_ssl_config(&private_key, file_name)) { - return; + if (update_ssl_config(&private_key, file_name)) { + stream_ssl_set_private_key_file__(file_name); } - if (SSL_CTX_use_PrivateKey_file(ctx, file_name, SSL_FILETYPE_PEM) != 1) { - VLOG_ERR("SSL_use_PrivateKey_file: %s", +} + +static void +stream_ssl_set_certificate_file__(const char *file_name) +{ + if (SSL_CTX_use_certificate_chain_file(ctx, file_name) == 1) { + certificate.read = true; + } else { + VLOG_ERR("SSL_use_certificate_file: %s", ERR_error_string(ERR_get_error(), NULL)); - return; } - private_key.read = true; } void stream_ssl_set_certificate_file(const char *file_name) { - if (!update_ssl_config(&certificate, file_name)) { - return; + if (update_ssl_config(&certificate, file_name)) { + stream_ssl_set_certificate_file__(file_name); } - if (SSL_CTX_use_certificate_chain_file(ctx, file_name) != 1) { - VLOG_ERR("SSL_use_certificate_file: %s", - ERR_error_string(ERR_get_error(), NULL)); - return; +} + +/* Sets the private key and certificate files in one operation. Use this + * interface, instead of calling stream_ssl_set_private_key_file() and + * stream_ssl_set_certificate_file() individually, in the main loop of a + * long-running program whose key and certificate might change at runtime. + * + * This is important because of OpenSSL's behavior. If an OpenSSL context + * already has a certificate, and stream_ssl_set_private_key_file() is called + * to install a new private key, OpenSSL will report an error because the new + * private key does not match the old certificate. The other order, of setting + * a new certificate, then setting a new private key, does work. + * + * If this were the only problem, calling stream_ssl_set_certificate_file() + * before stream_ssl_set_private_key_file() would fix it. But, if the private + * key is changed before the certificate (e.g. someone "scp"s or "mv"s the new + * private key in place before the certificate), then OpenSSL would reject that + * change, and then the change of certificate would succeed, but there would be + * no associated private key (because it had only changed once and therefore + * there was no point in re-reading it). + * + * This function avoids both problems by, whenever either the certificate or + * the private key file changes, re-reading both of them, in the correct order. + */ +void +stream_ssl_set_key_and_cert(const char *private_key_file, + const char *certificate_file) +{ + if (update_ssl_config(&private_key, private_key_file) + || update_ssl_config(&certificate, certificate_file)) { + stream_ssl_set_certificate_file__(certificate_file); + stream_ssl_set_private_key_file__(private_key_file); } - certificate.read = true; } /* Reads the X509 certificate or certificates in file 'file_name'. On success, diff --git a/lib/stream-ssl.h b/lib/stream-ssl.h index dd2a16ee..ba6e422e 100644 --- a/lib/stream-ssl.h +++ b/lib/stream-ssl.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009 Nicira Networks. + * Copyright (c) 2008, 2009, 2010 Nicira Networks. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,9 +20,15 @@ #ifdef HAVE_OPENSSL bool stream_ssl_is_configured(void); + void stream_ssl_set_private_key_file(const char *file_name); void stream_ssl_set_certificate_file(const char *file_name); void stream_ssl_set_ca_cert_file(const char *file_name, bool bootstrap); + +void stream_ssl_set_key_and_cert(const char *private_key_file, + const char *certificate_file); + + void stream_ssl_set_peer_ca_cert_file(const char *file_name); /* Define the long options for SSL support. diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c index 4ca9c2d1..27db0702 100644 --- a/ovsdb/ovsdb-server.c +++ b/ovsdb/ovsdb-server.c @@ -283,8 +283,8 @@ reconfigure_from_db(struct ovsdb_jsonrpc_server *jsonrpc, #if HAVE_OPENSSL /* Configure SSL. */ - stream_ssl_set_private_key_file(query_db_string(db, private_key_file)); - stream_ssl_set_certificate_file(query_db_string(db, certificate_file)); + stream_ssl_set_key_and_cert(query_db_string(db, private_key_file), + query_db_string(db, certificate_file)); stream_ssl_set_ca_cert_file(query_db_string(db, ca_cert_file), bootstrap_ca_cert); #endif diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 12bad0bb..0397e0a2 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -350,8 +350,7 @@ bridge_configure_ssl(const struct ovsrec_ssl *ssl) { /* XXX SSL should be configurable on a per-bridge basis. */ if (ssl) { - stream_ssl_set_private_key_file(ssl->private_key); - stream_ssl_set_certificate_file(ssl->certificate); + stream_ssl_set_key_and_cert(ssl->private_key, ssl->certificate); stream_ssl_set_ca_cert_file(ssl->ca_cert, ssl->bootstrap_ca_cert); } }