diff mbox

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

Message ID 20170515153925.23102-1-nusiddiq@redhat.com
State Accepted
Headers show

Commit Message

Numan Siddique May 15, 2017, 3:39 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 by using 'python -c "import OpenSSL.SSL"'.

 - The SSL.Context class do not have the function "set_session_cache_mode"
   defined. Setting the session cache mode has an effect for server-side
   sessions and doesn't make much sense for client-side sessions.
   Since python ovs doesn't support "pssl" connection mode, this patch
   deletes the reference to this function.

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

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

Comments

Lance Richardson May 16, 2017, 6:15 p.m. UTC | #1
> From: nusiddiq@redhat.com
> To: dev@openvswitch.org
> Sent: Monday, 15 May, 2017 11:39:25 AM
> Subject: [ovs-dev] [PATCH v2] 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 by using 'python -c "import OpenSSL.SSL"'.
> 
>  - The SSL.Context class do not have the function "set_session_cache_mode"
>    defined. Setting the session cache mode has an effect for server-side
>    sessions and doesn't make much sense for client-side sessions.
>    Since python ovs doesn't support "pssl" connection mode, this patch
>    deletes the reference to this function.
> 
> I have not tested with older versions (< 0.13) of pyOpenSSL.
> 
> Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
> ---

LGTM

Acked-by: Lance Richardson <lrichard@redhat.com>
Marcin Mirecki May 19, 2017, 11:48 a.m. UTC | #2
> From: nusiddiq@redhat.com
> To: dev@openvswitch.org
> Sent: Monday, 15 May, 2017 11:39:25 AM
> Subject: [ovs-dev] [PATCH v2] 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 by using 'python -c "import OpenSSL.SSL"'.
>
>  - The SSL.Context class do not have the function "set_session_cache_mode"
>    defined. Setting the session cache mode has an effect for server-side
>    sessions and doesn't make much sense for client-side sessions.
>    Since python ovs doesn't support "pssl" connection mode, this patch
>    deletes the reference to this function.
>
> I have not tested with older versions (< 0.13) of pyOpenSSL.
>
> Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
> ---

> LGTM

> Acked-by: Lance Richardson <lrichard@redhat.com>

Tested-by: Marcin Mirecki <mmirecki@redhat.com>
Russell Bryant May 23, 2017, 5:33 p.m. UTC | #3
On Mon, May 15, 2017 at 11:39 AM,  <nusiddiq@redhat.com> wrote:
> 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 by using 'python -c "import OpenSSL.SSL"'.
>
>  - The SSL.Context class do not have the function "set_session_cache_mode"
>    defined. Setting the session cache mode has an effect for server-side
>    sessions and doesn't make much sense for client-side sessions.
>    Since python ovs doesn't support "pssl" connection mode, this patch
>    deletes the reference to this function.

I'd like to tweak this wording a bit.  Setting the cache mode in
general *does* have an effect for client side connections, as there is
a client mode you can set it to.  However, the usage being removed was
only relevant for server side connections so it was a no-op for our
client-only usage.  I'll tweak the commit message because I'm sure you
won't mind.  :-)

Otherwise, this looks good to me and I'll apply it to master and branch-2.7.

Thanks!

>
> I have not tested with older versions (< 0.13) of pyOpenSSL.
>
> Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
> ---
>  python/ovs/stream.py | 1 -
>  tests/ovsdb-idl.at   | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/python/ovs/stream.py b/python/ovs/stream.py
> index fc0368c..660d8bb 100644
> --- a/python/ovs/stream.py
> +++ b/python/ovs/stream.py
> @@ -767,7 +767,6 @@ 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)
>          # 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 23, 2017, 7:37 p.m. UTC | #4
On May 23, 2017 11:04 PM, "Russell Bryant" <russell@ovn.org> wrote:

On Mon, May 15, 2017 at 11:39 AM,  <nusiddiq@redhat.com> wrote:
> 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 by using 'python -c "import OpenSSL.SSL"'.
>
>  - The SSL.Context class do not have the function "set_session_cache_mode"
>    defined. Setting the session cache mode has an effect for server-side
>    sessions and doesn't make much sense for client-side sessions.
>    Since python ovs doesn't support "pssl" connection mode, this patch
>    deletes the reference to this function.

I'd like to tweak this wording a bit.  Setting the cache mode in
general *does* have an effect for client side connections, as there is
a client mode you can set it to.  However, the usage being removed was
only relevant for server side connections so it was a no-op for our
client-only usage.  I'll tweak the commit message because I'm sure you
won't mind.  :-)


Please do so :). Thanks for the proper commit message.

Numan


Otherwise, this looks good to me and I'll apply it to master and branch-2.7.

Thanks!

>
> I have not tested with older versions (< 0.13) of pyOpenSSL.
>
> Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
> ---
>  python/ovs/stream.py | 1 -
>  tests/ovsdb-idl.at   | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/python/ovs/stream.py b/python/ovs/stream.py
> index fc0368c..660d8bb 100644
> --- a/python/ovs/stream.py
> +++ b/python/ovs/stream.py
> @@ -767,7 +767,6 @@ 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)
>          # 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



--
Russell Bryant
diff mbox

Patch

diff --git a/python/ovs/stream.py b/python/ovs/stream.py
index fc0368c..660d8bb 100644
--- a/python/ovs/stream.py
+++ b/python/ovs/stream.py
@@ -767,7 +767,6 @@  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)
         # 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])