From 4e8489fc4727d59d33d0710f01d96aa347a07e06 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adrien=20B=C3=A9raud?= <adrien.beraud@savoirfairelinux.com>
Date: Tue, 24 Jan 2017 18:09:03 -0500
Subject: [PATCH] ringaccount: use relative paths in configuration

By default for new accounts, use relative paths
for certificates, keys and archive.
Absolute paths are still supported.
This make it easier to move ring installs between
computers.

Change-Id: I5a34c8174225446e6b5d6cb17c05f147cdff0b5b
Reviewed-by: Guillaume Roguez <guillaume.roguez@savoirfairelinux.com>
---
 src/account.cpp             |  9 ++++++
 src/account.h               |  1 +
 src/config/yamlparser.cpp   | 15 +++++++--
 src/config/yamlparser.h     |  7 ++---
 src/fileutils.cpp           | 22 +++++++++++--
 src/fileutils.h             | 14 +++++++--
 src/ringdht/ringaccount.cpp | 61 ++++++++++++++++++++++---------------
 src/ringdht/ringaccount.h   |  4 +--
 src/sip/sipaccount.cpp      | 11 ++++++-
 src/sip/sipaccountbase.cpp  | 18 -----------
 10 files changed, 106 insertions(+), 56 deletions(-)

diff --git a/src/account.cpp b/src/account.cpp
index 244d470591..4ca0850c32 100644
--- a/src/account.cpp
+++ b/src/account.cpp
@@ -42,6 +42,7 @@ using random_device = dht::crypto::random_device;
 #include "client/ring_signal.h"
 #include "account_schema.h"
 #include "string_utils.h"
+#include "fileutils.h"
 #include "config/yamlparser.h"
 #include "system_codec_container.h"
 #include "ice_transport.h"
@@ -410,6 +411,14 @@ Account::parseString(const std::map<std::string, std::string>& details,
     s = iter->second;
 }
 
+void
+Account::parsePath(const std::map<std::string, std::string>& details,
+                   const char *key, std::string &s, const std::string& base)
+{
+    find_iter();
+    s = fileutils::getCleanPath(base, iter->second);
+}
+
 void
 Account::parseBool(const std::map<std::string, std::string>& details,
                    const char* key, bool &b)
diff --git a/src/account.h b/src/account.h
index f0d4375d5d..7ee94b5ef7 100644
--- a/src/account.h
+++ b/src/account.h
@@ -326,6 +326,7 @@ class Account : public Serializable, public std::enable_shared_from_this<Account
     protected:
         static void parseString(const std::map<std::string, std::string> &details, const char *key, std::string &s);
         static void parseBool(const std::map<std::string, std::string> &details, const char *key, bool &b);
+        static void parsePath(const std::map<std::string, std::string> &details, const char *key, std::string &s, const std::string& base);
 
         template<class T>
         static inline void
diff --git a/src/config/yamlparser.cpp b/src/config/yamlparser.cpp
index 1aac3d7caa..17160ba24d 100644
--- a/src/config/yamlparser.cpp
+++ b/src/config/yamlparser.cpp
@@ -19,13 +19,22 @@
  */
 
 #include "yamlparser.h"
+#include "fileutils.h"
 
-namespace ring {
+namespace ring { namespace yaml_utils {
+
+void
+parsePath(const YAML::Node &node, const char *key, std::string& path, const std::string& base)
+{
+    std::string val;
+    parseValue(node, key, val);
+    path = fileutils::getCleanPath(base, val);
+}
 
 // FIXME: Maybe think of something more clever, this is due to yaml-cpp's poor
 // handling of empty values for nested collections.
 std::vector<std::map<std::string, std::string>>
-yaml_utils::parseVectorMap(const YAML::Node &node, const std::initializer_list<std::string> &keys)
+parseVectorMap(const YAML::Node &node, const std::initializer_list<std::string> &keys)
 {
     std::vector<std::map<std::string, std::string>> result;
     for (const auto &n : node) {
@@ -38,4 +47,4 @@ yaml_utils::parseVectorMap(const YAML::Node &node, const std::initializer_list<s
     return result;
 }
 
-} // namespace ring
+}} // namespace ring::yaml_utils
diff --git a/src/config/yamlparser.h b/src/config/yamlparser.h
index a1234ec752..eaab6be237 100644
--- a/src/config/yamlparser.h
+++ b/src/config/yamlparser.h
@@ -18,8 +18,7 @@
  *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA.
  */
 
-#ifndef __YAMLPARSER_H__
-#define __YAMLPARSER_H__
+#pragma once
 
 #include <yaml-cpp/yaml.h>
 
@@ -33,9 +32,9 @@ void parseValue(const YAML::Node &node, const char *key, T &value)
     value = node[key].as<T>(value);
 }
 
+void parsePath(const YAML::Node &node, const char *key, std::string& path, const std::string& base);
+
 std::vector<std::map<std::string, std::string>>
 parseVectorMap(const YAML::Node &node, const std::initializer_list<std::string> &keys);
 
 }} // namespace ring::yaml_utils
-
-#endif
diff --git a/src/fileutils.cpp b/src/fileutils.cpp
index bd040d5a70..5e87286f13 100644
--- a/src/fileutils.cpp
+++ b/src/fileutils.cpp
@@ -299,12 +299,30 @@ bool isPathRelative(const std::string& path)
 #endif
 }
 
+std::string
+getCleanPath(const std::string& base, const std::string& path)
+{
+    if (base.empty() or path.size() < base.size())
+        return path;
+    auto base_sep = base + DIR_SEPARATOR_STR;
+    if (path.compare(0, base_sep.size(), base_sep) == 0)
+        return path.substr(base_sep.size());
+    else
+        return path;
+}
+
+std::string
+getFullPath(const std::string& base, const std::string& path)
+{
+    bool isRelative {not base.empty() and isPathRelative(path)};
+    return isRelative ? base + DIR_SEPARATOR_STR + path : path;
+}
+
 std::vector<uint8_t>
 loadFile(const std::string& path, const std::string& default_dir)
 {
-    bool isRelative {not default_dir.empty() and isPathRelative(path)};
     std::vector<uint8_t> buffer;
-    std::ifstream file(isRelative ? default_dir + DIR_SEPARATOR_STR + path : path, std::ios::binary);
+    std::ifstream file(getFullPath(default_dir, path), std::ios::binary);
     if (!file)
         throw std::runtime_error("Can't read file: "+path);
     file.seekg(0, std::ios::end);
diff --git a/src/fileutils.h b/src/fileutils.h
index 5d3d3c64f3..10e65c28d9 100644
--- a/src/fileutils.h
+++ b/src/fileutils.h
@@ -59,8 +59,8 @@ namespace ring { namespace fileutils {
 
     /**
      * Check directory existance and create it with given mode if it doesn't.
-     * @path path to check, relative or absolute
-     * @dir last directory creation mode
+     * @param path to check, relative or absolute
+     * @param dir last directory creation mode
      * @param parents default mode for all created directories except the last
      */
     bool check_dir(const char *path, mode_t dir=0755, mode_t parents=0755);
@@ -71,6 +71,16 @@ namespace ring { namespace fileutils {
     bool recursive_mkdir(const std::string& path, mode_t mode=0755);
 
     bool isPathRelative(const std::string& path);
+    /**
+     * If path is contained in base, return the suffix, otherwise return the full path.
+     * @param base must not finish with DIR_SEPARATOR_STR, can be empty
+     * @param path the path
+     */
+    std::string getCleanPath(const std::string& base, const std::string& path);
+    /**
+     * If path is relative, it is appended to base.
+     */
+    std::string getFullPath(const std::string& base, const std::string& path);
 
     bool isFile(const std::string& path);
     bool isDirectory(const std::string& path);
diff --git a/src/ringdht/ringaccount.cpp b/src/ringdht/ringaccount.cpp
index 49005d43cd..cfdd69c642 100644
--- a/src/ringdht/ringaccount.cpp
+++ b/src/ringdht/ringaccount.cpp
@@ -695,8 +695,17 @@ void RingAccount::serialize(YAML::Emitter &out)
 void RingAccount::unserialize(const YAML::Node &node)
 {
     using yaml_utils::parseValue;
+    using yaml_utils::parsePath;
 
     SIPAccountBase::unserialize(node);
+
+    // get tls submap
+    const auto &tlsMap = node[Conf::TLS_KEY];
+    parsePath(tlsMap, Conf::CERTIFICATE_KEY, tlsCertificateFile_, idPath_);
+    parsePath(tlsMap, Conf::CALIST_KEY, tlsCaListFile_, idPath_);
+    parseValue(tlsMap, Conf::TLS_PASSWORD_KEY, tlsPassword_);
+    parsePath(tlsMap, Conf::PRIVATE_KEY_KEY, tlsPrivateKeyFile_, idPath_);
+
     parseValue(node, Conf::DHT_ALLOW_PEERS_FROM_HISTORY, allowPeersFromHistory_);
     parseValue(node, Conf::DHT_ALLOW_PEERS_FROM_CONTACT, allowPeersFromContact_);
     parseValue(node, Conf::DHT_ALLOW_PEERS_FROM_TRUSTED, allowPeersFromTrusted_);
@@ -707,7 +716,7 @@ void RingAccount::unserialize(const YAML::Node &node)
     }
 
     try {
-        parseValue(node, DRing::Account::ConfProperties::ARCHIVE_PATH, archivePath_);
+        parsePath(node, DRing::Account::ConfProperties::ARCHIVE_PATH, archivePath_, idPath_);
     } catch (const std::exception& e) {
         RING_WARN("can't read archive path: %s", e.what());
     }
@@ -753,7 +762,7 @@ RingAccount::createRingDevice(const dht::crypto::Identity& id)
     fileutils::check_dir(idPath_.c_str(), 0700);
 
     // save the chain including CA
-    std::tie(tlsPrivateKeyFile_, tlsCertificateFile_) = saveIdentity(dev_id, idPath_ + DIR_SEPARATOR_STR "ring_device");
+    std::tie(tlsPrivateKeyFile_, tlsCertificateFile_) = saveIdentity(dev_id, idPath_, "ring_device");
     tlsPassword_ = {};
     identity_ = dev_id;
     ringDeviceId_ = dev_id.first->getPublicKey().getId().toString();
@@ -877,13 +886,13 @@ RingAccount::useIdentity(const dht::crypto::Identity& identity)
 }
 
 dht::crypto::Identity
-RingAccount::loadIdentity(const std::string& crt_path, const std::string& key_path, const std::string& key_pwd)
+RingAccount::loadIdentity(const std::string& crt_path, const std::string& key_path, const std::string& key_pwd) const
 {
     RING_DBG("Loading identity: %s %s", crt_path.c_str(), key_path.c_str());
     dht::crypto::Identity id;
     try {
-        dht::crypto::Certificate dht_cert(fileutils::loadFile(crt_path));
-        dht::crypto::PrivateKey  dht_key(fileutils::loadFile(key_path), key_pwd);
+        dht::crypto::Certificate dht_cert(fileutils::loadFile(crt_path, idPath_));
+        dht::crypto::PrivateKey  dht_key(fileutils::loadFile(key_path, idPath_), key_pwd);
         auto crt_id = dht_cert.getId();
         if (crt_id != dht_key.getPublicKey().getId())
             return {};
@@ -916,7 +925,7 @@ RingAccount::readArchive(const std::string& pwd) const
 
     // Read file
     try {
-        data = fileutils::loadFile(archivePath_);
+        data = fileutils::loadFile(archivePath_, idPath_);
     } catch (const std::exception& e) {
         RING_ERR("[Account %s] archive loading error: %s", getAccountID().c_str(), e.what());
         throw;
@@ -1063,8 +1072,8 @@ RingAccount::saveArchive(const ArchiveContent& archive_content, const std::strin
     // Write
     try {
         if (archivePath_.empty())
-            archivePath_ = idPath_ + DIR_SEPARATOR_STR "export.gz";
-        fileutils::saveFile(archivePath_, encrypted);
+            archivePath_ = "export.gz";
+        fileutils::saveFile(fileutils::getFullPath(idPath_, archivePath_), encrypted);
     } catch (const std::runtime_error& ex) {
         RING_ERR("Export failed: %s", ex.what());
         return;
@@ -1177,14 +1186,14 @@ RingAccount::revokeDevice(const std::string& password, const std::string& device
 }
 
 std::pair<std::string, std::string>
-RingAccount::saveIdentity(const dht::crypto::Identity id, const std::string& path) const
+RingAccount::saveIdentity(const dht::crypto::Identity id, const std::string& path, const std::string& name)
 {
-    auto paths = std::make_pair(path + ".key", path + ".crt");
+    auto names = std::make_pair(name + ".key", name + ".crt");
     if (id.first)
-        fileutils::saveFile(paths.first, id.first->serialize(), 0600);
+        fileutils::saveFile(path + DIR_SEPARATOR_STR + names.first, id.first->serialize(), 0600);
     if (id.second)
-        fileutils::saveFile(paths.second, id.second->getPacked(), 0600);
-    return paths;
+        fileutils::saveFile(path + DIR_SEPARATOR_STR + names.second, id.second->getPacked(), 0600);
+    return names;
 }
 
 void
@@ -1404,7 +1413,7 @@ RingAccount::migrateAccount(const std::string& pwd)
     }
 
     if (updateCertificates(archive, identity_)) {
-        std::tie(tlsPrivateKeyFile_, tlsCertificateFile_) = saveIdentity(identity_, idPath_ + DIR_SEPARATOR_STR "ring_device");
+        std::tie(tlsPrivateKeyFile_, tlsCertificateFile_) = saveIdentity(identity_, idPath_, "ring_device");
         saveArchive(archive, pwd);
         setRegistrationState(RegistrationState::UNREGISTERED);
         Migration::setState(accountID_, Migration::State::SUCCESS);
@@ -1420,14 +1429,7 @@ RingAccount::loadAccount(const std::string& archive_password, const std::string&
 
     RING_DBG("[Account %s] loading Ring account", getAccountID().c_str());
     try {
-#if TARGET_OS_IPHONE
-        const auto certPath = idPath_ + DIR_SEPARATOR_STR + tlsCertificateFile_;
-        const auto keyPath = idPath_ + DIR_SEPARATOR_STR + tlsPrivateKeyFile_;
-#else
-        const auto& certPath = tlsCertificateFile_;
-        const auto& keyPath = tlsPrivateKeyFile_;
-#endif
-        auto id = loadIdentity(certPath, keyPath, tlsPassword_);
+        auto id = loadIdentity(tlsCertificateFile_, tlsPrivateKeyFile_, tlsPassword_);
         bool hasValidId = useIdentity(id);
         bool needMigration = hasValidId and needsMigration(id);
         bool hasArchive = not archivePath_.empty() and fileutils::isFile(archivePath_);
@@ -1483,9 +1485,16 @@ RingAccount::loadAccount(const std::string& archive_password, const std::string&
 }
 
 void
-RingAccount::setAccountDetails(const std::map<std::string, std::string> &details)
+RingAccount::setAccountDetails(const std::map<std::string, std::string>& details)
 {
     SIPAccountBase::setAccountDetails(details);
+
+    // TLS
+    parsePath(details, Conf::CONFIG_TLS_CA_LIST_FILE, tlsCaListFile_, idPath_);
+    parsePath(details, Conf::CONFIG_TLS_CERTIFICATE_FILE, tlsCertificateFile_, idPath_);
+    parsePath(details, Conf::CONFIG_TLS_PRIVATE_KEY_FILE, tlsPrivateKeyFile_, idPath_);
+    parseString(details, Conf::CONFIG_TLS_PASSWORD, tlsPassword_);
+
     if (hostname_.empty())
         hostname_ = DHT_DEFAULT_BOOTSTRAP;
     parseInt(details, Conf::CONFIG_DHT_PORT, dhtPort_);
@@ -1502,7 +1511,7 @@ RingAccount::setAccountDetails(const std::map<std::string, std::string> &details
     parseString(details, DRing::Account::ConfProperties::ARCHIVE_PASSWORD, archive_password);
     parseString(details, DRing::Account::ConfProperties::ARCHIVE_PIN,      archive_pin);
     std::transform(archive_pin.begin(), archive_pin.end(), archive_pin.begin(), ::toupper);
-    parseString(details, DRing::Account::ConfProperties::ARCHIVE_PATH,     archivePath_);
+    parsePath(details, DRing::Account::ConfProperties::ARCHIVE_PATH,     archivePath_, idPath_);
     parseString(details, DRing::Account::ConfProperties::RING_DEVICE_NAME, ringDeviceName_);
 
 #if HAVE_RINGNS
@@ -1530,6 +1539,10 @@ RingAccount::getAccountDetails() const
     a.emplace(Conf::CONFIG_SRTP_ENABLE,       isSrtpEnabled() ? TRUE_STR : FALSE_STR);
     a.emplace(Conf::CONFIG_SRTP_RTP_FALLBACK, getSrtpFallback() ? TRUE_STR : FALSE_STR);
 
+    a.emplace(Conf::CONFIG_TLS_CA_LIST_FILE,        fileutils::getFullPath(idPath_, tlsCaListFile_));
+    a.emplace(Conf::CONFIG_TLS_CERTIFICATE_FILE,    fileutils::getFullPath(idPath_, tlsCertificateFile_));
+    a.emplace(Conf::CONFIG_TLS_PRIVATE_KEY_FILE,    fileutils::getFullPath(idPath_, tlsPrivateKeyFile_));
+    a.emplace(Conf::CONFIG_TLS_PASSWORD,            tlsPassword_);
     a.emplace(Conf::CONFIG_TLS_METHOD,                     "Automatic");
     a.emplace(Conf::CONFIG_TLS_CIPHERS,                    "");
     a.emplace(Conf::CONFIG_TLS_SERVER_NAME,                "");
diff --git a/src/ringdht/ringaccount.h b/src/ringdht/ringaccount.h
index e1882754de..3024c7fa6f 100644
--- a/src/ringdht/ringaccount.h
+++ b/src/ringdht/ringaccount.h
@@ -499,7 +499,7 @@ class RingAccount : public SIPAccountBase {
         static ArchiveContent loadArchive(const std::vector<uint8_t>& data);
         std::vector<std::pair<sockaddr_storage, socklen_t>> loadBootstrap() const;
 
-        std::pair<std::string, std::string> saveIdentity(const dht::crypto::Identity id, const std::string& path) const;
+        static std::pair<std::string, std::string> saveIdentity(const dht::crypto::Identity id, const std::string& path, const std::string& name);
         void saveNodes(const std::vector<dht::NodeExport>&) const;
         void saveValues(const std::vector<dht::ValuesExport>&) const;
 
@@ -525,7 +525,7 @@ class RingAccount : public SIPAccountBase {
          * and certPath_ a valid certificate file, load and returns them.
          * Otherwise, generate a new identity and returns it.
          */
-        static dht::crypto::Identity loadIdentity(const std::string& crt_path, const std::string& key_path, const std::string& key_pwd);
+        dht::crypto::Identity loadIdentity(const std::string& crt_path, const std::string& key_path, const std::string& key_pwd) const;
         std::vector<dht::NodeExport> loadNodes() const;
         std::vector<dht::ValuesExport> loadValues() const;
 
diff --git a/src/sip/sipaccount.cpp b/src/sip/sipaccount.cpp
index da6a6fa4e3..1bafebdd92 100644
--- a/src/sip/sipaccount.cpp
+++ b/src/sip/sipaccount.cpp
@@ -499,7 +499,10 @@ void SIPAccount::unserialize(const YAML::Node &node)
 
     // get tls submap
     const auto &tlsMap = node[Conf::TLS_KEY];
-
+    parseValue(tlsMap, Conf::CERTIFICATE_KEY, tlsCertificateFile_);
+    parseValue(tlsMap, Conf::CALIST_KEY, tlsCaListFile_);
+    parseValue(tlsMap, Conf::TLS_PASSWORD_KEY, tlsPassword_);
+    parseValue(tlsMap, Conf::PRIVATE_KEY_KEY, tlsPrivateKeyFile_);
     parseValue(tlsMap, Conf::TLS_ENABLE_KEY, tlsEnable_);
     parseValue(tlsMap, Conf::TLS_PORT_KEY, tlsListenerPort_);
     parseValue(tlsMap, Conf::CIPHERS_KEY, tlsCiphers_);
@@ -530,6 +533,12 @@ void SIPAccount::setAccountDetails(const std::map<std::string, std::string> &det
 
     parseInt(details, Conf::CONFIG_LOCAL_PORT, localPort_);
 
+    // TLS
+    parseString(details, Conf::CONFIG_TLS_CA_LIST_FILE, tlsCaListFile_);
+    parseString(details, Conf::CONFIG_TLS_CERTIFICATE_FILE, tlsCertificateFile_);
+    parseString(details, Conf::CONFIG_TLS_PRIVATE_KEY_FILE, tlsPrivateKeyFile_);
+    parseString(details, Conf::CONFIG_TLS_PASSWORD, tlsPassword_);
+
     // SIP specific account settings
     parseString(details, Conf::CONFIG_ACCOUNT_ROUTESET, serviceRoute_);
 
diff --git a/src/sip/sipaccountbase.cpp b/src/sip/sipaccountbase.cpp
index 098995d53f..823558b14a 100644
--- a/src/sip/sipaccountbase.cpp
+++ b/src/sip/sipaccountbase.cpp
@@ -196,13 +196,6 @@ void SIPAccountBase::unserialize(const YAML::Node &node)
 
     parseValue(node, Conf::DTMF_TYPE_KEY, dtmfType_);
 
-    // get tls submap
-    const auto &tlsMap = node[Conf::TLS_KEY];
-    parseValue(tlsMap, Conf::CERTIFICATE_KEY, tlsCertificateFile_);
-    parseValue(tlsMap, Conf::CALIST_KEY, tlsCaListFile_);
-    parseValue(tlsMap, Conf::TLS_PASSWORD_KEY, tlsPassword_);
-    parseValue(tlsMap, Conf::PRIVATE_KEY_KEY, tlsPrivateKeyFile_);
-
     unserializeRange(node, Conf::AUDIO_PORT_MIN_KEY, Conf::AUDIO_PORT_MAX_KEY, audioPortRange_);
     unserializeRange(node, Conf::VIDEO_PORT_MIN_KEY, Conf::VIDEO_PORT_MAX_KEY, videoPortRange_);
 
@@ -245,12 +238,6 @@ void SIPAccountBase::setAccountDetails(const std::map<std::string, std::string>
     updateRange(tmpMin, tmpMax, videoPortRange_);
 #endif
 
-    // TLS
-    parseString(details, Conf::CONFIG_TLS_CA_LIST_FILE, tlsCaListFile_);
-    parseString(details, Conf::CONFIG_TLS_CERTIFICATE_FILE, tlsCertificateFile_);
-    parseString(details, Conf::CONFIG_TLS_PRIVATE_KEY_FILE, tlsPrivateKeyFile_);
-    parseString(details, Conf::CONFIG_TLS_PASSWORD, tlsPassword_);
-
     // ICE - STUN
     parseBool(details, Conf::CONFIG_STUN_ENABLE, stunEnabled_);
     parseString(details, Conf::CONFIG_STUN_SERVER, stunServer_);
@@ -288,11 +275,6 @@ SIPAccountBase::getAccountDetails() const
     a.emplace(Conf::CONFIG_TURN_SERVER_PWD, turnServerPwd_);
     a.emplace(Conf::CONFIG_TURN_SERVER_REALM, turnServerRealm_);
 
-    a.emplace(Conf::CONFIG_TLS_CA_LIST_FILE,        tlsCaListFile_);
-    a.emplace(Conf::CONFIG_TLS_CERTIFICATE_FILE,    tlsCertificateFile_);
-    a.emplace(Conf::CONFIG_TLS_PRIVATE_KEY_FILE,    tlsPrivateKeyFile_);
-    a.emplace(Conf::CONFIG_TLS_PASSWORD,            tlsPassword_);
-
     return a;
 }
 
-- 
GitLab