stream-ssl: Improve messages when configuring SSL if it is unsupported.
authorBen Pfaff <blp@nicira.com>
Tue, 10 May 2011 16:17:37 +0000 (09:17 -0700)
committerBen Pfaff <blp@nicira.com>
Tue, 10 May 2011 16:17:37 +0000 (09:17 -0700)
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 <arosen@clemson.edu>
Feature #5325.

12 files changed:
lib/automake.mk
lib/stream-nossl.c [new file with mode: 0644]
lib/stream-ssl.h
ovsdb/ovsdb-client.c
ovsdb/ovsdb-server.c
tests/test-jsonrpc.c
utilities/ovs-controller.c
utilities/ovs-ofctl.c
utilities/ovs-openflowd.c
utilities/ovs-vsctl.c
vswitchd/bridge.c
vswitchd/ovs-vswitchd.c

index 7c1977f..efc1fd7 100644 (file)
@@ -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 (file)
index 0000000..cdbbf5d
--- /dev/null
@@ -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 <config.h>
+#include "stream-ssl.h"
+#include "vlog.h"
+
+VLOG_DEFINE_THIS_MODULE(stream_nossl);
+\f
+/* 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);
+}
index 6bea577..29c3120 100644 (file)
@@ -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.
 
 #include <stdbool.h>
 
-#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 */
index a66b013..e8afdd6 100644 (file)
@@ -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);
index c9b0fdd..14f0fbf 100644 (file)
 
 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);
index 12bbc97..5d93850 100644 (file)
@@ -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);
index ae0ea3d..3844666 100644 (file)
@@ -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);
index c0ff4dc..6c2ca17 100644 (file)
@@ -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);
index 86f5ca5..33ebc68 100644 (file)
@@ -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);
index 6c2fbec..8516966 100644 (file)
@@ -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);
index 56943f6..55c9f40 100644 (file)
@@ -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);
index b9a2461..626cba4 100644 (file)
@@ -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();