libweave: Remove device_kind and embedded_code_path Keep only embedded_code, implementation will decide how to get it. device_kind is not supported by server already. BUG=brillo:1257 TEST=`FEATURES=test emerge-gizmo libweave buffet` Change-Id: I6a4a438d78eab188f3e454ebf4f12a4d4684622a Reviewed-on: https://chromium-review.googlesource.com/293896 Reviewed-by: Alex Vakulenko <avakulenko@chromium.org> Reviewed-by: Vitaly Buka <vitalybuka@chromium.org> Tested-by: Vitaly Buka <vitalybuka@chromium.org> Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
diff --git a/libweave/include/weave/config.h b/libweave/include/weave/config.h index 523adf7..e2512cb 100644 --- a/libweave/include/weave/config.h +++ b/libweave/include/weave/config.h
@@ -29,14 +29,13 @@ std::string oem_name; std::string model_name; std::string model_id; - std::string device_kind; base::TimeDelta polling_period; base::TimeDelta backup_polling_period; bool wifi_auto_setup_enabled{true}; bool ble_setup_enabled{false}; std::set<PairingType> pairing_modes; - base::FilePath embedded_code_path; + std::string embedded_code; std::string device_id; std::string refresh_token;
diff --git a/libweave/src/buffet_config.cc b/libweave/src/buffet_config.cc index 2698630..3f5bd8a 100644 --- a/libweave/src/buffet_config.cc +++ b/libweave/src/buffet_config.cc
@@ -17,36 +17,6 @@ namespace { -// TODO(vitalybuka): Remove this when deviceKind is gone from server. -std::string GetDeviceKind(const std::string& manifest_id) { - CHECK_EQ(5u, manifest_id.size()); - std::string kind = manifest_id.substr(0, 2); - if (kind == "AC") - return "accessPoint"; - if (kind == "AK") - return "aggregator"; - if (kind == "AM") - return "camera"; - if (kind == "AB") - return "developmentBoard"; - if (kind == "AE") - return "printer"; - if (kind == "AF") - return "scanner"; - if (kind == "AD") - return "speaker"; - if (kind == "AL") - return "storage"; - if (kind == "AJ") - return "toy"; - if (kind == "AA") - return "vendor"; - if (kind == "AN") - return "video"; - LOG(FATAL) << "Invalid model id: " << manifest_id; - return std::string(); -} - bool IsValidAccessRole(const std::string& role) { return role == "none" || role == "viewer" || role == "user"; } @@ -86,7 +56,7 @@ const char kRobotAccount[] = "robot_account"; const char kWifiAutoSetupEnabled[] = "wifi_auto_setup_enabled"; const char kBleSetupEnabled[] = "ble_setup_enabled"; -const char kEmbeddedCodePath[] = "embedded_code_path"; +const char kEmbeddedCode[] = "embedded_code"; const char kPairingModes[] = "pairing_modes"; const char kLastConfiguredSsid[] = "last_configured_ssid"; @@ -106,7 +76,6 @@ result.oem_name = "Chromium"; result.model_name = "Brillo"; result.model_id = "AAAAA"; - result.device_kind = "vendor"; result.polling_period = base::TimeDelta::FromSeconds(7); result.backup_polling_period = base::TimeDelta::FromMinutes(30); result.wifi_auto_setup_enabled = true; @@ -169,7 +138,7 @@ CHECK(!settings_.model_name.empty()); store.GetString(config_keys::kModelId, &settings_.model_id); - settings_.device_kind = GetDeviceKind(settings_.model_id); + CHECK(!settings_.model_id.empty()); std::string polling_period_str; if (store.GetString(config_keys::kPollingPeriodMs, &polling_period_str)) @@ -185,12 +154,7 @@ store.GetBoolean(config_keys::kBleSetupEnabled, &settings_.ble_setup_enabled); - std::string embedded_code_path; - if (store.GetString(config_keys::kEmbeddedCodePath, &embedded_code_path)) { - settings_.embedded_code_path = base::FilePath(embedded_code_path); - if (!settings_.embedded_code_path.empty()) - settings_.pairing_modes = {PairingType::kEmbeddedCode}; - } + store.GetString(config_keys::kEmbeddedCode, &settings_.embedded_code); std::string modes_str; if (store.GetString(config_keys::kPairingModes, &modes_str)) {
diff --git a/libweave/src/buffet_config.h b/libweave/src/buffet_config.h index 2d161e1..f8a387f 100644 --- a/libweave/src/buffet_config.h +++ b/libweave/src/buffet_config.h
@@ -103,7 +103,6 @@ const std::string& oem_name() const { return settings_.oem_name; } const std::string& model_name() const { return settings_.model_name; } const std::string& model_id() const { return settings_.model_id; } - const std::string& device_kind() const { return settings_.device_kind; } base::TimeDelta polling_period() const { return settings_.polling_period; } base::TimeDelta backup_polling_period() const { return settings_.backup_polling_period; @@ -118,9 +117,7 @@ const std::set<PairingType>& pairing_modes() const { return settings_.pairing_modes; } - const base::FilePath& embedded_code_path() const { - return settings_.embedded_code_path; - } + const std::string& embedded_code() const { return settings_.embedded_code; } const std::string& name() const { return settings_.name; } const std::string& description() const { return settings_.description; }
diff --git a/libweave/src/buffet_config_unittest.cc b/libweave/src/buffet_config_unittest.cc index 7c80b06..38bb5b8 100644 --- a/libweave/src/buffet_config_unittest.cc +++ b/libweave/src/buffet_config_unittest.cc
@@ -54,14 +54,13 @@ EXPECT_EQ("Chromium", config_->oem_name()); EXPECT_EQ("Brillo", config_->model_name()); EXPECT_EQ("AAAAA", config_->model_id()); - EXPECT_EQ("vendor", config_->device_kind()); EXPECT_EQ(base::TimeDelta::FromSeconds(7), config_->polling_period()); EXPECT_EQ(base::TimeDelta::FromMinutes(30), config_->backup_polling_period()); EXPECT_TRUE(config_->wifi_auto_setup_enabled()); EXPECT_FALSE(config_->ble_setup_enabled()); EXPECT_EQ(std::set<PairingType>{PairingType::kPinCode}, config_->pairing_modes()); - EXPECT_EQ("", config_->embedded_code_path().value()); + EXPECT_EQ("", config_->embedded_code()); EXPECT_EQ("Developer device", config_->name()); EXPECT_EQ("", config_->description()); EXPECT_EQ("", config_->location()); @@ -90,7 +89,7 @@ config_store.SetBoolean("ble_setup_enabled", true); config_store.SetString("pairing_modes", "pinCode,embeddedCode,ultrasound32,audible32"); - config_store.SetString("embedded_code_path", "/conf_code"); + config_store.SetString("embedded_code", "1234"); config_store.SetString("name", "conf_name"); config_store.SetString("description", "conf_description"); config_store.SetString("location", "conf_location"); @@ -116,7 +115,6 @@ EXPECT_EQ("conf_oem_name", config_->oem_name()); EXPECT_EQ("conf_model_name", config_->model_name()); EXPECT_EQ("ABCDE", config_->model_id()); - EXPECT_EQ("developmentBoard", config_->device_kind()); EXPECT_EQ(base::TimeDelta::FromMilliseconds(12345), config_->polling_period()); EXPECT_EQ(base::TimeDelta::FromMilliseconds(6589), @@ -127,7 +125,7 @@ PairingType::kPinCode, PairingType::kEmbeddedCode, PairingType::kUltrasound32, PairingType::kAudible32}; EXPECT_EQ(pairing_types, config_->pairing_modes()); - EXPECT_EQ("/conf_code", config_->embedded_code_path().value()); + EXPECT_EQ("1234", config_->embedded_code()); EXPECT_EQ("conf_name", config_->name()); EXPECT_EQ("conf_description", config_->description()); EXPECT_EQ("conf_location", config_->location()); @@ -203,7 +201,6 @@ EXPECT_EQ(default_.oem_name(), config_->oem_name()); EXPECT_EQ(default_.model_name(), config_->model_name()); EXPECT_EQ(default_.model_id(), config_->model_id()); - EXPECT_EQ(default_.device_kind(), config_->device_kind()); EXPECT_EQ(default_.polling_period(), config_->polling_period()); EXPECT_EQ(default_.backup_polling_period(), config_->backup_polling_period()); EXPECT_EQ(default_.wifi_auto_setup_enabled(), @@ -211,7 +208,7 @@ EXPECT_EQ(default_.ble_setup_enabled(), config_->ble_setup_enabled()); EXPECT_EQ(default_.pairing_modes(), config_->pairing_modes()); - EXPECT_EQ(default_.embedded_code_path(), config_->embedded_code_path()); + EXPECT_EQ(default_.embedded_code(), config_->embedded_code()); EXPECT_EQ("state_name", config_->name()); EXPECT_EQ("state_description", config_->description()); EXPECT_EQ("state_location", config_->location());
diff --git a/libweave/src/device_registration_info.cc b/libweave/src/device_registration_info.cc index d3dc1c0..0f150e1 100644 --- a/libweave/src/device_registration_info.cc +++ b/libweave/src/device_registration_info.cc
@@ -506,7 +506,6 @@ if (!config_->location().empty()) resource->SetString("location", config_->location()); resource->SetString("modelManifestId", config_->model_id()); - resource->SetString("deviceKind", config_->device_kind()); std::unique_ptr<base::DictionaryValue> channel{new base::DictionaryValue}; if (current_notification_channel_) { channel->SetString("supportedType",
diff --git a/libweave/src/device_registration_info_unittest.cc b/libweave/src/device_registration_info_unittest.cc index c73b55a..9c70207 100644 --- a/libweave/src/device_registration_info_unittest.cc +++ b/libweave/src/device_registration_info_unittest.cc
@@ -389,8 +389,6 @@ EXPECT_EQ("pull", value); EXPECT_TRUE(json->GetString("oauthClientId", &value)); EXPECT_EQ(test_data::kClientId, value); - EXPECT_TRUE(json->GetString("deviceDraft.deviceKind", &value)); - EXPECT_EQ("vendor", value); EXPECT_TRUE(json->GetString("deviceDraft.description", &value)); EXPECT_EQ("Easy to clean", value); EXPECT_TRUE(json->GetString("deviceDraft.location", &value));
diff --git a/libweave/src/privet/privet_manager.cc b/libweave/src/privet/privet_manager.cc index c64430a..16bbb4d 100644 --- a/libweave/src/privet/privet_manager.cc +++ b/libweave/src/privet/privet_manager.cc
@@ -16,7 +16,6 @@ #include <base/scoped_observer.h> #include <base/strings/string_number_conversions.h> #include <base/values.h> -#include <chromeos/key_value_store.h> #include <weave/network.h> #include "libweave/src/device_registration_info.h" @@ -52,8 +51,8 @@ state_manager); cloud_observer_.Add(cloud_.get()); security_.reset(new SecurityManager(device->GetConfig().pairing_modes(), - device->GetConfig().embedded_code_path(), - task_runner, disable_security_)); + device->GetConfig().embedded_code(), + disable_security_, task_runner)); network->AddOnConnectionChangedCallback( base::Bind(&Manager::OnConnectivityChanged, base::Unretained(this)));
diff --git a/libweave/src/privet/security_manager.cc b/libweave/src/privet/security_manager.cc index 8f90cc4..409f09a 100644 --- a/libweave/src/privet/security_manager.cc +++ b/libweave/src/privet/security_manager.cc
@@ -17,7 +17,6 @@ #include <base/strings/string_number_conversions.h> #include <base/strings/stringprintf.h> #include <base/time/time.h> -#include <chromeos/key_value_store.h> #include <weave/task_runner.h> #include "libweave/external/crypto/p224_spake.h" @@ -37,8 +36,6 @@ const int kMaxAllowedPairingAttemts = 3; const int kPairingBlockingTimeMinutes = 1; -const char kEmbeddedCode[] = "embedded_code"; - // Returns "scope:id:time". std::string CreateTokenData(const UserInfo& user_info, const base::Time& time) { return base::IntToString(static_cast<int>(user_info.scope())) + @@ -70,14 +67,6 @@ return UserInfo{static_cast<AuthScope>(scope), id}; } -std::string LoadEmbeddedCode(const base::FilePath& path) { - std::string code; - chromeos::KeyValueStore store; - if (store.Load(path)) - store.GetString(kEmbeddedCode, &code); - return code; -} - class Spakep224Exchanger : public SecurityManager::KeyExchanger { public: explicit Spakep224Exchanger(const std::string& password) @@ -131,17 +120,17 @@ } // namespace SecurityManager::SecurityManager(const std::set<PairingType>& pairing_modes, - const base::FilePath& embedded_code_path, - TaskRunner* task_runner, - bool disable_security) + const std::string& embedded_code, + bool disable_security, + TaskRunner* task_runner) : is_security_disabled_(disable_security), pairing_modes_(pairing_modes), - embedded_code_path_(embedded_code_path), + embedded_code_(embedded_code), task_runner_{task_runner}, secret_(kSha256OutputSize) { base::RandBytes(secret_.data(), kSha256OutputSize); - CHECK_EQ(embedded_code_path_.empty(), + CHECK_EQ(embedded_code_.empty(), std::find(pairing_modes_.begin(), pairing_modes_.end(), PairingType::kEmbeddedCode) == pairing_modes_.end()); } @@ -225,17 +214,7 @@ std::string code; switch (mode) { case PairingType::kEmbeddedCode: - CHECK(!embedded_code_path_.empty()); - - if (embedded_code_.empty()) - embedded_code_ = LoadEmbeddedCode(embedded_code_path_); - - if (embedded_code_.empty()) { // File is not created yet. - Error::AddTo(error, FROM_HERE, errors::kDomain, errors::kDeviceBusy, - "Embedded code is not ready"); - return false; - } - + CHECK(!embedded_code_.empty()); code = embedded_code_; break; case PairingType::kUltrasound32:
diff --git a/libweave/src/privet/security_manager.h b/libweave/src/privet/security_manager.h index f0a6de4..eda7946 100644 --- a/libweave/src/privet/security_manager.h +++ b/libweave/src/privet/security_manager.h
@@ -48,9 +48,9 @@ }; SecurityManager(const std::set<PairingType>& pairing_modes, - const base::FilePath& embedded_code_path, - TaskRunner* task_runner, - bool disable_security); + const std::string& embedded_code, + bool disable_security, + TaskRunner* task_runner); ~SecurityManager() override; // SecurityDelegate methods @@ -92,7 +92,6 @@ // If true allows unencrypted pairing and accepts any access code. bool is_security_disabled_{false}; std::set<PairingType> pairing_modes_; - const base::FilePath embedded_code_path_; std::string embedded_code_; // TODO(vitalybuka): Session cleanup can be done without posting tasks. TaskRunner* task_runner_{nullptr};
diff --git a/libweave/src/privet/security_manager_unittest.cc b/libweave/src/privet/security_manager_unittest.cc index 8ce0ba9..2b7babb 100644 --- a/libweave/src/privet/security_manager_unittest.cc +++ b/libweave/src/privet/security_manager_unittest.cc
@@ -13,17 +13,15 @@ #include <vector> #include <base/bind.h> -#include <base/files/file_util.h> #include <base/logging.h> #include <base/rand_util.h> #include <base/strings/string_number_conversions.h> #include <base/strings/string_util.h> -#include <chromeos/key_value_store.h> -#include "libweave/external/crypto/p224_spake.h" #include <gmock/gmock.h> #include <gtest/gtest.h> #include <weave/mock_task_runner.h> +#include "libweave/external/crypto/p224_spake.h" #include "libweave/src/data_encoding.h" #include "libweave/src/privet/openssl_utils.h" @@ -54,12 +52,6 @@ MOCK_METHOD1(OnPairingEnd, void(const std::string& session_id)); }; -base::FilePath GetTempFilePath() { - base::FilePath file_path; - EXPECT_TRUE(base::CreateTemporaryFile(&file_path)); - return file_path; -} - } // namespace class SecurityManagerTest : public testing::Test { @@ -69,14 +61,8 @@ fingerprint.resize(256 / 8); base::RandBytes(fingerprint.data(), fingerprint.size()); security_.SetCertificateFingerprint(fingerprint); - - chromeos::KeyValueStore store; - store.SetString("embedded_code", "1234"); - EXPECT_TRUE(store.Save(embedded_code_path_)); } - void TearDown() override { base::DeleteFile(embedded_code_path_, false); } - protected: void PairAndAuthenticate(std::string* fingerprint, std::string* signature) { std::string session_id; @@ -114,12 +100,11 @@ } const base::Time time_ = base::Time::FromTimeT(1410000000); - base::FilePath embedded_code_path_{GetTempFilePath()}; unittests::MockTaskRunner task_runner_; SecurityManager security_{{PairingType::kEmbeddedCode}, - embedded_code_path_, - &task_runner_, - false}; + "1234", + false, + &task_runner_}; }; TEST_F(SecurityManagerTest, IsBase64) { @@ -154,14 +139,14 @@ TEST_F(SecurityManagerTest, CreateTokenDifferentInstance) { EXPECT_NE(security_.CreateAccessToken(UserInfo{AuthScope::kUser, 123}, time_), - SecurityManager({}, base::FilePath{}, &task_runner_, false) + SecurityManager({}, "", false, &task_runner_) .CreateAccessToken(UserInfo{AuthScope::kUser, 123}, time_)); } TEST_F(SecurityManagerTest, ParseAccessToken) { // Multiple attempts with random secrets. for (size_t i = 0; i < 1000; ++i) { - SecurityManager security{{}, base::FilePath{}, &task_runner_, false}; + SecurityManager security{{}, "", false, &task_runner_}; std::string token = security.CreateAccessToken(UserInfo{AuthScope::kUser, 5}, time_); @@ -302,16 +287,5 @@ } } -TEST_F(SecurityManagerTest, EmbeddedCodeNotReady) { - std::string session_id; - std::string device_commitment; - base::DeleteFile(embedded_code_path_, false); - ErrorPtr error; - ASSERT_FALSE(security_.StartPairing(PairingType::kEmbeddedCode, - CryptoType::kSpake_p224, &session_id, - &device_commitment, &error)); - EXPECT_EQ("deviceBusy", error->GetCode()); -} - } // namespace privet } // namespace weave
diff --git a/libweave/src/privet/wifi_bootstrap_manager.cc b/libweave/src/privet/wifi_bootstrap_manager.cc index d5f7916..5529030 100644 --- a/libweave/src/privet/wifi_bootstrap_manager.cc +++ b/libweave/src/privet/wifi_bootstrap_manager.cc
@@ -6,7 +6,6 @@ #include <base/logging.h> #include <base/memory/weak_ptr.h> -#include <chromeos/key_value_store.h> #include <weave/enum_to_string.h> #include <weave/network.h> #include <weave/task_runner.h>