login: Abstract out the interface to crossystem.
This is required for SessionManagerImpl::UpdateSystemSettings() unit
tests which will be added in a separate CL.
BUG=chrome-os-partner:50142
TEST=verified that block_devmode is still updated on policy changes
Change-Id: I11fe94b14d6e1bf9d98d8e0920fea48c0c12588f
Reviewed-on: https://chromium-review.googlesource.com/328993
Commit-Ready: Thiemo Nagel <tnagel@chromium.org>
Tested-by: Thiemo Nagel <tnagel@chromium.org>
Reviewed-by: Thiemo Nagel <tnagel@chromium.org>
diff --git a/login_manager/crossystem.cc b/login_manager/crossystem.cc
new file mode 100644
index 0000000..8c7ab2f
--- /dev/null
+++ b/login_manager/crossystem.cc
@@ -0,0 +1,10 @@
+// Copyright 2016 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.
+
+#include "login_manager/crossystem.h"
+
+#include <vboot/crossystem.h>
+
+static_assert(Crossystem::kVbMaxStringProperty == VB_MAX_STRING_PROPERTY,
+ "VB_MAX_STRING_PROPERTY got out of sync!");
diff --git a/login_manager/crossystem.h b/login_manager/crossystem.h
new file mode 100644
index 0000000..2ace3a1
--- /dev/null
+++ b/login_manager/crossystem.h
@@ -0,0 +1,50 @@
+// Copyright 2016 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.
+
+#ifndef LOGIN_MANAGER_CROSSYSTEM_H_
+#define LOGIN_MANAGER_CROSSYSTEM_H_
+
+#include <cstddef>
+
+// Light-weight interface to crossystem keeping the original C semantics to make
+// it an easy drop-in replacement. (C++ semantics may be added in the future.)
+class Crossystem {
+ public:
+ // Recommended size for string property buffers used with
+ // VbGetSystemPropertyString().
+ static const std::size_t kVbMaxStringProperty = 8192;
+
+ // Reads a system property integer.
+ //
+ // Returns the property value, or -1 if error.
+ virtual int VbGetSystemPropertyInt(const char* name) = 0;
+
+ // Sets a system property integer.
+ //
+ // Returns 0 if success, -1 if error.
+ virtual int VbSetSystemPropertyInt(const char* name, int value) = 0;
+
+ // Reads a system property string into a destination buffer of the specified
+ // size. Returned string will be null-terminated. If the buffer is too
+ // small, the returned string will be truncated.
+ //
+ // The caller can expect an un-truncated value if the size provided is at
+ // least kVbMaxStringProperty.
+ //
+ // Returns the passed buffer, or nullptr if error.
+ virtual const char* VbGetSystemPropertyString(const char* name,
+ char* dest,
+ std::size_t size) = 0;
+
+ // Sets a system property string.
+ //
+ // The maximum length of the value accepted depends on the specific
+ // property, not on kVbMaxStringProperty.
+ //
+ // Returns 0 if success, -1 if error.
+ virtual int VbSetSystemPropertyString(const char* name,
+ const char* value) = 0;
+};
+
+#endif // LOGIN_MANAGER_CROSSYSTEM_H_
diff --git a/login_manager/crossystem_impl.cc b/login_manager/crossystem_impl.cc
new file mode 100644
index 0000000..5529548
--- /dev/null
+++ b/login_manager/crossystem_impl.cc
@@ -0,0 +1,26 @@
+// Copyright 2016 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.
+
+#include "login_manager/crossystem_impl.h"
+
+#include <vboot/crossystem.h>
+
+int CrossystemImpl::VbGetSystemPropertyInt(const char* name) {
+ return ::VbGetSystemPropertyInt(name);
+}
+
+int CrossystemImpl::VbSetSystemPropertyInt(const char* name, int value) {
+ return ::VbSetSystemPropertyInt(name, value);
+}
+
+const char* CrossystemImpl::VbGetSystemPropertyString(const char* name,
+ char* dest,
+ std::size_t size) {
+ return ::VbGetSystemPropertyString(name, dest, size);
+}
+
+int CrossystemImpl::VbSetSystemPropertyString(const char* name,
+ const char* value) {
+ return ::VbSetSystemPropertyString(name, value);
+}
diff --git a/login_manager/crossystem_impl.h b/login_manager/crossystem_impl.h
new file mode 100644
index 0000000..23860dc
--- /dev/null
+++ b/login_manager/crossystem_impl.h
@@ -0,0 +1,27 @@
+// Copyright 2016 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.
+
+#ifndef LOGIN_MANAGER_CROSSYSTEM_IMPL_H_
+#define LOGIN_MANAGER_CROSSYSTEM_IMPL_H_
+
+#include "login_manager/crossystem.h"
+
+class CrossystemImpl : public Crossystem {
+ public:
+ // Reads a system property integer.
+ int VbGetSystemPropertyInt(const char* name);
+
+ // Sets a system property integer.
+ int VbSetSystemPropertyInt(const char* name, int value);
+
+ // Reads a system property string into a destination buffer.
+ const char* VbGetSystemPropertyString(const char* name,
+ char* dest,
+ std::size_t size);
+
+ // Sets a system property string.
+ int VbSetSystemPropertyString(const char* name, const char* value);
+};
+
+#endif // LOGIN_MANAGER_CROSSYSTEM_IMPL_H_
diff --git a/login_manager/fake_crossystem.cc b/login_manager/fake_crossystem.cc
new file mode 100644
index 0000000..a58c93c
--- /dev/null
+++ b/login_manager/fake_crossystem.cc
@@ -0,0 +1,33 @@
+// Copyright 2016 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.
+
+#include "login_manager/fake_crossystem.h"
+
+int FakeCrossystem::VbGetSystemPropertyInt(const char* name) {
+ if (int_map_.find(name) == int_map_.end())
+ return -1;
+
+ return int_map_[name];
+}
+
+int FakeCrossystem::VbSetSystemPropertyInt(const char* name, int value) {
+ int_map_[name] = value;
+ return 0;
+}
+
+const char* FakeCrossystem::VbGetSystemPropertyString(const char* name,
+ char* dest,
+ std::size_t size) {
+ if (string_map_.find(name) == string_map_.end())
+ return nullptr;
+
+ string_map_[name].copy(dest, size);
+ return dest;
+}
+
+int FakeCrossystem::VbSetSystemPropertyString(const char* name,
+ const char* value) {
+ string_map_[name] = std::string(value);
+ return 0;
+}
diff --git a/login_manager/fake_crossystem.h b/login_manager/fake_crossystem.h
new file mode 100644
index 0000000..049baca
--- /dev/null
+++ b/login_manager/fake_crossystem.h
@@ -0,0 +1,27 @@
+// Copyright 2016 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.
+
+#ifndef LOGIN_MANAGER_FAKE_CROSSYSTEM_H_
+#define LOGIN_MANAGER_FAKE_CROSSYSTEM_H_
+
+#include <map>
+#include <string>
+
+#include "login_manager/crossystem.h"
+
+class FakeCrossystem : public Crossystem {
+ public:
+ int VbGetSystemPropertyInt(const char* name);
+ int VbSetSystemPropertyInt(const char* name, int value);
+ const char* VbGetSystemPropertyString(const char* name,
+ char* dest,
+ std::size_t size);
+ int VbSetSystemPropertyString(const char* name, const char* value);
+
+ private:
+ std::map<std::string, int> int_map_;
+ std::map<std::string, std::string> string_map_;
+};
+
+#endif // LOGIN_MANAGER_FAKE_CROSSYSTEM_H_
diff --git a/login_manager/login_manager.gyp b/login_manager/login_manager.gyp
index 6be5c5b..29442da 100644
--- a/login_manager/login_manager.gyp
+++ b/login_manager/login_manager.gyp
@@ -56,6 +56,8 @@
'child_exit_handler.cc',
'child_job.cc',
'chrome_setup.cc',
+ 'crossystem.cc',
+ 'crossystem_impl.cc',
'dbus_signal_emitter.cc',
'device_local_account_policy_service.cc',
'device_policy_service.cc',
@@ -121,6 +123,7 @@
'device_policy_service_unittest.cc',
'fake_browser_job.cc',
'fake_child_process.cc',
+ 'fake_crossystem.cc',
'fake_generated_key_handler.cc',
'fake_generator_job.cc',
'keygen_worker.cc',
diff --git a/login_manager/session_manager_impl.cc b/login_manager/session_manager_impl.cc
index 2241c49..6d5e41b 100644
--- a/login_manager/session_manager_impl.cc
+++ b/login_manager/session_manager_impl.cc
@@ -22,10 +22,10 @@
#include <brillo/cryptohome.h>
#include <crypto/scoped_nss_types.h>
#include <dbus/message.h>
-#include <vboot/crossystem.h>
#include "bindings/chrome_device_policy.pb.h"
#include "bindings/device_management_backend.pb.h"
+#include "login_manager/crossystem.h"
#include "login_manager/dbus_error_types.h"
#include "login_manager/dbus_signal_emitter.h"
#include "login_manager/device_local_account_policy_service.h"
@@ -149,7 +149,8 @@
ProcessManagerServiceInterface* manager,
LoginMetrics* metrics,
NssUtil* nss,
- SystemUtils* utils)
+ SystemUtils* utils,
+ Crossystem* crossystem)
: session_started_(false),
session_stopping_(false),
screen_locked_(false),
@@ -165,6 +166,7 @@
login_metrics_(metrics),
nss_(nss),
system_(utils),
+ crossystem_(crossystem),
owner_key_(nss->GetOwnerKeyFilePath(), nss),
mitigator_(key_gen) {
}
@@ -741,19 +743,20 @@
return;
// Only write verified boot settings if running on Chrome OS firmware.
- char buffer[VB_MAX_STRING_PROPERTY];
- if (VbGetSystemPropertyString(kCrossystemMainfwType, buffer,
- sizeof(buffer)) &&
+ char buffer[Crossystem::kVbMaxStringProperty];
+ if (crossystem_->VbGetSystemPropertyString(kCrossystemMainfwType, buffer,
+ sizeof(buffer)) &&
strcmp(kCrossystemMainfwTypeNonchrome, buffer)) {
int block_devmode_setting =
device_policy_->GetSettings().system_settings().block_devmode() ? 1 : 0;
- int block_devmode_value = VbGetSystemPropertyInt(kCrossystemBlockDevmode);
+ int block_devmode_value =
+ crossystem_->VbGetSystemPropertyInt(kCrossystemBlockDevmode);
if (block_devmode_value == -1)
LOG(ERROR) << "Failed to read block_devmode flag!";
if (block_devmode_setting != block_devmode_value) {
- if (VbSetSystemPropertyInt(kCrossystemBlockDevmode,
- block_devmode_setting)) {
+ if (crossystem_->VbSetSystemPropertyInt(kCrossystemBlockDevmode,
+ block_devmode_setting)) {
LOG(ERROR) << "Failed to write block_devmode flag!";
}
}
diff --git a/login_manager/session_manager_impl.h b/login_manager/session_manager_impl.h
index 6470f9d..2213d70 100644
--- a/login_manager/session_manager_impl.h
+++ b/login_manager/session_manager_impl.h
@@ -23,6 +23,8 @@
#include "login_manager/regen_mitigator.h"
#include "login_manager/server_backed_state_key_generator.h"
+class Crossystem;
+
namespace login_manager {
class DBusSignalEmitterInterface;
class DeviceLocalAccountPolicyService;
@@ -86,7 +88,8 @@
ProcessManagerServiceInterface* manager,
LoginMetrics* metrics,
NssUtil* nss,
- SystemUtils* utils);
+ SystemUtils* utils,
+ Crossystem* crossystem);
virtual ~SessionManagerImpl();
void InjectPolicyServices(
@@ -228,6 +231,7 @@
LoginMetrics* login_metrics_; // Owned by the caller.
NssUtil* nss_; // Owned by the caller.
SystemUtils* system_; // Owned by the caller.
+ Crossystem* crossystem_; // Owned by the caller.
scoped_ptr<DevicePolicyService> device_policy_;
scoped_ptr<UserPolicyServiceFactory> user_policy_factory_;
diff --git a/login_manager/session_manager_impl_unittest.cc b/login_manager/session_manager_impl_unittest.cc
index 8158c30..b11cb77 100644
--- a/login_manager/session_manager_impl_unittest.cc
+++ b/login_manager/session_manager_impl_unittest.cc
@@ -30,6 +30,7 @@
#include "bindings/device_management_backend.pb.h"
#include "login_manager/dbus_error_types.h"
#include "login_manager/device_local_account_policy_service.h"
+#include "login_manager/fake_crossystem.h"
#include "login_manager/file_checker.h"
#include "login_manager/matchers.h"
#include "login_manager/mock_dbus_signal_emitter.h"
@@ -87,7 +88,8 @@
&manager_,
&metrics_,
&nss_,
- &utils_),
+ &utils_,
+ &crossystem_),
fake_salt_("fake salt"),
actual_locks_(0),
expected_locks_(0),
@@ -196,6 +198,7 @@
MockMetrics metrics_;
MockNssUtil nss_;
MockSystemUtils utils_;
+ FakeCrossystem crossystem_;
SessionManagerImpl impl_;
SessionManagerImpl::Error error_;
diff --git a/login_manager/session_manager_main.cc b/login_manager/session_manager_main.cc
index 4474d37..84557b0 100644
--- a/login_manager/session_manager_main.cc
+++ b/login_manager/session_manager_main.cc
@@ -33,6 +33,7 @@
#include "login_manager/browser_job.h"
#include "login_manager/chrome_setup.h"
+#include "login_manager/crossystem_impl.h"
#include "login_manager/file_checker.h"
#include "login_manager/login_metrics.h"
#include "login_manager/regen_mitigator.h"
@@ -152,6 +153,7 @@
// Shim that wraps system calls, file system ops, etc.
SystemUtilsImpl system;
+ CrossystemImpl crossystem;
// Checks magic file that causes the session_manager to stop managing the
// browser process. Devs and tests can use this to keep the session_manager
@@ -204,7 +206,8 @@
enable_hang_detection,
base::TimeDelta::FromSeconds(hang_detection_interval),
&metrics,
- &system);
+ &system,
+ &crossystem);
if (manager->Initialize()) {
// Allows devs to start/stop browser manually.
diff --git a/login_manager/session_manager_process_unittest.cc b/login_manager/session_manager_process_unittest.cc
index 6dfcbc9..3a51011 100644
--- a/login_manager/session_manager_process_unittest.cc
+++ b/login_manager/session_manager_process_unittest.cc
@@ -22,6 +22,7 @@
#include "login_manager/browser_job.h"
#include "login_manager/fake_browser_job.h"
#include "login_manager/fake_child_process.h"
+#include "login_manager/fake_crossystem.h"
#include "login_manager/fake_generator_job.h"
#include "login_manager/mock_device_policy_service.h"
#include "login_manager/mock_file_checker.h"
@@ -112,7 +113,8 @@
false,
base::TimeDelta(),
&metrics_,
- &real_utils_);
+ &real_utils_,
+ &crossystem_);
manager_->test_api().set_liveness_checker(liveness_checker_);
manager_->test_api().set_session_manager(session_manager_impl_);
}
@@ -143,6 +145,7 @@
scoped_refptr<SessionManagerService> manager_;
SystemUtilsImpl real_utils_;
+ FakeCrossystem crossystem_;
MockMetrics metrics_;
MockSystemUtils utils_;
diff --git a/login_manager/session_manager_service.cc b/login_manager/session_manager_service.cc
index ae568d3..4027cc5 100644
--- a/login_manager/session_manager_service.cc
+++ b/login_manager/session_manager_service.cc
@@ -31,6 +31,7 @@
#include "login_manager/browser_job.h"
#include "login_manager/child_exit_handler.h"
+#include "login_manager/crossystem_impl.h"
#include "login_manager/dbus_signal_emitter.h"
#include "login_manager/key_generator.h"
#include "login_manager/liveness_checker_impl.h"
@@ -125,7 +126,8 @@
bool enable_browser_abort_on_hang,
base::TimeDelta hang_detection_interval,
LoginMetrics* metrics,
- SystemUtils* utils)
+ SystemUtils* utils,
+ Crossystem* crossystem)
: browser_(std::move(child_job)),
exit_on_child_done_(false),
kill_timeout_(base::TimeDelta::FromSeconds(kill_timeout)),
@@ -135,6 +137,7 @@
suspend_delay_id_(-1),
login_metrics_(metrics),
system_(utils),
+ crossystem_(crossystem),
nss_(NssUtil::Create()),
key_gen_(uid, utils),
state_key_generator_(utils, metrics),
@@ -194,7 +197,8 @@
this,
login_metrics_,
nss_.get(),
- system_);
+ system_,
+ crossystem_);
adaptor_.reset(new SessionManagerDBusAdaptor(impl));
impl_.reset(impl);
diff --git a/login_manager/session_manager_service.h b/login_manager/session_manager_service.h
index d1e4e73..a0140b6 100644
--- a/login_manager/session_manager_service.h
+++ b/login_manager/session_manager_service.h
@@ -29,6 +29,7 @@
struct signalfd_siginfo;
+class Crossystem;
class MessageLoop;
namespace dbus {
@@ -142,7 +143,8 @@
bool enable_browser_abort_on_hang,
base::TimeDelta hang_detection_interval,
LoginMetrics* metrics,
- SystemUtils* system);
+ SystemUtils* system,
+ Crossystem* crossystem);
virtual ~SessionManagerService();
// TestApi exposes internal routines for testing purposes.
@@ -269,7 +271,8 @@
int suspend_delay_id_;
LoginMetrics* login_metrics_; // Owned by the caller.
- SystemUtils* system_; // Owned by the caller.
+ SystemUtils* system_; // Owned by the caller.
+ Crossystem* crossystem_; // Owned by the caller.
scoped_ptr<NssUtil> nss_;
KeyGenerator key_gen_;
