Bug 1192496

Summary: vdsm is failing against M2Crypto vanilla code
Product: [oVirt] vdsm Reporter: Simone Tiraboschi <stirabos>
Component: GeneralAssignee: Piotr Kliczewski <pkliczew>
Status: CLOSED CURRENTRELEASE QA Contact: Pavel Stehlik <pstehlik>
Severity: low Docs Contact:
Priority: unspecified    
Version: 4.17.0CC: amureini, bazulay, bugs, ecohen, gklein, iheim, lsurette, mgoldboi, nsoffer, oourfali, pkliczew, rbalakri, sbonazzo, s.kieske, stirabos, ybronhei, yeylon
Target Milestone: ovirt-3.6.1Keywords: CodeChange
Target Release: 4.17.11Flags: pkliczew: ovirt-3.6.z?
rule-engine: planning_ack?
oourfali: devel_ack+
rule-engine: testing_ack+
Hardware: Unspecified   
OS: Unspecified   
Whiteboard: infra
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Cause: VDSM uses m2crypto package to allow ssl communication. Over debian distribution this package is buggy and we prefer to use python's default ssl implementation. Consequence: VDSM cannot communicate with ssl over debian Fix: In this bug scope we added config value to allow choosing between python ssl to m2crypto. During vdsm installation we set the default to python ssl over debian and to m2crypto over rhel\centos and fedora. The user can change the default value in vdsm.conf Result: Over debian we use now different implementation for ssl
 Story Points: ---
Clone Of: Environment:
Last Closed: 2016-01-13 14:37:33 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: Infra RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1163069    

Description Simone Tiraboschi 2015-02-13 14:33:09 UTC 
Description of problem:
vdsm is failing against M2Crypto vanilla code.

vdsm is calling 'settimeout' method on an instance of M2Crypto.SSL.Connection; this class doesn't provide 'settimeout' method on vanilla code.
https://github.com/M2Crypto/M2Crypto/blob/master/M2Crypto/SSL/Connection.py

That method has been added since fedora 7 with a custom patch never merged upstream due to portability issues.
http://pkgs.fedoraproject.org/cgit/m2crypto.git/tree/m2crypto-0.21.1-timeouts.patch

M2Crypto.SSL.Connection.settimeout is in charge of setting the the timeout of the underling socket and setting the blocking/non-blocking behavior depending 
on the specific timeout value.


Version-Release number of selected component (if applicable):
vdsm 4.17.0 (but also previous ones)

How reproducible:
100%

Steps to Reproduce:
1. use M2Crypto from https://pypi.python.org/pypi/M2Crypto (upstream) instead of fedora/RHEL/centos rpm
2. try to run vdsm ssl unit tests: tests/run_tests_local.sh sslhelper.py sslTests.py
3.

Actual results:
SSLTests
    testConnectWithCertificateSucceeds                          FAIL
    testConnectWithoutCertificateFails                          OK
    testSessionIsCached                                         FAIL
SocketTests
    test_block_socket                                           Exception in thread Thread-4:

This is the exception we get:
Traceback (most recent call last):
  File "/usr/lib/python2.7/threading.py", line 810, in __bootstrap_inner
    self.run()
  File "/usr/lib/python2.7/threading.py", line 763, in run
    self.__target(*self.__args, **self.__kwargs)
  File "/home/stirabos/vdsmdebian/vdsm/tests/sslTests.py", line 67, in serve_forever
    self.server.serve_forever()
  File "/usr/lib/python2.7/SocketServer.py", line 238, in serve_forever
    self._handle_request_noblock()
  File "/usr/lib/python2.7/SocketServer.py", line 290, in _handle_request_noblock
    request, client_address = self.get_request()
  File "/usr/lib/python2.7/SocketServer.py", line 467, in get_request
    return self.socket.accept()
  File "/home/stirabos/vdsmdebian/vdsm/lib/vdsm/sslutils.py", line 123, in accept
    client.settimeout(self.accept_timeout)
AttributeError: Connection instance has no attribute 'settimeout'

If we simply try to set the timeout value on the underling socket ignoring the blocking/non-blocking behavior of the read/write operations, the test blocks indefinitely.

Expected results:
It pass the tests using M2Crypto vanilla code.

Additional info:
Using vdsm with vanilla M2Crypto code is part of the effort to have oVirt managing debian based hosts.
Comment 1 Oved Ourfali 2015-03-08 09:17:58 UTC 
Simone - I saw you posted a fix. Shall I make you the assignee of this bug?
Comment 2 Simone Tiraboschi 2015-03-09 10:18:38 UTC 
No, as from first test I understood that my patch is absolutely unuseful: it sets the timeout on the underlying socket but M2Crypto still miss a lot of pieces to correctly handle and report it, so it's not that easy to fix it without a patch on M2Crypto if we need, as i thought, timeouts on SSL connections.
Comment 3 Piotr Kliczewski 2015-04-10 08:56:54 UTC 
I am confused why we want to have our code working with different version of M2Crypto. The code was written to work with library provided by different distributions (fedora, centos, rhel).

In my opinion this is not a bug because we are planning on migrating to standard ssl module and stop using m2crypto.
Comment 4 Sandro Bonazzola 2015-04-10 09:02:31 UTC 
(In reply to Piotr Kliczewski from comment #3)
> I am confused why we want to have our code working with different version of
> M2Crypto. The code was written to work with library provided by different
> distributions (fedora, centos, rhel).
> 
> In my opinion this is not a bug because we are planning on migrating to
> standard ssl module and stop using m2crypto.

+1 for stopping using m2crypto. But we need to either stopping using it or having vdsm working on vanilla m2crypto in order to support Debian.
Comment 5 Oved Ourfali 2015-06-11 07:38:12 UTC 
Piotr - once the m2crypto removal is in place, move this one to MODIFIED.
Comment 6 Sandro Bonazzola 2015-09-29 09:09:45 UTC 
This is an automated message.
oVirt 3.6.0 RC1 has been released. This bug has no target release and still have target milestone set to 3.6.0-rc.
Please review this bug and set target milestone and release to one of the next releases.
Comment 7 Piotr Kliczewski 2015-09-29 10:04:30 UTC 
Yaniv I can see that you removed Target release. Can you please set it back?
Comment 8 Yaniv Lavi 2015-10-14 12:43:57 UTC 
(In reply to Piotr Kliczewski from comment #7)
> Yaniv I can see that you removed Target release. Can you please set it back?


Target release should be set in build time.
Target milestone is the one that matters to state when this issue will be fixed.
Comment 9 Oved Ourfali 2015-10-25 11:24:45 UTC 
Piotr - when will this be in?
Need doc-text?
Comment 10 Red Hat Bugzilla Rules Engine 2015-10-26 10:31:43 UTC 
This bug is not marked for z-stream, yet the milestone is for a z-stream version, therefore the milestone has been reset.
Please set the correct milestone or add the z-stream flag.
Comment 11 Red Hat Bugzilla Rules Engine 2015-10-26 10:31:56 UTC 
This request has been proposed for two releases. This is invalid flag usage. The higher numbered release flag has been cleared. If you wish to change the release flag, you must clear one release flag and then set the other release flag to ?.
Comment 12 Piotr Kliczewski 2015-10-26 11:16:46 UTC 
Configurable ssl implementation was merged on master yesterday. I am currently working on verification of the patches for 3.6.

Now instead of having dependency on m2crypto we can choose to use ssl module.
Comment 13 Piotr Kliczewski 2015-10-26 12:56:30 UTC 
Changes verified for 3.6 but since this bug is targeted for master I added corresponding patches and moving to MODIFIED.
Comment 14 Red Hat Bugzilla Rules Engine 2015-10-28 09:16:11 UTC 
This bug is marked for z-stream, yet the milestone is for a major version, therefore the milestone has been reset.
Please set the correct milestone or drop the z stream flag.
Comment 15 Red Hat Bugzilla Rules Engine 2015-10-28 09:16:11 UTC 
Fixed bug tickets must have target milestone set prior to fixing them. Please set the correct milestone and move the bugs back to the previous status after this is corrected.
Comment 16 Red Hat Bugzilla Rules Engine 2015-10-28 09:16:11 UTC 
Target release should be placed once a package build is known to fix a issue. Since this bug is not modified, the target version has been reset. Please use target milestone to plan a fix for a oVirt release.
Comment 17 Red Hat Bugzilla Rules Engine 2015-11-03 12:51:05 UTC 
Target release should be placed once a package build is known to fix a issue. Since this bug is not modified, the target version has been reset. Please use target milestone to plan a fix for a oVirt release.
Comment 18 Sandro Bonazzola 2016-01-13 14:37:33 UTC 
oVirt 3.6.1 has been released, closing current release