Change in vdsm[ovirt-3.4]: vdscli: avoid ssl-specific localhost heuristic

danken at redhat.com danken at redhat.com
Wed Jun 25 08:56:53 UTC 2014


Hello Alon Bar-Lev, Nir Soffer, Federico Simoncelli,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/29182

to review the following change.

Change subject: vdscli: avoid ssl-specific localhost heuristic
......................................................................

vdscli: avoid ssl-specific localhost heuristic

Unlike it used to, Vdsm no longer checks its client's identity versus
the content of the client certificate.

Therefore, there is no need for the too-clever mechanism of guessing the
local host adderess from the ssl certificate. That mechanism is known to
have caused pains when the subject name of the certificate became
non-resolvable for whatever reason (Cf. http://gerrit.ovirt.org/28107 ).

Change-Id: I421e466621e2a172f9cf8158913654c49cfecca1
Signed-off-by: Dan Kenigsberg <danken at redhat.com>
Bug-Url: https://bugzilla.redhat.com/1111075
Reviewed-on: http://gerrit.ovirt.org/28674
Reviewed-by: Alon Bar-Lev <alonbl at redhat.com>
Reviewed-by: Nir Soffer <nsoffer at redhat.com>
Reviewed-by: Federico Simoncelli <fsimonce at redhat.com>
---
M lib/vdsm/vdscli.py.in
M tests/vdsClientTests.py
2 files changed, 1 insertion(+), 50 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/82/29182/1

diff --git a/lib/vdsm/vdscli.py.in b/lib/vdsm/vdscli.py.in
index 5fa7528..4b7b503 100644
--- a/lib/vdsm/vdscli.py.in
+++ b/lib/vdsm/vdscli.py.in
@@ -20,7 +20,6 @@
 #
 
 import xmlrpclib
-import subprocess
 import os
 import re
 import sys
@@ -70,10 +69,7 @@
             d_useSSL = config.getboolean('vars', 'ssl')
             d_tsPath = config.get('vars', 'trust_store_path')
             d_port = config.get('addresses', 'management_port')
-            if d_useSSL:
-                d_addr = __getLocalVdsName(d_tsPath)
-            else:
-                d_addr = config.get('addresses', 'management_ip')
+            d_addr = config.get('addresses', 'management_ip')
         except:
             pass
         if os.name == 'nt':
@@ -87,29 +83,6 @@
             d_tsPath = os.path.join(getRHEVMInstallPath(), "Service", "ca")
     except:
         pass
-
-
-def __getLocalVdsName(tsPath):
-    LOOPBACK_ADDR = '0'
-
-    p = subprocess.Popen(['openssl', 'x509', '-noout', '-subject', '-in',
-                          '%s/certs/vdsmcert.pem' % tsPath],
-                         stdout=subprocess.PIPE, stderr=subprocess.PIPE,
-                         close_fds=True)
-    out, err = p.communicate()
-    if p.returncode != 0:
-        return LOOPBACK_ADDR
-
-    for keyval in out.split('/'):
-        try:
-            key, val = keyval.split('=', 1)
-        except ValueError:
-            pass
-        else:
-            if key == 'CN':
-                return val.strip()
-
-    return LOOPBACK_ADDR
 
 
 __guessDefaults()
diff --git a/tests/vdsClientTests.py b/tests/vdsClientTests.py
index 908ff24..a405007 100644
--- a/tests/vdsClientTests.py
+++ b/tests/vdsClientTests.py
@@ -21,13 +21,10 @@
 import os
 from tempfile import mkstemp
 from contextlib import contextmanager
-import subprocess
 
 from testrunner import VdsmTestCase as TestCaseBase
-from monkeypatch import MonkeyPatch
 
 import vdsClient
-from vdsm.vdscli import __getLocalVdsName as getLocalVdsName
 
 
 @contextmanager
@@ -134,22 +131,3 @@
 
     def communicate(self):
         return self._output, ''
-
-
-class vdscliTests(TestCaseBase):
-    @MonkeyPatch(subprocess, 'Popen', lambda *y, **x: _FakePopen(
-        'subject= /O=VDSM Certificate/CN=myhost\n'))
-    def test__getLocalVdsName1(self):
-        cn = getLocalVdsName('fake')
-        self.assertEquals('myhost', cn)
-
-    @MonkeyPatch(subprocess, 'Popen', lambda *y, **x: _FakePopen(
-        'subject= /CN=myhost/O=VDSM Certificate\n'))
-    def test__getLocalVdsName2(self):
-        cn = getLocalVdsName('fake')
-        self.assertEquals('myhost', cn)
-
-    @MonkeyPatch(subprocess, 'Popen', lambda *y, **x: _FakePopen('garbled='))
-    def test__getLocalVdsName3(self):
-        cn = getLocalVdsName('fake')
-        self.assertEquals('0', cn)


-- 
To view, visit http://gerrit.ovirt.org/29182
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I421e466621e2a172f9cf8158913654c49cfecca1
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>


More information about the vdsm-patches mailing list