SSLv23_method() is deprecated since OpenSSL 1.1.0. In practice, it is
just renamed into TLS_method(). Use the new name instead.
For the python version of the code, we can use PROTOCOL_TLS_CLIENT,
since we only support client side of the connection. It turns on
the hostname check by default, though. So, we need to turn it off,
otherwise we would have to provide the server_hostname for every
wrap_socket. We would just use generic PROTOCOL_TLS as we do in C,
but unfortunately PROTOCOL_TLS is deprecated since Python 3.10.
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
The NO options are deprecated since OpenSSL 1.1.0:
* SSL_OP_NO_SSLv3
* SSL_OP_NO_TLSv1
* SSL_OP_NO_TLSv1_1
* SSL_OP_NO_TLSv1_2
SSL_CTX_set_min/max_proto_version API should be used instead.
Change the "ssl-protocols" configuration option to parse values and
enable ranges with this new API instead. This means that we'll start
enabling protocols that may not be enabled by the user, e.g.
--ssl-protocols="TLSv1,TLSv1.2" will now enable TLSv1.1 as well.
But it's probably not a big deal, and there will be no way to turn off
one protocol in the middle in the future anyway, since the OpenSSL
API required to do so is deprecated. And such configurations are
very unlikely to be used in practice. At least, that was one of the
reasons for OpenSSL to change the API in the first place.
While at it, allow users to configure simple ranges, instead of lists.
For example, OVS will now allow values like "TLSv1-TLSv1.2" to enable
all versions between TLSv1 and TLSv1.2, or "TLSv1.1+" to allow TLSv1.1
or any later version. The option still accepts a list of protocols or
exactly one range.
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
OpenSSL 1.1.0 reached EoL 5 years ago on 11 Sep 2019. Vast majority
of distributions moved to newer versions long time ago.
OpenSSL 1.1.1 introduced a lot of new APIs and deprecated a lot of
old ones. It also introduced support for TLSv1.3 with a pack of
APIs specific to that version.
Requiring OpenSSL 1.1.1 or newer will allow us to get rid of use of
many deprecated APIs as well as introduce explicit support for TLSv1.3
without polluting the code with conditional compiling.
Python community did an exceptional investigation on benefits of
dropping support for OpenSSL 1.1.0 when they did the same in 2021:
https://peps.python.org/pep-0644/
We do not officially support building with LibreSSL, but all the
ifdefs for it are not necessary today, as LibreSSL implemented all
the missing APIs. Also, most major distributions either moved away
from LibreSSL or provide OpenSSL as an alternative.
This commit only removes explicit workarounds. We'll start replacing
deprecated APIs in the next ones.
OpenSSL 1.1.1 also reached end of life in 2023, but it's not a big
burden to support, and many distributions are still using it and
will continue using it for quite some time.
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
SSL protocol family is not actually being used or supported in OVS.
What we use is actually TLS.
Terms "SSL" and "TLS" are often used interchangeably in modern
software and refer to the same thing, which is normally just TLS.
Let's replace "SSL" with "SSL/TLS" in documentation and user-visible
messages, where it makes sense. This may make it more clear what
is meant for a less experienced user that may look for TLS support
in OVS and not find much.
We're not changing any actual code, because, for example, most of
OpenSSL APIs are using just SSL, for historical reasons. And our
database is using "SSL" table. We may consider migrating to "TLS"
naming for user-visible configuration like command line arguments
and database names, but that will require extra work on making sure
upgrades can still work. In general, a slightly more clear
documentation should be enough for now, especially since term SSL
is still widely used in the industry.
"SSL/TLS" is chosen over "TLS/SSL" simply because our user-visible
configuration knobs are using "SSL" naming, e.g. '--ssl-cyphers'
or 'ovs-vsctl set-ssl'. So, it might be less confusing this way.
We may switch that, if we decide on re-working the user-visible
commands towards "TLS" naming, or providing both alternatives.
Some other projects did similar changes. For example, the python ssl
library is now using "TLS/SSL" in the documentation whenever possible.
Same goes for OpenSSL itself.
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
TLSv1 and TLSv1.1 are officially deprecated by RFC 8996 since March
of 2021: https://datatracker.ietf.org/doc/rfc8996/
Both protocols should not generally be used (RFC says MUST NOT) and
are being actively removed from support by major distributions and
libraries.
Deprecate these protocols in OVS and turn them off by default.
Ability to use them preserved for now with a warning. We'll fully
remove support in OVS 3.6.
Before this change, OVS would use TLSv1 or later, if the protocols
are not specified in the database or command line (this includes
TLSv1.3 that is not supported explicitly). After the change, this
becomes TLSv1.2 or later.
Python library only supports client side of SSL/TLS and doesn't
support configuring protocols. So, just turning off TLSv1 and
TLSv1.1. Meaning, new python clients will not be able to connect
to servers that only have TLSv1.1 or lower. This is a strange
configuration for a modern server and can be fixed by allowing the
server to use newer protocols. So, there might not be a real need
in making client side configurable. If the server is so old that
it doesn't support TLSv1.2, it may be a time to update it.
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
OpenSSL 3.0 enabled alerts for unexpected EOF by default. It supposed
to alert the application whenever the connection terminated without
a proper close_notify. And that should allow applications to take
actions to protect themselves from potential TLS truncation attack.
This is how it looks like in the log:
|stream_ssl|WARN|SSL_read: error:0A000126:SSL routines::unexpected eof while reading
|jsonrpc|WARN|ssl:127.0.0.1:34288: receive error: Input/output error
|reconnect|WARN|ssl:127.0.0.1:34288: connection dropped (Input/output error)
The problem is that clients based on OVS libraries do not wait for
the proper termination if it didn't happen right away. It means that
chances to have alerts on the server side for every single disconnection
are very high.
None of the high level protocols supported by OVS daemons can carry
state between re-connections, e.g., there are no session cookies or
anything like that. So, the TLS truncation attack is no applicable.
Disable the alert to avoid unnecessary warnings in the log.
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
In OpenSSL 3.0 some functions were deprecated and replaced.
This commit adds some #ifdef to build without warning on both
OpenSSL 1.x and OpenSSL 3.x.
For OpenSSL 3.x, the default built-in DH parameters are used (as
suggested by SSL_CTX_set_dh_auto manpage).
Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
ssl_send() clones the data before sending, but if SSL_write() succeeds
at the first attempt, this is only a waste of CPU cycles.
Trying to send the original buffer instead and only copying remaining
data if it's not possible to send it all right away.
This should save a few cycles on every send.
Note:
It's probably possible to avoid the copy even if we can't send
everything at once, but will, likely, require some major change
of the stream-sll module in order to take into account all the
corner cases related to SSL connection. So, not trying to do that
for now.
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Using 1024 bit params for DH is considered unsafe [1]. Additionally,
from [2]:
"Modern servers that do not support export ciphersuites are advised to
either use SSL_CTX_set_tmp_dh() or alternatively, use the callback but
ignore keylength and is_export and simply supply at least 2048-bit
parameters in the callback."
Additionally, using 1024 bit dh params may block clients running on
recent openssl version from connecting given the stricter default
security requirements of those new openssl versions. The error message
for these clients looks like:
error:141A318A:SSL routines:tls_process_ske_dhe:dh key too small:ssl/statem/statem_clnt.c:2150
As a workaround, this error can be suppressed tweaking the cipher list
(--ssl-ciphers) to either 'HIGH:!aNULL:!MD5:@SECLEVEL=1' to reduce
security requirements or 'HIGH:!aNULL:!MD5:!DH' to avoid using fixed
param DH based ciphers. The first option is recommended though as it
likely a fixed param DH cipher is the best possible option in that
situation.
[1] https://weakdh.org/
[2] https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_tmp_dh_callback.html
Signed-off-by: Jaime Caamaño Ruiz <jcaamano@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
While checking valgrind reports after running "make check-valgrind" I have noticed
reports for several tests similar to the following:
....
==5345== Memcheck, a memory error detector
==5345== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==5345== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==5345== Command: ovsdb-client --private-key=/home/damijan.skvarc/doma/ovs/tests/testpki-privkey.pem --certificate=/home/damijan.skvarc/doma/ovs/tests/testpki-cert.pem --ca-cert=/home/damijan.skvarc/doma/ovs/tests/testpki-cacert.pem transact ssl:127.0.0.1:40111 \ \ \ ["ordinals",
==5345== \ \ \ \ \ \ {"op":\ "update",
==5345== \ \ \ \ \ \ \ "table":\ "ordinals",
==5345== \ \ \ \ \ \ \ "where":\ [["number",\ "==",\ 1]],
==5345== \ \ \ \ \ \ \ "row":\ {"number":\ 2,\ "name":\ "old\ two"}},
==5345== \ \ \ \ \ \ {"op":\ "update",
==5345== \ \ \ \ \ \ \ "table":\ "ordinals",
==5345== \ \ \ \ \ \ \ "where":\ [["name",\ "==",\ "two"]],
==5345== \ \ \ \ \ \ \ "row":\ {"number":\ 1,\ "name":\ "old\ one"}}]
==5345== Parent PID: 5344
==5345==
==5345==
==5345== HEAP SUMMARY:
==5345== in use at exit: 116,551 bytes in 3,341 blocks
==5345== total heap usage: 5,134 allocs, 1,793 frees, 412,290 bytes allocated
==5345==
==5345== 6,221 (184 direct, 6,037 indirect) bytes in 1 blocks are definitely lost in loss record 498 of 500
==5345== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5345== by 0x5105E77: CRYPTO_malloc (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==5345== by 0x51E1D23: ??? (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==5345== by 0x51E4861: ??? (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==5345== by 0x51E5414: ASN1_item_ex_d2i (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==5345== by 0x51E546A: ASN1_item_d2i (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==5345== by 0x4E56B27: ??? (in /lib/x86_64-linux-gnu/libssl.so.1.0.0)
==5345== by 0x4E5BA11: ??? (in /lib/x86_64-linux-gnu/libssl.so.1.0.0)
==5345== by 0x4E65145: ??? (in /lib/x86_64-linux-gnu/libssl.so.1.0.0)
==5345== by 0x4522DF: ssl_connect (stream-ssl.c:530)
==5345== by 0x443D38: scs_connecting (stream.c:315)
==5345== by 0x443D38: stream_connect (stream.c:338)
==5345== by 0x443FA1: stream_open_block (stream.c:266)
==5345== by 0x40AB79: open_jsonrpc (ovsdb-client.c:507)
==5345== by 0x40AB79: open_rpc (ovsdb-client.c:143)
==5345== by 0x40B06B: do_transact__ (ovsdb-client.c:871)
==5345== by 0x40B245: do_transact (ovsdb-client.c:893)
==5345== by 0x405F76: main (ovsdb-client.c:282)
==5345==
==5345== LEAK SUMMARY:
==5345== definitely lost: 184 bytes in 1 blocks
==5345== indirectly lost: 6,037 bytes in 117 blocks
==5345== possibly lost: 0 bytes in 0 blocks
==5345== still reachable: 110,330 bytes in 3,223 blocks
==5345== suppressed: 0 bytes in 0 blocks
==5345== Reachable blocks (those to which a pointer was found) are not shown.
==5345== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==5345==
==5345== For counts of detected and suppressed errors, rerun with: -v
==5345== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
....
This report was extracted from "index uniqueness checking" test and complains about
leaking memory in ovsdb-client application. The problem is not huge, since ovsdb-client
is CLI tool which is constantly reinvoked/restarted, thus leaked memory is not accumulated.
More problematic issue is that for the same test valgrind reports the similar problem also for
ovsdb-server:
....
==5290== Memcheck, a memory error detector
==5290== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==5290== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==5290== Command: ovsdb-server --log-file --detach --no-chdir --pidfile --private-key=/home/damijan.skvarc/doma/ovs/tests/testpki-privkey2.pem --certificate=/home/damijan.skvarc/doma/ovs/tests/testpki-cert2.pem --ca-cert=/home/damijan.skvarc/doma/ovs/tests/testpki-cacert.pem --remote=pssl:0:127.0.0.1 db
==5290== Parent PID: 5289
==5290==
==5292== Warning: noted but unhandled ioctl 0x2403 with no size/direction hints.
==5292== This could cause spurious value errors to appear.
==5292== See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==5292== Warning: noted but unhandled ioctl 0x2400 with no size/direction hints.
==5292== This could cause spurious value errors to appear.
==5292== See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==5290==
==5290== HEAP SUMMARY:
==5290== in use at exit: 2,066 bytes in 48 blocks
==5290== total heap usage: 87 allocs, 39 frees, 14,152 bytes allocated
==5290==
==5290== LEAK SUMMARY:
==5290== definitely lost: 0 bytes in 0 blocks
==5290== indirectly lost: 0 bytes in 0 blocks
==5290== possibly lost: 0 bytes in 0 blocks
==5290== still reachable: 2,066 bytes in 48 blocks
==5290== suppressed: 0 bytes in 0 blocks
==5290== Reachable blocks (those to which a pointer was found) are not shown.
==5290== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==5290==
==5290== For counts of detected and suppressed errors, rerun with: -v
==5290== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 1 from 1)
==5292== Warning: noted but unhandled ioctl 0x2401 with no size/direction hints.
==5292== This could cause spurious value errors to appear.
==5292== See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==5292==
==5292== HEAP SUMMARY:
==5292== in use at exit: 164,018 bytes in 4,252 blocks
==5292== total heap usage: 17,910 allocs, 13,658 frees, 1,907,468 bytes allocated
==5292==
==5292== 49,720 (1,472 direct, 48,248 indirect) bytes in 8 blocks are definitely lost in loss record 580 of 580
==5292== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5292== by 0x5105E77: CRYPTO_malloc (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==5292== by 0x51E1D23: ??? (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==5292== by 0x51E4861: ??? (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==5292== by 0x51E5414: ASN1_item_ex_d2i (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==5292== by 0x51E546A: ASN1_item_d2i (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==5292== by 0x4E53E00: ??? (in /lib/x86_64-linux-gnu/libssl.so.1.0.0)
==5292== by 0x4E55727: ??? (in /lib/x86_64-linux-gnu/libssl.so.1.0.0)
==5292== by 0x452C4B: ssl_connect (stream-ssl.c:530)
==5292== by 0x445B18: scs_connecting (stream.c:315)
==5292== by 0x445B18: stream_connect (stream.c:338)
==5292== by 0x445B91: stream_recv (stream.c:369)
==5292== by 0x432A9C: jsonrpc_recv.part.7 (jsonrpc.c:310)
==5292== by 0x433977: jsonrpc_recv (jsonrpc.c:1139)
==5292== by 0x433977: jsonrpc_session_recv (jsonrpc.c:1112)
==5292== by 0x40CCE3: ovsdb_jsonrpc_session_run (jsonrpc-server.c:553)
==5292== by 0x40CCE3: ovsdb_jsonrpc_session_run_all (jsonrpc-server.c:586)
==5292== by 0x40CCE3: ovsdb_jsonrpc_server_run (jsonrpc-server.c:401)
==5292== by 0x40682E: main_loop (ovsdb-server.c:209)
==5292== by 0x40682E: main (ovsdb-server.c:460)
==5292==
==5292== LEAK SUMMARY:
==5292== definitely lost: 1,472 bytes in 8 blocks
==5292== indirectly lost: 48,248 bytes in 936 blocks
==5292== possibly lost: 0 bytes in 0 blocks
==5292== still reachable: 114,298 bytes in 3,308 blocks
==5292== suppressed: 0 bytes in 0 blocks
==5292== Reachable blocks (those to which a pointer was found) are not shown.
==5292== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==5292==
==5292== For counts of detected and suppressed errors, rerun with: -v
==5292== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 1 from 1)
....
In this case ovsdb-server is running as daemon process (--detach option) and leaking memory is
accumulated whenever ovsdb-client is reconnected. Within observed test ovsdb-client CLI tool
connects 8 times to ovsdb-server. Leaked memory in ovsdb-client (for each invocation) is approx.
6K bytes, while leaked memory in ovsdb-server is aprox. 48Kbytes what is actually 8*6K. Thus per
each connection both ovsdb-client and ovsdb-server leak approx. 6K bytes.
I have done a small manual test to check if ovsdb-server is indeed accumulating leaked memory
by dumping ovsdb-server in a loop:
console1:
ovsdb-server \
--log-file \
--detach --no-chdir --pidfile \
--private-key=testpki-privkey2.pem \
--certificate=testpki-cert2.pem \
--ca-cert=testpki-cacert.pem \
--remote=pssl:0:127.0.0.1 \
db
while (true); do \
ovsdb-client \
--private-key=testpki-privkey.pem \
--certificate=testpki-cert.pem \
--ca-cert=testpki-cacert.pem \
dump ssl:127.0.0.1:42067; \
done
console2:
watch -n 0.5 'cat /proc/$(pidof ovsdb-server)/status | grep VmSize'
In console2 it was evidently seen ovsdb-server is constantly leaking memory. After a while
(i.e. after a certain number of reconnections) the OOM killer jumps out and kills ovsdb-server.
Very similar situation was already noticed and described in
https://github.com/openvswitch/ovs-issues/issues/168. There, the problem pops up while connecting
controller to ovs-vswitchd daemon.
Valgrind reports point to a problem in openssl library, however after studying openssl code for
a while I have found out the problem is actually in ovs. When connection through SSL channel is
taken place openssl library allocates memory for keeping track of certificate. Reference to this
memory works very similar as std::shared_ptr pointer in recent C++ dialects. i.e. when allocated
memory is referenced its reference counter is incremented and decremented after the memory is
derefered. When reference counter becomes zero allocated memory is automatically deallocated.
In openssl library environment certificate is retrieved by calling SSL_get_peer_certificate()
where its reference counter is incremented. After retrieved certificate is not used any more its
reference counter must be decremented by calling X509_free(). If not, allocated memory is never
freed despite the ssl connection is properly closed.
The problem was caused in stream-ssl.c in function ssl_connect(), which retrieves common peer name
by calling SSL_get_peer_certificate() function and without calling X509_free() function afterwards.
Signed-off-by: Damijan Skvarc <damjan.skvarc@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Running ovsdb-server with empty private-key and non-empty certificate
(or otherwise) causes crash:
# ovsdb-tool create ./etc/openvswitch/conf.db ./vswitch.ovsschema
# ovsdb-server --remote=punix:./db.sock \
--remote=db:Open_vSwitch,Open_vSwitch,manager_options \
--private-key=db:Open_vSwitch,SSL,private_key \
--certificate=db:Open_vSwitch,SSL,certificate \
--bootstrap-ca-cert=db:Open_vSwitch,SSL,ca_cert
# ovs-vsctl --no-wait init
# ovs-vsctl --no-wait set-ssl pkey.key cert.cert ca.cert
# ovs-vsctl --no-wait set SSL . private_key='""'
# ovs-vsctl --no-wait set SSL . certificate='cert.new'
==25513==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000
==25513==The signal is caused by a READ memory access.
==25513==Hint: address points to the zero page.
#0 0x7ff7582aa0a9 in __GI___strlen_sse2
#1 0x7ff759bdde81 (/lib64/libasan.so.5+0xace81)
#2 0x7ff759479932 (/lib64/libcrypto.so.1.1+0xb3932)
#3 0x7ff759473c5a in BIO_ctrl (/lib64/libcrypto.so.1.1+0xadc5a)
#4 0x7ff7598decc1 in SSL_CTX_use_certificate_file (/lib64/libssl.so.1.1+0x40cc1)
#5 0x4dbaa7 in stream_ssl_set_certificate_file__ lib/stream-ssl.c:1170
#6 0x4dca2e in stream_ssl_set_key_and_cert lib/stream-ssl.c:1216
#7 0x4146b2 in reconfigure_ssl ovsdb/ovsdb-server.c:1254
#8 0x409c83 in main ovsdb/ovsdb-server.c:368
#9 0x7ff758233812 in __libc_start_main
#10 0x40f6bd in _start (ovsdb-server+0x40f6bd)
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/lib64/libc.so.6+0x9a0a9) in __GI___strlen_sse2
==25513==ABORTING
Another way to reproduce is to use non-initialized DB entry for
private-key and a file for certificate in ovsdb-server cmdline.
The root cause is that stream_ssl_set_key_and_cert() triggers
configuration for both key and cert if any of them is valid, keeping
it possible for one of them to be NULL.
Fixes: 6f1e91b1d7c0 ("stream-ssl: Make changing keys and certificate at runtime reliable.")
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Acked-by: Ben Pfaff <blp@ovn.org>
This TLS extension, introduced in RFC 3546, allows the server to know what
host the client believes it is contacting, the TLS equivalent of the Host:
header in HTTP.
Tested-by: Yifeng Sun <pkusunyifeng@gmail.com>
Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
Requested-by: Shivaram Mysore <smysore@servicefractal.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
10 of the travis builds are failing such as
TESTSUITE=1 KERNEL=3.16.54 for gcc and clang.
Fixes: ab16d2c2871b ("stream-ssl: Don't enable new TLS versions by default")
CC: Timothy Redaelli <tredaelli@redhat.com>
Signed-off-by: Darrell Ball <dlu998@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Han Zhou <hzhou8@ebay.com>
Acked-by: Darrell Ball <dlu998@gmail.com>
Currently protocol_flags is populated by the list of SSL and TLS
protocols by hand. This means that when a new TLS version is added to
openssl (in this case TLS v1.3 is added to openssl 1.1.1 beta)
ovsdb-server automatically enable support to it with the default ciphers.
This can be a security problem (since other ciphers can be enabled) and it
also makes a test (SSL db: implementation) to fail.
This commit changes the 'protocol_flags' to use the list of all protocol
flags as provided by openssl library (SSL_OP_NO_SSL_MASK) so there is no
need to keep the list updated by hand.
Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
In openssl 1.1, there is no need to initialize the library. It is
automatically done when first used. This allows to compile openvswitch
with openssl 1.1.0 with deprecated API disabled.
Signed-off-by: Eneas U de Queiroz <cote2004-github@yahoo.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Rename the remaining variables that were shadowing another definition.
Signed-off-by: Justin Pettit <jpettit@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>
Poll-loop is the core to implement main loop. It should be available in
libopenvswitch.
Signed-off-by: Xiao Liang <shaw.leon@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
fixes undefined reference to ASN1_STRING_get0_data
Submitted-at: https://github.com/openvswitch/ovs/pull/202
Signed-off-by: Stuart Cardall <developer@it-offshore.co.uk>
Signed-off-by: Russell Bryant <russell@ovn.org>
ssl_new_stream() takes ownership of the passed-in 'name' parameter.
In error scenarios, the name is leaked. I was able to trigger this
leak by attempting to connect to an ovsdb over SSL and specifying
non-existent certificate, private key, and CA cert files.
This patch fixes the problem by freeing 'name' in the error label.
Signed-off-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Russell Bryant <russell@ovn.org>
It's occasionally convenient to format into a fixed-size buffer, but
as the use cases, and the text to be formatted, get more sophisticated,
it becomes easier to deal with "struct ds *" than a buffer pointer and
length pair. An upcoming commit will make ss_format_address() do more
work, and I think that this is the point at which it becomes easier to
take a dynamic string. This commit makes the parameter type change
without yet changing what is formatted.
Signed-off-by: Ben Pfaff <blp@ovn.org>
Tested-by: Numan Siddique <nusiddiq@redhat.com>
Acked-by: Numan Siddique <nusiddiq@redhat.com>
This will be a more sensible interface in an upcoming commit where many of
the callers are assembling dynamic name strings anyway.
Signed-off-by: Ben Pfaff <blp@ovn.org>
Tested-by: Numan Siddique <nusiddiq@redhat.com>
Acked-by: Numan Siddique <nusiddiq@redhat.com>
Track authenticated stream peer ID. For SSL connections, the
authenticated ID is the CN (Common Name) field extracted from
the peer's SSL certificate.
Signed-off-by: Lance Richardson <lrichard@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
The commit that this fixes is from 2009.
Reported-by: Kai-Wei Fan <fank@vmware.com>
Fixes: 9467fe624698 ("Add SSL support to "stream" library and OVSDB.")
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Andy Zhou <azhou@ovn.org>
To easily allow both in- and out-of-tree building of the Python
wrapper for the OVS JSON parser (e.g. w/ pip), move json.h to
include/openvswitch. This also requires moving lib/{hmap,shash}.h.
Both hmap.h and shash.h were #include-ing "util.h" even though the
headers themselves did not use anything from there, but rather from
include/openvswitch/util.h. Fixing that required including util.h
in several C files mostly due to OVS_NOT_REACHED and things like
xmalloc.
Signed-off-by: Terry Wilson <twilson@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Most vlog calls are for the log module owned by the translation unit being
compiled, but this module was referenced indirectly through a pointer
variable. That seems silly, so this commit changes the code so that the
local vlog module is referred to directly, as &this_module.
We could get rid of the global variables for vlog modules entirely, but
I like getting linker errors when there's a duplicate module name.
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Russell Bryant <russell@ovn.org>
test case 1628: peer ca cert
ASN1_item_dup
do_ca_cert_bootstrap (stream-ssl.c:413)
ssl_connect (stream-ssl.c:468)
scs_connecting (stream.c:297)
stream_connect (stream.c:320)
Fix by removing the X509_dup().
Signed-off-by: William Tu <u9012063@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
When the SSL code presents the name of the address to which it is bound,
it should include an "ssl:" or "pssl:" prefix instead of "tcp:" or "ptcp:".
Reported-by: meishengxin <meishengxin@huawei.com>
Reported-at: http://openvswitch.org/pipermail/discuss/2015-December/019694.html
Fixes: e731d71bf47b ("Add IPv6 support for OpenFlow, OVSDB, NetFlow, and sFlow.")
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Russell Bryant <russell@ovn.org>
SSL_CTX_add_client_CA() appends to the client CA list without replacing any
already on the list, and furthermore wastes memory if the certificate in
the file is already on the list. This commit thus fixes an effective
memory leak.
Signed-off-by: YongQiangLiu <liu.liuyongqiang@huawei.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
When --certificate option is provided, we currently use
SSL_CTX_use_certificate_chain_file() function to add
that certificate. If our single certificate file had multiple
certificates (as a chain), all of them would get added and sent
to the remote peer. But once you call
SSL_CTX_use_certificate_chain_file(), any future calls to
SSL_CTX_add_extra_chain_cert() (called when --peer-ca-cert option
is used) had no effect.
Since our man pages and INSTALL.SSL.md say that --certificate
is used to specify one certificate and additional certificates
are sent via --peer-ca-cert, this commit changes
SSL_CTX_use_certificate_chain_file() use to
SSL_CTX_use_certificate_file(). With this, additional certificates
can now be added via --peer-ca-cert option.
The test case added with this commit would fail without the
above changes.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
To enable SSL clients to reconnect with the ovs-testcontoller without being
rejected, one must either set the SSL Session ID flag or disable the
SSL caching. This patch disables the SSL caching/reuse.
In the absence of this fix, the error message from ovs-testcontroller is as
below:
SSL protocol error: SSL_accept (error:140D9115:SSL
routines:SSL_GET_PREV_SESSION:session id context uninitialized)
See <https://www.openssl.org/docs/ssl/SSL_CTX_set_session_id_context.html>.
Validation: Tested with ovs-testcontroller, by performing SSL reconnection
with OpenSSL based SSL client.
Signed-off-by: Guru Chaitanya Perakam <gperakam@brocade.com>
Reported-by: Guru Chaitanya Perakam <gperakam@brocade.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
We've been warning about the change since 2.1, which was released a year
ago.
Signed-off-by: Justin Pettit <jpettit@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
In all the churn around ofpbuf and dp_packet, this code seems to have been
overlooked. This fixes the problem.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
This function is really of marginal utility. This commit drops it and
makes the existing callers instead open a new pstream with the desired
dscp.
The ulterior motive here is that the set_dscp() function that actually sets
the DSCP on a socket really wants to know the address family (AF_INET vs.
AF_INET6). We could plumb that down through the stream code, and that's
one reasonable option, but I thought that simply eliminating some calls
to set_dscp() where we don't already have the address family handy was
another reasonable way to go.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Alex Wang <alexw@nicira.com>
A new function vlog_insert_module() is introduced to avoid using
list_insert() from the vlog.h header.
Signed-off-by: Thomas Graf <tgraf@noironetworks.com>
Acked-by: Ben Pfaff <blp@nicira.com>
On Windows platform, TCP_NODELAY can only be set when TCP is established.
(This is an observed behavior and not written in any MSDN documentation.)
The current code does not create any problems while running unit tests
(because connections get established immediately) but is reportedly
observed while connecting to a different machine.
commit 8b76839(Move setsockopt TCP_NODELAY to when TCP is connected.)
made changes to call setsockopt with TCP_NODELAY after TCP is connected
only in lib/stream-ssl.c. We need the same change for stream-tcp too and
this commit does that.
Currently, a failure of setting TCP_NODELAY results in reporting
the error and then closing the socket. This commit changes that
behavior such that an error is reported if setting TCP_NODELAY
fails, but the connection itself is not torn down.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Commit a8d819675f3 (Remove stream, vconn, and rconn functions to get
local/remote IPs/ports.) removed the code that used the local socket
address but neglected to remove the code to fetch that address. This
commit removes the latter code also.
Reported-by: Eitan Eliahu <eliahue@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Eitan Eliahu <eliahue@vmware.com>
We currently have a poll_fd_wait_event(fd, wevent, events) function that
is used at places common to Windows and Linux where we have to wait on
sockets. On Linux, 'wevent' is always set as zero. On Windows, for sockets,
when we send both 'fd' and 'wevent', we associate them with each other for
'events' and then wait on 'wevent'. Also on Windows, when we only send 'wevent'
to this function, we would simply wait for all events for that 'wevent'.
There is a disadvantage with this approach.
* Windows clients need to create a 'wevent' and then pass it along. This
means that at a lot of places where we create sockets, we also are forced
to create a 'wevent'.
With this commit, we pass the responsibility of creating a 'wevent' to
poll_fd_wait() in case of sockets. That way, a client using poll_fd_wait()
is only concerned about sockets and not about 'wevents'. There is a potential
disadvantage with this change in that we create events more often and that
may have a performance penalty. If that turns out to be the case, we will
eventually need to create a pool of wevents that can be re-used.
In Windows, there are cases where we want to wait on a event (not
associated with any sockets) and then control it using functions
like SetEvent() etc. For that purpose, introduce a new function
poll_wevent_wait(). For this function, the client needs to create a event
and then pass it along as an argument.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-By: Ben Pfaff <blp@nicira.com>
On windows platform, TCP_NODELAY can only be set when TCP
is established. If the conection is not immediately returning
success, call it when state is changed from TCP_CONNECTING
to SSL_CONNECTING.
Signed-off-by: Linda Sun <lsun@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Otherwise the indeterminate 'wevent' could frustrate poll_fd_wait_at()'s
attempt to merge "poll_node"s for the same fd.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Gurucharan Shetty <gshetty@nicira.com>
So far, we log the kernel assigned port number when the port number is
not specified. On Windows, this happens multiple times because "unix"
sockets are implemented internally via TCP ports. This means that many tests,
specially the ovs-ofctl monitor tests, need to filter out the
additional messages. Doing that is not a big deal, but I think it will
keep manifesting in future tests added by Linux developers.
With this commit, we simply don't print the kernel assigned TCP ports
on Windows when done for "unix" sockets.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
For Windows sockets, one has to call closesocket() to
close the sockets.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
This commit creates events and through poll_fd_wait_event()
associates them with socket file descriptors to get woken up
from poll_block().
Some other changes:
* Windows does not have sys/fcntl.h but has a fcntl.h
On Linux, there is fctnl.h too.
* include <openssl/applink.c> to handle different C-Runtime linking
of OVS and openssl libraries as suggested at
https://www.openssl.org/support/faq.html#PROG2
The above include will not be needed if we compile Open vSwitch with
/MD compiler option.
* SHUT_RDWR is equivalent to SD_BOTH on Windows.
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
Does not add IPv6 support for in-band control.
Co-authored-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: Nandan Nivgune <nandan.nivgune@calsoftinc.com>
Signed-off-by: Abhijit Bhopatkar <abhijit.bhopatkar@calsoftinc.com>
Signed-off-by: Arun Sharma <arun.sharma@calsoftinc.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
