Use local state file instead of myClientId file

src/cryfs/CMakeLists.txt

@@ -40,8 +40,8 @@ set(LIB_SOURCES
         filesystem/cachingfsblobstore/SymlinkBlobRef.cpp
         filesystem/CryFile.cpp
         filesystem/CryDevice.cpp
-        localstate/MyClientId.cpp
         localstate/LocalStateDir.cpp
+        localstate/LocalStateMetadata.cpp
 )
 
 add_library(${PROJECT_NAME} STATIC ${LIB_SOURCES})

src/cryfs/config/CryConfigCreator.cpp

@@ -3,7 +3,7 @@
 #include <gitversion/gitversion.h>
 #include <cpp-utils/random/Random.h>
 #include <cryfs/localstate/LocalStateDir.h>
-#include <cryfs/localstate/MyClientId.h>
+#include <cryfs/localstate/LocalStateMetadata.h>
 
 using cpputils::Console;
 using cpputils::unique_ref;
@@ -30,7 +30,8 @@ namespace cryfs {
         config.SetRootBlob(_generateRootBlobId());
         config.SetEncryptionKey(_generateEncKey(config.Cipher()));
         config.SetFilesystemId(_generateFilesystemID());
-        uint32_t myClientId = MyClientId(LocalStateDir::forFilesystemId(config.FilesystemId())).loadOrGenerate();
+        auto localState = LocalStateMetadata::loadOrGenerate(LocalStateDir::forFilesystemId(config.FilesystemId()));
+        uint32_t myClientId = localState.myClientId();
         config.SetExclusiveClientId(_generateExclusiveClientId(missingBlockIsIntegrityViolationFromCommandLine, myClientId));
 #ifndef CRYFS_NO_COMPATIBILITY
         config.SetHasVersionNumbers(true);

src/cryfs/config/CryConfigLoader.cpp

@@ -7,7 +7,7 @@
 #include <gitversion/gitversion.h>
 #include <gitversion/VersionCompare.h>
 #include "../localstate/LocalStateDir.h"
-#include "../localstate/MyClientId.h"
+#include "../localstate/LocalStateMetadata.h"
 
 namespace bf = boost::filesystem;
 using cpputils::unique_ref;
@@ -57,7 +57,8 @@ optional<CryConfigLoader::ConfigLoadResult> CryConfigLoader::_loadConfig(const b
     config->save();
   }
   _checkCipher(*config->config());
-  uint32_t myClientId = MyClientId(LocalStateDir::forFilesystemId(config->config()->FilesystemId())).loadOrGenerate();
+  auto localState = LocalStateMetadata::loadOrGenerate(LocalStateDir::forFilesystemId(config->config()->FilesystemId()));
+  uint32_t myClientId = localState.myClientId();
   _checkMissingBlocksAreIntegrityViolations(&*config, myClientId);
   return ConfigLoadResult {std::move(*config), myClientId};
 }

src/cryfs/filesystem/CryDevice.cpp

@@ -19,7 +19,6 @@
 #include <cpp-utils/system/homedir.h>
 #include <gitversion/VersionCompare.h>
 #include <blockstore/interface/BlockStore2.h>
-#include "cryfs/localstate/MyClientId.h"
 #include "cryfs/localstate/LocalStateDir.h"
 
 using std::string;

src/cryfs/localstate/LocalStateMetadata.cpp

@@ -0,0 +1,108 @@
+#include "LocalStateMetadata.h"
+#include <boost/property_tree/ptree.hpp>
+#include <boost/property_tree/json_parser.hpp>
+#include <cpp-utils/random/Random.h>
+#include <boost/filesystem.hpp>
+#include <blockstore/implementations/integrity/KnownBlockVersions.h>
+
+using boost::optional;
+using boost::none;
+using boost::property_tree::ptree;
+using boost::property_tree::write_json;
+using boost::property_tree::read_json;
+using std::ifstream;
+using std::ofstream;
+using std::istream;
+using std::ostream;
+using cpputils::Random;
+using blockstore::integrity::KnownBlockVersions;
+namespace bf = boost::filesystem;
+
+namespace cryfs {
+
+LocalStateMetadata::LocalStateMetadata(uint32_t myClientId)
+: _myClientId(myClientId) {}
+
+LocalStateMetadata LocalStateMetadata::loadOrGenerate(const bf::path &statePath) {
+  auto metadataFile = statePath / "metadata";
+  auto loaded = _load(metadataFile);
+  if (loaded != none) {
+      return *loaded;
+  }
+  // If it couldn't be loaded, generate a new client id.
+  return _generate(metadataFile);
+}
+
+optional<LocalStateMetadata> LocalStateMetadata::_load(const bf::path &metadataFilePath) {
+  ifstream file(metadataFilePath.native());
+  if (!file.good()) {
+    // State file doesn't exist
+    return none;
+  }
+  return _deserialize(file);
+}
+
+void LocalStateMetadata::_save(const bf::path &metadataFilePath) const {
+  ofstream file(metadataFilePath.native(), std::ios::trunc);
+  _serialize(file);
+}
+
+namespace {
+uint32_t _generateClientId() {
+  uint32_t result;
+  do {
+    result = *reinterpret_cast<uint32_t*>(Random::PseudoRandom().getFixedSize<sizeof(uint32_t)>().data());
+  } while(result == KnownBlockVersions::CLIENT_ID_FOR_DELETED_BLOCK); // Safety check - CLIENT_ID_FOR_DELETED_BLOCK shouldn't be used by any valid client.
+  return result;
+}
+
+#ifndef CRYFS_NO_COMPATIBILITY
+optional<uint32_t> _tryLoadClientIdFromLegacyFile(const bf::path &metadataFilePath) {
+  auto myClientIdFile = metadataFilePath.parent_path() / "myClientId";
+  ifstream file(myClientIdFile.native());
+  if (!file.good()) {
+    return none;
+  }
+
+  uint32_t value;
+  file >> value;
+  file.close();
+  //bf::remove(myClientIdFile);
+  return value;
+}
+#endif
+}
+
+LocalStateMetadata LocalStateMetadata::_generate(const bf::path &metadataFilePath) {
+  uint32_t myClientId = _generateClientId();
+#ifndef CRYFS_NO_COMPATIBILITY
+  // In the old format, this was stored in a "myClientId" file. If that file exists, load it from there.
+  optional<uint32_t> legacy = _tryLoadClientIdFromLegacyFile(metadataFilePath);
+  if (legacy != none) {
+    myClientId = *legacy;
+  }
+#endif
+
+  LocalStateMetadata result(myClientId);
+  result._save(metadataFilePath);
+  return result;
+}
+
+void LocalStateMetadata::_serialize(ostream& stream) const {
+  ptree pt;
+  pt.put<uint32_t>("myClientId", myClientId());
+
+  write_json(stream, pt);
+}
+
+LocalStateMetadata LocalStateMetadata::_deserialize(istream& stream) {
+  ptree pt;
+  read_json(stream, pt);
+
+  uint32_t myClientId = pt.get<uint32_t>("myClientId");
+
+  return LocalStateMetadata(myClientId);
+}
+
+
+}

src/cryfs/localstate/LocalStateMetadata.h

@@ -0,0 +1,36 @@
+#pragma once
+#ifndef MESSMER_CRYFS_LOCALSTATE_LOCALSTATEMETADATA_H_
+#define MESSMER_CRYFS_LOCALSTATE_LOCALSTATEMETADATA_H_
+
+#include <boost/filesystem/path.hpp>
+#include <boost/optional.hpp>
+#include <iostream>
+
+namespace cryfs {
+
+class LocalStateMetadata final {
+public:
+
+  static LocalStateMetadata loadOrGenerate(const boost::filesystem::path &statePath);
+
+  uint32_t myClientId() const;
+
+private:
+  LocalStateMetadata(uint32_t myClientId);
+
+  static boost::optional<LocalStateMetadata> _load(const boost::filesystem::path &metadataFilePath);
+  static LocalStateMetadata _deserialize(std::istream& stream);
+  static LocalStateMetadata _generate(const boost::filesystem::path &metadataFilePath);
+  void _save(const boost::filesystem::path &metadataFilePath) const;
+  void _serialize(std::ostream& stream) const;
+
+  const uint32_t _myClientId;
+};
+
+inline uint32_t LocalStateMetadata::myClientId() const {
+  return _myClientId;
+}
+
+}
+
+#endif

src/cryfs/localstate/MyClientId.cpp

@@ -1,54 +0,0 @@
-#include "MyClientId.h"
-#include <fstream>
-#include <cpp-utils/random/Random.h>
-#include <blockstore/implementations/integrity/KnownBlockVersions.h>
-
-using boost::optional;
-using boost::none;
-using std::ifstream;
-using std::ofstream;
-using cpputils::Random;
-using blockstore::integrity::KnownBlockVersions;
-namespace bf = boost::filesystem;
-
-namespace cryfs {
-
-    MyClientId::MyClientId(const bf::path &statePath)
-            :_stateFilePath(statePath / "myClientId") {
-    }
-
-    uint32_t MyClientId::loadOrGenerate() const {
-        auto loaded = _load();
-        if (loaded != none) {
-            return *loaded;
-        }
-        // If it couldn't be loaded, generate a new client id.
-        auto generated = _generate();
-        _save(generated);
-        return generated;
-    }
-
-    uint32_t MyClientId::_generate() {
-        uint32_t result;
-        do {
-            result = *reinterpret_cast<uint32_t*>(Random::PseudoRandom().getFixedSize<sizeof(uint32_t)>().data());
-        } while(result == KnownBlockVersions::CLIENT_ID_FOR_DELETED_BLOCK); // Safety check - CLIENT_ID_FOR_DELETED_BLOCK shouldn't be used by any valid client.
-        return result;
-    }
-
-    optional<uint32_t> MyClientId::_load() const {
-        ifstream file(_stateFilePath.native());
-        if (!file.good()) {
-            // State file doesn't exist
-            return none;
-        }
-        uint32_t value;
-        file >> value;
-        return value;
-    }
-
-    void MyClientId::_save(uint32_t clientId) const {
-        ofstream file(_stateFilePath.native());
-        file << clientId;
-    }
-}

src/cryfs/localstate/MyClientId.h

@@ -1,30 +0,0 @@
-#pragma once
-#ifndef MESSMER_CRYFS_LOCALSTATE_MYCLIENTID_H_
-#define MESSMER_CRYFS_LOCALSTATE_MYCLIENTID_H_
-
-#include <cpp-utils/macros.h>
-#include <boost/filesystem/path.hpp>
-#include <boost/optional.hpp>
-
-namespace cryfs {
-
-    class MyClientId final {
-    public:
-        MyClientId(const boost::filesystem::path &statePath);
-
-        uint32_t loadOrGenerate() const;
-
-    private:
-        const boost::filesystem::path _stateFilePath;
-
-        static uint32_t _generate();
-        boost::optional<uint32_t> _load() const;
-        void _save(uint32_t clientId) const;
-
-        DISALLOW_COPY_AND_ASSIGN(MyClientId);
-    };
-
-}
-
-
-#endif

test/cryfs/CMakeLists.txt

@@ -17,7 +17,7 @@ set(SOURCES
     filesystem/CryFsTest.cpp
     filesystem/CryNodeTest.cpp
     filesystem/FileSystemTest.cpp
-    localstate/MyClientIdTest.cpp
+        localstate/LocalStateMetadataTest.cpp
 )
 
 add_executable(${PROJECT_NAME} ${SOURCES})

test/cryfs/localstate/LocalStateMetadataTest.cpp

@@ -0,0 +1,38 @@
+#include <gtest/gtest.h>
+
+#include <cryfs/localstate/LocalStateMetadata.h>
+#include <cpp-utils/tempfile/TempDir.h>
+#include <fstream>
+
+using cpputils::TempDir;
+using cryfs::LocalStateMetadata;
+using std::ofstream;
+
+class LocalStateMetadataTest : public ::testing::Test {
+public:
+    TempDir stateDir;
+    TempDir stateDir2;
+};
+
+TEST_F(LocalStateMetadataTest, myClientId_ValueIsConsistent) {
+    LocalStateMetadata metadata1 = LocalStateMetadata::loadOrGenerate(stateDir.path());
+    LocalStateMetadata metadata2 = LocalStateMetadata::loadOrGenerate(stateDir.path());
+    EXPECT_EQ(metadata1.myClientId(), metadata2.myClientId());
+}
+
+TEST_F(LocalStateMetadataTest, myClientId_ValueIsRandomForNewClient) {
+    LocalStateMetadata metadata1 = LocalStateMetadata::loadOrGenerate(stateDir.path());
+    LocalStateMetadata metadata2 = LocalStateMetadata::loadOrGenerate(stateDir2.path());
+    EXPECT_NE(metadata1.myClientId(), metadata2.myClientId());
+}
+
+#ifndef CRYFS_NO_COMPATIBILITY
+TEST_F(LocalStateMetadataTest, myClientId_TakesLegacyValueIfSpecified) {
+  ofstream file((stateDir.path() / "myClientId").native());
+  file << 12345u;
+  file.close();
+
+  LocalStateMetadata metadata = LocalStateMetadata::loadOrGenerate(stateDir.path());
+  EXPECT_EQ(12345u, metadata.myClientId());
+}
+#endif

test/cryfs/localstate/MyClientIdTest.cpp

@@ -1,23 +0,0 @@
-#include <gtest/gtest.h>
-
-#include "cryfs/localstate/MyClientId.h"
-#include <cpp-utils/tempfile/TempDir.h>
-
-using cpputils::TempDir;
-using cryfs::MyClientId;
-
-class MyClientIdTest : public ::testing::Test {
-public:
-    TempDir stateDir;
-    TempDir stateDir2;
-};
-
-TEST_F(MyClientIdTest, ValueIsConsistent) {
-    uint32_t myClientId = MyClientId(stateDir.path()).loadOrGenerate();
-    EXPECT_EQ(myClientId, MyClientId(stateDir.path()).loadOrGenerate());
-}
-
-TEST_F(MyClientIdTest, ValueIsRandomForNewClient) {
-    uint32_t myClientId = MyClientId(stateDir.path()).loadOrGenerate();
-    EXPECT_NE(myClientId, MyClientId(stateDir2.path()).loadOrGenerate());
-}