Message ID | ad48b2cb4cf274fcf64c253ba01e7c8f0ad1c5bc.1635548862.git.tredaelli@redhat.com |
---|---|
State | Accepted |
Headers | show |
Series | None | expand |
Acked-by: Terry Wilson <twilson@redhat.com> Tested-by: Terry Wilson <twilson@redhat.com> On Fri, Oct 29, 2021 at 6:12 PM Timothy Redaelli <tredaelli@redhat.com> wrote: > > Currently, pyOpenSSL is half-deprecated upstream and so it's removed on > some distributions (for example on CentOS Stream 9, > https://issues.redhat.com/browse/CS-336), but since OVS only > supports Python 3 it's possible to replace pyOpenSSL with "import ssl" > included in base Python 3. > > Stream recv and send had to be splitted as _recv and _send, since SSLError > is a subclass of socket.error and so it was not possible to except for > SSLWantReadError and SSLWantWriteError in recv and send of SSLStream. > > TCPstream._open cannot be used in SSLStream, since Python ssl module > requires the SSL socket to be created before connecting it, so > SSLStream._open needs to create the socket, create SSL socket and then > connect the SSL socket. > > Reported-by: Timothy Redaelli <tredaelli@redhat.com> > Reported-at: https://bugzilla.redhat.com/1988429 > Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> > --- > v3 -> v4: > - Remove useless ValueError in self.socket.shutdown, as reported by > Terry Wilson. It was wrongly here due to some previous tests, but > it's not needed. > --- > .ci/linux-prepare.sh | 2 +- > .cirrus.yml | 2 +- > .travis.yml | 1 - > python/ovs/poller.py | 6 +-- > python/ovs/stream.py | 91 ++++++++++++++++++++++++++------------------ > tests/ovsdb-idl.at | 2 +- > 6 files changed, 60 insertions(+), 44 deletions(-) > > diff --git a/.ci/linux-prepare.sh b/.ci/linux-prepare.sh > index c55125cf7..b9b499bad 100755 > --- a/.ci/linux-prepare.sh > +++ b/.ci/linux-prepare.sh > @@ -21,7 +21,7 @@ make -j4 HAVE_LLVM= HAVE_SQLITE= install > cd .. > > pip3 install --disable-pip-version-check --user \ > - flake8 hacking sphinx pyOpenSSL wheel setuptools > + flake8 hacking sphinx wheel setuptools > pip3 install --user --upgrade docutils > pip3 install --user 'meson==0.47.1' > > diff --git a/.cirrus.yml b/.cirrus.yml > index 480fea242..a7ae793bc 100644 > --- a/.cirrus.yml > +++ b/.cirrus.yml > @@ -9,7 +9,7 @@ freebsd_build_task: > > env: > DEPENDENCIES: automake libtool gmake gcc wget openssl python3 > - PY_DEPS: sphinx|openssl > + PY_DEPS: sphinx > matrix: > COMPILER: gcc > COMPILER: clang > diff --git a/.travis.yml b/.travis.yml > index 51d051108..c7aeede06 100644 > --- a/.travis.yml > +++ b/.travis.yml > @@ -17,7 +17,6 @@ addons: > - libjemalloc-dev > - libnuma-dev > - libpcap-dev > - - python3-openssl > - python3-pip > - python3-sphinx > - libelf-dev > diff --git a/python/ovs/poller.py b/python/ovs/poller.py > index 3624ec865..157719c3a 100644 > --- a/python/ovs/poller.py > +++ b/python/ovs/poller.py > @@ -26,9 +26,9 @@ if sys.platform == "win32": > import ovs.winutils as winutils > > try: > - from OpenSSL import SSL > + import ssl > except ImportError: > - SSL = None > + ssl = None > > try: > from eventlet import patcher as eventlet_patcher > @@ -73,7 +73,7 @@ class _SelectSelect(object): > def register(self, fd, events): > if isinstance(fd, socket.socket): > fd = fd.fileno() > - if SSL and isinstance(fd, SSL.Connection): > + if ssl and isinstance(fd, ssl.SSLSocket): > fd = fd.fileno() > > if sys.platform != 'win32': > diff --git a/python/ovs/stream.py b/python/ovs/stream.py > index f5a520862..ac5b0fd0c 100644 > --- a/python/ovs/stream.py > +++ b/python/ovs/stream.py > @@ -22,9 +22,9 @@ import ovs.socket_util > import ovs.vlog > > try: > - from OpenSSL import SSL > + import ssl > except ImportError: > - SSL = None > + ssl = None > > if sys.platform == 'win32': > import ovs.winutils as winutils > @@ -322,6 +322,12 @@ class Stream(object): > The recv function will not block waiting for data to arrive. If no > data have been received, it returns (errno.EAGAIN, "") immediately.""" > > + try: > + return self._recv(n) > + except socket.error as e: > + return (ovs.socket_util.get_exception_errno(e), "") > + > + def _recv(self, n): > retval = self.connect() > if retval != 0: > return (retval, "") > @@ -331,10 +337,7 @@ class Stream(object): > if sys.platform == 'win32' and self.socket is None: > return self.__recv_windows(n) > > - try: > - return (0, self.socket.recv(n)) > - except socket.error as e: > - return (ovs.socket_util.get_exception_errno(e), "") > + return (0, self.socket.recv(n)) > > def __recv_windows(self, n): > if self._read_pending: > @@ -396,6 +399,12 @@ class Stream(object): > Will not block. If no bytes can be immediately accepted for > transmission, returns -errno.EAGAIN immediately.""" > > + try: > + return self._send(buf) > + except socket.error as e: > + return -ovs.socket_util.get_exception_errno(e) > + > + def _send(self, buf): > retval = self.connect() > if retval != 0: > return -retval > @@ -409,10 +418,7 @@ class Stream(object): > if sys.platform == 'win32' and self.socket is None: > return self.__send_windows(buf) > > - try: > - return self.socket.send(buf) > - except socket.error as e: > - return -ovs.socket_util.get_exception_errno(e) > + return self.socket.send(buf) > > def __send_windows(self, buf): > if self._write_pending: > @@ -769,35 +775,42 @@ class SSLStream(Stream): > def check_connection_completion(sock): > try: > return Stream.check_connection_completion(sock) > - except SSL.SysCallError as e: > + except ssl.SSLSyscallError as e: > return ovs.socket_util.get_exception_errno(e) > > @staticmethod > def needs_probes(): > return True > > - @staticmethod > - def verify_cb(conn, cert, errnum, depth, ok): > - return ok > - > @staticmethod > def _open(suffix, dscp): > - error, sock = TCPStream._open(suffix, dscp) > - if error: > - return error, None > + address = ovs.socket_util.inet_parse_active(suffix, 0) > + family, sock = ovs.socket_util.inet_create_socket_active( > + socket.SOCK_STREAM, address) > + if sock is None: > + return family, sock > > # Create an SSL context > - ctx = SSL.Context(SSL.SSLv23_METHOD) > - ctx.set_verify(SSL.VERIFY_PEER, SSLStream.verify_cb) > - ctx.set_options(SSL.OP_NO_SSLv2 | SSL.OP_NO_SSLv3) > + ctx = ssl.SSLContext(ssl.PROTOCOL_SSLv23) > + ctx.verify_mode = ssl.CERT_REQUIRED > + ctx.options |= ssl.OP_NO_SSLv2 > + ctx.options |= ssl.OP_NO_SSLv3 > # If the client has not set the SSL configuration files > # exception would be raised. > - ctx.use_privatekey_file(Stream._SSL_private_key_file) > - ctx.use_certificate_file(Stream._SSL_certificate_file) > ctx.load_verify_locations(Stream._SSL_ca_cert_file) > + ctx.load_cert_chain(Stream._SSL_certificate_file, > + Stream._SSL_private_key_file) > + ssl_sock = ctx.wrap_socket(sock, do_handshake_on_connect=False) > > - ssl_sock = SSL.Connection(ctx, sock) > - ssl_sock.set_connect_state() > + # Connect > + error = ovs.socket_util.inet_connect_active(ssl_sock, address, family, > + dscp) > + if not error: > + try: > + ssl_sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1) > + except socket.error as e: > + ssl_sock.close() > + return ovs.socket_util.get_exception_errno(e), None > return error, ssl_sock > > def connect(self): > @@ -809,40 +822,44 @@ class SSLStream(Stream): > # TCP Connection is successful. Now do the SSL handshake > try: > self.socket.do_handshake() > - except SSL.WantReadError: > + except ssl.SSLWantReadError: > return errno.EAGAIN > - except SSL.SysCallError as e: > + except ssl.SSLSyscallError as e: > return ovs.socket_util.get_exception_errno(e) > > return 0 > > def recv(self, n): > try: > - return super(SSLStream, self).recv(n) > - except SSL.WantReadError: > + return super(SSLStream, self)._recv(n) > + except ssl.SSLWantReadError: > return (errno.EAGAIN, "") > - except SSL.SysCallError as e: > + except ssl.SSLSyscallError as e: > return (ovs.socket_util.get_exception_errno(e), "") > - except SSL.ZeroReturnError: > + except ssl.SSLZeroReturnError: > return (0, "") > + except socket.error as e: > + return (ovs.socket_util.get_exception_errno(e), "") > > def send(self, buf): > try: > - return super(SSLStream, self).send(buf) > - except SSL.WantWriteError: > + return super(SSLStream, self)._send(buf) > + except ssl.SSLWantWriteError: > return -errno.EAGAIN > - except SSL.SysCallError as e: > + except ssl.SSLSyscallError as e: > + return -ovs.socket_util.get_exception_errno(e) > + except socket.error as e: > return -ovs.socket_util.get_exception_errno(e) > > def close(self): > if self.socket: > try: > - self.socket.shutdown() > - except SSL.Error: > + self.socket.shutdown(socket.SHUT_RDWR) > + except socket.error: > pass > return super(SSLStream, self).close() > > > -if SSL: > +if ssl: > # Register SSL only if the OpenSSL module is available > Stream.register_method("ssl", SSLStream) > diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at > index 501c13b81..0f229b2f9 100644 > --- a/tests/ovsdb-idl.at > +++ b/tests/ovsdb-idl.at > @@ -225,7 +225,7 @@ m4_define([OVSDB_CHECK_IDL_TCP6_MULTIPLE_REMOTES_PY], > m4_define([OVSDB_CHECK_IDL_SSL_PY], > [AT_SETUP([$1 - Python3 - SSL]) > AT_SKIP_IF([test "$HAVE_OPENSSL" = no]) > - $PYTHON3 -c "import OpenSSL.SSL" > + $PYTHON3 -c "import ssl" > SSL_PRESENT=$? > AT_SKIP_IF([test $SSL_PRESENT != 0]) > AT_KEYWORDS([ovsdb server idl positive Python with ssl socket $5]) > -- > 2.31.1 >
On 11/1/21 15:27, Terry Wilson wrote: > Acked-by: Terry Wilson <twilson@redhat.com> > Tested-by: Terry Wilson <twilson@redhat.com> > > On Fri, Oct 29, 2021 at 6:12 PM Timothy Redaelli <tredaelli@redhat.com> wrote: >> >> Currently, pyOpenSSL is half-deprecated upstream and so it's removed on >> some distributions (for example on CentOS Stream 9, >> https://issues.redhat.com/browse/CS-336), but since OVS only >> supports Python 3 it's possible to replace pyOpenSSL with "import ssl" >> included in base Python 3. >> >> Stream recv and send had to be splitted as _recv and _send, since SSLError >> is a subclass of socket.error and so it was not possible to except for >> SSLWantReadError and SSLWantWriteError in recv and send of SSLStream. >> >> TCPstream._open cannot be used in SSLStream, since Python ssl module >> requires the SSL socket to be created before connecting it, so >> SSLStream._open needs to create the socket, create SSL socket and then >> connect the SSL socket. >> >> Reported-by: Timothy Redaelli <tredaelli@redhat.com> >> Reported-at: https://bugzilla.redhat.com/1988429 >> Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> >> --- >> v3 -> v4: >> - Remove useless ValueError in self.socket.shutdown, as reported by >> Terry Wilson. It was wrongly here due to some previous tests, but >> it's not needed. >> --- >> .ci/linux-prepare.sh | 2 +- >> .cirrus.yml | 2 +- >> .travis.yml | 1 - >> python/ovs/poller.py | 6 +-- >> python/ovs/stream.py | 91 ++++++++++++++++++++++++++------------------ >> tests/ovsdb-idl.at | 2 +- >> 6 files changed, 60 insertions(+), 44 deletions(-) Thanks, Terry and Timothy! I added a NEWS entry about this change, since it's kind of user-visible in terms of required dependencies, and applied. Bets regards, Ilya Maximets.
diff --git a/.ci/linux-prepare.sh b/.ci/linux-prepare.sh index c55125cf7..b9b499bad 100755 --- a/.ci/linux-prepare.sh +++ b/.ci/linux-prepare.sh @@ -21,7 +21,7 @@ make -j4 HAVE_LLVM= HAVE_SQLITE= install cd .. pip3 install --disable-pip-version-check --user \ - flake8 hacking sphinx pyOpenSSL wheel setuptools + flake8 hacking sphinx wheel setuptools pip3 install --user --upgrade docutils pip3 install --user 'meson==0.47.1' diff --git a/.cirrus.yml b/.cirrus.yml index 480fea242..a7ae793bc 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -9,7 +9,7 @@ freebsd_build_task: env: DEPENDENCIES: automake libtool gmake gcc wget openssl python3 - PY_DEPS: sphinx|openssl + PY_DEPS: sphinx matrix: COMPILER: gcc COMPILER: clang diff --git a/.travis.yml b/.travis.yml index 51d051108..c7aeede06 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,7 +17,6 @@ addons: - libjemalloc-dev - libnuma-dev - libpcap-dev - - python3-openssl - python3-pip - python3-sphinx - libelf-dev diff --git a/python/ovs/poller.py b/python/ovs/poller.py index 3624ec865..157719c3a 100644 --- a/python/ovs/poller.py +++ b/python/ovs/poller.py @@ -26,9 +26,9 @@ if sys.platform == "win32": import ovs.winutils as winutils try: - from OpenSSL import SSL + import ssl except ImportError: - SSL = None + ssl = None try: from eventlet import patcher as eventlet_patcher @@ -73,7 +73,7 @@ class _SelectSelect(object): def register(self, fd, events): if isinstance(fd, socket.socket): fd = fd.fileno() - if SSL and isinstance(fd, SSL.Connection): + if ssl and isinstance(fd, ssl.SSLSocket): fd = fd.fileno() if sys.platform != 'win32': diff --git a/python/ovs/stream.py b/python/ovs/stream.py index f5a520862..ac5b0fd0c 100644 --- a/python/ovs/stream.py +++ b/python/ovs/stream.py @@ -22,9 +22,9 @@ import ovs.socket_util import ovs.vlog try: - from OpenSSL import SSL + import ssl except ImportError: - SSL = None + ssl = None if sys.platform == 'win32': import ovs.winutils as winutils @@ -322,6 +322,12 @@ class Stream(object): The recv function will not block waiting for data to arrive. If no data have been received, it returns (errno.EAGAIN, "") immediately.""" + try: + return self._recv(n) + except socket.error as e: + return (ovs.socket_util.get_exception_errno(e), "") + + def _recv(self, n): retval = self.connect() if retval != 0: return (retval, "") @@ -331,10 +337,7 @@ class Stream(object): if sys.platform == 'win32' and self.socket is None: return self.__recv_windows(n) - try: - return (0, self.socket.recv(n)) - except socket.error as e: - return (ovs.socket_util.get_exception_errno(e), "") + return (0, self.socket.recv(n)) def __recv_windows(self, n): if self._read_pending: @@ -396,6 +399,12 @@ class Stream(object): Will not block. If no bytes can be immediately accepted for transmission, returns -errno.EAGAIN immediately.""" + try: + return self._send(buf) + except socket.error as e: + return -ovs.socket_util.get_exception_errno(e) + + def _send(self, buf): retval = self.connect() if retval != 0: return -retval @@ -409,10 +418,7 @@ class Stream(object): if sys.platform == 'win32' and self.socket is None: return self.__send_windows(buf) - try: - return self.socket.send(buf) - except socket.error as e: - return -ovs.socket_util.get_exception_errno(e) + return self.socket.send(buf) def __send_windows(self, buf): if self._write_pending: @@ -769,35 +775,42 @@ class SSLStream(Stream): def check_connection_completion(sock): try: return Stream.check_connection_completion(sock) - except SSL.SysCallError as e: + except ssl.SSLSyscallError as e: return ovs.socket_util.get_exception_errno(e) @staticmethod def needs_probes(): return True - @staticmethod - def verify_cb(conn, cert, errnum, depth, ok): - return ok - @staticmethod def _open(suffix, dscp): - error, sock = TCPStream._open(suffix, dscp) - if error: - return error, None + address = ovs.socket_util.inet_parse_active(suffix, 0) + family, sock = ovs.socket_util.inet_create_socket_active( + socket.SOCK_STREAM, address) + if sock is None: + return family, sock # Create an SSL context - ctx = SSL.Context(SSL.SSLv23_METHOD) - ctx.set_verify(SSL.VERIFY_PEER, SSLStream.verify_cb) - ctx.set_options(SSL.OP_NO_SSLv2 | SSL.OP_NO_SSLv3) + ctx = ssl.SSLContext(ssl.PROTOCOL_SSLv23) + ctx.verify_mode = ssl.CERT_REQUIRED + ctx.options |= ssl.OP_NO_SSLv2 + ctx.options |= ssl.OP_NO_SSLv3 # If the client has not set the SSL configuration files # exception would be raised. - ctx.use_privatekey_file(Stream._SSL_private_key_file) - ctx.use_certificate_file(Stream._SSL_certificate_file) ctx.load_verify_locations(Stream._SSL_ca_cert_file) + ctx.load_cert_chain(Stream._SSL_certificate_file, + Stream._SSL_private_key_file) + ssl_sock = ctx.wrap_socket(sock, do_handshake_on_connect=False) - ssl_sock = SSL.Connection(ctx, sock) - ssl_sock.set_connect_state() + # Connect + error = ovs.socket_util.inet_connect_active(ssl_sock, address, family, + dscp) + if not error: + try: + ssl_sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1) + except socket.error as e: + ssl_sock.close() + return ovs.socket_util.get_exception_errno(e), None return error, ssl_sock def connect(self): @@ -809,40 +822,44 @@ class SSLStream(Stream): # TCP Connection is successful. Now do the SSL handshake try: self.socket.do_handshake() - except SSL.WantReadError: + except ssl.SSLWantReadError: return errno.EAGAIN - except SSL.SysCallError as e: + except ssl.SSLSyscallError as e: return ovs.socket_util.get_exception_errno(e) return 0 def recv(self, n): try: - return super(SSLStream, self).recv(n) - except SSL.WantReadError: + return super(SSLStream, self)._recv(n) + except ssl.SSLWantReadError: return (errno.EAGAIN, "") - except SSL.SysCallError as e: + except ssl.SSLSyscallError as e: return (ovs.socket_util.get_exception_errno(e), "") - except SSL.ZeroReturnError: + except ssl.SSLZeroReturnError: return (0, "") + except socket.error as e: + return (ovs.socket_util.get_exception_errno(e), "") def send(self, buf): try: - return super(SSLStream, self).send(buf) - except SSL.WantWriteError: + return super(SSLStream, self)._send(buf) + except ssl.SSLWantWriteError: return -errno.EAGAIN - except SSL.SysCallError as e: + except ssl.SSLSyscallError as e: + return -ovs.socket_util.get_exception_errno(e) + except socket.error as e: return -ovs.socket_util.get_exception_errno(e) def close(self): if self.socket: try: - self.socket.shutdown() - except SSL.Error: + self.socket.shutdown(socket.SHUT_RDWR) + except socket.error: pass return super(SSLStream, self).close() -if SSL: +if ssl: # Register SSL only if the OpenSSL module is available Stream.register_method("ssl", SSLStream) diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at index 501c13b81..0f229b2f9 100644 --- a/tests/ovsdb-idl.at +++ b/tests/ovsdb-idl.at @@ -225,7 +225,7 @@ m4_define([OVSDB_CHECK_IDL_TCP6_MULTIPLE_REMOTES_PY], m4_define([OVSDB_CHECK_IDL_SSL_PY], [AT_SETUP([$1 - Python3 - SSL]) AT_SKIP_IF([test "$HAVE_OPENSSL" = no]) - $PYTHON3 -c "import OpenSSL.SSL" + $PYTHON3 -c "import ssl" SSL_PRESENT=$? AT_SKIP_IF([test $SSL_PRESENT != 0]) AT_KEYWORDS([ovsdb server idl positive Python with ssl socket $5])
Currently, pyOpenSSL is half-deprecated upstream and so it's removed on some distributions (for example on CentOS Stream 9, https://issues.redhat.com/browse/CS-336), but since OVS only supports Python 3 it's possible to replace pyOpenSSL with "import ssl" included in base Python 3. Stream recv and send had to be splitted as _recv and _send, since SSLError is a subclass of socket.error and so it was not possible to except for SSLWantReadError and SSLWantWriteError in recv and send of SSLStream. TCPstream._open cannot be used in SSLStream, since Python ssl module requires the SSL socket to be created before connecting it, so SSLStream._open needs to create the socket, create SSL socket and then connect the SSL socket. Reported-by: Timothy Redaelli <tredaelli@redhat.com> Reported-at: https://bugzilla.redhat.com/1988429 Signed-off-by: Timothy Redaelli <tredaelli@redhat.com> --- v3 -> v4: - Remove useless ValueError in self.socket.shutdown, as reported by Terry Wilson. It was wrongly here due to some previous tests, but it's not needed. --- .ci/linux-prepare.sh | 2 +- .cirrus.yml | 2 +- .travis.yml | 1 - python/ovs/poller.py | 6 +-- python/ovs/stream.py | 91 ++++++++++++++++++++++++++------------------ tests/ovsdb-idl.at | 2 +- 6 files changed, 60 insertions(+), 44 deletions(-)