platform_Cryptohome, cryptohome.py: kill c-homed on dbus silence
Move platform_Cryptohome over to using the CryptohomeProxy and
ensure that any unreplied DBus call results in cryptohomed being
ABRT'd with the hope that we'll get a crash dump on daisy flake.
As we discover blocking operations, we can move them off the main
thread.
TEST=passes; kills -ABRT if cryptohomed gets a SIGSTOP during the test (as a proof of hang detection)
BUG=chromium:303677
Change-Id: Ia40b0120f4f7a918c8288bf65b5c251fb0671223
Reviewed-on: https://chromium-review.googlesource.com/178881
Reviewed-by: Kees Cook <keescook@chromium.org>
Commit-Queue: Will Drewry <wad@chromium.org>
Tested-by: Will Drewry <wad@chromium.org>
diff --git a/client/cros/cryptohome.py b/client/cros/cryptohome.py
index 8948827..bd4c0c6 100644
--- a/client/cros/cryptohome.py
+++ b/client/cros/cryptohome.py
@@ -2,7 +2,7 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
-import dbus, logging, os, random, re, shutil, string
+import dbus, logging, os, random, re, shutil, string, time
import common, constants
from autotest_lib.client.bin import utils
@@ -15,12 +15,10 @@
"""Generic error for ChromiumOS-specific exceptions."""
pass
-
def __run_cmd(cmd):
return utils.system_output(cmd + ' 2>&1', retain_output=True,
ignore_status=True).strip()
-
def get_user_hash(user):
"""Get the user hash for the given user."""
return utils.system_output(['cryptohome', '--action=obfuscate_user',
@@ -268,6 +266,11 @@
return '@'.join([name, domain]).lower()
+def crash_cryptohomed():
+ # Try to kill cryptohomed so we get something to work with.
+ utils.system('pkill -ABRT cryptohomed')
+ time.sleep(2) # Give it 2 seconds to dump
+
class CryptohomeProxy:
def __init__(self):
BUSNAME = 'org.chromium.Cryptohome'
@@ -277,6 +280,17 @@
obj = bus.get_object(BUSNAME, PATH)
self.iface = dbus.Interface(obj, INTERFACE)
+ # Wrap all proxied calls to catch cryptohomed failures.
+ def __call(self, method, *args):
+ try:
+ return method(*args)
+ except dbus.exceptions.DBusException, e:
+ if e.get_dbus_name() == 'org.freedesktop.DBus.Error.NoReply':
+ logging.error('Cryptohome is not responding. Sending ABRT')
+ crash_cryptohomed()
+ raise ChromiumOSError('cryptohomed aborted. Check crashes!')
+ raise e
+
def mount(self, user, password, create=False):
"""Mounts a cryptohome.
@@ -284,7 +298,10 @@
TODO(ellyjones): Migrate mount_vault() to use a multi-user-safe
heuristic, then remove this method. See <crosbug.com/20778>.
"""
- return self.iface.Mount(user, password, create, False, [])[1]
+ out = self.__call(self.iface.Mount, user, password, create, False, [])
+ if not out:
+ return out
+ return out[1]
def unmount(self, user):
"""Unmounts a cryptohome.
@@ -293,7 +310,7 @@
TODO(ellyjones): Once there's a per-user unmount method, use it. See
<crosbug.com/20778>.
"""
- return self.iface.Unmount()
+ return self.__call(self.iface.Unmount)
def is_mounted(self, user):
"""Tests whether a user's cryptohome is mounted."""
@@ -307,7 +324,7 @@
def migrate(self, user, oldkey, newkey):
"""Migrates the specified user's cryptohome from one key to another."""
- return self.iface.MigrateKey(user, oldkey, newkey)
+ return self.__call(self.iface.MigrateKey, user, oldkey, newkey)
def remove(self, user):
- return self.iface.Remove(user)
+ return self.__call(self.iface.Remove, user)
diff --git a/client/site_tests/platform_CryptohomeMount/platform_CryptohomeMount.py b/client/site_tests/platform_CryptohomeMount/platform_CryptohomeMount.py
index fbb2107..88c2ca0 100644
--- a/client/site_tests/platform_CryptohomeMount/platform_CryptohomeMount.py
+++ b/client/site_tests/platform_CryptohomeMount/platform_CryptohomeMount.py
@@ -1,72 +1,43 @@
# Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
-
-import logging
-import os
-import re
-import shutil
-
from autotest_lib.client.bin import test
-from autotest_lib.client.common_lib import error, utils
-from autotest_lib.client.cros import constants
+from autotest_lib.client.common_lib import error
+from autotest_lib.client.cros import cryptohome
class platform_CryptohomeMount(test.test):
version = 1
- def __run_cmd(self, cmd):
- result = utils.system_output(cmd + ' 2>&1', retain_output=True,
- ignore_status=True)
- return result
-
-
def run_once(self):
test_user = 'this_is_a_local_test_account@chromium.org';
test_password = 'this_is_a_test_password';
# Get the hash for the test user account
- cmd = ('/usr/sbin/cryptohome --action=obfuscate_user --user='
- + test_user)
- user_hash = self.__run_cmd(cmd).strip()
+ user_hash = cryptohome.get_user_hash(test_user)
+ proxy = cryptohome.CryptohomeProxy()
# Remove the test user account
- cmd = ('/usr/sbin/cryptohome --action=remove --force --user='
- + test_user)
- self.__run_cmd(cmd)
- # Ensure that the user directory does not exist
- if os.path.exists(os.path.join(constants.SHADOW_ROOT, user_hash)):
- raise error.TestFail('Cryptohome could not remove the test user.')
+ proxy.remove(test_user)
# Mount the test user account
- cmd = ('/usr/sbin/cryptohome --async --action=mount --user=' + test_user
- + ' --password=' + test_password + ' --create')
- self.__run_cmd(cmd)
- # Ensure that the user directory exists
- if not os.path.exists(os.path.join(constants.SHADOW_ROOT, user_hash)):
- raise error.TestFail('Cryptohome could not create the test user.')
- # Ensure that the user directory is mounted
- cmd = ('/usr/sbin/cryptohome --action=is_mounted')
- if (self.__run_cmd(cmd).strip() == 'false'):
- raise error.TestFail('Cryptohome created the user but did not mount.')
+ if not proxy.mount(test_user, test_password, create=True):
+ raise error.TestFail('Failed to create and mount the test user')
# Unmount the directory
- cmd = ('/usr/sbin/cryptohome --action=unmount')
- self.__run_cmd(cmd)
+ if not proxy.unmount(test_user):
+ raise error.TestFail('Failed to unmount test user')
+
# Ensure that the user directory is not mounted
- cmd = ('/usr/sbin/cryptohome --action=is_mounted')
- if (self.__run_cmd(cmd).strip() != 'false'):
- raise error.TestFail('Cryptohome did not unmount the user.')
+ if proxy.is_mounted(test_user):
+ raise error.TestFail('Cryptohome mounted after unmount!')
# Make sure that an incorrect password fails
incorrect_password = 'this_is_an_incorrect_password'
- cmd = ('/usr/sbin/cryptohome --async --action=mount --user=' + test_user
- + ' --password=' + incorrect_password)
- self.__run_cmd(cmd)
- # Ensure that the user directory is not mounted
- cmd = ('/usr/sbin/cryptohome --action=is_mounted')
- if (self.__run_cmd(cmd).strip() != 'false'):
+ if proxy.mount(test_user, incorrect_password):
raise error.TestFail('Cryptohome mounted with a bad password.')
+ # Ensure that the user directory is not mounted
+ if proxy.is_mounted(test_user):
+ raise error.TestFail('Cryptohome mounted even though mount() failed')
# Remove the test user account
- cmd = ('/usr/sbin/cryptohome --action=remove --force --user='
- + test_user)
- self.__run_cmd(cmd)
+ if not proxy.remove(test_user):
+ raise error.TestFail('Cryptohome could not clean up vault')
