diff mbox

[ovs-dev] python ovs: Fix SSL exceptions with pyOpenSSL v0.13

Message ID 20170515150347.18961-1-nusiddiq@redhat.com
State Superseded
Headers show

Commit Message

Numan Siddique May 15, 2017, 3:03 p.m. UTC
From: Numan Siddique <nusiddiq@redhat.com>

Centos provides pyOpenSSL version pyOpenSSL-0.13.1-3.el7.x86_64.
There are 2 issues using this version, which this patch fixes

 - The test case "simple idl verify notify - SSL" is skipped.
   This is because "python -m OpenSSL.SSL" is used to detect the
   presence of pyOpenSSL package. pyOpenSSL v0.13 has C python
   modules because of which the above command returns 1.
   So this patch fixes this using 'python -c "import OpenSSL.SSL"'.

 - The SSL.Context class do not the function "set_session_cache_mode"
   defined. So this patch uses hasattr() to detect this function
   before accessing it.

I have not tested with older versions (< 0.13) of pyOpenSSL.

Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
---
 python/ovs/stream.py | 7 ++++++-
 tests/ovsdb-idl.at   | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

Comments

Lance Richardson May 15, 2017, 3:16 p.m. UTC | #1
> From: nusiddiq@redhat.com
> To: dev@openvswitch.org
> Sent: Monday, 15 May, 2017 11:03:47 AM
> Subject: [ovs-dev] [PATCH] python ovs: Fix SSL exceptions with pyOpenSSL	v0.13
> 
> From: Numan Siddique <nusiddiq@redhat.com>
> 
> Centos provides pyOpenSSL version pyOpenSSL-0.13.1-3.el7.x86_64.
> There are 2 issues using this version, which this patch fixes
> 
>  - The test case "simple idl verify notify - SSL" is skipped.
>    This is because "python -m OpenSSL.SSL" is used to detect the
>    presence of pyOpenSSL package. pyOpenSSL v0.13 has C python
>    modules because of which the above command returns 1.
>    So this patch fixes this using 'python -c "import OpenSSL.SSL"'.
> 
>  - The SSL.Context class do not the function "set_session_cache_mode"
>    defined. So this patch uses hasattr() to detect this function
>    before accessing it.
> 
> I have not tested with older versions (< 0.13) of pyOpenSSL.
> 
> Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
> ---
>  python/ovs/stream.py | 7 ++++++-
>  tests/ovsdb-idl.at   | 2 +-
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/python/ovs/stream.py b/python/ovs/stream.py
> index fc0368c..c037df5 100644
> --- a/python/ovs/stream.py
> +++ b/python/ovs/stream.py
> @@ -767,7 +767,12 @@ class SSLStream(Stream):
>          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.set_session_cache_mode(SSL.SESS_CACHE_OFF)
> +
> +        # 'set_session_cache_mode' attribute in SSL.Context is not present
> +        # in pyOpenSSL version < 0.14. So check it before accessing it.
> +        if hasattr(ctx, 'set_session_cache_mode'):
> +            ctx.set_session_cache_mode(SSL.SESS_CACHE_OFF)
> +

Hi Numan,

The default session cache mode is SSL_SESS_CACHE_SERVER, which only
has an effect for server-side sessions (the openssl library considers
the passive connection side to be the server side).

But python/ovs/stream.py only supports client-side (active) SSL connections
(ssl:, not pssl:), so wouldn't it make more sense to simply delete this call?

Regards,

  Lance


>          # If the client has not set the SSL configuration files
>          # exception would be raised.
>          ctx.use_privatekey_file(Stream._SSL_private_key_file)
> diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
> index d28dfc1..4eaf87f 100644
> --- a/tests/ovsdb-idl.at
> +++ b/tests/ovsdb-idl.at
> @@ -1185,7 +1185,7 @@ m4_define([OVSDB_CHECK_IDL_NOTIFY_SSL_PY],
>    [AT_SETUP([$1 - SSL])
>     AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
>     AT_SKIP_IF([test $HAVE_PYTHON = no])
> -   $PYTHON -m OpenSSL.SSL
> +   $PYTHON -c "import OpenSSL.SSL"
>     SSL_PRESENT=$?
>     AT_SKIP_IF([test $SSL_PRESENT != 0])
>     AT_KEYWORDS([ovsdb server idl Python notify - ssl socket])
> --
> 2.9.3
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Numan Siddique May 15, 2017, 3:29 p.m. UTC | #2
On Mon, May 15, 2017 at 8:46 PM, Lance Richardson <lrichard@redhat.com>
wrote:

> > From: nusiddiq@redhat.com
> > To: dev@openvswitch.org
> > Sent: Monday, 15 May, 2017 11:03:47 AM
> > Subject: [ovs-dev] [PATCH] python ovs: Fix SSL exceptions with
> pyOpenSSL      v0.13
> >
> > From: Numan Siddique <nusiddiq@redhat.com>
> >
> > Centos provides pyOpenSSL version pyOpenSSL-0.13.1-3.el7.x86_64.
> > There are 2 issues using this version, which this patch fixes
> >
> >  - The test case "simple idl verify notify - SSL" is skipped.
> >    This is because "python -m OpenSSL.SSL" is used to detect the
> >    presence of pyOpenSSL package. pyOpenSSL v0.13 has C python
> >    modules because of which the above command returns 1.
> >    So this patch fixes this using 'python -c "import OpenSSL.SSL"'.
> >
> >  - The SSL.Context class do not the function "set_session_cache_mode"
> >    defined. So this patch uses hasattr() to detect this function
> >    before accessing it.
> >
> > I have not tested with older versions (< 0.13) of pyOpenSSL.
> >
> > Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
> > ---
> >  python/ovs/stream.py | 7 ++++++-
> >  tests/ovsdb-idl.at   | 2 +-
> >  2 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/python/ovs/stream.py b/python/ovs/stream.py
> > index fc0368c..c037df5 100644
> > --- a/python/ovs/stream.py
> > +++ b/python/ovs/stream.py
> > @@ -767,7 +767,12 @@ class SSLStream(Stream):
> >          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.set_session_cache_mode(SSL.SESS_CACHE_OFF)
> > +
> > +        # 'set_session_cache_mode' attribute in SSL.Context is not
> present
> > +        # in pyOpenSSL version < 0.14. So check it before accessing it.
> > +        if hasattr(ctx, 'set_session_cache_mode'):
> > +            ctx.set_session_cache_mode(SSL.SESS_CACHE_OFF)
> > +
>
> Hi Numan,
>
> The default session cache mode is SSL_SESS_CACHE_SERVER, which only
> has an effect for server-side sessions (the openssl library considers
> the passive connection side to be the server side).
>
> But python/ovs/stream.py only supports client-side (active) SSL connections
> (ssl:, not pssl:), so wouldn't it make more sense to simply delete this
> call?
>
>
Thanks Lance for the comments. I agree. It makes more sense to delete it. I
will spin another patch with the changes.

Numan

Regards,
>
>   Lance
>
>
> >          # If the client has not set the SSL configuration files
> >          # exception would be raised.
> >          ctx.use_privatekey_file(Stream._SSL_private_key_file)
> > diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
> > index d28dfc1..4eaf87f 100644
> > --- a/tests/ovsdb-idl.at
> > +++ b/tests/ovsdb-idl.at
> > @@ -1185,7 +1185,7 @@ m4_define([OVSDB_CHECK_IDL_NOTIFY_SSL_PY],
> >    [AT_SETUP([$1 - SSL])
> >     AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
> >     AT_SKIP_IF([test $HAVE_PYTHON = no])
> > -   $PYTHON -m OpenSSL.SSL
> > +   $PYTHON -c "import OpenSSL.SSL"
> >     SSL_PRESENT=$?
> >     AT_SKIP_IF([test $SSL_PRESENT != 0])
> >     AT_KEYWORDS([ovsdb server idl Python notify - ssl socket])
> > --
> > 2.9.3
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
diff mbox

Patch

diff --git a/python/ovs/stream.py b/python/ovs/stream.py
index fc0368c..c037df5 100644
--- a/python/ovs/stream.py
+++ b/python/ovs/stream.py
@@ -767,7 +767,12 @@  class SSLStream(Stream):
         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.set_session_cache_mode(SSL.SESS_CACHE_OFF)
+
+        # 'set_session_cache_mode' attribute in SSL.Context is not present
+        # in pyOpenSSL version < 0.14. So check it before accessing it.
+        if hasattr(ctx, 'set_session_cache_mode'):
+            ctx.set_session_cache_mode(SSL.SESS_CACHE_OFF)
+
         # If the client has not set the SSL configuration files
         # exception would be raised.
         ctx.use_privatekey_file(Stream._SSL_private_key_file)
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index d28dfc1..4eaf87f 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -1185,7 +1185,7 @@  m4_define([OVSDB_CHECK_IDL_NOTIFY_SSL_PY],
   [AT_SETUP([$1 - SSL])
    AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
    AT_SKIP_IF([test $HAVE_PYTHON = no])
-   $PYTHON -m OpenSSL.SSL
+   $PYTHON -c "import OpenSSL.SSL"
    SSL_PRESENT=$?
    AT_SKIP_IF([test $SSL_PRESENT != 0])
    AT_KEYWORDS([ovsdb server idl Python notify - ssl socket])