Hello Dan Kenigsberg,

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

    http://gerrit.ovirt.org/32052

to review the following change.

Change subject: vdsm-tool: roll out self signed certificates in vdsm-tool.
......................................................................

vdsm-tool: roll out self signed certificates in vdsm-tool.

This is handled in a new 'ModuleConfigure' named 'certificates'.
Libvirt module now requires 'certificates'.

certificates are not remove during removeConf.

previously certificates where generated on init.

Change-Id: I53d927e49920e8c45d02718f2b6ef5dff63eead0
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1127877
Signed-off-by: Mooli Tayer <[email protected]>
Reviewed-on: http://gerrit.ovirt.org/31562
Reviewed-by: Dan Kenigsberg <[email protected]>
---
M init/vdsmd_init_common.sh.in
M lib/vdsm/tool/configurator.py
M lib/vdsm/tool/configurators/Makefile.am
A lib/vdsm/tool/configurators/certificates.py
M lib/vdsm/tool/configurators/libvirt.py
M vdsm.spec.in
M vdsm/vdsm-gencerts.sh.in
7 files changed, 103 insertions(+), 49 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/52/32052/1

diff --git a/init/vdsmd_init_common.sh.in b/init/vdsmd_init_common.sh.in
index 19dea08..298b658 100644
--- a/init/vdsmd_init_common.sh.in
+++ b/init/vdsmd_init_common.sh.in
@@ -54,15 +54,6 @@
 }
 
 
-task_gencerts(){
-    if ! "@LIBEXECDIR@/vdsm-gencerts.sh" --check; then
-        printf "Configuring a self-signed VDSM host certificate"
-        "@LIBEXECDIR@/vdsm-gencerts.sh" || return 1
-    fi
-    return 0
-}
-
-
 task_check_is_configured() {
     "$VDSM_TOOL" is-configured
 }
@@ -242,7 +233,6 @@
             configure_coredump \
             configure_vdsm_logs \
             run_init_hooks \
-            gencerts \
             check_is_configured \
             validate_configuration \
             prepare_transient_repository \
diff --git a/lib/vdsm/tool/configurator.py b/lib/vdsm/tool/configurator.py
index 1e185f6..2f71075 100644
--- a/lib/vdsm/tool/configurator.py
+++ b/lib/vdsm/tool/configurator.py
@@ -24,6 +24,7 @@
 
 from . import service, expose
 from .configurators import \
+    certificates, \
     CONFIGURED, \
     InvalidConfig, \
     InvalidRun, \
@@ -35,8 +36,9 @@
 def _getConfigurers():
     return {
         m.getName(): m for m in (
+            certificates.Certificates(),
             libvirt.Libvirt(),
-            sanlock.Sanlock()
+            sanlock.Sanlock(),
         )
     }
 
diff --git a/lib/vdsm/tool/configurators/Makefile.am 
b/lib/vdsm/tool/configurators/Makefile.am
index 36ddbe2..02c6f82 100644
--- a/lib/vdsm/tool/configurators/Makefile.am
+++ b/lib/vdsm/tool/configurators/Makefile.am
@@ -21,6 +21,7 @@
 
 dist_configurators_PYTHON = \
        __init__.py \
+       certificates.py \
        configfile.py \
        libvirt.py \
        sanlock.py \
diff --git a/lib/vdsm/tool/configurators/certificates.py 
b/lib/vdsm/tool/configurators/certificates.py
new file mode 100644
index 0000000..7a9a7c6
--- /dev/null
+++ b/lib/vdsm/tool/configurators/certificates.py
@@ -0,0 +1,88 @@
+# 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
+#
+import os
+import sys
+
+from vdsm.config import config
+
+from . import \
+    CONFIGURED, \
+    ModuleConfigure, \
+    NOT_CONFIGURED
+from .. import validate_ovirt_certs
+from ...constants import \
+    P_VDSM_EXEC, \
+    SYSCONF_PATH
+from ...utils import \
+    execCmd, \
+    isOvirtNode
+
+PKI_DIR = os.path.join(SYSCONF_PATH, 'pki/vdsm')
+CA_FILE = os.path.join(PKI_DIR, 'certs/cacert.pem')
+CERT_FILE = os.path.join(PKI_DIR, 'certs/vdsmcert.pem')
+KEY_FILE = os.path.join(PKI_DIR, 'keys/vdsmkey.pem')
+
+
+class Certificates(ModuleConfigure):
+    """
+    Responsible for rolling out self signed certificates if vdsm's
+    configuration is ssl_enabled and no certificates exist.
+    """
+    # Todo: validate_ovirt_certs.py
+    def getName(self):
+        return 'certificates'
+
+    def validate(self):
+        return self._certsExist()
+
+    def _exec_vdsm_gencerts(self):
+        rc, out, err = execCmd(
+            (
+                os.path.join(
+                    P_VDSM_EXEC,
+                    'vdsm-gencerts.sh'
+                ),
+                CA_FILE,
+                KEY_FILE,
+                CERT_FILE,
+            ),
+            raw=True,
+        )
+        sys.stdout.write(out)
+        sys.stderr.write(err)
+        if rc != 0:
+            raise RuntimeError("Failed to perform vdsm-gencerts action.")
+
+    def configure(self):
+        self._exec_vdsm_gencerts()
+        if isOvirtNode():
+            validate_ovirt_certs.validate_ovirt_certs()
+
+    def isconfigured(self):
+        return CONFIGURED if self._certsExist() else NOT_CONFIGURED
+
+    def _certsExist(self):
+        config.read(
+            os.path.join(
+                SYSCONF_PATH,
+                'vdsm/vdsm.conf'
+            )
+        )
+        return not config.getboolean('vars', 'ssl') or\
+            os.path.isfile(CERT_FILE)
diff --git a/lib/vdsm/tool/configurators/libvirt.py 
b/lib/vdsm/tool/configurators/libvirt.py
index 25a744f..c1ee527 100644
--- a/lib/vdsm/tool/configurators/libvirt.py
+++ b/lib/vdsm/tool/configurators/libvirt.py
@@ -38,6 +38,10 @@
 from . configfile import \
     ConfigFile, \
     ParserWrapper
+from . certificates import \
+    CA_FILE, \
+    CERT_FILE, \
+    KEY_FILE
 from ... import utils
 from ... import constants
 
@@ -73,11 +77,6 @@
 
         config.read(self._getFile('VDSM_CONF'))
         vdsmConfiguration = {
-            'certs_exist': all(os.path.isfile(f) for f in [
-                self.CA_FILE,
-                self.CERT_FILE,
-                self.KEY_FILE
-            ]),
             'ssl_enabled': config.getboolean('vars', 'ssl'),
             'sanlock_enabled': constants.SANLOCK_ENABLED,
             'libvirt_selinux': constants.LIBVIRT_SELINUX
@@ -111,6 +110,9 @@
     def removeConf(self):
         for cfile, content in Libvirt.FILES.items():
             content['removeConf'](self, content['path'])
+
+    def getRequires(self):
+        return {'certificates'}
 
     def _getPersistedFiles(self):
         """
@@ -304,9 +306,6 @@
     CONF_VERSION = '4.13.0'
 
     PKI_DIR = os.path.join(constants.SYSCONF_PATH, 'pki/vdsm')
-    CA_FILE = os.path.join(PKI_DIR, 'certs/cacert.pem')
-    CERT_FILE = os.path.join(PKI_DIR, 'certs/vdsmcert.pem')
-    KEY_FILE = os.path.join(PKI_DIR, 'keys/vdsmkey.pem')
     LS_CERT_DIR = os.path.join(PKI_DIR, 'libvirt-spice')
 
     # be sure to update CONF_VERSION accordingly when updating FILES.
@@ -365,7 +364,6 @@
                 {
                     'conditions': {
                         "ssl_enabled": True,
-                        "certs_exist": True,
                     },
                     'content': {
                         'ca_file': '\"' + CA_FILE + '\"',
@@ -374,18 +372,6 @@
                     },
 
                 },
-                {
-                    'conditions': {
-                        "ssl_enabled": True,
-                        "certs_exist": False,
-                    },
-                    'content': {
-                        'auth_tcp': '"none"',
-                        'listen_tcp': 1,
-                        'listen_tls': 0,
-                    },
-
-                }
             ]
         },
 
@@ -424,15 +410,6 @@
                     },
                     'content': {
                         'spice_tls': 1,
-                    },
-
-                },
-                {
-                    'conditions': {
-                        "ssl_enabled": True,
-                        "certs_exist": True,
-                    },
-                    'content': {
                         'spice_tls_x509_cert_dir': '\"' + LS_CERT_DIR + '\"',
                     },
 
diff --git a/vdsm.spec.in b/vdsm.spec.in
index 4ebbedf..a07ce56 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -1176,6 +1176,7 @@
 %{python_sitearch}/%{vdsm_name}/tool/configurator.py*
 %{python_sitearch}/%{vdsm_name}/tool/configurators/__init__*
 %{python_sitearch}/%{vdsm_name}/tool/configurators/configfile.py*
+%{python_sitearch}/%{vdsm_name}/tool/configurators/certificates.py*
 %{python_sitearch}/%{vdsm_name}/tool/configurators/libvirt.py*
 %{python_sitearch}/%{vdsm_name}/tool/configurators/sanlock.py*
 %{python_sitearch}/%{vdsm_name}/tool/passwd.py*
diff --git a/vdsm/vdsm-gencerts.sh.in b/vdsm/vdsm-gencerts.sh.in
index adfd56e..4ad59a8 100755
--- a/vdsm/vdsm-gencerts.sh.in
+++ b/vdsm/vdsm-gencerts.sh.in
@@ -19,10 +19,9 @@
 # Refer to the README and COPYING files for full details of the license
 #
 
-VDSM_PKI="@TRUSTSTORE@"
-VDSM_KEY="$VDSM_PKI/keys/vdsmkey.pem"
-VDSM_CRT="$VDSM_PKI/certs/vdsmcert.pem"
-VDSM_CA="$VDSM_PKI/certs/cacert.pem"
+VDSM_CA="$1"
+VDSM_KEY="$2"
+VDSM_CRT="$3"
 
 VDSM_TEMPLATE="$(mktemp)"
 
@@ -32,10 +31,6 @@
 VDSM_PERMS="@VDSMUSER@:@VDSMGROUP@"
 
 umask 077
-
-if [ "$1" = "--check" ]; then
-    [ -s "$VDSM_KEY" -a -s "$VDSM_CA" -a -s "$VDSM_CRT" ] && exit 0 || exit 1
-fi
 
 if [ ! -f "$VDSM_KEY" ]; then
     /usr/bin/certtool --generate-privkey --outfile "$VDSM_KEY" 2> /dev/null


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I53d927e49920e8c45d02718f2b6ef5dff63eead0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: mooli tayer <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to