Piotr Kliczewski has uploaded a new change for review.
Change subject: ssl: configurable implementation ......................................................................
ssl: configurable implementation
We want to choose ssl implementation during build process.
Change-Id: I6501981bbd5276c49731b0d9eba4794286b0f823 Signed-off-by: pkliczewski piotr.kliczewski@gmail.com --- M debian/vdsm-python.install M lib/vdsm/Makefile.am M lib/vdsm/jsonrpcvdscli.py A lib/vdsm/sslutils.py M lib/vdsm/utils.py M lib/vdsm/vdscli.py M lib/yajsonrpc/stompreactor.py M tests/crossImportsTests.py.in M tests/integration/Makefile.am M tests/integration/jsonRpcHelper.py M tests/integration/m2chelper.py A tests/integration/sslhelper.py M tests/protocoldetectorTests.py M tests/sslTests.py M tests/stompTests.py M tests/vdscliTests.py M vdsm.spec.in M vdsm/clientIF.py M vdsm/kaxmlrpclib.py M vdsm/protocoldetector.py M vdsm/virt/migration.py 21 files changed, 562 insertions(+), 88 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/94/44494/1
diff --git a/debian/vdsm-python.install b/debian/vdsm-python.install index 36c0d9f..d9402c3 100644 --- a/debian/vdsm-python.install +++ b/debian/vdsm-python.install @@ -12,7 +12,6 @@ ./usr/lib/python2.7/dist-packages/vdsm/ipwrapper.py ./usr/lib/python2.7/dist-packages/vdsm/jsonrpcvdscli.py ./usr/lib/python2.7/dist-packages/vdsm/libvirtconnection.py -./usr/lib/python2.7/dist-packages/vdsm/m2cutils.py ./usr/lib/python2.7/dist-packages/vdsm/netconfpersistence.py ./usr/lib/python2.7/dist-packages/vdsm/netinfo.py ./usr/lib/python2.7/dist-packages/vdsm/netlink/__init__.py @@ -30,6 +29,7 @@ ./usr/lib/python2.7/dist-packages/vdsm/qemuimg.py ./usr/lib/python2.7/dist-packages/vdsm/response.py ./usr/lib/python2.7/dist-packages/vdsm/schedule.py +./usr/lib/python2.7/dist-packages/vdsm/sslutils.py ./usr/lib/python2.7/dist-packages/vdsm/tool/__init__.py ./usr/lib/python2.7/dist-packages/vdsm/tool/dummybr.py ./usr/lib/python2.7/dist-packages/vdsm/tool/dump_bonding_defaults.py diff --git a/lib/vdsm/Makefile.am b/lib/vdsm/Makefile.am index ec1a140..4a2bcb1 100644 --- a/lib/vdsm/Makefile.am +++ b/lib/vdsm/Makefile.am @@ -40,6 +40,7 @@ qemuimg.py \ response.py \ schedule.py \ + sslutils.py \ sysctl.py \ udevadm.py \ utils.py \ diff --git a/lib/vdsm/jsonrpcvdscli.py b/lib/vdsm/jsonrpcvdscli.py index 1687b6f..391d79e 100644 --- a/lib/vdsm/jsonrpcvdscli.py +++ b/lib/vdsm/jsonrpcvdscli.py @@ -22,7 +22,6 @@ from uuid import uuid4 import socket
-from . import m2cutils from yajsonrpc import stompreactor from yajsonrpc import \ JsonRpcRequest, \ @@ -30,6 +29,10 @@
from vdsm import response from .config import config +try: + from . import m2cutils as sslutils +except ImportError: + from . import sslutils
_COMMAND_CONVERTER = { diff --git a/lib/vdsm/sslutils.py b/lib/vdsm/sslutils.py new file mode 100644 index 0000000..63fc9cc --- /dev/null +++ b/lib/vdsm/sslutils.py @@ -0,0 +1,302 @@ +# +# Copyright 2014 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +# +# Refer to the README and COPYING files for full details of the license +# +from __future__ import absolute_import +import httplib +import logging +import os +import socket +import ssl +import xmlrpclib + +from vdsm.utils import ( + monotonic_time, +) +from .config import config + + +DEFAULT_ACCEPT_TIMEOUT = 5 +SOCKET_DEFAULT_TIMEOUT = socket._GLOBAL_DEFAULT_TIMEOUT + + +class SSLSocket(object): + def __init__(self, sock): + self.sock = sock + self._data = '' + + # ssl do not accept flag other than 0 + def read(self, size=4096, flag=None): + result = None + if flag == socket.MSG_PEEK: + bytes_left = size - len(self._data) + if bytes_left > 0: + self._data += self.sock.read(bytes_left) + result = self._data + else: + if self._data: + result = self._data + self._data = '' + else: + result = self.sock.read(size) + return result + recv = read + + def pending(self): + pending = self.sock.pending() + if self._data: + pending = pending + len(self._data) + return pending + + def __getattr__(self, name): + return getattr(self.sock, name) + + def makefile(self, mode='rb', bufsize=-1): + if mode == 'rb': + return socket._fileobject(self, mode, bufsize) + else: + return self.sock.makefile(mode, bufsize) + + +class SSLContext(object): + def __init__(self, cert_file, key_file, ca_certs=None, + protocol=ssl.PROTOCOL_TLSv1): + self.cert_file = cert_file + self.key_file = key_file + self.ca_certs = ca_certs + self.protocol = protocol + + +class VerifyingHTTPSConnection(httplib.HTTPSConnection): + def __init__(self, host, port=None, key_file=None, cert_file=None, + strict=None, timeout=SOCKET_DEFAULT_TIMEOUT, + ca_certs=None, cert_reqs=ssl.CERT_REQUIRED): + httplib.HTTPSConnection.__init__(self, host, port, key_file, cert_file, + strict, timeout) + self.ca_certs = ca_certs + self.cert_reqs = cert_reqs + + def connect(self): + "Connect to a host on a given (SSL) port." + + sock = socket.create_connection((self.host, self.port), self.timeout) + if self._tunnel_host: + self.sock = sock + self._tunnel() + # DK added: pass ca_cert to sslsocket + self.sock = ssl.wrap_socket(sock, self.key_file, self.cert_file, + ca_certs=self.ca_certs, server_side=False, + cert_reqs=self.cert_reqs) + + +class VerifyingSafeTransport(xmlrpclib.SafeTransport): + def __init__(self, use_datetime=0, key_file=None, cert_file=None, + ca_certs=None, cert_reqs=ssl.CERT_REQUIRED, + timeout=SOCKET_DEFAULT_TIMEOUT): + xmlrpclib.SafeTransport.__init__(self, use_datetime) + self.key_file = key_file + self.cert_file = cert_file + self.ca_certs = ca_certs + self.cert_reqs = cert_reqs + self._timeout = timeout + + def make_connection(self, host): + """Return VerifyingHTTPS object that is aware of ca_certs, and will + create VerifyingHTTPSConnection. + In Python 2.7, return VerifyingHTTPSConnection object + """ + chost, self._extra_headers, x509 = self.get_host_info(host) + if hasattr(xmlrpclib.SafeTransport, "single_request"): # Python 2.7 + return VerifyingHTTPSConnection( + chost, None, key_file=self.key_file, strict=None, + cert_file=self.cert_file, timeout=self._timeout, + ca_certs=self.ca_certs, + cert_reqs=self.cert_reqs) + else: + return VerifyingHTTPS( + chost, None, key_file=self.key_file, + cert_file=self.cert_file, timeout=self._timeout, + ca_certs=self.ca_certs, + cert_reqs=self.cert_reqs) + + +class VerifyingHTTPS(httplib.HTTPS): + _connection_class = VerifyingHTTPSConnection + + def __init__(self, host='', port=None, key_file=None, cert_file=None, + strict=None, timeout=SOCKET_DEFAULT_TIMEOUT, + ca_certs=None, cert_reqs=ssl.CERT_REQUIRED): + """A ca_cert-aware HTTPS object, + that creates a VerifyingHTTPSConnection + """ + # provide a default host, pass the X509 cert info + + # urf. compensate for bad input. + if port == 0: + port = None + self._setup(self._connection_class(host=host, + port=port, + key_file=key_file, + cert_file=cert_file, + strict=strict, + timeout=timeout, + ca_certs=ca_certs, + cert_reqs=cert_reqs)) + + # we never actually use these for anything, but we keep them + # here for compatibility with post-1.5.2 CVS. + self.key_file = key_file + self.cert_file = cert_file + + +class SSLHandshakeDispatcher(object): + """ + SSLHandshakeDispatcher is dispatcher implementation to process ssl + handshake in asynchronous way. Once we are done with handshaking we + we need to swap our dispatcher implementation with message processing + dispatcher. We use handshake_finished_handler function to perform + swapping. The handler implementation need to invoke + + dispatcher.switch_implementation() + + where we provide message processing dispatcher as parameter. + """ + log = logging.getLogger("ProtocolDetector.SSLHandshakeDispatcher") + SSL_HANDSHAKE_TIMEOUT = 10 + + def __init__( + self, + sslinfo, + handshake_finished_handler, + handshake_timeout=SSL_HANDSHAKE_TIMEOUT, + ): + self._give_up_at = monotonic_time() + handshake_timeout + self._has_been_set_up = False + self._is_handshaking = True + self.want_read = True + self.want_write = True + self._sslinfo = sslinfo + self._handshake_finished_handler = handshake_finished_handler + + def _set_up_socket(self, dispatcher): + client_socket = dispatcher.socket + client_socket = SSLSocket( + ssl.wrap_socket(client_socket, + keyfile=self._sslinfo.key_file, + certfile=self._sslinfo.cert_file, + server_side=True, + cert_reqs=ssl.CERT_REQUIRED, + ssl_version=self._sslinfo.protocol, + ca_certs=self._sslinfo.ca_certs, + do_handshake_on_connect=False)) + + dispatcher.socket = client_socket + self._has_been_set_up = True + + def next_check_interval(self): + return max(self._give_up_at - monotonic_time(), 0) + + def readable(self, dispatcher): + if self.has_expired(): + dispatcher.close() + return False + + return self.want_read + + def writable(self, dispatcher): + if self.has_expired(): + dispatcher.close() + return False + + return self.want_write + + def has_expired(self): + return monotonic_time() >= self._give_up_at + + def handle_read(self, dispatcher): + self._handle_io(dispatcher) + + def handle_write(self, dispatcher): + self._handle_io(dispatcher) + + def _handle_io(self, dispatcher): + if not self._has_been_set_up: + self._set_up_socket(dispatcher) + + if self._is_handshaking: + self._handshake(dispatcher) + else: + if not self._verify_host(dispatcher.socket.getpeercert(), + dispatcher.socket.getpeername()[0]): + self.log.error("peer certificate does not match host name") + dispatcher.socket.close() + return + + self._handshake_finished_handler(dispatcher) + + def _verify_host(self, peercert, addr): + if not peercert: + return False + + for sub in peercert.get("subject", ()): + for key, value in sub: + if key == "commonName": + return self._compare_names(addr, value) + + return False + + def _compare_names(self, addr, name): + if addr == name or addr == '127.0.0.1': + return True + else: + return name.lower() == socket.gethostbyaddr(addr)[0].lower() + + def _handshake(self, dispatcher): + try: + dispatcher.socket.do_handshake() + except ssl.SSLError as err: + self.want_read = self.want_write = False + if err.args[0] == ssl.SSL_ERROR_WANT_READ: + self.want_read = True + elif err.args[0] == ssl.SSL_ERROR_WANT_WRITE: + self.want_write = True + else: + dispatcher.close() + raise + else: + self.want_read = self.want_write = True + self._is_handshaking = False + + +def create_ssl_context(): + sslinfo = None + if config.getboolean('vars', 'ssl'): + truststore_path = config.get('vars', 'trust_store_path') + protocol = ( + ssl.PROTOCOL_SSLv23 + if config.get('vars', 'ssl_protocol') == 'sslv23' + else ssl.PROTOCOL_TLSv1 + ) + sslinfo = SSLContext( + key_file=os.path.join(truststore_path, 'keys', 'vdsmkey.pem'), + cert_file=os.path.join(truststore_path, 'certs', + 'vdsmcert.pem'), + ca_certs=os.path.join(truststore_path, 'certs', 'cacert.pem'), + protocol=protocol + ) + return sslinfo diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py index d847a23..b7818eb 100644 --- a/lib/vdsm/utils.py +++ b/lib/vdsm/utils.py @@ -54,11 +54,16 @@ import threading import time import vdsm.infra.zombiereaper as zombiereaper -from M2Crypto import SSL
from cpopen import CPopen from . import cmdutils from . import constants +try: + from M2Crypto import SSL + _m2cEnabled = True +except ImportError: + import ssl + _m2cEnabled = False
try: # If failing to import old code, then try importing the legacy code @@ -1251,11 +1256,18 @@
def create_connected_socket(host, port, sslctx=None, timeout=None): + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) if sslctx: - sock = SSL.Connection(sslctx.context) - else: - sock = socket.socket(socket.AF_INET, - socket.SOCK_STREAM) + if _m2cEnabled: + sock = SSL.Connection(sslctx.context) + else: + sock = ssl.wrap_socket(sock, + keyfile=sslctx.key_file, + certfile=sslctx.cert_file, + server_side=False, + cert_reqs=ssl.CERT_REQUIRED, + ssl_version=sslctx.protocol, + ca_certs=sslctx.ca_certs) sock.settimeout(timeout) sock.connect((host, port)) return sock diff --git a/lib/vdsm/vdscli.py b/lib/vdsm/vdscli.py index a78a67a..cf083e3 100644 --- a/lib/vdsm/vdscli.py +++ b/lib/vdsm/vdscli.py @@ -26,7 +26,10 @@ import re import sys from xml.parsers.expat import ExpatError -from . import m2cutils +try: + from . import m2cutils as sslutils +except ImportError: + from . import sslutils
_USE_SSL = False @@ -60,7 +63,7 @@ self.timeout = kwargs['timeout'] del kwargs['timeout'] else: - self.timeout = m2cutils.SOCKET_DEFAULT_TIMEOUT + self.timeout = sslutils.SOCKET_DEFAULT_TIMEOUT
xmlrpclib.Transport.__init__(self, *args, **kwargs)
@@ -107,8 +110,8 @@
def connect(hostPort=None, useSSL=None, tsPath=None, - TransportClass=m2cutils.VerifyingSafeTransport, - timeout=m2cutils.SOCKET_DEFAULT_TIMEOUT): + TransportClass=sslutils.VerifyingSafeTransport, + timeout=sslutils.SOCKET_DEFAULT_TIMEOUT):
hostPort = cannonizeHostPort(hostPort) if useSSL is None: diff --git a/lib/yajsonrpc/stompreactor.py b/lib/yajsonrpc/stompreactor.py index ee710f5..5dfa805 100644 --- a/lib/yajsonrpc/stompreactor.py +++ b/lib/yajsonrpc/stompreactor.py @@ -23,10 +23,14 @@ from vdsm import utils from vdsm.config import config from vdsm.compat import json -from vdsm.m2cutils import SSLSocket from . import JsonRpcClient, JsonRpcServer from . import stomp from .betterAsyncore import Dispatcher, Reactor +try: + from vdsm.m2cutils import SSLSocket +except ImportError: + from ssl import SSLSocket +
_STATE_LEN = "Waiting for message length" _STATE_MSG = "Waiting for message" diff --git a/tests/crossImportsTests.py.in b/tests/crossImportsTests.py.in index a3e1d81..a997211 100644 --- a/tests/crossImportsTests.py.in +++ b/tests/crossImportsTests.py.in @@ -51,4 +51,7 @@ else: mods = get_mods(os.path.join(get_python_lib(), pkg_name))
+ # ignore M2Crypto + mods.remove('m2cutils') + __import__(pkg_name, fromlist=mods) diff --git a/tests/integration/Makefile.am b/tests/integration/Makefile.am index 525ddd8..7fd327d 100644 --- a/tests/integration/Makefile.am +++ b/tests/integration/Makefile.am @@ -25,4 +25,5 @@ jsonRpcHelper.py \ jsonRpcTests.py \ m2chelper.py \ + sslhelper.py \ $(NULL) diff --git a/tests/integration/jsonRpcHelper.py b/tests/integration/jsonRpcHelper.py index 65ec5e3..fbb74c1 100644 --- a/tests/integration/jsonRpcHelper.py +++ b/tests/integration/jsonRpcHelper.py @@ -37,7 +37,10 @@ from protocoldetector import MultiProtocolAcceptor from rpc.bindingjsonrpc import BindingJsonRpc from vdsm import utils -from m2chelper import DEAFAULT_SSL_CONTEXT +try: + from integration.m2chelper import DEAFAULT_SSL_CONTEXT +except ImportError: + from integration.sslhelper import DEAFAULT_SSL_CONTEXT
PERMUTATIONS = tuple(product((True, False), ("xml", "stomp")))
diff --git a/tests/integration/m2chelper.py b/tests/integration/m2chelper.py index 77a2767..7ece453 100644 --- a/tests/integration/m2chelper.py +++ b/tests/integration/m2chelper.py @@ -17,9 +17,11 @@ # # Refer to the README and COPYING files for full details of the license # - import os -from vdsm.m2cutils import SSLContext +import SimpleXMLRPCServer +import threading +from M2Crypto import SSL +from vdsm.m2cutils import SSLContext, SSLServerSocket
CERT_DIR = os.path.join(os.path.abspath(os.path.dirname(__file__)), '..') CRT_FILE = os.path.join(CERT_DIR, "server.crt") @@ -29,3 +31,43 @@
DEAFAULT_SSL_CONTEXT = SSLContext( CRT_FILE, KEY_FILE, session_id="server-tests") + + +def get_server_socket(key_file, cert_file, socket): + return SSLServerSocket(raw=socket, + keyfile=key_file, + certfile=cert_file, + ca_certs=cert_file) + + +class TestServer(): + + def __init__(self, host, service): + self.server = SimpleXMLRPCServer.SimpleXMLRPCServer((host, 0), + logRequests=False) + self.server.socket = SSLServerSocket(raw=self.server.socket, + keyfile=KEY_FILE, + certfile=CRT_FILE, + ca_certs=CRT_FILE) + _, self.port = self.server.socket.getsockname() + self.server.register_instance(service) + + def start(self): + self.thread = threading.Thread(target=self.serve_forever) + self.thread.daemon = True + self.thread.start() + + def serve_forever(self): + try: + self.server.serve_forever() + except SSL.SSLError: + # expected sslerror is thrown in server side during test_invalid + # method we do not want to pollute test console output + pass + + def stop(self): + self.server.shutdown() + + def get_timeout(self): + self.server.socket.accept_timeout = 1 + return self.server.socket.accept_timeout + 1 diff --git a/tests/integration/sslhelper.py b/tests/integration/sslhelper.py new file mode 100644 index 0000000..acc2516 --- /dev/null +++ b/tests/integration/sslhelper.py @@ -0,0 +1,82 @@ +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +# +# Refer to the README and COPYING files for full details of the license +# +import os +import SimpleXMLRPCServer +import ssl +import threading +from vdsm.sslutils import SSLContext + +CERT_DIR = os.path.join(os.path.abspath(os.path.dirname(__file__)), '..') +CRT_FILE = os.path.join(CERT_DIR, "server.crt") +KEY_FILE = os.path.join(CERT_DIR, "server.key") +OTHER_CRT_FILE = os.path.join(CERT_DIR, "other.crt") +OTHER_KEY_FILE = os.path.join(CERT_DIR, "other.key") + +DEAFAULT_SSL_CONTEXT = SSLContext(cert_file=CRT_FILE, key_file=KEY_FILE, + ca_certs=CRT_FILE) + + +def get_server_socket(key_file, cert_file, socket): + return ssl.wrap_socket(socket, + keyfile=key_file, + certfile=cert_file, + server_side=False, + cert_reqs=ssl.CERT_REQUIRED, + ssl_version=ssl.PROTOCOL_TLSv1, + ca_certs=cert_file) + + +class TestServer(SimpleXMLRPCServer.SimpleXMLRPCServer): + + def __init__(self, host, service): + SimpleXMLRPCServer.SimpleXMLRPCServer.__init__(self, (host, 0), + logRequests=False, + bind_and_activate=False) + + self.socket = ssl.wrap_socket(self.socket, + keyfile=KEY_FILE, + certfile=CRT_FILE, + server_side=True, + cert_reqs=ssl.CERT_REQUIRED, + ssl_version=ssl.PROTOCOL_TLSv1, + ca_certs=CRT_FILE, + do_handshake_on_connect=False) + + self.server_bind() + self.server_activate() + + _, self.port = self.socket.getsockname() + self.register_instance(service) + + def finish_request(self, request, client_address): + if self.timeout is not None: + request.settimeout(self.timeout) + + request.do_handshake() + + return SimpleXMLRPCServer.SimpleXMLRPCServer.finish_request( + self, + request, + client_address) + + def handle_error(self, request, client_address): + # ignored due to expected sslerrors when perorming plain connection + pass + + def start(self): + self.thread = threading.Thread(target=self.serve_forever) + self.thread.daemon = True + self.thread.start() + + def stop(self): + self.shutdown() + + def get_timeout(self): + self.timeout = 1 + return self.timeout + 1 diff --git a/tests/protocoldetectorTests.py b/tests/protocoldetectorTests.py index f90eb43..927b44a 100644 --- a/tests/protocoldetectorTests.py +++ b/tests/protocoldetectorTests.py @@ -26,10 +26,12 @@ from contextlib import contextmanager
from yajsonrpc.betterAsyncore import Reactor -from vdsm import m2cutils from protocoldetector import MultiProtocolAcceptor -from integration.m2chelper import KEY_FILE, CRT_FILE from testlib import VdsmTestCase, expandPermutations, permutations +try: + from integration.m2chelper import KEY_FILE, CRT_FILE, DEAFAULT_SSL_CONTEXT +except: + from integration.sslhelper import KEY_FILE, CRT_FILE, DEAFAULT_SSL_CONTEXT
class Detector(object): @@ -97,7 +99,7 @@ GRACETIME = 0.5 CONCURRENCY = 5 PERMUTATIONS = ((False,), (True,)) - SSLCTX = m2cutils.SSLContext(CRT_FILE, KEY_FILE, ca_cert=CRT_FILE) + SSLCTX = DEAFAULT_SSL_CONTEXT BUFSIZE = 512
def setUp(self): diff --git a/tests/sslTests.py b/tests/sslTests.py index b40f263..b7a4bfb 100644 --- a/tests/sslTests.py +++ b/tests/sslTests.py @@ -22,7 +22,6 @@ import errno import os import re -import SimpleXMLRPCServer import socket import ssl import subprocess @@ -31,12 +30,21 @@ import xmlrpclib
from contextlib import contextmanager, closing -from M2Crypto import SSL -from integration.m2chelper import KEY_FILE, \ - CRT_FILE, OTHER_KEY_FILE, OTHER_CRT_FILE from testlib import VdsmTestCase as TestCaseBase -from vdsm.m2cutils import SSLServerSocket -from vdsm.m2cutils import VerifyingSafeTransport +from nose.plugins.skip import SkipTest +try: + from vdsm.m2cutils import VerifyingSafeTransport + from integration.m2chelper import TestServer, \ + get_server_socket, KEY_FILE, \ + CRT_FILE, OTHER_KEY_FILE, OTHER_CRT_FILE + _m2cEnabled = True +except ImportError: + from vdsm.sslutils import VerifyingSafeTransport + from integration.sslhelper import TestServer, \ + get_server_socket, KEY_FILE, \ + CRT_FILE, OTHER_KEY_FILE, OTHER_CRT_FILE + _m2cEnabled = False +
HOST = '127.0.0.1'
@@ -45,35 +53,6 @@
def add(self, x, y): return x + y - - -class TestServer(): - - def __init__(self): - self.server = SimpleXMLRPCServer.SimpleXMLRPCServer((HOST, 0), - logRequests=False) - self.server.socket = SSLServerSocket(raw=self.server.socket, - keyfile=KEY_FILE, - certfile=CRT_FILE, - ca_certs=CRT_FILE) - _, self.port = self.server.socket.getsockname() - self.server.register_instance(MathService()) - - def start(self): - self.thread = threading.Thread(target=self.serve_forever) - self.thread.daemon = True - self.thread.start() - - def serve_forever(self): - try: - self.server.serve_forever() - except SSL.SSLError: - # expected sslerror is thrown in server side during test_invalid - # method we do not want to pollute test console output - pass - - def stop(self): - self.server.shutdown()
class VerifyingClient(): @@ -97,7 +76,7 @@
@contextmanager def verifyingclient(key_file, crt_file): - server = TestServer() + server = TestServer(HOST, MathService()) server.start() try: client = VerifyingClient(server.port, key_file, crt_file) @@ -112,9 +91,8 @@ class SocketTests(TestCaseBase):
def test_block_socket(self): - server = TestServer() - server.server.socket.accept_timeout = 1 - timeout = server.server.socket.accept_timeout + 1 + server = TestServer(HOST, MathService()) + timeout = server.get_timeout() server.start() try: with closing(socket.socket(socket.AF_INET, @@ -222,11 +200,8 @@
# Create the server socket: self.server = socket.socket() - self.server = SSLServerSocket( - raw=self.server, - keyfile=self.keyfile, - certfile=self.certfile, - ca_certs=self.certfile) + self.server = get_server_socket(self.keyfile, self.certfile, + self.server) self.address = self.tryBind(ADDRESS) self.server.listen(5)
@@ -357,6 +332,11 @@ Verify that SSL the session identifier is preserved when connecting two times without stopping the server. """ + if not _m2cEnabled: + raise SkipTest( + 'support for SSL sessions discontinued and may be reintroduced' + ' when we move to python 3' + )
# Create a temporary file to store the session details: sessionDetailsFile = tempfile.NamedTemporaryFile(delete=False) diff --git a/tests/stompTests.py b/tests/stompTests.py index 3b38c95..3eabbb7 100644 --- a/tests/stompTests.py +++ b/tests/stompTests.py @@ -26,9 +26,12 @@ dummyTextGenerator
from integration.jsonRpcHelper import constructAcceptor -from integration.m2chelper import DEAFAULT_SSL_CONTEXT from yajsonrpc.stompreactor import StandAloneRpcClient -from vdsm.utils import running +from vdsm import utils +try: + from integration.m2chelper import DEAFAULT_SSL_CONTEXT +except ImportError: + from integration.sslhelper import DEAFAULT_SSL_CONTEXT
CALL_TIMEOUT = 15 @@ -61,9 +64,11 @@ with constructAcceptor(self.log, use_ssl, _SampleBridge()) as acceptor: sslctx = DEAFAULT_SSL_CONTEXT if use_ssl else None
- with running(StandAloneRpcClient(acceptor._host, acceptor._port, - 'jms.topic.vdsm_requests', - str(uuid4()), sslctx)) as client: + with utils.running(StandAloneRpcClient(acceptor._host, + acceptor._port, + 'jms.topic.vdsm_requests', + str(uuid4()), + sslctx)) as client: self.assertEquals(client.callMethod('echo', (data,), str(uuid4())), data) @@ -75,7 +80,6 @@ with constructAcceptor(self.log, use_ssl, _SampleBridge(), 'jms.queue.events') as acceptor: sslctx = DEAFAULT_SSL_CONTEXT if use_ssl else None - client = StandAloneRpcClient(acceptor._host, acceptor._port, 'jms.topic.vdsm_requests', 'jms.queue.events', sslctx, False) diff --git a/tests/vdscliTests.py b/tests/vdscliTests.py index ba2cf36..bfcbddf 100644 --- a/tests/vdscliTests.py +++ b/tests/vdscliTests.py @@ -29,9 +29,14 @@
from testlib import VdsmTestCase as TestCaseBase from testValidation import ValidateRunningAsRoot - -from vdsm import m2cutils from vdsm import vdscli +try: + from vdsm import m2cutils as sslutils + from integration.m2chelper import get_server_socket +except ImportError: + from vdsm import sslutils + from integration.sslhelper import get_server_socket +
HOST = '127.0.0.1'
@@ -51,11 +56,8 @@ if useSSL: KEY_FILE = os.path.join(path, 'keys/vdsmkey.pem') CRT_FILE = os.path.join(path, 'certs/vdsmcert.pem') - self.server.socket = m2cutils.SSLServerSocket( - raw=self.server.socket, - keyfile=KEY_FILE, - certfile=CRT_FILE, - ca_certs=CRT_FILE) + self.server.socket = get_server_socket(KEY_FILE, CRT_FILE, + self.server.socket)
_, self.port = self.server.socket.getsockname() self.server.register_instance(TestingService()) @@ -74,7 +76,7 @@
@contextmanager def setupclient(useSSL, tsPath, - timeout=m2cutils.SOCKET_DEFAULT_TIMEOUT): + timeout=sslutils.SOCKET_DEFAULT_TIMEOUT): server = TestServer(useSSL, tsPath) server.start() hostPort = '0:' + str(server.port) diff --git a/vdsm.spec.in b/vdsm.spec.in index ddb3184..7bcea09 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -42,6 +42,9 @@ %global _polkitdir %{_localstatedir}/lib/polkit-1/localauthority/10-vendor.d %endif
+# enable m2crypto by default +%global with_m2c 1 + # Gluster should not be shipped with RHEV %if ! 0%{?rhev_build} %global with_gluster 1 @@ -83,7 +86,9 @@ BuildRequires: libnl3 BuildRequires: libselinux-python BuildRequires: libvirt-python +%if %{with_m2c} BuildRequires: m2crypto +%endif BuildRequires: mom >= 0.4.5 BuildRequires: openssl BuildRequires: policycoreutils-python @@ -129,7 +134,9 @@ Requires: %{name}-infra = %{version}-%{release} Requires: rpm-python Requires: nfs-utils +%if %{with_m2c} Requires: m2crypto +%endif Requires: libnl3 Requires: curl Requires: %{name}-xmlrpc = %{version}-%{release} @@ -295,7 +302,9 @@ %package reg Summary: VDSM registration package Requires: %{name} = %{version}-%{release} +%if %{with_m2c} Requires: m2crypto +%endif Requires: openssl Conflicts: ovirt-node < 3.0.4
@@ -307,7 +316,9 @@ Summary: VDSM python libraries Requires: %{name}-infra = %{version}-%{release} Requires: python-cpopen >= 1.2.3-5 +%if %{with_m2c} Requires: m2crypto +%endif Requires: python-ioprocess >= 0.14
%description python @@ -1049,7 +1060,6 @@ %{python_sitelib}/%{vdsm_name}/ipwrapper.py* %{python_sitelib}/%{vdsm_name}/jsonrpcvdscli.py* %{python_sitelib}/%{vdsm_name}/libvirtconnection.py* -%{python_sitelib}/%{vdsm_name}/m2cutils.py* %{python_sitelib}/%{vdsm_name}/netinfo.py* %{python_sitelib}/%{vdsm_name}/netlink/__init__.py* %{python_sitelib}/%{vdsm_name}/netlink/addr.py* @@ -1067,6 +1077,13 @@ %{python_sitelib}/%{vdsm_name}/response.py* %{python_sitelib}/%{vdsm_name}/netconfpersistence.py* %{python_sitelib}/%{vdsm_name}/schedule.py* +%if %{with_m2c} +%{python_sitelib}/%{vdsm_name}/m2cutils.py* +%exclude %{python_sitelib}/%{vdsm_name}/sslutils.py* +%else +%exclude %{python_sitelib}/%{vdsm_name}/m2cutils.py* +%{python_sitelib}/%{vdsm_name}/sslutils.py* +%endif %{python_sitelib}/%{vdsm_name}/sysctl.py* %{python_sitelib}/%{vdsm_name}/udevadm.py* %{python_sitelib}/%{vdsm_name}/utils.py* diff --git a/vdsm/clientIF.py b/vdsm/clientIF.py index 5590db0..578b349 100644 --- a/vdsm/clientIF.py +++ b/vdsm/clientIF.py @@ -37,7 +37,6 @@ from vdsm.compat import pickle from vdsm.define import doneCode, errCode import libvirt -from vdsm import m2cutils from vdsm import libvirtconnection from vdsm import constants from vdsm import utils @@ -55,6 +54,10 @@ from virt.vmchannels import Listener from virt.vmdevices import hwclass from virt.utils import isVdsmImage +try: + from vdsm import m2cutils as sslutils +except ImportError: + from vdsm import sslutils try: import gluster.api as gapi _glusterEnabled = True @@ -193,7 +196,7 @@ return cls._instance
def _createAcceptor(self, host, port): - sslctx = m2cutils.create_ssl_context() + sslctx = sslutils.create_ssl_context() self._reactor = Reactor()
self._acceptor = MultiProtocolAcceptor(self._reactor, host, @@ -205,7 +208,7 @@ broker_port = config.getint('addresses', 'broker_port') request_queues = config.get('addresses', 'request_queues')
- sslctx = m2cutils.create_ssl_context() + sslctx = sslutils.create_ssl_context() sock = socket.socket() sock.connect((broker_address, broker_port)) if sslctx: diff --git a/vdsm/kaxmlrpclib.py b/vdsm/kaxmlrpclib.py index b38031e..b0640e8 100644 --- a/vdsm/kaxmlrpclib.py +++ b/vdsm/kaxmlrpclib.py @@ -110,7 +110,10 @@
################### # the same, for ssl -from vdsm import m2cutils +try: + from vdsm import m2cutils as sslutils +except ImportError: + from vdsm import sslutils import ssl
@@ -122,7 +125,7 @@ SslServerProxy = SslServer
-class TcpkeepSafeTransport(m2cutils.VerifyingSafeTransport): +class TcpkeepSafeTransport(sslutils.VerifyingSafeTransport):
def make_connection(self, host): chost, self._extra_headers, x509 = self.get_host_info(host) @@ -139,17 +142,17 @@ cert_reqs=self.cert_reqs)
-class TcpkeepHTTPSConnection(m2cutils.VerifyingHTTPSConnection): +class TcpkeepHTTPSConnection(sslutils.VerifyingHTTPSConnection): def __init__(self, host, port=None, key_file=None, cert_file=None, strict=None, timeout=CONNECTTIMEOUT, ca_certs=None, cert_reqs=ssl.CERT_REQUIRED): - m2cutils.VerifyingHTTPSConnection.__init__( + sslutils.VerifyingHTTPSConnection.__init__( self, host, port=port, key_file=key_file, cert_file=cert_file, strict=strict, timeout=timeout, ca_certs=ca_certs, cert_reqs=cert_reqs)
def connect(self): - m2cutils.VerifyingHTTPSConnection.connect(self) + sslutils.VerifyingHTTPSConnection.connect(self)
# after TCP_KEEPIDLE seconds of silence, TCP_KEEPCNT probes would be # sent, TCP_KEEPINTVL seconds apart of each other. If all of them fail, @@ -160,5 +163,5 @@ self.sock.setsockopt(socket.SOL_TCP, socket.TCP_KEEPCNT, KEEPCNT)
-class TcpkeepHTTPS(m2cutils.VerifyingHTTPS): +class TcpkeepHTTPS(sslutils.VerifyingHTTPS): _connection_class = TcpkeepHTTPSConnection diff --git a/vdsm/protocoldetector.py b/vdsm/protocoldetector.py index 6d01a1c..dbdc730 100644 --- a/vdsm/protocoldetector.py +++ b/vdsm/protocoldetector.py @@ -24,7 +24,10 @@ import vdsm.infra.filecontrol as filecontrol
from vdsm.utils import monotonic_time -from vdsm.m2cutils import SSLHandshakeDispatcher +try: + from vdsm.m2cutils import SSLHandshakeDispatcher +except ImportError: + from vdsm.sslutils import SSLHandshakeDispatcher
def _create_socket(host, port): diff --git a/vdsm/virt/migration.py b/vdsm/virt/migration.py index 32caccb..6c41114 100644 --- a/vdsm/virt/migration.py +++ b/vdsm/virt/migration.py @@ -28,7 +28,6 @@ from vdsm import utils from vdsm import vdscli from vdsm import jsonrpcvdscli -from vdsm import m2cutils from vdsm.compat import pickle from vdsm.config import config from vdsm.define import NORMAL, Mbytes @@ -40,6 +39,11 @@ from . import vmexitreason from . import vmstatus
+try: + from vdsm import m2cutils as sslutils +except ImportError: + from vdsm import sslutils +
MODE_REMOTE = 'remote' MODE_FILE = 'file'
automation@ovirt.org has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 1:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 1:
Both patches verified together. Build vdsm with m2c and ssl and for both performed host install. I can see that host deploy installs m2crypto when it is not needed (ssl module used).
I noticed that with the latest engine host install always fallback to xmlrpc but when editing manually it works with jsonrpc.
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 1: Verified+1
Nir Soffer has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 1:
It would be more useful as runtime option, using the configuration - like oop (ioprocess|rfh).
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 1:
I thought about having it as runtime option but we would still need to require m2crypto as dependency. I am OK changing it. Dan do you have preference?
Nir Soffer has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 1:
Piotr, sure we still need to depend on m2c, but this will allow using builtin ssl when it works better, and in the next version we would have better and tested ssl and hopefully we can drop m2c.
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 1:
OK. Let's have runtime option.
automation@ovirt.org has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 2:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
automation@ovirt.org has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 3:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 3: Verified+1
Verified all 3 patches together by building with_m2c both with 0 and 1. Installed both build and seeing that communication is OK. For option 1 I tested both implementations by setting both implementation in config py and restarting vdsm.
CI failures are not related to these patches.
Nir Soffer has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 3:
(1 comment)
https://gerrit.ovirt.org/#/c/44494/3/lib/vdsm/jsonrpcvdscli.py File lib/vdsm/jsonrpcvdscli.py:
Line 31: from .config import config Line 32: try: Line 33: from . import m2cutils as sslutils Line 34: except ImportError: Line 35: from . import sslutils Adding this import everywhere is not a good idea. This should be moved to the compat module, so we can use:
from vdsm.compat import sslutils
And we get some implementation depending on the build and runtime settings. Line 36: Line 37: Line 38: _COMMAND_CONVERTER = { Line 39: 'ping': 'Host.ping',
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 3:
(1 comment)
https://gerrit.ovirt.org/#/c/44494/3/lib/vdsm/jsonrpcvdscli.py File lib/vdsm/jsonrpcvdscli.py:
Line 31: from .config import config Line 32: try: Line 33: from . import m2cutils as sslutils Line 34: except ImportError: Line 35: from . import sslutils
Adding this import everywhere is not a good idea. This should be moved to t
Done Line 36: Line 37: Line 38: _COMMAND_CONVERTER = { Line 39: 'ping': 'Host.ping',
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 3: Code-Review-1
When I try to create a vm using ssl standard module I loose connectivity. This can be related to the perf issue. For visibility giving -1.
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 3: -Code-Review Verified-1
automation@ovirt.org has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 4:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 4: Verified+1
All three patches verified together. Vdsm was built with both options set to 1 and tested with both configurations m2c and ssl. For both cases handshake was successful.
In this patch set I introduced sslcompat module. Nir asked to use compat module for sslutils but it depends on utils which creates a dependency cycle.
automation@ovirt.org has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 5:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 5: Verified+1
Maunual rebase, no code changes. Conflicting lib/vdsm/Makefile, debian/vdsm-python.install and vdsm.spec.in.
Rebuilt to check packaging for both with_ssl set to 0 and 1
Dan Kenigsberg has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 5:
(5 comments)
https://gerrit.ovirt.org/#/c/44494/5/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 58: from cpopen import CPopen Line 59: from . import cmdutils Line 60: from . import constants Line 61: try: Line 62: from M2Crypto import SSL this logic can be hidden in sslcompat Line 63: _m2cEnabled = True Line 64: except ImportError: Line 65: import ssl Line 66: _m2cEnabled = False
Line 1255: return count * size Line 1256: Line 1257: Line 1258: def create_connected_socket(host, port, sslctx=None, timeout=None): Line 1259: sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) sock is not always used. can you assign it only when it is? Line 1260: if sslctx: Line 1261: if _m2cEnabled: Line 1262: sock = SSL.Connection(sslctx.context) Line 1263: else:
https://gerrit.ovirt.org/#/c/44494/5/lib/yajsonrpc/stompreactor.py File lib/yajsonrpc/stompreactor.py:
Line 27: from . import JsonRpcClient, JsonRpcServer Line 28: from . import stomp Line 29: from .betterAsyncore import Dispatcher, Reactor Line 30: Line 31: unrelated newline Line 32: _STATE_LEN = "Waiting for message length" Line 33: _STATE_MSG = "Waiting for message" Line 34: Line 35:
https://gerrit.ovirt.org/#/c/44494/5/tests/crossImportsTests.py.in File tests/crossImportsTests.py.in:
Line 50: mods = get_mods(src_dir) Line 51: else: Line 52: mods = get_mods(os.path.join(get_python_lib(), pkg_name)) Line 53: Line 54: # ignore M2Crypto # skip the test for m2cutils as we cannot even import it on systems with no M2Crypto. Line 55: mods.remove('m2cutils') Line 56:
https://gerrit.ovirt.org/#/c/44494/5/tests/integration/m2chelper.py File tests/integration/m2chelper.py:
Line 16: # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA Line 17: # Line 18: # Refer to the README and COPYING files for full details of the license Line 19: # Line 20: import os unrelated newline removal Line 21: import SimpleXMLRPCServer Line 22: import threading Line 23: from M2Crypto import SSL Line 24: from vdsm.m2cutils import SSLContext, SSLServerSocket
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 5:
(5 comments)
https://gerrit.ovirt.org/#/c/44494/5/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 58: from cpopen import CPopen Line 59: from . import cmdutils Line 60: from . import constants Line 61: try: Line 62: from M2Crypto import SSL
this logic can be hidden in sslcompat
Done Line 63: _m2cEnabled = True Line 64: except ImportError: Line 65: import ssl Line 66: _m2cEnabled = False
Line 1255: return count * size Line 1256: Line 1257: Line 1258: def create_connected_socket(host, port, sslctx=None, timeout=None): Line 1259: sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
sock is not always used. can you assign it only when it is?
Done Line 1260: if sslctx: Line 1261: if _m2cEnabled: Line 1262: sock = SSL.Connection(sslctx.context) Line 1263: else:
https://gerrit.ovirt.org/#/c/44494/5/lib/yajsonrpc/stompreactor.py File lib/yajsonrpc/stompreactor.py:
Line 27: from . import JsonRpcClient, JsonRpcServer Line 28: from . import stomp Line 29: from .betterAsyncore import Dispatcher, Reactor Line 30: Line 31:
unrelated newline
Done Line 32: _STATE_LEN = "Waiting for message length" Line 33: _STATE_MSG = "Waiting for message" Line 34: Line 35:
https://gerrit.ovirt.org/#/c/44494/5/tests/crossImportsTests.py.in File tests/crossImportsTests.py.in:
Line 50: mods = get_mods(src_dir) Line 51: else: Line 52: mods = get_mods(os.path.join(get_python_lib(), pkg_name)) Line 53: Line 54: # ignore M2Crypto
# skip the test for m2cutils as we cannot even import it on systems with no
Done Line 55: mods.remove('m2cutils') Line 56:
https://gerrit.ovirt.org/#/c/44494/5/tests/integration/m2chelper.py File tests/integration/m2chelper.py:
Line 16: # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA Line 17: # Line 18: # Refer to the README and COPYING files for full details of the license Line 19: # Line 20: import os
unrelated newline removal
Done Line 21: import SimpleXMLRPCServer Line 22: import threading Line 23: from M2Crypto import SSL Line 24: from vdsm.m2cutils import SSLContext, SSLServerSocket
Yeela Kaplan has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 5: Code-Review-1
(1 comment)
https://gerrit.ovirt.org/#/c/44494/5/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 1260: if sslctx: Line 1261: if _m2cEnabled: Line 1262: sock = SSL.Connection(sslctx.context) Line 1263: else: Line 1264: sock = ssl.wrap_socket(sock, why not use sslctx.wrapSocket(sock) ?
It already has all parameters, so its doing this call for you and you don't have to work hard here... Line 1265: keyfile=sslctx.key_file, Line 1266: certfile=sslctx.cert_file, Line 1267: server_side=False, Line 1268: cert_reqs=ssl.CERT_REQUIRED,
automation@ovirt.org has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 6:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 5:
(1 comment)
https://gerrit.ovirt.org/#/c/44494/5/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 1260: if sslctx: Line 1261: if _m2cEnabled: Line 1262: sock = SSL.Connection(sslctx.context) Line 1263: else: Line 1264: sock = ssl.wrap_socket(sock,
why not use sslctx.wrapSocket(sock) ?
Done Line 1265: keyfile=sslctx.key_file, Line 1266: certfile=sslctx.cert_file, Line 1267: server_side=False, Line 1268: cert_reqs=ssl.CERT_REQUIRED,
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 5:
(1 comment)
https://gerrit.ovirt.org/#/c/44494/5/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 1255: return count * size Line 1256: Line 1257: Line 1258: def create_connected_socket(host, port, sslctx=None, timeout=None): Line 1259: sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
Done
Looking one more time on it I see that it is used for plain connection as well so I think it should stay here. Line 1260: if sslctx: Line 1261: if _m2cEnabled: Line 1262: sock = SSL.Connection(sslctx.context) Line 1263: else:
automation@ovirt.org has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 7:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 7: Verified+1
Fixed comments and rebased. Verified by building with and without m2c and tested communication with m2c by host deploying vdsm and seeing that the connection was OK.
Dan Kenigsberg has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 7: Code-Review+2
automation@ovirt.org has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 8:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Francesco Romani has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 8:
(3 comments)
initial review, need more time to difest sslutils.py and friends.
First scan looks good however.
https://gerrit.ovirt.org/#/c/44494/8/lib/vdsm/sslcompat.py File lib/vdsm/sslcompat.py:
Line 16: # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA Line 17: # Line 18: # Refer to the README and COPYING files for full details of the license Line 19: # Line 20: try: I'm not sure this is what Nir meant in a past comment, IIUC he suggested to use the existing compat.py.
Not important enough to stop this patch, however. Line 21: from . import m2cutils as sslutils Line 22: from .m2cutils import SSLSocket Line 23: from .m2cutils import SSLHandshakeDispatcher Line 24: except ImportError:
Line 25: from . import sslutils Line 26: from .sslutils import SSLHandshakeDispatcher Line 27: from ssl import SSLSocket Line 28: Line 29: # we need it to satisfy pyflakes does this block need to be indented? Just asking, not sure about what python wants here. This can raise some warning/error in the ssl path. Line 30: sslutils Line 31: SSLHandshakeDispatcher
https://gerrit.ovirt.org/#/c/44494/8/lib/vdsm/sslutils.py File lib/vdsm/sslutils.py:
Line 25: import ssl Line 26: import xmlrpclib Line 27: Line 28: from vdsm.utils import ( Line 29: monotonic_time, no need for parens, but not that important. Line 30: ) Line 31: from .config import config Line 32: Line 33:
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 8:
(3 comments)
https://gerrit.ovirt.org/#/c/44494/8/lib/vdsm/sslcompat.py File lib/vdsm/sslcompat.py:
Line 16: # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA Line 17: # Line 18: # Refer to the README and COPYING files for full details of the license Line 19: # Line 20: try:
I'm not sure this is what Nir meant in a past comment, IIUC he suggested to
We can't use existing compat module because if we do we would create module dependency cycle. Line 21: from . import m2cutils as sslutils Line 22: from .m2cutils import SSLSocket Line 23: from .m2cutils import SSLHandshakeDispatcher Line 24: except ImportError:
Line 25: from . import sslutils Line 26: from .sslutils import SSLHandshakeDispatcher Line 27: from ssl import SSLSocket Line 28: Line 29: # we need it to satisfy pyflakes
does this block need to be indented? Just asking, not sure about what pytho
pyflakes complains that modules are imported but not used. Line 30: sslutils Line 31: SSLHandshakeDispatcher
https://gerrit.ovirt.org/#/c/44494/8/lib/vdsm/sslutils.py File lib/vdsm/sslutils.py:
Line 25: import ssl Line 26: import xmlrpclib Line 27: Line 28: from vdsm.utils import ( Line 29: monotonic_time,
no need for parens, but not that important.
Copied from m2cutils. Will change in different patch. Line 30: ) Line 31: from .config import config Line 32: Line 33:
automation@ovirt.org has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 9:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 9: Verified+1
Verified by building only with ssl and testing runtime settings with m2c and ssl. As part of the test vdsm was built with m2c and verified that communication works.
Patch rebased, and fixed import in sslutils.
Francesco Romani has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 9:
(1 comment)
seems OK, but I need more time to process sslutils.py.
https://gerrit.ovirt.org/#/c/44494/9/lib/vdsm/sslutils.py File lib/vdsm/sslutils.py:
Line 111: sock = socket.create_connection((self.host, self.port), self.timeout) Line 112: if self._tunnel_host: Line 113: self.sock = sock Line 114: self._tunnel() Line 115: # DK added: pass ca_cert to sslsocket What does "DK" mean? (link to past revisions is fine I most likely missed something trivial) Line 116: self.sock = ssl.wrap_socket(sock, self.key_file, self.cert_file, Line 117: ca_certs=self.ca_certs, server_side=False, Line 118: cert_reqs=self.cert_reqs) Line 119:
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 9:
(1 comment)
https://gerrit.ovirt.org/#/c/44494/9/lib/vdsm/sslutils.py File lib/vdsm/sslutils.py:
Line 111: sock = socket.create_connection((self.host, self.port), self.timeout) Line 112: if self._tunnel_host: Line 113: self.sock = sock Line 114: self._tunnel() Line 115: # DK added: pass ca_cert to sslsocket
What does "DK" mean? (link to past revisions is fine I most likely missed s
This comment was in m2cutils (old sslutils). The logic is the same so the comment is the same.
Dan I think it was your comment. Anything you want me to fix here? Line 116: self.sock = ssl.wrap_socket(sock, self.key_file, self.cert_file, Line 117: ca_certs=self.ca_certs, server_side=False, Line 118: cert_reqs=self.cert_reqs) Line 119:
automation@ovirt.org has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 10:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Yaniv Bronhaim has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 10: Code-Review-1
(8 comments)
https://gerrit.ovirt.org/#/c/44494/10//COMMIT_MSG Commit Message:
Line 5: CommitDate: 2015-09-15 14:47:04 +0200 Line 6: Line 7: ssl: configurable implementation Line 8: Line 9: We want to choose ssl implementation during build process. both lines can merge to - adding ssl implementation config value. and the commit message should explain the two options and why we need it (which you explained in the following patch) Line 10: Line 11: Change-Id: I6501981bbd5276c49731b0d9eba4794286b0f823
https://gerrit.ovirt.org/#/c/44494/10/debian/vdsm-python.install File debian/vdsm-python.install:
Line 16: ./ you still have this m2cutils.py if im not wrong :/
https://gerrit.ovirt.org/#/c/44494/10/lib/vdsm/Makefile.am File lib/vdsm/Makefile.am:
Line 32: executor.py \ Line 33: ipwrapper.py \ Line 34: jsonrpcvdscli.py \ Line 35: libvirtconnection.py \ Line 36: m2cutils.py \ you left it here.. err how did I miss the m2c shortcut . with m2cryptoUtils\m2cryptoWrapper you can at least guess what it is Line 37: netconfpersistence.py \ Line 38: netinfo.py \ Line 39: password.py \ Line 40: ppc64HardwareInfo.py \
https://gerrit.ovirt.org/#/c/44494/10/lib/vdsm/sslutils.py File lib/vdsm/sslutils.py:
Line 1: # Line 2: # Copyright 2014 Red Hat, Inc. 2015
did you change something in this file? Line 3: # Line 4: # This program is free software; you can redistribute it and/or modify Line 5: # it under the terms of the GNU General Public License as published by Line 6: # the Free Software Foundation; either version 2 of the License, or
https://gerrit.ovirt.org/#/c/44494/10/tests/integration/m2chelper.py File tests/integration/m2chelper.py:
Line 43: Line 44: class TestServer(): Line 45: Line 46: def __init__(self, host, service): Line 47: self.server = SimpleXMLRPCServer.SimpleXMLRPCServer((host, 0), why not jsonrpc? Line 48: logRequests=False) Line 49: self.server.socket = SSLServerSocket(raw=self.server.socket, Line 50: keyfile=KEY_FILE, Line 51: certfile=CRT_FILE,
https://gerrit.ovirt.org/#/c/44494/10/tests/sslTests.py File tests/sslTests.py:
Line 42: from vdsm.sslutils import VerifyingSafeTransport Line 43: from integration.sslhelper import TestServer, \ Line 44: get_server_socket, KEY_FILE, \ Line 45: CRT_FILE, OTHER_KEY_FILE, OTHER_CRT_FILE Line 46: _m2cEnabled = False don't you want to test both? Line 47: Line 48: Line 49: HOST = '127.0.0.1' Line 50:
https://gerrit.ovirt.org/#/c/44494/10/tests/vdscliTests.py File tests/vdscliTests.py:
Line 34: from vdsm import m2cutils as sslutils Line 35: from integration.m2chelper import get_server_socket Line 36: except ImportError: Line 37: from vdsm import sslutils Line 38: from integration.sslhelper import get_server_socket don't you want to test both? Line 39: Line 40: Line 41: HOST = '127.0.0.1' Line 42:
https://gerrit.ovirt.org/#/c/44494/10/vdsm.spec.in File vdsm.spec.in:
Line 1072: %{python_sitelib}/%{vdsm_name}/sslcompat.py* Line 1073: %if %{with_m2c} Line 1074: %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1075: %exclude %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1076: %else why do you need to exclude it? Line 1077: %exclude %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1078: %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1079: %endif Line 1080: %{python_sitelib}/%{vdsm_name}/supervdsm.py*
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 10:
(8 comments)
https://gerrit.ovirt.org/#/c/44494/10//COMMIT_MSG Commit Message:
Line 5: CommitDate: 2015-09-15 14:47:04 +0200 Line 6: Line 7: ssl: configurable implementation Line 8: Line 9: We want to choose ssl implementation during build process.
both lines can merge to - adding ssl implementation config value. and the c
This patch provide ability to choose ssl implementation during build process. I am not sure what exactly do you want here. Line 10: Line 11: Change-Id: I6501981bbd5276c49731b0d9eba4794286b0f823
https://gerrit.ovirt.org/#/c/44494/10/debian/vdsm-python.install File debian/vdsm-python.install:
Line 16: ./
you still have this m2cutils.py if im not wrong :/
I forwarded you email thread where we discussed what need to be implemented.
It was removed by design.
https://gerrit.ovirt.org/#/c/44494/10/lib/vdsm/Makefile.am File lib/vdsm/Makefile.am:
Line 32: executor.py \ Line 33: ipwrapper.py \ Line 34: jsonrpcvdscli.py \ Line 35: libvirtconnection.py \ Line 36: m2cutils.py \
you left it here.. err how did I miss the m2c shortcut . with m2cryptoUtils
Please see my comment in debian file.
It is here by design. Line 37: netconfpersistence.py \ Line 38: netinfo.py \ Line 39: password.py \ Line 40: ppc64HardwareInfo.py \
https://gerrit.ovirt.org/#/c/44494/10/lib/vdsm/sslutils.py File lib/vdsm/sslutils.py:
Line 1: # Line 2: # Copyright 2014 Red Hat, Inc.
2015
Done Line 3: # Line 4: # This program is free software; you can redistribute it and/or modify Line 5: # it under the terms of the GNU General Public License as published by Line 6: # the Free Software Foundation; either version 2 of the License, or
https://gerrit.ovirt.org/#/c/44494/10/tests/integration/m2chelper.py File tests/integration/m2chelper.py:
Line 43: Line 44: class TestServer(): Line 45: Line 46: def __init__(self, host, service): Line 47: self.server = SimpleXMLRPCServer.SimpleXMLRPCServer((host, 0),
why not jsonrpc?
This is old test and we want to keep it as it was. Hopefully we can manage to merge jsonrpc tests soon.
It will be removed once we drop support for xmlrpc. Line 48: logRequests=False) Line 49: self.server.socket = SSLServerSocket(raw=self.server.socket, Line 50: keyfile=KEY_FILE, Line 51: certfile=CRT_FILE,
https://gerrit.ovirt.org/#/c/44494/10/tests/sslTests.py File tests/sslTests.py:
Line 42: from vdsm.sslutils import VerifyingSafeTransport Line 43: from integration.sslhelper import TestServer, \ Line 44: get_server_socket, KEY_FILE, \ Line 45: CRT_FILE, OTHER_KEY_FILE, OTHER_CRT_FILE Line 46: _m2cEnabled = False
don't you want to test both?
I want to test only implementation that I configured before building. There is no point testing something which we are not going to use. Line 47: Line 48: Line 49: HOST = '127.0.0.1' Line 50:
https://gerrit.ovirt.org/#/c/44494/10/tests/vdscliTests.py File tests/vdscliTests.py:
Line 34: from vdsm import m2cutils as sslutils Line 35: from integration.m2chelper import get_server_socket Line 36: except ImportError: Line 37: from vdsm import sslutils Line 38: from integration.sslhelper import get_server_socket
don't you want to test both?
Please see my response in other test module. Line 39: Line 40: Line 41: HOST = '127.0.0.1' Line 42:
https://gerrit.ovirt.org/#/c/44494/10/vdsm.spec.in File vdsm.spec.in:
Line 1072: %{python_sitelib}/%{vdsm_name}/sslcompat.py* Line 1073: %if %{with_m2c} Line 1074: %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1075: %exclude %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1076: %else
why do you need to exclude it?
when I compile for debian I do not want any m2c files. They are not going to be used. Line 1077: %exclude %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1078: %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1079: %endif Line 1080: %{python_sitelib}/%{vdsm_name}/supervdsm.py*
automation@ovirt.org has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 11:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 11: Verified+1
Verified all combinations of config settings m2c and ssl with and without m2crypto rpm installed. Every time no issues with communication.
Yaniv Bronhaim has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 10:
(2 comments)
https://gerrit.ovirt.org/#/c/44494/10//COMMIT_MSG Commit Message:
Line 5: CommitDate: 2015-09-15 14:47:04 +0200 Line 6: Line 7: ssl: configurable implementation Line 8: Line 9: We want to choose ssl implementation during build process.
This patch provide ability to choose ssl implementation during build proces
for what? why do we need to different implementation? is it debian support fault? Line 10: Line 11: Change-Id: I6501981bbd5276c49731b0d9eba4794286b0f823
https://gerrit.ovirt.org/#/c/44494/10/vdsm.spec.in File vdsm.spec.in:
Line 1072: %{python_sitelib}/%{vdsm_name}/sslcompat.py* Line 1073: %if %{with_m2c} Line 1074: %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1075: %exclude %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1076: %else
when I compile for debian I do not want any m2c files. They are not going t
first, in debian you don't use rpm so spec change for that is not relevant. and having such global that can be modified only manually and not by "configure" is redundant
second, I don't connect to this import dependency that you create here. the mail talks about having config value that decides the implementation vdsm will use. debian users should modify the default value and I think its enough to request than having this installation magic tricks that will take care for you which implementation will be available for you to use. I want to keep the same tree both in debian and fedora and have configuration values for the differences - if you want to make the user's life easier you can specify new vdsm-tool configure for debian that sets whatever needed in vdsm.conf for it to work properly. but currently we have just one value and it can stay as a note for the user. if later on engine will provide ability to deploy debian host, the host-deploy will take care of the config values. Line 1077: %exclude %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1078: %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1079: %endif Line 1080: %{python_sitelib}/%{vdsm_name}/supervdsm.py*
Nir Soffer has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 10:
(1 comment)
https://gerrit.ovirt.org/#/c/44494/10/vdsm.spec.in File vdsm.spec.in:
Line 1072: %{python_sitelib}/%{vdsm_name}/sslcompat.py* Line 1073: %if %{with_m2c} Line 1074: %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1075: %exclude %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1076: %else
first, in debian you don't use rpm so spec change for that is not relevant.
debian cannot work with the m2c, since we depend on fedora/el version which contains features not in upstream or debian version.
I think what we need is:
rpm based: - package both ssl and m2c - default should be m2c until we are confident that ssl does not introduce performance regression.
deb based: - package only ssl, since m2c code is not compatible - default should be ssl since there is no other option
When ssl is proven, we can drop m2c compatibility layer and the configuration.
So I agree with Yaniv that we don't need to exclude anything here. Line 1077: %exclude %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1078: %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1079: %endif Line 1080: %{python_sitelib}/%{vdsm_name}/supervdsm.py*
Nir Soffer has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 11:
(1 comment)
https://gerrit.ovirt.org/#/c/44494/11/vdsm.spec.in File vdsm.spec.in:
Line 42: %global _polkitdir %{_localstatedir}/lib/polkit-1/localauthority/10-vendor.d Line 43: %endif Line 44: Line 45: # enable m2crypto by default Line 46: %global with_m2c 1 Do we need now a way to build only with m2c or ssl? I think we do not.
Lets add this feature later when building without m2c is a valid option. When this will be true, we may simply drop it instead of adding a build option. Line 47: Line 48: # Gluster should not be shipped with RHEV Line 49: %if ! 0%{?rhev_build} Line 50: %global with_gluster 1
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 11:
(1 comment)
https://gerrit.ovirt.org/#/c/44494/11/vdsm.spec.in File vdsm.spec.in:
Line 42: %global _polkitdir %{_localstatedir}/lib/polkit-1/localauthority/10-vendor.d Line 43: %endif Line 44: Line 45: # enable m2crypto by default Line 46: %global with_m2c 1
Do we need now a way to build only with m2c or ssl? I think we do not.
I would love to drop it but for now until we solve perf issue we can't use standard ssl module and we can't use m2c for debian.
My short answer is we need it. Line 47: Line 48: # Gluster should not be shipped with RHEV Line 49: %if ! 0%{?rhev_build} Line 50: %global with_gluster 1
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 10:
(2 comments)
https://gerrit.ovirt.org/#/c/44494/10//COMMIT_MSG Commit Message:
Line 5: CommitDate: 2015-09-15 14:47:04 +0200 Line 6: Line 7: ssl: configurable implementation Line 8: Line 9: We want to choose ssl implementation during build process.
for what? why do we need to different implementation? is it debian support
There is custom patch on top of m2c provided by fedora. The patch is not applied for any other distribution and vdsm fails without it. Line 10: Line 11: Change-Id: I6501981bbd5276c49731b0d9eba4794286b0f823
https://gerrit.ovirt.org/#/c/44494/10/vdsm.spec.in File vdsm.spec.in:
Line 1072: %{python_sitelib}/%{vdsm_name}/sslcompat.py* Line 1073: %if %{with_m2c} Line 1074: %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1075: %exclude %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1076: %else
debian cannot work with the m2c, since we depend on fedora/el version which
I do not want to introduce different approaches for different OSes. In my opinion we should have the same behavior for debian and fedora so if we do not have m2c based ssl in one than we should exclude it in the other.
I goal here is to be consistent. Line 1077: %exclude %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1078: %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1079: %endif Line 1080: %{python_sitelib}/%{vdsm_name}/supervdsm.py*
Yaniv Bronhaim has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 10:
(1 comment)
https://gerrit.ovirt.org/#/c/44494/10/vdsm.spec.in File vdsm.spec.in:
Line 1072: %{python_sitelib}/%{vdsm_name}/sslcompat.py* Line 1073: %if %{with_m2c} Line 1074: %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1075: %exclude %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1076: %else
I do not want to introduce different approaches for different OSes. In my o
you introduce something much bigger - different installation tree between distributions. I understand the reasons and I quite familiar with the thread. but people that reads the spec have no clue and can't understand why we need this two implementations so it should be understandable from the commit message
m2cryptoUtil and sslUtil should be available in vdsm installation. debian admin (or vdsm-tool configure or host-deploy) should set the new config value to use the ssl implementation over debian. This patch should add this config value only as the commit message states, the rest is up to the user. Line 1077: %exclude %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1078: %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1079: %endif Line 1080: %{python_sitelib}/%{vdsm_name}/supervdsm.py*
Yaniv Bronhaim has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 11: Code-Review-1
(1 comment)
Sorry, I still think this patch needs only to introduce the config value and depend on it. you can still use the sslcompat trick, but avoiding to install a module and depend on it is not something I want to add
https://gerrit.ovirt.org/#/c/44494/11/vdsm.spec.in File vdsm.spec.in:
Line 42: %global _polkitdir %{_localstatedir}/lib/polkit-1/localauthority/10-vendor.d Line 43: %endif Line 44: Line 45: # enable m2crypto by default Line 46: %global with_m2c 1
I would love to drop it but for now until we solve perf issue we can't use
we don't ... what will happen if you have this py file always? Line 47: Line 48: # Gluster should not be shipped with RHEV Line 49: %if ! 0%{?rhev_build} Line 50: %global with_gluster 1
Nir Soffer has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 11:
(1 comment)
https://gerrit.ovirt.org/#/c/44494/11/vdsm.spec.in File vdsm.spec.in:
Line 42: %global _polkitdir %{_localstatedir}/lib/polkit-1/localauthority/10-vendor.d Line 43: %endif Line 44: Line 45: # enable m2crypto by default Line 46: %global with_m2c 1
we don't ... what will happen if you have this py file always?
Pitor, this file is used only for rpm, not for debian. On rpm based systems, we can have both for now.
On debian, we can have only ssl, unless we change our m2c code to work with upstream/debian version - did we check this option? Line 47: Line 48: # Gluster should not be shipped with RHEV Line 49: %if ! 0%{?rhev_build} Line 50: %global with_gluster 1
Nir Soffer has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 10:
(1 comment)
https://gerrit.ovirt.org/#/c/44494/10/vdsm.spec.in File vdsm.spec.in:
Line 1072: %{python_sitelib}/%{vdsm_name}/sslcompat.py* Line 1073: %if %{with_m2c} Line 1074: %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1075: %exclude %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1076: %else
you introduce something much bigger - different installation tree between d
Yaniv, we cannot ship vdsm on debian using m2c, since our code is not compatible with m2c on debian. The only option is to ship only ssl and the default on debian can be only ssl.
On rpm based systems, we can choose any default. Line 1077: %exclude %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1078: %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1079: %endif Line 1080: %{python_sitelib}/%{vdsm_name}/supervdsm.py*
Yaniv Bronhaim has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 10:
(1 comment)
https://gerrit.ovirt.org/#/c/44494/10/vdsm.spec.in File vdsm.spec.in:
Line 1072: %{python_sitelib}/%{vdsm_name}/sslcompat.py* Line 1073: %if %{with_m2c} Line 1074: %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1075: %exclude %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1076: %else
Yaniv, we cannot ship vdsm on debian using m2c, since our code is not compa
what is shipping? having a py file as part of the installation? alert the user that over debian he must config 1 2 3 otherwise default ssl implementation will cause failures. if you want you can add "check code" in vdsm/vdsm to validate that if debian is the os and m2crypto is configured, alert and exit. Line 1077: %exclude %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1078: %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1079: %endif Line 1080: %{python_sitelib}/%{vdsm_name}/supervdsm.py*
Nir Soffer has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 10:
(1 comment)
https://gerrit.ovirt.org/#/c/44494/10/vdsm.spec.in File vdsm.spec.in:
Line 1072: %{python_sitelib}/%{vdsm_name}/sslcompat.py* Line 1073: %if %{with_m2c} Line 1074: %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1075: %exclude %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1076: %else
what is shipping? having a py file as part of the installation? alert the u
Yaniv, can you explain what is wrong with default configuration that works? why do you want to provide users a broken configuration? Line 1077: %exclude %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1078: %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1079: %endif Line 1080: %{python_sitelib}/%{vdsm_name}/supervdsm.py*
Yaniv Bronhaim has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 10:
(1 comment)
https://gerrit.ovirt.org/#/c/44494/10/vdsm.spec.in File vdsm.spec.in:
Line 1072: %{python_sitelib}/%{vdsm_name}/sslcompat.py* Line 1073: %if %{with_m2c} Line 1074: %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1075: %exclude %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1076: %else
Yaniv, can you explain what is wrong with default configuration that works?
of course I want to have working defaults.. but currently we don't deploy by the engine debian hosts, and this the location where we modify vdsm.conf. so it's not possible to set different value just for debian unless we hardcoded it or depending on different installation as this patch propose and I nack Line 1077: %exclude %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1078: %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1079: %endif Line 1080: %{python_sitelib}/%{vdsm_name}/supervdsm.py*
Nir Soffer has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 10:
(1 comment)
https://gerrit.ovirt.org/#/c/44494/10/vdsm.spec.in File vdsm.spec.in:
Line 1072: %{python_sitelib}/%{vdsm_name}/sslcompat.py* Line 1073: %if %{with_m2c} Line 1074: %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1075: %exclude %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1076: %else
of course I want to have working defaults.. but currently we don't deploy b
We should set the default ssl_impl during build - just add configure rule for this. Line 1077: %exclude %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1078: %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1079: %endif Line 1080: %{python_sitelib}/%{vdsm_name}/supervdsm.py*
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 11:
(1 comment)
https://gerrit.ovirt.org/#/c/44494/11/vdsm.spec.in File vdsm.spec.in:
Line 42: %global _polkitdir %{_localstatedir}/lib/polkit-1/localauthority/10-vendor.d Line 43: %endif Line 44: Line 45: # enable m2crypto by default Line 46: %global with_m2c 1
Pitor, this file is used only for rpm, not for debian. On rpm based systems
I know that we use it for rpm build. In debian we have issue that m2crypto is not patched and vdsm fails so it is not an option to have it there. Line 47: Line 48: # Gluster should not be shipped with RHEV Line 49: %if ! 0%{?rhev_build} Line 50: %global with_gluster 1
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 10:
(1 comment)
https://gerrit.ovirt.org/#/c/44494/10/vdsm.spec.in File vdsm.spec.in:
Line 1072: %{python_sitelib}/%{vdsm_name}/sslcompat.py* Line 1073: %if %{with_m2c} Line 1074: %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1075: %exclude %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1076: %else
We should set the default ssl_impl during build - just add configure rule f
OK, during the build or during installation we can do it. In my opinion that if we remove dependency on m2crypto we can't assume that it is there so there is no reason why we need to keep m2cutls in our rpm. Line 1077: %exclude %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1078: %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1079: %endif Line 1080: %{python_sitelib}/%{vdsm_name}/supervdsm.py*
Yaniv Bronhaim has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 10:
(1 comment)
https://gerrit.ovirt.org/#/c/44494/10/vdsm.spec.in File vdsm.spec.in:
Line 1072: %{python_sitelib}/%{vdsm_name}/sslcompat.py* Line 1073: %if %{with_m2c} Line 1074: %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1075: %exclude %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1076: %else
OK, during the build or during installation we can do it. In my opinion tha
why? there are places in code that we check if something installed and if not we fallback to another implementation. its quite common, and here, if user configures vdsm to use m2crypto and its not exist, vdsm will fail to start with an error saying to install the package - sounds perfectly reasonable to me. Line 1077: %exclude %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1078: %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1079: %endif Line 1080: %{python_sitelib}/%{vdsm_name}/supervdsm.py*
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 10:
(1 comment)
https://gerrit.ovirt.org/#/c/44494/10/vdsm.spec.in File vdsm.spec.in:
Line 1072: %{python_sitelib}/%{vdsm_name}/sslcompat.py* Line 1073: %if %{with_m2c} Line 1074: %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1075: %exclude %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1076: %else
why? there are places in code that we check if something installed and if n
Not for me. We know that we do not require m2crypto to be installed and so why we want to have a file which depends on it? If we built for ssl only we should keep our code base ssl only. We know what we expect. Line 1077: %exclude %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1078: %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1079: %endif Line 1080: %{python_sitelib}/%{vdsm_name}/supervdsm.py*
Yaniv Bronhaim has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 10:
(1 comment)
https://gerrit.ovirt.org/#/c/44494/10/vdsm.spec.in File vdsm.spec.in:
Line 1072: %{python_sitelib}/%{vdsm_name}/sslcompat.py* Line 1073: %if %{with_m2c} Line 1074: %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1075: %exclude %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1076: %else
Not for me. We know that we do not require m2crypto to be installed and so
how do you build for ssl? there won't be such thing. you build vdsm and you config which implementation to use.. Line 1077: %exclude %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1078: %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1079: %endif Line 1080: %{python_sitelib}/%{vdsm_name}/supervdsm.py*
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 10:
(1 comment)
https://gerrit.ovirt.org/#/c/44494/10/vdsm.spec.in File vdsm.spec.in:
Line 1072: %{python_sitelib}/%{vdsm_name}/sslcompat.py* Line 1073: %if %{with_m2c} Line 1074: %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1075: %exclude %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1076: %else
how do you build for ssl? there won't be such thing. you build vdsm and you
I see your confusion. please check next patch and you will see what is the way to go. You are looking at mid setp with provided way of building vdsm with m2c or ssl. As a result we are able to use one or the other even though ssl module is always there. Line 1077: %exclude %{python_sitelib}/%{vdsm_name}/m2cutils.py* Line 1078: %{python_sitelib}/%{vdsm_name}/sslutils.py* Line 1079: %endif Line 1080: %{python_sitelib}/%{vdsm_name}/supervdsm.py*
automation@ovirt.org has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 12:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
automation@ovirt.org has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 13:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 13: Verified+1
Verified by building vdsm for both implementation and tested with both implementation settings. There was separate test by building vdsm for ssl module only and verified runtime with ssl setting. Both building and runtime machines had no m2crypto rpm installed.
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 13:
CI failure is not related to this change
Francesco Romani has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 13: Code-Review+1
I don't have comments and I haven't spotted any obvious flaw, so +1 from me. I'll keep watching this change.
automation@ovirt.org has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 14:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 14: Verified+1
There were implementation fallback issues during tests which were discovered while running build in mock. Fixed by checking which ssl implementation is in configuration.
Non-test functionality was not change and verified in previous patch set.
automation@ovirt.org has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 15:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 15: Verified+1
Moved test fixes to next patch. No other changes. Verified by running build in mock env locally.
automation@ovirt.org has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 16:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 16:
Rebase only, no code changes
Yaniv Bronhaim has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 16:
(3 comments)
https://gerrit.ovirt.org/#/c/44494/16/lib/vdsm/m2cutils.py File lib/vdsm/m2cutils.py:
Line 164: def _loadCAs(self): Line 165: context = self.context Line 166: Line 167: if self.ca_certs: Line 168: context.load_verify_locations(self.ca_certs) less related and can get in before this patch Line 169: context.set_verify( Line 170: mode=SSL.verify_peer | SSL.verify_fail_if_no_peer_cert, Line 171: depth=10, Line 172: callback=self._verify)
https://gerrit.ovirt.org/#/c/44494/16/lib/vdsm/sslcompat.py File lib/vdsm/sslcompat.py:
Line 16: # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA Line 17: # Line 18: # Refer to the README and COPYING files for full details of the license Line 19: # Line 20: try: instead of rely on ImportError why not to have - https://gerrit.ovirt.org/#/c/44689/15/lib/vdsm/config.py.in here and just check it to know what to import Line 21: from . import m2cutils as sslutils Line 22: from .m2cutils import SSLHandshakeDispatcher Line 23: from .m2cutils import SSLSocket Line 24: except ImportError:
https://gerrit.ovirt.org/#/c/44494/16/vdsm.spec.in File vdsm.spec.in:
Line 42: %global _polkitdir %{_localstatedir}/lib/polkit-1/localauthority/10-vendor.d Line 43: %endif Line 44: Line 45: # enable m2crypto by default Line 46: %global with_m2c 1 I still prefer to require m2crypto always to keep the spec less complicated to understand. and why not? you have it in debian as well.. its just implementation thing that we want to avoid Line 47: Line 48: # Gluster should not be shipped with RHEV Line 49: %if ! 0%{?rhev_build} Line 50: %global with_gluster 1
Yeela Kaplan has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 16:
Looks good to me. Except for Yaniv's comments, which I agree with. Piotr, waiting for your response.
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 16:
(3 comments)
https://gerrit.ovirt.org/#/c/44494/16/lib/vdsm/m2cutils.py File lib/vdsm/m2cutils.py:
Line 164: def _loadCAs(self): Line 165: context = self.context Line 166: Line 167: if self.ca_certs: Line 168: context.load_verify_locations(self.ca_certs)
less related and can get in before this patch
I discovered it when I had wrongly configured setup. We need to be consistent between sslutils and m2cutils. This is exactly what this patch is about. Line 169: context.set_verify( Line 170: mode=SSL.verify_peer | SSL.verify_fail_if_no_peer_cert, Line 171: depth=10, Line 172: callback=self._verify)
https://gerrit.ovirt.org/#/c/44494/16/lib/vdsm/sslcompat.py File lib/vdsm/sslcompat.py:
Line 16: # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA Line 17: # Line 18: # Refer to the README and COPYING files for full details of the license Line 19: # Line 20: try:
instead of rely on ImportError why not to have - https://gerrit.ovirt.org/#
This patch is interim step and the change that you propose is in the next patch. Both patches are split based on functional scope. Line 21: from . import m2cutils as sslutils Line 22: from .m2cutils import SSLHandshakeDispatcher Line 23: from .m2cutils import SSLSocket Line 24: except ImportError:
https://gerrit.ovirt.org/#/c/44494/16/vdsm.spec.in File vdsm.spec.in:
Line 42: %global _polkitdir %{_localstatedir}/lib/polkit-1/localauthority/10-vendor.d Line 43: %endif Line 44: Line 45: # enable m2crypto by default Line 46: %global with_m2c 1
I still prefer to require m2crypto always to keep the spec less complicated
We can't required it always. We could have it in debian as well but please in mind that it is a bit different and it makes vdsm to fail. In order to keep it simple we need to remove m2c dependency if not needed. Line 47: Line 48: # Gluster should not be shipped with RHEV Line 49: %if ! 0%{?rhev_build} Line 50: %global with_gluster 1
Yeela Kaplan has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 16: Code-Review+1
Yaniv Bronhaim has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 16:
(2 comments)
https://gerrit.ovirt.org/#/c/44494/16/lib/vdsm/sslcompat.py File lib/vdsm/sslcompat.py:
Line 16: # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA Line 17: # Line 18: # Refer to the README and COPYING files for full details of the license Line 19: # Line 20: try:
This patch is interim step and the change that you propose is in the next p
I also don't see a reason for two patches.. it can be done in one single and simple patch. having new config value, having the compact trick and fix the imports. Line 21: from . import m2cutils as sslutils Line 22: from .m2cutils import SSLHandshakeDispatcher Line 23: from .m2cutils import SSLSocket Line 24: except ImportError:
https://gerrit.ovirt.org/#/c/44494/16/vdsm.spec.in File vdsm.spec.in:
Line 42: %global _polkitdir %{_localstatedir}/lib/polkit-1/localauthority/10-vendor.d Line 43: %endif Line 44: Line 45: # enable m2crypto by default Line 46: %global with_m2c 1
We can't required it always. We could have it in debian as well but please
first of all - this is a spec, spec is for fedora only, and we will always want m2c here. how does it make anything simple? second, as I said before - if users run it over debian, they must be aware that they need to change vdsm.conf for that. which should do the magic Line 47: Line 48: # Gluster should not be shipped with RHEV Line 49: %if ! 0%{?rhev_build} Line 50: %global with_gluster 1
automation@ovirt.org has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 17:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 17:
Only rebase no code changes. Pending changes to remove spec changes but not yet agreed on.
automation@ovirt.org has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 18:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Piotr Kliczewski has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 18:
Rebase only.
Francesco Romani has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 18: Code-Review+1
+1 for the concept and for the parts of this patch I know about.
Yaniv Bronhaim has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 18: Code-Review-1
we agreed to change the implementation
Piotr Kliczewski has abandoned this change.
Change subject: ssl: configurable implementation ......................................................................
Abandoned
Due to squashing this patch with https://gerrit.ovirt.org/#/c/44689/ we do not need it anymore.
automation@ovirt.org has posted comments on this change.
Change subject: ssl: configurable implementation ......................................................................
Patch Set 18:
* Update tracker::IGNORE, no Bug-Url found
vdsm-patches@lists.fedorahosted.org