On Fri, Aug 21, 2015 at 2:43 PM, Ben Pfaff <b...@nicira.com> wrote:
> On Thu, Aug 20, 2015 at 10:03:39AM -0700, Gurucharan Shetty wrote:
>> In OVN, ovsdb-server is the daemon that manages the databases
>> and can be called as the central controller. So it would be
>> nice for ovsdb-server to be able to push its self-signed
>> certificate to all the other nodes where ovn-controller runs.
>>
>> Signed-off-by: Gurucharan Shetty <gshe...@nicira.com>
>
> This is a good idea, especially the test.
>
> The test passes a plain --log-file option and a --log-file option with a
> full path. The first one fails:
>
> 2015-08-21T21:34:19Z|00001|vlog|WARN|failed to open
> /var/log/openvswitch/ovsdb-server.log for logging: No such file or
> directory
>
> so I'd replace it by the one with the full path.
>
> The log message from ovs-vsctl on the disconnection is confusing:
>
> ovs-vsctl: ssl:127.0.0.1:34766: database connection failed ()
>
> It looks like this improves it, at least to "(Protocol error)":
>
> diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c
> index ae51b42..1e312a2 100644
> --- a/lib/jsonrpc.c
> +++ b/lib/jsonrpc.c
> @@ -948,6 +948,7 @@ jsonrpc_session_run(struct jsonrpc_session *s)
> reconnect_connect_failed(s->reconnect, time_msec(), error);
> stream_close(s->stream);
> s->stream = NULL;
> + s->last_error = error;
> }
> }
>
>
> The test runs over 10x faster on my system with 1024-bit keys:
>
> diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
> index 6a2189a..caaa497 100644
> --- a/tests/ovs-vsctl.at
> +++ b/tests/ovs-vsctl.at
> @@ -1315,9 +1315,9 @@ AT_KEYWORDS([ovs-vsctl ssl])
> AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
> PKIDIR=`pwd`
> OVS_PKI="sh $abs_top_srcdir/utilities/ovs-pki.in --dir=$PKIDIR/pki
> --log=$PKIDIR/ovs-pki.log"
> -$OVS_PKI init && \
> -$OVS_PKI req+sign vsctl switch && \
> -$OVS_PKI req ovsdbserver && $OVS_PKI self-sign ovsdbserver
> +$OVS_PKI -B 1024 init && \
> +$OVS_PKI -B 1024 req+sign vsctl switch && \
> +$OVS_PKI -B 1024 req ovsdbserver && $OVS_PKI self-sign ovsdbserver
>
> dnl Create database.
> touch .conf.db.~lock~
>
> Acked-by: Ben Pfaff <b...@nicira.com>
I took all your suggestions and pushed the series. I have one question
for you though.
In lib/stream-ssl.c there is this piece of code:
/* Check that 'cert' is self-signed. Otherwise it is not a CA
* certificate and we should not attempt to use it as one. */
error = X509_check_issued(cert, cert);
if (error) {
VLOG_ERR("could not bootstrap CA cert: obtained certificate is "
"not self-signed (%s)",
X509_verify_cert_error_string(error));
if (sk_X509_num(chain) < 2) {
VLOG_ERR("only one certificate was received, so probably the peer "
"is not configured to send its CA certificate");
}
return EPROTO;
}
Now, what the above does is that it will only let boot-strap happen if
the controller certificate is self-signed (which is what the unit test
in this commit does). The bootstrap fails if the controller
certificate is signed by a CA. The check looks to be explicit and was
present many years ago, so there must have been a reason for that. Do
you remember why? The man pages do not mandate this requirement and
makes you believe that CA certificates are OK.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
