On Mon, 8 Aug 2022 11:42:55 +0200 Ilya Maximets <i.maxim...@ovn.org> wrote:
> On 8/8/22 04:27, Miro Tomaska wrote: > > Python std library SSLSocket.send does not allow non-zero value for the > > optional flag. > > > > pyOpenSSL was recently switched for the Python standard library ssl module > > commit 68543dd523bd00f53fa7b91777b962ccb22ce679 (python: Replace pyOpenSSL > > with ssl). > > Python SSLsocket.send() does not allow non-zero optional flag and it will > > explicitly > > raise an exception for that. pyOpenSSL did not nothing with this flag but > > kept > > it to be compatible with socket API. > > https://github.com/pyca/pyopenssl/blob/main/src/OpenSSL/SSL.py#L1844 > > Hi, Miro. Thanks for the patch! > I'm puzzled though, how come our unit tests are not all failing? > I'm sure we have tests for SSL connections with python. And the > cited commit was tested with the neutron ml2/ovn tests over SSL > before being accepted with no issues observed. Because this code is only executed when you have an error (POLLERR or POLLHUP) and so it's very unlucky you enter this path of the code, that should definitely be fixed with your patch. > > > > In addition, expect for ImportError is not necessary anymore as ssl is part > > of > > the Python standard library. This type of exception should not happen. > > IIRC, if openssl is not installed in the system, import will fail. > Python itself doesn't contain the implementation for SSL protocols, > it relies on shared libraries which are not obligatory. So, we, > likely, still need an import check. Yes, you may build python without ssl support (if ssl libraries are not available when you build it) and so try is still necessary (they also do that in cpython, for example in asyncio https://github.com/python/cpython/blob/main/Lib/asyncio/sslproto.py) So, please do a v2 by restoring the try: / except ImportError: part and I'll ack your patch. Thank you > Best regards, Ilya Maximets. > > > > > Signed-off-by: Miro Tomaska <mtoma...@redhat.com> > > Reported-at: https://bugzilla.redhat.com/2115035 > > --- > > python/ovs/poller.py | 5 +---- > > python/ovs/socket_util.py | 7 ++++++- > > python/ovs/stream.py | 9 ++------- > > 3 files changed, 9 insertions(+), 12 deletions(-) > > > > diff --git a/python/ovs/poller.py b/python/ovs/poller.py > > index 157719c3a..0723ed1f6 100644 > > --- a/python/ovs/poller.py > > +++ b/python/ovs/poller.py > > @@ -17,6 +17,7 @@ import os > > > > import select > > import socket > > +import ssl > > import sys > > > > import ovs.timeval > > @@ -25,10 +26,6 @@ import ovs.vlog > > if sys.platform == "win32": > > import ovs.winutils as winutils > > > > -try: > > - import ssl > > -except ImportError: > > - ssl = None > > > > try: > > from eventlet import patcher as eventlet_patcher > > diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py > > index 651012bf0..5f2201bbb 100644 > > --- a/python/ovs/socket_util.py > > +++ b/python/ovs/socket_util.py > > @@ -17,6 +17,7 @@ import os > > import os.path > > import random > > import socket > > +import ssl > > import sys > > > > import ovs.fatal_signal > > @@ -178,7 +179,11 @@ def check_connection_completion(sock): > > if revents & ovs.poller.POLLERR or revents & ovs.poller.POLLHUP: > > try: > > # The following should raise an exception. > > - sock.send("\0".encode(), socket.MSG_DONTWAIT) > > + if isinstance(sock, ssl.SSLSocket): > > + # a SSL wrapped socket does not allow non-zero > > optional flag > > + sock.send("\0".encode()) > > + else: > > + sock.send("\0".encode(), socket.MSG_DONTWAIT) > > > > # (Here's where we end up if it didn't.) > > # XXX rate-limit > > diff --git a/python/ovs/stream.py b/python/ovs/stream.py > > index ac5b0fd0c..14a3c4946 100644 > > --- a/python/ovs/stream.py > > +++ b/python/ovs/stream.py > > @@ -15,16 +15,13 @@ > > import errno > > import os > > import socket > > +import ssl > > import sys > > > > import ovs.poller > > import ovs.socket_util > > import ovs.vlog > > > > -try: > > - import ssl > > -except ImportError: > > - ssl = None > > > > if sys.platform == 'win32': > > import ovs.winutils as winutils > > @@ -860,6 +857,4 @@ class SSLStream(Stream): > > return super(SSLStream, self).close() > > > > > > -if ssl: > > - # Register SSL only if the OpenSSL module is available > > - Stream.register_method("ssl", SSLStream) > > +Stream.register_method("ssl", SSLStream) > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
