diff mbox series

[ovs-dev] python: Catch OpenSSL errors during socket send

Message ID 5f7c1db2.nXiSOjYcw4zBbmqq%thomas.neuman@nutanix.com
State Superseded
Headers show
Series [ovs-dev] python: Catch OpenSSL errors during socket send | expand

Commit Message

Thomas Neuman Oct. 6, 2020, 7:33 a.m. UTC
The Python socket util is able to catch and parse standard socket errors
over the course of the connection lifecycle, but the OpenSSL library
raises an altogether different class of exception. As a result, if the
caller is attempting to use the util in establishing an SSL connection,
these errors go uncaught when they arise; instead of returning an errno
to the caller, the exception bubbles back up. Therefore this patch amends
this issue by checking for an OpenSSL.SSL.Error in addition to the
standard socket.error, when running with OpenSSL support.

Signed-off-by: thomas-neuman <thomas.neuman@nutanix.com>
---
 python/ovs/socket_util.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Ilya Maximets Oct. 6, 2020, 8:46 a.m. UTC | #1
On 10/6/20 9:33 AM, thomas.neuman@nutanix.com wrote:
> The Python socket util is able to catch and parse standard socket errors
> over the course of the connection lifecycle, but the OpenSSL library
> raises an altogether different class of exception. As a result, if the
> caller is attempting to use the util in establishing an SSL connection,
> these errors go uncaught when they arise; instead of returning an errno
> to the caller, the exception bubbles back up. Therefore this patch amends
> this issue by checking for an OpenSSL.SSL.Error in addition to the
> standard socket.error, when running with OpenSSL support.
> 
> Signed-off-by: thomas-neuman <thomas.neuman@nutanix.com>
> ---

Hi.  Thanks for the patch!

I think that you're trying to solve the same issue as following patch:
https://patchwork.ozlabs.org/project/openvswitch/patch/20200915212906.679182-1-twilson@redhat.com/

It'll be great if you can help reviewing/testing it.

v1 of above patch was very similar to yours.  You can find it there
alng with my review comments:
https://patchwork.ozlabs.org/project/openvswitch/patch/20200915171740.526042-1-twilson@redhat.com/

Best regards, Ilya Maximets.

>  python/ovs/socket_util.py | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py
> index 3faa64e..c7c1649 100644
> --- a/python/ovs/socket_util.py
> +++ b/python/ovs/socket_util.py
> @@ -23,6 +23,15 @@ import ovs.fatal_signal
>  import ovs.poller
>  import ovs.vlog
>  
> +try:
> +    from OpenSSL import SSL
> +    SSLError = SSL.Error
> +except ImportError:
> +    SSL = None
> +    # Define an exception class to catch, even though it's never raised.
> +    class SSLError(Exception):
> +        pass
> +
>  if sys.platform == 'win32':
>      import ovs.winutils as winutils
>      import win32file
> @@ -186,6 +195,9 @@ def check_connection_completion(sock):
>                  return errno.EPROTO
>              except socket.error as e:
>                  return get_exception_errno(e)
> +            except SSLError as e:
> +                vlog.err("SSL error %s" % e)
> +                return errno.EPROTO
>          else:
>              return 0
>      else:
>
diff mbox series

Patch

diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py
index 3faa64e..c7c1649 100644
--- a/python/ovs/socket_util.py
+++ b/python/ovs/socket_util.py
@@ -23,6 +23,15 @@  import ovs.fatal_signal
 import ovs.poller
 import ovs.vlog
 
+try:
+    from OpenSSL import SSL
+    SSLError = SSL.Error
+except ImportError:
+    SSL = None
+    # Define an exception class to catch, even though it's never raised.
+    class SSLError(Exception):
+        pass
+
 if sys.platform == 'win32':
     import ovs.winutils as winutils
     import win32file
@@ -186,6 +195,9 @@  def check_connection_completion(sock):
                 return errno.EPROTO
             except socket.error as e:
                 return get_exception_errno(e)
+            except SSLError as e:
+                vlog.err("SSL error %s" % e)
+                return errno.EPROTO
         else:
             return 0
     else: