From bf8f2167fd3107f5513d487a69a6568cf51afd68 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 10 May 2011 09:17:37 -0700 Subject: [PATCH] stream-ssl: Improve messages when configuring SSL if it is unsupported. Previously, if --private-key or another option that requires SSL support was used, but OVS was built without OpenSSL support, then OVS would fail with an error message that the specified option was not supported. This confused users because it made them think that the option had been removed: http://openvswitch.org/pipermail/discuss/2011-April/005034.html This commit improves the error message: OVS will now report that it was built without SSL support. This should be make the problem clear to users. Reported-by: Aaron Rosen Feature #5325. --- lib/automake.mk | 2 + lib/stream-nossl.c | 76 ++++++++++++++++++++++++++++++++++++++ lib/stream-ssl.h | 26 ++----------- ovsdb/ovsdb-client.c | 6 +-- ovsdb/ovsdb-server.c | 8 ---- tests/test-jsonrpc.c | 6 +-- utilities/ovs-controller.c | 6 +-- utilities/ovs-ofctl.c | 2 +- utilities/ovs-openflowd.c | 6 +-- utilities/ovs-vsctl.c | 6 +-- vswitchd/bridge.c | 4 +- vswitchd/ovs-vswitchd.c | 7 +--- 12 files changed, 92 insertions(+), 63 deletions(-) create mode 100644 lib/stream-nossl.c diff --git a/lib/automake.mk b/lib/automake.mk index 7c1977f8..efc1fd7b 100644 --- a/lib/automake.mk +++ b/lib/automake.mk @@ -220,6 +220,8 @@ lib/dhparams.c: lib/dh1024.pem lib/dh2048.pem lib/dh4096.pem openssl dhparam -C -in $(srcdir)/lib/dh4096.pem -noout) \ | sed 's/\(get_dh[0-9]*\)()/\1(void)/' > lib/dhparams.c.tmp mv lib/dhparams.c.tmp lib/dhparams.c +else +lib_libopenvswitch_a_SOURCES += lib/stream-nossl.c endif EXTRA_DIST += \ diff --git a/lib/stream-nossl.c b/lib/stream-nossl.c new file mode 100644 index 00000000..cdbbf5d7 --- /dev/null +++ b/lib/stream-nossl.c @@ -0,0 +1,76 @@ +/* + * Copyright (c) 2011 Nicira Networks. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include "stream-ssl.h" +#include "vlog.h" + +VLOG_DEFINE_THIS_MODULE(stream_nossl); + +/* Dummy function definitions, used when OVS is built without OpenSSL. */ + +bool +stream_ssl_is_configured(void) +{ + return false; +} + +static void NO_RETURN +nossl_option(const char *detail) +{ + VLOG_FATAL("%s specified but Open vSwitch was built without SSL support", + detail); +} + +void +stream_ssl_set_private_key_file(const char *file_name) +{ + if (file_name != NULL) { + nossl_option("Private key"); + } +} + +void +stream_ssl_set_certificate_file(const char *file_name) +{ + if (file_name != NULL) { + nossl_option("Certificate"); + } +} + +void +stream_ssl_set_ca_cert_file(const char *file_name, bool bootstrap OVS_UNUSED) +{ + if (file_name != NULL) { + nossl_option("CA certificate"); + } +} + +void +stream_ssl_set_peer_ca_cert_file(const char *file_name) +{ + if (file_name != NULL) { + nossl_option("Peer CA certificate"); + } +} + +void +stream_ssl_set_key_and_cert(const char *private_key_file, + const char *certificate_file) +{ + stream_ssl_set_private_key_file(private_key_file); + stream_ssl_set_certificate_file(certificate_file); +} diff --git a/lib/stream-ssl.h b/lib/stream-ssl.h index 6bea577d..29c3120f 100644 --- a/lib/stream-ssl.h +++ b/lib/stream-ssl.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2009, 2010 Nicira Networks. + * Copyright (c) 2008, 2009, 2010, 2011 Nicira Networks. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,28 +18,18 @@ #include -#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_peer_ca_cert_file(const char *file_name); 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. - * - * Note that the definition includes a final comma, and therefore a comma - * must not be supplied when using the definition. This is done so that - * compilation succeeds whether or not HAVE_OPENSSL is defined. */ -#define STREAM_SSL_LONG_OPTIONS \ +#define STREAM_SSL_LONG_OPTIONS \ {"private-key", required_argument, 0, 'p'}, \ {"certificate", required_argument, 0, 'c'}, \ - {"ca-cert", required_argument, 0, 'C'}, + {"ca-cert", required_argument, 0, 'C'} #define STREAM_SSL_OPTION_HANDLERS \ case 'p': \ @@ -53,13 +43,5 @@ void stream_ssl_set_peer_ca_cert_file(const char *file_name); case 'C': \ stream_ssl_set_ca_cert_file(optarg, false); \ break; -#else /* !HAVE_OPENSSL */ -static inline bool stream_ssl_is_configured(void) -{ - return false; -} -#define STREAM_SSL_LONG_OPTIONS -#define STREAM_SSL_OPTION_HANDLERS -#endif /* !HAVE_OPENSSL */ #endif /* stream-ssl.h */ diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c index a66b013b..e8afdd6b 100644 --- a/ovsdb/ovsdb-client.c +++ b/ovsdb/ovsdb-client.c @@ -80,9 +80,9 @@ parse_options(int argc, char *argv[]) DAEMON_LONG_OPTIONS, #ifdef HAVE_OPENSSL {"bootstrap-ca-cert", required_argument, 0, OPT_BOOTSTRAP_CA_CERT}, - TABLE_LONG_OPTIONS, - STREAM_SSL_LONG_OPTIONS + STREAM_SSL_LONG_OPTIONS, #endif + TABLE_LONG_OPTIONS, {0, 0, 0, 0}, }; char *short_options = long_options_to_short_options(long_options); @@ -111,13 +111,11 @@ parse_options(int argc, char *argv[]) TABLE_OPTION_HANDLERS(&table_style) -#ifdef HAVE_OPENSSL STREAM_SSL_OPTION_HANDLERS case OPT_BOOTSTRAP_CA_CERT: stream_ssl_set_ca_cert_file(optarg, true); break; -#endif case '?': exit(EXIT_FAILURE); diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c index c9b0fdd9..14f0fbfb 100644 --- a/ovsdb/ovsdb-server.c +++ b/ovsdb/ovsdb-server.c @@ -51,13 +51,11 @@ VLOG_DEFINE_THIS_MODULE(ovsdb_server); -#if HAVE_OPENSSL /* SSL configuration. */ static char *private_key_file; static char *certificate_file; static char *ca_cert_file; static bool bootstrap_ca_cert; -#endif static unixctl_cb_func ovsdb_server_exit; static unixctl_cb_func ovsdb_server_compact; @@ -598,13 +596,11 @@ reconfigure_from_db(struct ovsdb_jsonrpc_server *jsonrpc, ovsdb_jsonrpc_server_set_remotes(jsonrpc, &resolved_remotes); shash_destroy_free_data(&resolved_remotes); -#if HAVE_OPENSSL /* Configure SSL. */ 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 } static void @@ -671,12 +667,10 @@ parse_options(int argc, char *argv[], char **file_namep, DAEMON_LONG_OPTIONS, VLOG_LONG_OPTIONS, LEAK_CHECKER_LONG_OPTIONS, -#ifdef HAVE_OPENSSL {"bootstrap-ca-cert", required_argument, 0, OPT_BOOTSTRAP_CA_CERT}, {"private-key", required_argument, 0, 'p'}, {"certificate", required_argument, 0, 'c'}, {"ca-cert", required_argument, 0, 'C'}, -#endif {0, 0, 0, 0}, }; char *short_options = long_options_to_short_options(long_options); @@ -714,7 +708,6 @@ parse_options(int argc, char *argv[], char **file_namep, DAEMON_OPTION_HANDLERS LEAK_CHECKER_OPTION_HANDLERS -#ifdef HAVE_OPENSSL case 'p': private_key_file = optarg; break; @@ -732,7 +725,6 @@ parse_options(int argc, char *argv[], char **file_namep, ca_cert_file = optarg; bootstrap_ca_cert = true; break; -#endif case '?': exit(EXIT_FAILURE); diff --git a/tests/test-jsonrpc.c b/tests/test-jsonrpc.c index 12bbc975..5d93850c 100644 --- a/tests/test-jsonrpc.c +++ b/tests/test-jsonrpc.c @@ -60,10 +60,8 @@ parse_options(int argc, char *argv[]) {"verbose", optional_argument, 0, 'v'}, {"help", no_argument, 0, 'h'}, DAEMON_LONG_OPTIONS, -#ifdef HAVE_OPENSSL {"bootstrap-ca-cert", required_argument, 0, OPT_BOOTSTRAP_CA_CERT}, - STREAM_SSL_LONG_OPTIONS -#endif + STREAM_SSL_LONG_OPTIONS, {0, 0, 0, 0}, }; char *short_options = long_options_to_short_options(long_options); @@ -84,13 +82,11 @@ parse_options(int argc, char *argv[]) DAEMON_OPTION_HANDLERS -#ifdef HAVE_OPENSSL STREAM_SSL_OPTION_HANDLERS case OPT_BOOTSTRAP_CA_CERT: stream_ssl_set_ca_cert_file(optarg, true); break; -#endif case '?': exit(EXIT_FAILURE); diff --git a/utilities/ovs-controller.c b/utilities/ovs-controller.c index ae0ea3da..3844666f 100644 --- a/utilities/ovs-controller.c +++ b/utilities/ovs-controller.c @@ -324,10 +324,8 @@ parse_options(int argc, char *argv[]) {"version", no_argument, 0, 'V'}, DAEMON_LONG_OPTIONS, VLOG_LONG_OPTIONS, -#ifdef HAVE_OPENSSL - STREAM_SSL_LONG_OPTIONS + STREAM_SSL_LONG_OPTIONS, {"peer-ca-cert", required_argument, 0, OPT_PEER_CA_CERT}, -#endif {0, 0, 0, 0}, }; char *short_options = long_options_to_short_options(long_options); @@ -400,13 +398,11 @@ parse_options(int argc, char *argv[]) VLOG_OPTION_HANDLERS DAEMON_OPTION_HANDLERS -#ifdef HAVE_OPENSSL STREAM_SSL_OPTION_HANDLERS case OPT_PEER_CA_CERT: stream_ssl_set_peer_ca_cert_file(optarg); break; -#endif case '?': exit(EXIT_FAILURE); diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index c0ff4dc1..6c2ca176 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -92,7 +92,7 @@ parse_options(int argc, char *argv[]) {"help", no_argument, 0, 'h'}, {"version", no_argument, 0, 'V'}, VLOG_LONG_OPTIONS, - STREAM_SSL_LONG_OPTIONS + STREAM_SSL_LONG_OPTIONS, {0, 0, 0, 0}, }; char *short_options = long_options_to_short_options(long_options); diff --git a/utilities/ovs-openflowd.c b/utilities/ovs-openflowd.c index 86f5ca59..33ebc681 100644 --- a/utilities/ovs-openflowd.c +++ b/utilities/ovs-openflowd.c @@ -277,10 +277,8 @@ parse_options(int argc, char *argv[], struct ofsettings *s) DAEMON_LONG_OPTIONS, VLOG_LONG_OPTIONS, LEAK_CHECKER_LONG_OPTIONS, -#ifdef HAVE_OPENSSL - STREAM_SSL_LONG_OPTIONS + STREAM_SSL_LONG_OPTIONS, {"bootstrap-ca-cert", required_argument, 0, OPT_BOOTSTRAP_CA_CERT}, -#endif {0, 0, 0, 0}, }; char *short_options = long_options_to_short_options(long_options); @@ -448,13 +446,11 @@ parse_options(int argc, char *argv[], struct ofsettings *s) LEAK_CHECKER_OPTION_HANDLERS -#ifdef HAVE_OPENSSL STREAM_SSL_OPTION_HANDLERS case OPT_BOOTSTRAP_CA_CERT: stream_ssl_set_ca_cert_file(optarg, true); break; -#endif case '?': exit(EXIT_FAILURE); diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c index 6c2fbece..85169665 100644 --- a/utilities/ovs-vsctl.c +++ b/utilities/ovs-vsctl.c @@ -217,10 +217,8 @@ parse_options(int argc, char *argv[]) {"version", no_argument, 0, 'V'}, VLOG_LONG_OPTIONS, TABLE_LONG_OPTIONS, -#ifdef HAVE_OPENSSL - STREAM_SSL_LONG_OPTIONS + STREAM_SSL_LONG_OPTIONS, {"peer-ca-cert", required_argument, 0, OPT_PEER_CA_CERT}, -#endif {0, 0, 0, 0}, }; char *tmp, *short_options; @@ -278,13 +276,11 @@ parse_options(int argc, char *argv[]) VLOG_OPTION_HANDLERS TABLE_OPTION_HANDLERS(&table_style) -#ifdef HAVE_OPENSSL STREAM_SSL_OPTION_HANDLERS case OPT_PEER_CA_CERT: stream_ssl_set_peer_ca_cert_file(optarg); break; -#endif case '?': exit(EXIT_FAILURE); diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 56943f67..55c9f40a 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -1401,7 +1401,7 @@ bridge_run(void) /* (Re)configure if necessary. */ database_changed = ovsdb_idl_run(idl); cfg = ovsrec_open_vswitch_first(idl); -#ifdef HAVE_OPENSSL + /* Re-configure SSL. We do this on every trip through the main loop, * instead of just when the database changes, because the contents of the * key and certificate files can change without the database changing. @@ -1414,7 +1414,7 @@ bridge_run(void) stream_ssl_set_key_and_cert(ssl->private_key, ssl->certificate); stream_ssl_set_ca_cert_file(ssl->ca_cert, ssl->bootstrap_ca_cert); } -#endif + if (database_changed || datapath_destroyed) { if (cfg) { struct ovsdb_idl_txn *txn = ovsdb_idl_txn_create(idl); diff --git a/vswitchd/ovs-vswitchd.c b/vswitchd/ovs-vswitchd.c index b9a24618..626cba4f 100644 --- a/vswitchd/ovs-vswitchd.c +++ b/vswitchd/ovs-vswitchd.c @@ -128,11 +128,9 @@ parse_options(int argc, char *argv[]) DAEMON_LONG_OPTIONS, VLOG_LONG_OPTIONS, LEAK_CHECKER_LONG_OPTIONS, -#ifdef HAVE_OPENSSL - STREAM_SSL_LONG_OPTIONS + STREAM_SSL_LONG_OPTIONS, {"peer-ca-cert", required_argument, 0, OPT_PEER_CA_CERT}, {"bootstrap-ca-cert", required_argument, 0, OPT_BOOTSTRAP_CA_CERT}, -#endif {"enable-dummy", no_argument, 0, OPT_ENABLE_DUMMY}, {0, 0, 0, 0}, }; @@ -168,8 +166,6 @@ parse_options(int argc, char *argv[]) VLOG_OPTION_HANDLERS DAEMON_OPTION_HANDLERS LEAK_CHECKER_OPTION_HANDLERS - -#ifdef HAVE_OPENSSL STREAM_SSL_OPTION_HANDLERS case OPT_PEER_CA_CERT: @@ -179,7 +175,6 @@ parse_options(int argc, char *argv[]) case OPT_BOOTSTRAP_CA_CERT: stream_ssl_set_ca_cert_file(optarg, true); break; -#endif case OPT_ENABLE_DUMMY: dummy_enable(); -- 2.30.2