Save device secret Device secret is needed for verification of auth tokens after device restart. BUG: 24110921 Change-Id: If956886fcd6325dd4b419b7ca39ccb6bc4708230 Reviewed-on: https://weave-review.googlesource.com/1180 Reviewed-by: Alex Vakulenko <avakulenko@google.com>
diff --git a/libweave/include/weave/settings.h b/libweave/include/weave/settings.h index 01f4c0d..264c6fa 100644 --- a/libweave/include/weave/settings.h +++ b/libweave/include/weave/settings.h
@@ -41,6 +41,7 @@ std::string refresh_token; std::string robot_account; std::string last_configured_ssid; + std::string secret; }; } // namespace weave
diff --git a/libweave/src/config.cc b/libweave/src/config.cc index 3aa57d5..0f01e2c 100644 --- a/libweave/src/config.cc +++ b/libweave/src/config.cc
@@ -36,6 +36,7 @@ const char kCloudId[] = "cloud_id"; const char kRobotAccount[] = "robot_account"; const char kLastConfiguredSsid[] = "last_configured_ssid"; +const char kSecret[] = "secret"; } // namespace config_keys @@ -109,6 +110,7 @@ CHECK(!settings_.model_name.empty()); CHECK(!settings_.model_id.empty()); CHECK(!settings_.name.empty()); + CHECK(IsValidAccessRole(settings_.local_anonymous_access_role)) << "Invalid role: " << settings_.local_anonymous_access_role; CHECK_EQ( @@ -116,6 +118,13 @@ std::find(settings_.pairing_modes.begin(), settings_.pairing_modes.end(), PairingType::kEmbeddedCode) == settings_.pairing_modes.end()); + // Values below will be generated at runtime. + CHECK(settings_.cloud_id.empty()); + CHECK(settings_.refresh_token.empty()); + CHECK(settings_.robot_account.empty()); + CHECK(settings_.last_configured_ssid.empty()); + CHECK(settings_.secret.empty()); + change.LoadState(); } @@ -169,6 +178,9 @@ if (dict->GetBoolean(config_keys::kLocalPairingEnabled, &tmp_bool)) set_local_pairing_enabled(tmp_bool); + if (dict->GetString(config_keys::kCloudId, &tmp)) + set_cloud_id(tmp); + if (dict->GetString(config_keys::kRefreshToken, &tmp)) set_refresh_token(tmp); @@ -178,8 +190,8 @@ if (dict->GetString(config_keys::kLastConfiguredSsid, &tmp)) set_last_configured_ssid(tmp); - if (dict->GetString(config_keys::kCloudId, &tmp)) - set_cloud_id(tmp); + if (dict->GetString(config_keys::kSecret, &tmp)) + set_secret(tmp); } void Config::Save() { @@ -197,6 +209,7 @@ dict.SetString(config_keys::kRobotAccount, settings_.robot_account); dict.SetString(config_keys::kLastConfiguredSsid, settings_.last_configured_ssid); + dict.SetString(config_keys::kSecret, settings_.secret); dict.SetString(config_keys::kName, settings_.name); dict.SetString(config_keys::kDescription, settings_.description); dict.SetString(config_keys::kLocation, settings_.location);
diff --git a/libweave/src/config.h b/libweave/src/config.h index 37b7cb6..c59cf86 100644 --- a/libweave/src/config.h +++ b/libweave/src/config.h
@@ -78,6 +78,7 @@ void set_last_configured_ssid(const std::string& ssid) { settings_->last_configured_ssid = ssid; } + void set_secret(const std::string& secret) { settings_->secret = secret; } void Commit();
diff --git a/libweave/src/config_unittest.cc b/libweave/src/config_unittest.cc index 2a81769..ada7fde 100644 --- a/libweave/src/config_unittest.cc +++ b/libweave/src/config_unittest.cc
@@ -75,6 +75,7 @@ EXPECT_EQ("", GetSettings().refresh_token); EXPECT_EQ("", GetSettings().robot_account); EXPECT_EQ("", GetSettings().last_configured_ssid); + EXPECT_EQ("", GetSettings().secret); } TEST_F(ConfigTest, LoadState) { @@ -82,8 +83,9 @@ "api_key": "state_api_key", "client_id": "state_client_id", "client_secret": "state_client_secret", - "description": "state_description", "cloud_id": "state_cloud_id", + "description": "state_description", + "last_configured_ssid": "state_last_configured_ssid", "local_anonymous_access_role": "user", "local_discovery_enabled": false, "local_pairing_enabled": false, @@ -92,7 +94,7 @@ "oauth_url": "state_oauth_url", "refresh_token": "state_refresh_token", "robot_account": "state_robot_account", - "last_configured_ssid": "state_last_configured_ssid", + "secret": "state_secret", "service_url": "state_service_url" })"; EXPECT_CALL(config_store_, LoadSettings()).WillOnce(Return(state)); @@ -127,6 +129,7 @@ EXPECT_EQ("state_refresh_token", GetSettings().refresh_token); EXPECT_EQ("state_robot_account", GetSettings().robot_account); EXPECT_EQ("state_last_configured_ssid", GetSettings().last_configured_ssid); + EXPECT_EQ("state_secret", GetSettings().secret); } TEST_F(ConfigTest, Setters) { @@ -189,6 +192,9 @@ change.set_last_configured_ssid("set_last_configured_ssid"); EXPECT_EQ("set_last_configured_ssid", GetSettings().last_configured_ssid); + change.set_secret("set_secret"); + EXPECT_EQ("set_secret", GetSettings().secret); + EXPECT_CALL(*this, OnConfigChanged(_)).Times(1); EXPECT_CALL(config_store_, SaveSettings(_)) @@ -197,8 +203,9 @@ 'api_key': 'set_api_key', 'client_id': 'set_client_id', 'client_secret': 'set_client_secret', - 'description': 'set_description', 'cloud_id': 'set_cloud_id', + 'description': 'set_description', + 'last_configured_ssid': 'set_last_configured_ssid', 'local_anonymous_access_role': 'user', 'local_discovery_enabled': true, 'local_pairing_enabled': true, @@ -207,7 +214,7 @@ 'oauth_url': 'set_oauth_url', 'refresh_token': 'set_token', 'robot_account': 'set_account', - 'last_configured_ssid': 'set_last_configured_ssid', + 'secret': 'set_secret', 'service_url': 'set_service_url' })"; EXPECT_JSON_EQ(expected, *test::CreateValue(json));
diff --git a/libweave/src/privet/privet_manager.cc b/libweave/src/privet/privet_manager.cc index 5b72a2a..e84fdf1 100644 --- a/libweave/src/privet/privet_manager.cc +++ b/libweave/src/privet/privet_manager.cc
@@ -54,9 +54,17 @@ cloud_ = CloudDelegate::CreateDefault(task_runner, device, command_manager, state_manager); cloud_observer_.Add(cloud_.get()); - security_.reset(new SecurityManager(device->GetSettings().pairing_modes, - device->GetSettings().embedded_code, - disable_security_, task_runner)); + security_.reset(new SecurityManager( + device->GetSettings().secret, device->GetSettings().pairing_modes, + device->GetSettings().embedded_code, disable_security_, task_runner)); + if (device->GetSettings().secret.empty()) { + // TODO(vitalybuka): Post all Config::Transaction to avoid following. + task_runner->PostDelayedTask( + FROM_HERE, + base::Bind(&Manager::SaveDeviceSecret, weak_ptr_factory_.GetWeakPtr(), + base::Unretained(device->GetMutableConfig())), + {}); + } network->AddConnectionChangedCallback( base::Bind(&Manager::OnConnectivityChanged, base::Unretained(this))); @@ -164,5 +172,10 @@ security_->SetCertificateFingerprint(server.GetHttpsCertificateFingerprint()); } +void Manager::SaveDeviceSecret(Config* config) { + Config::Transaction transaction(config); + transaction.set_secret(security_->GetSecret()); +} + } // namespace privet } // namespace weave
diff --git a/libweave/src/privet/privet_manager.h b/libweave/src/privet/privet_manager.h index 5e3df3f..a388967 100644 --- a/libweave/src/privet/privet_manager.h +++ b/libweave/src/privet/privet_manager.h
@@ -83,6 +83,7 @@ void OnConnectivityChanged(); void OnHttpServerStatusChanged(const provider::HttpServer& server); + void SaveDeviceSecret(Config* config); bool disable_security_{false}; std::unique_ptr<CloudDelegate> cloud_;
diff --git a/libweave/src/privet/publisher.cc b/libweave/src/privet/publisher.cc index 5d6567b..cd9cd66 100644 --- a/libweave/src/privet/publisher.cc +++ b/libweave/src/privet/publisher.cc
@@ -78,14 +78,17 @@ if (!cloud_->GetDescription().empty()) txt_record.emplace_back("note=" + cloud_->GetDescription()); - is_publishing_ = true; + auto new_data = std::make_pair(port, txt_record); + if (published_ == new_data) + return; + published_ = new_data; dns_sd_->PublishService(kPrivetServiceType, port, txt_record); } void Publisher::RemoveService() { - if (!is_publishing_) + if (!published_.first) return; - is_publishing_ = false; + published_ = {}; VLOG(1) << "Stopping service publishing."; dns_sd_->StopPublishing(kPrivetServiceType); }
diff --git a/libweave/src/privet/publisher.h b/libweave/src/privet/publisher.h index d330992..c1a9325 100644 --- a/libweave/src/privet/publisher.h +++ b/libweave/src/privet/publisher.h
@@ -7,6 +7,7 @@ #include <memory> #include <string> +#include <vector> #include <base/macros.h> @@ -43,13 +44,14 @@ void ExposeService(); void RemoveService(); - bool is_publishing_{false}; provider::DnsServiceDiscovery* dns_sd_{nullptr}; const DeviceDelegate* device_{nullptr}; const CloudDelegate* cloud_{nullptr}; const WifiDelegate* wifi_{nullptr}; + std::pair<uint16_t, std::vector<std::string>> published_; + DISALLOW_COPY_AND_ASSIGN(Publisher); };
diff --git a/libweave/src/privet/security_manager.cc b/libweave/src/privet/security_manager.cc index 1dfbab5..96b114f 100644 --- a/libweave/src/privet/security_manager.cc +++ b/libweave/src/privet/security_manager.cc
@@ -118,17 +118,19 @@ } // namespace -SecurityManager::SecurityManager(const std::set<PairingType>& pairing_modes, +SecurityManager::SecurityManager(const std::string& secret, + const std::set<PairingType>& pairing_modes, const std::string& embedded_code, bool disable_security, provider::TaskRunner* task_runner) : is_security_disabled_(disable_security), pairing_modes_(pairing_modes), embedded_code_(embedded_code), - task_runner_{task_runner}, - secret_(kSha256OutputSize) { - base::RandBytes(secret_.data(), kSha256OutputSize); - + task_runner_{task_runner} { + if (!Base64Decode(secret, &secret_) || secret_.size() != kSha256OutputSize) { + secret_.resize(kSha256OutputSize); + base::RandBytes(secret_.data(), secret_.size()); + } CHECK_EQ(embedded_code_.empty(), std::find(pairing_modes_.begin(), pairing_modes_.end(), PairingType::kEmbeddedCode) == pairing_modes_.end()); @@ -350,6 +352,10 @@ on_end_ = on_end; } +std::string SecurityManager::GetSecret() const { + return Base64Encode(secret_); +} + bool SecurityManager::CheckIfPairingAllowed(ErrorPtr* error) { if (is_security_disabled_) return true;
diff --git a/libweave/src/privet/security_manager.h b/libweave/src/privet/security_manager.h index 8acbd1d..4855778 100644 --- a/libweave/src/privet/security_manager.h +++ b/libweave/src/privet/security_manager.h
@@ -49,9 +49,11 @@ virtual const std::string& GetKey() const = 0; }; - SecurityManager(const std::set<PairingType>& pairing_modes, + SecurityManager(const std::string& secret, + const std::set<PairingType>& pairing_modes, const std::string& embedded_code, bool disable_security, + // TODO(vitalybuka): Remove task_runner. provider::TaskRunner* task_runner); ~SecurityManager() override; @@ -84,6 +86,8 @@ certificate_fingerprint_ = fingerprint; } + std::string GetSecret() const; + private: FRIEND_TEST_ALL_PREFIXES(SecurityManagerTest, ThrottlePairing); // Allows limited number of new sessions without successful authorization.
diff --git a/libweave/src/privet/security_manager_unittest.cc b/libweave/src/privet/security_manager_unittest.cc index c85bd5f..ed225ae 100644 --- a/libweave/src/privet/security_manager_unittest.cc +++ b/libweave/src/privet/security_manager_unittest.cc
@@ -58,7 +58,7 @@ public: void SetUp() override { std::vector<uint8_t> fingerprint; - fingerprint.resize(256 / 8); + fingerprint.resize(32); base::RandBytes(fingerprint.data(), fingerprint.size()); security_.SetCertificateFingerprint(fingerprint); } @@ -101,12 +101,29 @@ const base::Time time_ = base::Time::FromTimeT(1410000000); provider::test::FakeTaskRunner task_runner_; - SecurityManager security_{{PairingType::kEmbeddedCode}, + SecurityManager security_{{}, + {PairingType::kEmbeddedCode}, "1234", false, &task_runner_}; }; +TEST_F(SecurityManagerTest, RandomSecret) { + EXPECT_GE(security_.GetSecret().size(), 32); + EXPECT_TRUE(IsBase64(security_.GetSecret())); +} + +TEST_F(SecurityManagerTest, DifferentSecret) { + SecurityManager security{{}, {}, "", false, &task_runner_}; + EXPECT_NE(security_.GetSecret(), security.GetSecret()); +} + +TEST_F(SecurityManagerTest, ExternalSecret) { + const std::string kSecret = "T1SDv9CVGNO82zHKeRrUSzpAzjb1hmRyzXGotsn1gcU="; + SecurityManager security{kSecret, {}, "", false, &task_runner_}; + EXPECT_EQ(kSecret, security.GetSecret()); +} + TEST_F(SecurityManagerTest, IsBase64) { EXPECT_TRUE(IsBase64( security_.CreateAccessToken(UserInfo{AuthScope::kUser, 7}, time_))); @@ -139,14 +156,14 @@ TEST_F(SecurityManagerTest, CreateTokenDifferentInstance) { EXPECT_NE(security_.CreateAccessToken(UserInfo{AuthScope::kUser, 123}, time_), - SecurityManager({}, "", false, &task_runner_) + 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{{}, "", false, &task_runner_}; + SecurityManager security{{}, {}, "", false, &task_runner_}; std::string token = security.CreateAccessToken(UserInfo{AuthScope::kUser, 5}, time_);