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_);
