password: Prevent password logging and persisting

Vdsm should not expose passwords in log messages. We have code in HSM,
protecting some methods that accept connection lists, which may include
a password parameter. However this is not the right place to protect
passwords, as they are logged now by the jsonrpc server.

This patch introduces the password library, providing:

  ProtectedPassword     class for wrapping password value, so it is not
                        logged or persisted by mistake.

  protect_passwords     replace passwords value with ProtectedPassword
                        object in nested structures.

  unprotect_passwords   replace ProtectedPassword objects with the
                        actual password value.

The jsonprc server protect passwords in request parameters, and
unprotect passwords in response result.  This is safest and simplest
way, as we cannot forget to protect a method.

The xmlrpc server handle protecting and unprotecting in the method
level, since this is the only place where we can detect a password
parameter.

Passwords read from iscsi session and libvirt password file are also
protected when they enter the application.

Code that needs to access protected password value must access the
password.value attribute.

Note that xmlrpc server has incomplete and fragile password protection
for non-irs methods (see wrapApiMethod). Fixing this needs more work.

Change-Id: Icc849ad8bdc1b1fd09884e18038427d96e66110f
Bug-Url: https://bugzilla.redhat.com/1220039
Signed-off-by: Nir Soffer <nsoffer@redhat.com>
Reviewed-on: https://gerrit.ovirt.org/40707
Reviewed-by: Piotr Kliczewski <piotr.kliczewski@gmail.com>
Reviewed-by: Francesco Romani <fromani@redhat.com>
Reviewed-by: Dan Kenigsberg <danken@redhat.com>
Continuous-Integration: Jenkins CI

debian/vdsm-python.install

@@ -18,6 +18,7 @@
 ./usr/lib/python2.7/dist-packages/vdsm/netlink/link.py
 ./usr/lib/python2.7/dist-packages/vdsm/netlink/monitor.py
 ./usr/lib/python2.7/dist-packages/vdsm/netlink/route.py
+./usr/lib/python2.7/dist-packages/vdsm/password.py
 ./usr/lib/python2.7/dist-packages/vdsm/profiling/__init__.py
 ./usr/lib/python2.7/dist-packages/vdsm/profiling/cpu.py
 ./usr/lib/python2.7/dist-packages/vdsm/profiling/errors.py

lib/vdsm/Makefile.am

@@ -33,6 +33,7 @@ dist_vdsmpylib_PYTHON = \
 	libvirtconnection.py \
 	netconfpersistence.py \
 	netinfo.py \
+	password.py \
 	qemuimg.py \
 	response.py \
 	schedule.py \

lib/vdsm/libvirtconnection.py

@@ -27,6 +27,7 @@
 
 import libvirt
 from . import utils
+from .password import ProtectedPassword
 from .tool.configurators import passwd
 
 log = logging.getLogger()
@@ -87,7 +88,7 @@ def req(credentials, user_data):
             if cred[0] == libvirt.VIR_CRED_AUTHNAME:
                 cred[4] = username
             elif cred[0] == libvirt.VIR_CRED_PASSPHRASE:
-                cred[4] = passwd
+                cred[4] = passwd.value if passwd else None
         return 0
 
     auth = [[libvirt.VIR_CRED_AUTHNAME, libvirt.VIR_CRED_PASSPHRASE],
@@ -156,8 +157,9 @@ def wrapper(*args, **kwargs):
         conn = __connections.get(id(target))
         if not conn:
             log.debug('trying to connect libvirt')
+            password = ProtectedPassword(passwd.libvirt_password())
             conn = open_connection('qemu:///system', passwd.SASL_USERNAME,
-                                   passwd.libvirt_password())
+                                   password)
             __connections[id(target)] = conn
 
             setattr(conn, 'pingLibvirt', getattr(conn, 'getLibVersion'))

lib/vdsm/password.py

@@ -0,0 +1,80 @@
+#
+# Copyright 2015 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
+#
+
+
+class ProtectedPassword(object):
+    """
+    Protect a password so it will not be logged or serialized by mistake.
+    """
+    def __init__(self, value):
+        self.value = value
+
+    def __eq__(self, other):
+        return type(self) == type(other) and self.value == other.value
+
+    def __ne__(self, other):
+        return not self.__eq__(other)
+
+    def __getstate__(self):
+        """ Do not serialize the the real password """
+        raise TypeError("ProtectedPassword object cannot be pickled")
+
+    def __str__(self):
+        return "********"
+
+    def __repr__(self):
+        return repr(str(self))
+
+
+def protect_passwords(obj):
+    """
+    Replace "password" values with ProtectedPassword() object.
+
+    Accept a dict, list of dicts or nested structure containing these types.
+    """
+    for d, key, value in _walk(obj):
+        d[key] = ProtectedPassword(value)
+    return obj
+
+
+def unprotect_passwords(obj):
+    """
+    Replace ProtectedPassword() objects with the actual password value.
+
+    Accept a dict, list of dicts or nested structure containing these types.
+    """
+    for d, key, value in _walk(obj):
+        if isinstance(value, ProtectedPassword):
+            d[key] = value.value
+    return obj
+
+
+def _walk(obj):
+    if isinstance(obj, dict):
+        for key, value in obj.iteritems():
+            if key == "password":
+                yield obj, key, value
+            elif isinstance(value, (dict, list)):
+                for d, k, v in _walk(value):
+                    yield d, k, v
+    elif isinstance(obj, list):
+        for item in obj:
+            for d, k, v in _walk(item):
+                yield d, k, v

lib/yajsonrpc/__init__.py

@@ -20,6 +20,7 @@
 
 from vdsm.compat import json
 
+from vdsm.password import protect_passwords, unprotect_passwords
 from vdsm.utils import traceback
 
 __all__ = ["betterAsyncore", "stompReactor", "stomp"]
@@ -78,7 +79,7 @@ def __init__(self, msg=None):
 class JsonRpcRequest(object):
     def __init__(self, method, params=(), reqId=None):
         self.method = method
-        self.params = params
+        self.params = protect_passwords(params)
         self.id = reqId
 
     @classmethod
@@ -128,7 +129,7 @@ def isNotification(self):
 
 class JsonRpcResponse(object):
     def __init__(self, result=None, error=None, reqId=None):
-        self.result = result
+        self.result = unprotect_passwords(result)
         self.error = error
         self.id = reqId
 

tests/Makefile.am

@@ -74,6 +74,7 @@ test_modules = \
 	numaUtilsTests.py \
 	outOfProcessTests.py \
 	parted_utils_tests.py \
+	passwordsTests.py \
 	periodicTests.py \
 	permutationTests.py \
 	persistentDictTests.py \

tests/passwordsTests.py

@@ -0,0 +1,216 @@
+#
+# Copyright 2015 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 marshal
+from testlib import VdsmTestCase
+from testlib import expandPermutations, permutations
+from vdsm.compat import pickle, json
+from vdsm.password import (ProtectedPassword,
+                           protect_passwords,
+                           unprotect_passwords)
+
+
+class ProtectedPasswordTests(VdsmTestCase):
+
+    def test_str(self):
+        p = ProtectedPassword("12345678")
+        self.assertNotIn("12345678", str(p))
+
+    def test_repr(self):
+        p = ProtectedPassword("12345678")
+        self.assertNotIn("12345678", repr(p))
+
+    def test_value(self):
+        p = ProtectedPassword("12345678")
+        self.assertEqual("12345678", p.value)
+
+    def test_eq(self):
+        p1 = ProtectedPassword("12345678")
+        p2 = ProtectedPassword("12345678")
+        self.assertEqual(p1, p2)
+
+    def test_ne(self):
+        p1 = ProtectedPassword("12345678")
+        p2 = ProtectedPassword("12345678")
+        self.assertFalse(p1 != p2)
+
+    def test_no_pickle(self):
+        p1 = ProtectedPassword("12345678")
+        self.assertRaises(TypeError, pickle.dumps, p1)
+
+    def test_no_marshal(self):
+        p1 = ProtectedPassword("12345678")
+        self.assertRaises(ValueError, marshal.dumps, p1)
+
+    def test_no_json(self):
+        p1 = ProtectedPassword("12345678")
+        self.assertRaises(TypeError, json.dumps, p1)
+
+
+@expandPermutations
+class ProtectTests(VdsmTestCase):
+
+    @permutations([[list()], [dict()], [tuple()]])
+    def test_protect_empty(self, params):
+        self.assertEqual(params, protect_passwords(params))
+
+    @permutations([[list()], [dict()], [tuple()]])
+    def test_unprotect_empty(self, result):
+        self.assertEqual(result, unprotect_passwords(result))
+
+    def test_protect_dict(self):
+        unprotected = dict_unprotedted()
+        protected = dict_protected()
+        self.assertEqual(protected, protect_passwords(unprotected))
+
+    def test_unprotect_dict(self):
+        protected = dict_protected()
+        unprotected = dict_unprotedted()
+        self.assertEqual(unprotected, unprotect_passwords(protected))
+
+    def test_protect_nested_dicts(self):
+        unprotected = nested_dicts_unprotected()
+        protected = nested_dicts_protected()
+        self.assertEqual(protected, protect_passwords(unprotected))
+
+    def test_unprotect_nested_dicts(self):
+        protected = nested_dicts_protected()
+        unprotected = nested_dicts_unprotected()
+        self.assertEqual(unprotected, unprotect_passwords(protected))
+
+    def test_protect_lists_of_dicts(self):
+        unprotected = lists_of_dicts_unprotected()
+        protected = lists_of_dicts_protected()
+        self.assertEqual(protected, protect_passwords(unprotected))
+
+    def test_unprotect_lists_of_dicts(self):
+        protected = lists_of_dicts_protected()
+        unprotected = lists_of_dicts_unprotected()
+        self.assertEqual(unprotected, unprotect_passwords(protected))
+
+    def test_protect_nested_lists_of_dicts(self):
+        unprotected = nested_lists_of_dicts_unprotected()
+        protected = nested_lists_of_dicts_protected()
+        self.assertEqual(protected, protect_passwords(unprotected))
+
+    def test_unprotect_nested_lists_of_dicts(self):
+        protected = nested_lists_of_dicts_protected()
+        unprotected = nested_lists_of_dicts_unprotected()
+        self.assertEqual(unprotected, unprotect_passwords(protected))
+
+
+def dict_unprotedted():
+    return {
+        "key": "value",
+        "password": "12345678"
+    }
+
+
+def dict_protected():
+    return {
+        "key": "value",
+        "password": ProtectedPassword("12345678")
+    }
+
+
+def nested_dicts_unprotected():
+    return {
+        "key": "value",
+        "nested": {
+            "password": "12345678",
+            "nested": {
+                "key": "value",
+                "password": "87654321",
+            }
+        }
+    }
+
+
+def nested_dicts_protected():
+    return {
+        "key": "value",
+        "nested": {
+            "password": ProtectedPassword("12345678"),
+            "nested": {
+                "key": "value",
+                "password": ProtectedPassword("87654321"),
+            }
+        }
+    }
+
+
+def lists_of_dicts_unprotected():
+    return [
+        {
+            "key": "value",
+            "password": "12345678",
+        },
+        {
+            "key": "value",
+            "password": "87654321",
+        }
+    ]
+
+
+def lists_of_dicts_protected():
+    return [
+        {
+            "key": "value",
+            "password": ProtectedPassword("12345678"),
+        },
+        {
+            "key": "value",
+            "password": ProtectedPassword("87654321"),
+        }
+    ]
+
+
+def nested_lists_of_dicts_unprotected():
+    return {
+        "key": "value",
+        "nested": [
+            {
+                "key": "value",
+                "nested": [
+                    {
+                        "key": "value",
+                        "password": "12345678",
+                    }
+                ]
+            }
+        ]
+    }
+
+
+def nested_lists_of_dicts_protected():
+    return {
+        "key": "value",
+        "nested": [
+            {
+                "key": "value",
+                "nested": [
+                    {
+                        "key": "value",
+                        "password": ProtectedPassword("12345678"),
+                    }
+                ]
+            }
+        ]
+    }

tests/samplingTests.py

@@ -27,6 +27,7 @@
 import threading
 
 from vdsm import ipwrapper
+from vdsm.password import ProtectedPassword
 import virt.sampling as sampling
 
 from testValidation import brokentest, ValidateRunningAsRoot
@@ -135,7 +136,7 @@ def vlan(name, link, vlan_id):
 
 
 def read_password():
-    return 'password'
+    return ProtectedPassword('password')
 
 
 class InterfaceSampleTests(TestCaseBase):