[remoting host] Only watch for material changes to NSS DB files

remoting/host/linux/certificate_watcher.cc

Index: remoting/host/linux/certificate_watcher.cc
diff --git a/remoting/host/linux/certificate_watcher.cc b/remoting/host/linux/certificate_watcher.cc
index 42e3a7cc250293a6d3ba084f8f2fdc87ef101c42..c27b278451db7a4542f421861f73ea2faef35436 100644
--- a/remoting/host/linux/certificate_watcher.cc
+++ b/remoting/host/linux/certificate_watcher.cc
@@ -6,6 +6,8 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/files/file_util.h"
+#include "base/hash.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/path_service.h"
@@ -13,21 +15,152 @@
 
 namespace remoting {
 
-// Delay time to restart the host when a change of certificate is detected.
-// This is to avoid repeating restarts when continuous writes to the database
-// occur.
-const int kRestartDelayInSecond = 2;
+namespace {
+
+// Delay before re-reading the cert DB when a change is detected.
+const int kReadDelayInSeconds = 2;
 
 // Full Path: $HOME/.pki/nssdb
 const char kCertDirectoryPath[] = ".pki/nssdb";
 
+const char* const kCertFiles[] = {"cert9.db", "key4.db", "pkcs11.txt"};
+
+}  // namespace
+
+// This class lives on the IO thread, watches the certificate database files,
+// and notifies the caller_task_runner thread of any updates.
+class CertDbContentWatcher {

[Sergey Ulanov, 2016/05/14 05:43:46]

Please add ThreadChecker in this class to make sure it's called in the right
thread.

[Lambros, 2016/05/16 17:35:15]

Done.

+ public:
+  CertDbContentWatcher(
+      base::WeakPtr<CertificateWatcher> watcher,
+      scoped_refptr<base::SingleThreadTaskRunner> caller_task_runner,
+      base::FilePath cert_watch_path,
+      base::TimeDelta read_delay);
+  ~CertDbContentWatcher();
+
+  void StartWatching();
+
+ private:
+  // size_t is the return type of base::HashInts() which is used to accumulate
+  // a hash-code while iterating over the database files.
+  typedef size_t HashValue;
+
+  // Called by the FileWatcher when it detects any changes.
+  void OnCertDirectoryChanged(const base::FilePath& path, bool error);
+
+  // Called by |read_timer_| to trigger re-reading the DB content.
+  void OnTimer();
+
+  // Reads the certificate database files and returns a hash of their contents.
+  HashValue ComputeHash();
+
+  base::WeakPtr<CertificateWatcher> watcher_;
+
+  // The TaskRunner to be notified of any changes.
+  scoped_refptr<base::SingleThreadTaskRunner> caller_task_runner_;
+
+  // The file watcher to watch changes inside the certificate folder.
+  std::unique_ptr<base::FilePathWatcher> file_watcher_;
+
+  // Path of the certificate files/directories.
+  base::FilePath cert_watch_path_;
+
+  // Timer to delay reading the DB files after a change notification from the
+  // FileWatcher. This is done to avoid triggering multiple notifications when
+  // the DB is written to. It also avoids a false notification in case the NSS
+  // DB content is quickly changed and reverted.
+  std::unique_ptr<base::DelayTimer> read_timer_;
+
+  // The time to wait before re-reading the DB files after a change is
+  // detected.
+  base::TimeDelta delay_;
+
+  // The hash code of the current certificate database contents. When the
+  // FileWatcher detects changes, the code is re-computed and compared with
+  // this stored value.
+  HashValue current_hash_;
+
+  DISALLOW_COPY_AND_ASSIGN(CertDbContentWatcher);
+};
+
+CertDbContentWatcher::CertDbContentWatcher(
+    base::WeakPtr<CertificateWatcher> watcher,
+    scoped_refptr<base::SingleThreadTaskRunner> caller_task_runner,
+    base::FilePath cert_watch_path,
+    base::TimeDelta read_delay)
+    : watcher_(watcher),
+      caller_task_runner_(caller_task_runner),
+      cert_watch_path_(cert_watch_path),
+      delay_(read_delay) {}
+
+CertDbContentWatcher::~CertDbContentWatcher() {}
+
+void CertDbContentWatcher::StartWatching() {
+  DCHECK(!cert_watch_path_.empty());
+
+  file_watcher_.reset(new base::FilePathWatcher());
+
+  // Initialize hash value.
+  current_hash_ = ComputeHash();
+
+  // base::Unretained() is safe since this class owns the FileWatcher.
+  file_watcher_->Watch(cert_watch_path_, true,
+                       base::Bind(&CertDbContentWatcher::OnCertDirectoryChanged,
+                                  base::Unretained(this)));
+
+  read_timer_.reset(new base::DelayTimer(FROM_HERE, delay_, this,
+                                         &CertDbContentWatcher::OnTimer));
+}
+
+void CertDbContentWatcher::OnCertDirectoryChanged(const base::FilePath& path,
+                                                  bool error) {
+  DCHECK(path == cert_watch_path_);
+
+  if (error) {
+    LOG(FATAL) << "Error occurred while watching for changes of file: "
+               << cert_watch_path_.MaybeAsASCII();
+  }
+
+  read_timer_->Reset();
+}
+
+void CertDbContentWatcher::OnTimer() {
+  HashValue new_hash = ComputeHash();
+  if (new_hash != current_hash_) {
+    current_hash_ = new_hash;
+    caller_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&CertificateWatcher::DatabaseChanged, watcher_));
+  } else {
+    VLOG(1) << "Directory changed but contents are the same.";
+  }
+}
+
+CertDbContentWatcher::HashValue CertDbContentWatcher::ComputeHash() {
+  HashValue result = 0;
+
+  for (const char* file : kCertFiles) {
+    base::FilePath path = cert_watch_path_.AppendASCII(file);
+    std::string content;
+    HashValue file_hash = 0;
+
+    // It's possible the file might not exist. Compute the overall hash in a
+    // consistent way for the set of files that do exist. If a new file comes
+    // into existence, the resulting hash-code should change.
+    if (base::ReadFileToString(path, &content)) {
+      file_hash = base::Hash(content);
+    }
+    result = base::HashInts(result, file_hash);
+  }
+  return result;
+}
+
 CertificateWatcher::CertificateWatcher(
     const base::Closure& restart_action,
     scoped_refptr<base::SingleThreadTaskRunner> io_task_runner)
     : restart_action_(restart_action),
       caller_task_runner_(base::ThreadTaskRunnerHandle::Get()),
       io_task_runner_(io_task_runner),
-      delay_(base::TimeDelta::FromSeconds(kRestartDelayInSecond)),
+      delay_(base::TimeDelta::FromSeconds(kReadDelayInSeconds)),
       weak_factory_(this) {
   if (!base::PathService::Get(base::DIR_HOME, &cert_watch_path_)) {
     LOG(FATAL) << "Failed to get path of the home directory.";
@@ -44,7 +177,7 @@ CertificateWatcher::~CertificateWatcher() {
   if (monitor_) {
     monitor_->RemoveStatusObserver(this);
   }
-  io_task_runner_->DeleteSoon(FROM_HERE, file_watcher_.release());
+  io_task_runner_->DeleteSoon(FROM_HERE, content_watcher_.release());
 
   VLOG(1) << "Stopped watching certificate changes.";
 }
@@ -53,15 +186,13 @@ void CertificateWatcher::Start() {
   DCHECK(caller_task_runner_->BelongsToCurrentThread());
   DCHECK(!cert_watch_path_.empty());
 
-  file_watcher_.reset(new base::FilePathWatcher());
+  content_watcher_.reset(new CertDbContentWatcher(weak_factory_.GetWeakPtr(),
+                                                  caller_task_runner_,
+                                                  cert_watch_path_, delay_));
+
   io_task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(base::IgnoreResult(&base::FilePathWatcher::Watch),
-                 base::Unretained(file_watcher_.get()), cert_watch_path_, true,
-                 base::Bind(&CertificateWatcher::OnCertDirectoryChanged,
-                            caller_task_runner_, weak_factory_.GetWeakPtr())));
-  restart_timer_.reset(new base::DelayTimer(FROM_HERE, delay_, this,
-                                            &CertificateWatcher::OnTimer));
+      FROM_HERE, base::Bind(&CertDbContentWatcher::StartWatching,
+                            base::Unretained(content_watcher_.get())));
 
   VLOG(1) << "Started watching certificate changes.";
 }
@@ -103,34 +234,10 @@ void CertificateWatcher::SetWatchPathForTests(
 }
 
 bool CertificateWatcher::is_started() const {
-  return file_watcher_ != nullptr;
-}
-
-// static
-void CertificateWatcher::OnCertDirectoryChanged(
-    scoped_refptr<base::SingleThreadTaskRunner> network_task_runner,
-    base::WeakPtr<CertificateWatcher> watcher_,
-    const base::FilePath& path,
-    bool error) {
-  network_task_runner->PostTask(
-      FROM_HERE,
-      base::Bind(&CertificateWatcher::DirectoryChanged, watcher_, path, error));
-}
-
-void CertificateWatcher::DirectoryChanged(const base::FilePath& path,
-                                          bool error) {
-  DCHECK(caller_task_runner_->BelongsToCurrentThread());
-  DCHECK(path == cert_watch_path_);
-
-  if (error) {
-    LOG(FATAL) << "Error occurs when watching changes of file "
-               << cert_watch_path_.MaybeAsASCII();
-  }
-
-  restart_timer_->Reset();
+  return content_watcher_ != nullptr;
 }
 
-void CertificateWatcher::OnTimer() {
+void CertificateWatcher::DatabaseChanged() {
   DCHECK(caller_task_runner_->BelongsToCurrentThread());
 
   if (inhibit_mode_) {