CA-329845 - Remove usage of credentials file for CIF

Signed-off-by: ben sims <ben.sims@citrix.com>

Makefile

@@ -31,6 +31,7 @@ SM_LIBS += scsiutil
 SM_LIBS += scsi_host_rescan
 SM_LIBS += vhdutil
 SM_LIBS += lvhdutil
+SM_LIBS += cifutils
 SM_LIBS += xs_errors
 SM_LIBS += nfs
 SM_LIBS += devscan

drivers/ISOSR.py

@@ -21,6 +21,7 @@
 import nfs
 import os, re
 import xs_errors
+import cifutils
 
 CAPABILITIES = ["VDI_CREATE", "VDI_DELETE", "VDI_ATTACH", "VDI_DETACH", 
                 "SR_SCAN", "SR_ATTACH", "SR_DETACH"]
@@ -230,7 +231,6 @@ def load(self, sr_uuid):
 
         # Some info we need:
         self.sr_vditype = 'phy'
-        self.credentials = None
 
     def delete(self, sr_uuid):
         pass
@@ -254,7 +254,6 @@ def attach(self, sr_uuid):
 
         mountcmd=[]
         location = util.to_plain_string(self.dconf['location'])
-        self.credentials = os.path.join("/tmp", util.gen_uuid())
         # TODO: Have XC standardise iso type string
         protocol = 'nfs_iso'
         options = ''
@@ -305,7 +304,6 @@ def attach(self, sr_uuid):
             # Check the validity of 'smbversion'.
             # Raise an exception for any invalid version.
             if self.smbversion not in [SMB_VERSION_1, SMB_VERSION_3]:
-                self._cleanupcredentials()
                 raise xs_errors.XenError('ISOInvalidSMBversion')
 
         # Attempt mounting
@@ -344,21 +342,18 @@ def attach(self, sr_uuid):
                                 mountcmd.extend(options)
                             self.mountOverSMB(mountcmd)
                         else:
-                            self._cleanupcredentials()
                             raise xs_errors.XenError(
                                 'ISOMountFailure', opterr=inst.reason)
                 else:
                     util.SMlog('ISOSR mount over smb 1.0')
                     self.mountOverSMB(mountcmd)
         except util.CommandException, inst:
-            self._cleanupcredentials()
             if not self.is_smbversion_specified:
                 raise xs_errors.XenError(
                     'ISOMountFailure', opterr=smb3_fail_reason)
             else:
                 raise xs_errors.XenError(
                     'ISOMountFailure', opterr=inst.reason)
-        self._cleanupcredentials()
 
         # Check the iso_path is accessible
         if not self._checkmount():
@@ -384,7 +379,9 @@ def getSMBVersion(self):
 
     def mountOverSMB(self, mountcmd):
         """This function raises util.CommandException"""
-        util.pread(mountcmd, True)
+        new_env, domain = cifutils.getCIFCredentials(self.dconf, self.session)
+
+        util.pread(mountcmd, True, new_env)
         try:
             if not self.is_smbversion_specified:
                 # Store the successful smb version in PBD config
@@ -421,10 +418,16 @@ def appendCIFSMountOptions(self, mountcmd):
         """Append options to mount.cifs"""
         options = []
         try:
-            options.append(self.getCIFSPasswordOptions())
             options.append(self.getCacheOptions())
-            options.append('guest')
+
+            if not cifutils.containsCredentials(self.dconf):
+                options.append('guest')
+
             options.append(self.getSMBVersion())
+
+            username, domain = cifutils.splitDomainAndUsername(self.dconf['username'])
+            if domain:
+                options.append('domain='+ domain)
         except:
             util.SMlog("Exception while attempting to append mount options")
             raise
@@ -438,48 +441,6 @@ def getCacheOptions(self):
         """Pass cache options to mount.cifs"""
         return "cache=none"
 
-    def getCIFSPasswordOptions(self):
-        if self.dconf.has_key('username') \
-                and (self.dconf.has_key('cifspassword') or self.dconf.has_key('cifspassword_secret')):
-            dom_username = self.dconf['username'].split('\\')
-            if len(dom_username) == 1:
-                domain = None
-                username = dom_username[0]
-            elif len(dom_username) == 2:
-                domain = dom_username[0]
-                username = dom_username[1]
-            else:
-                err_str = ("A maximum of 2 tokens are expected "
-                           "(<domain>\<username>). {} were given."
-                           .format(len(dom_username)))
-                util.SMlog('CIFS ISO SR mount error: ' + err_str)
-                raise xs_errors.XenError('ISOMountFailure', opterr=err_str)
-
-            if self.dconf.has_key('cifspassword_secret'):
-                password = util.get_secret(self.session, self.dconf['cifspassword_secret'])
-            else:
-                password = self.dconf['cifspassword']
-
-            domain = util.to_plain_string(domain)
-            username = util.to_plain_string(username)
-            password = util.to_plain_string(password)
-
-            cred_str = 'username={}\npassword={}\n'.format(username, password)
-
-            if domain:
-                cred_str += 'domain={}\n'.format(domain)
-
-            # Open credentials file and truncate
-            f = open(self.credentials, 'w')
-            f.write(cred_str)
-            f.close()            
-            credentials = "credentials=%s" % self.credentials            
-            return credentials
-
-    def _cleanupcredentials(self):
-        if self.credentials and os.path.exists(self.credentials):
-            os.unlink(self.credentials)
-
     def detach(self, sr_uuid):
         """Std. detach"""
         # This handles legacy mode too, so no need to check

drivers/SMBSR.py

@@ -25,6 +25,7 @@
 import vhdutil
 from lock import Lock
 import cleanup
+import cifutils
 
 CAPABILITIES = ["SR_PROBE","SR_UPDATE", "SR_CACHING",
                 "VDI_CREATE","VDI_DELETE","VDI_ATTACH","VDI_DETACH",
@@ -80,7 +81,6 @@ def load(self, sr_uuid):
             self.sm_config = self.session.xenapi.SR.get_sm_config(self.sr_ref)
         else:
             self.sm_config = self.srcmd.params.get('sr_sm_config') or {}
-        self.credentials = None
         self.mountpoint = os.path.join(SR.MOUNT_BASE, 'SMB', self.__extract_server(), sr_uuid)
         self.linkpath = os.path.join(self.mountpoint,
                                            sr_uuid or "")
@@ -95,7 +95,7 @@ def checkmount(self):
 				util.ismount(self.mountpoint)) and \
                                 util.pathexists(self.linkpath)))
 
-    def mount(self, mountpoint=None):
+    def makeMountPoint(self, mountpoint):
         """Mount the remote SMB export at 'mountpoint'"""
         if mountpoint == None:
             mountpoint = self.mountpoint
@@ -108,27 +108,27 @@ def mount(self, mountpoint=None):
         except util.CommandException, inst:
             raise SMBException("Failed to make directory: code is %d" %
                                 inst.code)
+        return mountpoint
+
+    def mount(self, mountpoint=None):
+
+        mountpoint = self.makeMountPoint(mountpoint)
 
-        self.credentials = os.path.join("/tmp", util.gen_uuid())
+        new_env, domain = cifutils.getCIFCredentials(self.dconf, self.session, smbsr=True)
 
-        options = self.getMountOptions()
+        options = self.getMountOptions(domain)
         if options:
             options = ",".join(str(x) for x in options if x)
 
         try:
+
             util.ioretry(lambda:
                 util.pread(["mount.cifs", self.remoteserver,
-                mountpoint, "-o", options]),
+                mountpoint, "-o", options], new_env),
                 errlist=[errno.EPIPE, errno.EIO],
                 maxretry=2, nofail=True)
         except util.CommandException, inst:
             raise SMBException("mount failed with return code %d" % inst.code)
-        finally:
-            try:
-                os.unlink(self.credentials)
-            except OSError:
-                util.SMlog("Error when trying to delete "
-                           "credentials files /tmp/<uuid>")
 
         # Sanity check to ensure that the user has at least RO access to the
         # mounted share. Windows sharing and security settings can be tricky.
@@ -142,57 +142,18 @@ def mount(self, mountpoint=None):
             raise SMBException("Permission denied. "
                                 "Please check user privileges.")
 
-    def getMountOptions(self):
+    def getMountOptions(self, domain):
         """Creates option string based on parameters provided"""
         options = ['cache=loose',
                 'vers=3.0',
                 'actimeo=0'
         ]
 
-        if self.dconf.has_key('username') and \
-                (self.dconf.has_key('password') or
-                self.dconf.has_key('password_secret')):
-
-            dom_username = self.dconf['username'].split('\\')
-
-            if len(dom_username) == 1:
-                domain = None
-                username = dom_username[0]
-            elif len(dom_username) == 2:
-                domain = dom_username[0]
-                username = dom_username[1]
-            else:
-                raise SMBException("A maximum of 2 tokens are expected "
-                                   "(<domain>\<username>). {} were given."
-                                   .format(len(dom_username)))
-
-            domain = util.to_plain_string(domain)
-            username = util.to_plain_string(username)
-
-            if self.dconf.has_key('password_secret'):
-                password = util.get_secret(
-                           self.session,
-                           self.dconf['password_secret']
-            )
-            else:
-                password = self.dconf['password']
-
-            password = util.to_plain_string(password)
-
-            cred_str = 'username={}\npassword={}\n'.format(username, password)
-
-            if domain:
-                cred_str += 'domain={}\n'.format(domain)
-
-            # Open credentials file and truncate
-            try:
-                with open(self.credentials, 'w') as f:
-                    f.write(cred_str)
-            except IOError, e:
-                raise SMBException("Failed to create credentials file")
+        if domain:
+            options.append('domain='+domain)
 
-	    options.append('credentials=%s' % self.credentials)
-	else:
+        if not cifutils.containsCredentials(self.dconf, smbsr=True):
+            # No login details provided.
 	    options.append('guest')
 
         return options

drivers/cifutils.py

@@ -0,0 +1,84 @@
+#!/usr/bin/python
+#
+# Copyright (C) Citrix Systems Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License as published
+# by the Free Software Foundation; version 2.1 only.
+#
+# 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 Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser 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
+#
+# ISOSR: remote iso storage repository
+
+import util
+import xs_errors
+
+
+class CIFSException(Exception):
+    def __init__(self, errstr):
+        self.errstr = errstr
+
+def getDconfPasswordKey(smbsr=False):
+    key_password = 'cifspassword'
+    key_secret = 'cifspassword_secret'
+
+    if smbsr:
+        key_password = 'password'
+        key_secret = 'password_secret'
+    return key_password, key_secret
+
+def containsPassword(dconf, smbsr=False):
+    key_password, key_secret = getDconfPasswordKey(smbsr)
+    return ((key_password in dconf) or (key_secret in dconf))
+
+
+def containsCredentials(dconf, smbsr=False):
+    return ((('username' in dconf)) and (containsPassword(dconf, smbsr)))
+
+
+def splitDomainAndUsername(uname):
+
+    username = None
+    domain = None
+    dom_username = uname.split('\\')
+
+    if len(dom_username) == 1:
+        domain = None
+        username = dom_username[0]
+    elif len(dom_username) == 2:
+        domain = dom_username[0]
+        username = dom_username[1]
+    else:
+        raise CIFSException("A maximum of 2 tokens are expected "
+                            "(<domain>\<username>). {} were given."
+                            .format(len(dom_username)))
+    return username, domain
+
+
+def getCIFCredentials(dconf, session, smbsr=False):
+    credentials = {}
+    domain = None
+    if (containsCredentials(dconf, smbsr)):
+
+        username, domain = splitDomainAndUsername(dconf['username'])
+
+        credentials["USER"] = util.to_plain_string(username)
+
+        key_password, key_secret = getDconfPasswordKey(smbsr)
+        if key_secret in dconf:
+            password = util.get_secret(session, dconf[key_secret])
+        else:
+            password = dconf[key_password]
+
+        credentials["PASSWD"] = util.to_plain_string(password)
+
+        domain = util.to_plain_string(domain)
+
+    return credentials, domain

drivers/util.py

@@ -130,9 +130,11 @@ def _getDateString():
     return "%s-%s-%s:%s:%s:%s" % \
           (t[0],t[1],t[2],t[3],t[4],t[5])
 
-def doexec(args, inputtext=None):
+def doexec(args, inputtext=None, new_env={}):
     """Execute a subprocess, then return its return code, stdout and stderr"""
-    proc = subprocess.Popen(args,stdin=subprocess.PIPE,stdout=subprocess.PIPE,stderr=subprocess.PIPE,close_fds=True)
+    env=dict(os.environ)
+    env.update(new_env)
+    proc = subprocess.Popen(args,stdin=subprocess.PIPE,stdout=subprocess.PIPE,stderr=subprocess.PIPE,close_fds=True, env=env)
     (stdout,stderr) = proc.communicate(inputtext)
     # Workaround for a pylint bug, can be removed after upgrade to
     # python 3.x or maybe a newer version of pylint in the future
@@ -152,7 +154,7 @@ def is_string(value):
 # each pair, the first component is passed to exec while the second is
 # written to the logs.
 def pread(cmdlist, close_stdin = False, scramble = None, expect_rc = 0,
-        quiet = False):
+        quiet = False, new_env = {}):
     cmdlist_for_exec = []
     cmdlist_for_log = []
     for item in cmdlist:
@@ -171,7 +173,7 @@ def pread(cmdlist, close_stdin = False, scramble = None, expect_rc = 0,
 
     if not quiet:
         SMlog(cmdlist_for_log)
-    (rc,stdout,stderr) = doexec(cmdlist_for_exec)
+    (rc,stdout,stderr) = doexec(cmdlist_for_exec, new_env=new_env)
     if rc != expect_rc:
         SMlog("FAILED in util.pread: (rc %d) stdout: '%s', stderr: '%s'" % \
                 (rc, stdout, stderr))

mk/sm.spec.in

@@ -262,6 +262,9 @@ tests/run_python_unittests.sh
 /opt/xensource/sm/mpathutil.py
 /opt/xensource/sm/mpathutil.pyc
 /opt/xensource/sm/mpathutil.pyo
+/opt/xensource/sm/cifutils.py
+/opt/xensource/sm/cifutils.pyc
+/opt/xensource/sm/cifutils.pyo
 /opt/xensource/sm/mpp_mpathutil.py
 /opt/xensource/sm/mpp_mpathutil.pyc
 /opt/xensource/sm/mpp_mpathutil.pyo