Merge pull request #2144 from chriseth/cleanupSecretStore

Cleanup of SecretStore.

libdevcore/CommonIO.cpp

@@ -23,13 +23,14 @@
 #include <iostream>
 #include <cstdlib>
 #include <fstream>
-#include "Exceptions.h"
 #include <stdio.h>
 #ifdef _WIN32
 #include <windows.h>
 #else
 #include <termios.h>
 #endif
+#include <boost/filesystem.hpp>
+#include "Exceptions.h"
 using namespace std;
 using namespace dev;
 
@@ -95,9 +96,23 @@ string dev::contentsString(string const& _file)
 	return contentsGeneric<string>(_file);
 }
 
-void dev::writeFile(std::string const& _file, bytesConstRef _data)
+void dev::writeFile(std::string const& _file, bytesConstRef _data, bool _writeDeleteRename)
 {
-	ofstream(_file, ios::trunc|ios::binary).write((char const*)_data.data(), _data.size());
+	if (_writeDeleteRename)
+	{
+		namespace fs = boost::filesystem;
+		fs::path tempPath = fs::unique_path(_file + "-%%%%%%");
+		writeFile(tempPath.string(), _data, false);
+		// will delete _file if it exists
+		fs::rename(tempPath, _file);
+	}
+	else
+	{
+		ofstream s(_file, ios::trunc | ios::binary);
+		s.write(reinterpret_cast<char const*>(_data.data()), _data.size());
+		if (!s)
+			BOOST_THROW_EXCEPTION(FileError());
+	}
 }
 
 std::string dev::getPassword(std::string const& _prompt)

libdevcore/CommonIO.h

@@ -56,10 +56,13 @@ std::string contentsString(std::string const& _file);
 bytesRef contentsNew(std::string const& _file, bytesRef _dest = bytesRef());
 
 /// Write the given binary data into the given file, replacing the file if it pre-exists.
-void writeFile(std::string const& _file, bytesConstRef _data);
+/// Throws exception on error.
+/// @param _writeDeleteRename useful not to lose any data: If set, first writes to another file in
+/// the same directory and then moves that file.
+void writeFile(std::string const& _file, bytesConstRef _data, bool _writeDeleteRename = false);
 /// Write the given binary data into the given file, replacing the file if it pre-exists.
-inline void writeFile(std::string const& _file, bytes const& _data) { writeFile(_file, bytesConstRef(&_data)); }
-inline void writeFile(std::string const& _file, std::string const& _data) { writeFile(_file, bytesConstRef(_data)); }
+inline void writeFile(std::string const& _file, bytes const& _data, bool _writeDeleteRename = false) { writeFile(_file, bytesConstRef(&_data), _writeDeleteRename); }
+inline void writeFile(std::string const& _file, std::string const& _data, bool _writeDeleteRename = false) { writeFile(_file, bytesConstRef(_data), _writeDeleteRename); }
 
 /// Nicely renders the given bytes to a string, optionally as HTML.
 /// @a _bytes: bytes array to be rendered as string. @a _width of a bytes line.

libdevcrypto/Common.cpp

@@ -31,6 +31,7 @@
 #include <libdevcore/FileSystem.h>
 #include "AES.h"
 #include "CryptoPP.h"
+#include "Exceptions.h"
 using namespace std;
 using namespace dev;
 using namespace dev::crypto;
@@ -178,15 +179,35 @@ bool dev::verify(Public const& _p, Signature const& _s, h256 const& _hash)
 bytes dev::pbkdf2(string const& _pass, bytes const& _salt, unsigned _iterations, unsigned _dkLen)
 {
 	bytes ret(_dkLen);
-	PKCS5_PBKDF2_HMAC<SHA256> pbkdf;
-	pbkdf.DeriveKey(ret.data(), ret.size(), 0, (byte*)_pass.data(), _pass.size(), _salt.data(), _salt.size(), _iterations);
+	if (PKCS5_PBKDF2_HMAC<SHA256>().DeriveKey(
+		ret.data(),
+		ret.size(),
+		0,
+		reinterpret_cast<byte const*>(_pass.data()),
+		_pass.size(),
+		_salt.data(),
+		_salt.size(),
+		_iterations
+	) != _iterations)
+		BOOST_THROW_EXCEPTION(CryptoException() << errinfo_comment("Key derivation failed."));
 	return ret;
 }
 
 bytes dev::scrypt(std::string const& _pass, bytes const& _salt, uint64_t _n, uint32_t _r, uint32_t _p, unsigned _dkLen)
 {
 	bytes ret(_dkLen);
-	libscrypt_scrypt((uint8_t const*)_pass.data(), _pass.size(), _salt.data(), _salt.size(), _n, _r, _p, ret.data(), ret.size());
+	if (libscrypt_scrypt(
+		reinterpret_cast<uint8_t const*>(_pass.data()),
+		_pass.size(),
+		_salt.data(),
+		_salt.size(),
+		_n,
+		_r,
+		_p,
+		ret.data(),
+		ret.size()
+	) != 0)
+		BOOST_THROW_EXCEPTION(CryptoException() << errinfo_comment("Key derivation failed."));
 	return ret;
 }
 
@@ -233,42 +254,84 @@ h256 crypto::kdf(Secret const& _priv, h256 const& _hash)
 	return s;
 }
 
-h256 Nonce::get(bool _commit)
+mutex Nonce::s_x;
+static string s_seedFile;
+
+h256 Nonce::get()
 {
 	// todo: atomic efface bit, periodic save, kdf, rr, rng
 	// todo: encrypt
-	static h256 s_seed;
-	static string s_seedFile(getDataDir() + "/seed");
-	static mutex s_x;
-	Guard l(s_x);
-	if (!s_seed)
+	Guard l(Nonce::s_x);
+	return Nonce::singleton().next();
+}
+
+void Nonce::reset()
+{
+	Guard l(Nonce::s_x);
+	Nonce::singleton().resetInternal();
+}
+
+void Nonce::setSeedFilePath(string const& _filePath)
+{
+	s_seedFile = _filePath;
+}
+
+Nonce::~Nonce()
+{
+	Guard l(Nonce::s_x);
+	if (m_value)
+		// this might throw
+		resetInternal();
+}
+
+Nonce& Nonce::singleton()
+{
+	static Nonce s;
+	return s;
+}
+
+void Nonce::initialiseIfNeeded()
+{
+	if (m_value)
+		return;
+
+	bytes b = contents(seedFile());
+	if (b.size() == 32)
+		memcpy(m_value.data(), b.data(), 32);
+	else
 	{
-		static Nonce s_nonce;
-		bytes b = contents(s_seedFile);
-		if (b.size() == 32)
-			memcpy(s_seed.data(), b.data(), 32);
-		else
-		{
-			// todo: replace w/entropy from user and system
-			std::mt19937_64 s_eng(time(0) + chrono::high_resolution_clock::now().time_since_epoch().count());
-			std::uniform_int_distribution<uint16_t> d(0, 255);
-			for (unsigned i = 0; i < 32; ++i)
-				s_seed[i] = (byte)d(s_eng);
-		}
-		if (!s_seed)
-			BOOST_THROW_EXCEPTION(InvalidState());
-		
-		// prevent seed reuse if process terminates abnormally
-		writeFile(s_seedFile, bytes());
+		// todo: replace w/entropy from user and system
+		std::mt19937_64 s_eng(time(0) + chrono::high_resolution_clock::now().time_since_epoch().count());
+		std::uniform_int_distribution<uint16_t> d(0, 255);
+		for (unsigned i = 0; i < 32; ++i)
+			m_value[i] = byte(d(s_eng));
 	}
-	h256 prev(s_seed);
-	sha3(prev.ref(), s_seed.ref());
-	if (_commit)
-		writeFile(s_seedFile, s_seed.asBytes());
-	return std::move(s_seed);
+	if (!m_value)
+		BOOST_THROW_EXCEPTION(InvalidState());
+
+	// prevent seed reuse if process terminates abnormally
+	// this might throw
+	writeFile(seedFile(), bytes());
 }
 
-Nonce::~Nonce()
+h256 Nonce::next()
+{
+	initialiseIfNeeded();
+	m_value = sha3(m_value);
+	return m_value;
+}
+
+void Nonce::resetInternal()
+{
+	// this might throw
+	next();
+	writeFile(seedFile(), m_value.asBytes());
+	m_value = h256();
+}
+
+string const& Nonce::seedFile()
 {
-	Nonce::get(true);
+	if (s_seedFile.empty())
+		s_seedFile = getDataDir() + "/seed";
+	return s_seedFile;
 }

libdevcrypto/Common.h

@@ -24,6 +24,7 @@
 
 #pragma once
 
+#include <mutex>
 #include <libdevcore/Common.h>
 #include <libdevcore/FixedHash.h>
 #include <libdevcore/Exceptions.h>
@@ -180,14 +181,36 @@ struct InvalidState: public dev::Exception {};
 h256 kdf(Secret const& _priv, h256 const& _hash);
 
 /**
- * @brief Generator for nonce material
+ * @brief Generator for nonce material.
  */
 struct Nonce
 {
-	static h256 get(bool _commit = false);
+	/// Returns the next nonce (might be read from a file).
+	static h256 get();
+	/// Stores the current nonce in a file and resets Nonce to the uninitialised state.
+	static void reset();
+	/// Sets the location of the seed file to a non-default place. Used for testing.
+	static void setSeedFilePath(std::string const& _filePath);
+
 private:
 	Nonce() {}
 	~Nonce();
+	/// @returns the singleton instance.
+	static Nonce& singleton();
+	/// Reads the last seed from the seed file.
+	void initialiseIfNeeded();
+	/// @returns the next nonce.
+	h256 next();
+	/// Stores the current seed in the seed file.
+	void resetInternal();
+	/// @returns the path of the seed file.
+	static std::string const& seedFile();
+
+	/// Mutex for the singleton object.
+	/// @note Every access to any private function has to be guarded by this mutex.
+	static std::mutex s_x;
+
+	h256 m_value;
 };
 }
 

libdevcrypto/Exceptions.h

@@ -0,0 +1,35 @@
+/*
+	This file is part of cpp-ethereum.
+
+	cpp-ethereum is free software: you can redistribute it and/or modify
+	it under the terms of the GNU General Public License as published by
+	the Free Software Foundation, either version 3 of the License, or
+	(at your option) any later version.
+
+	cpp-ethereum is distributed in the hope that it will be useful,
+	but WITHOUT ANY WARRANTY; without even the implied warranty of
+	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+	GNU General Public License for more details.
+
+	You should have received a copy of the GNU General Public License
+	along with cpp-ethereum.  If not, see <http://www.gnu.org/licenses/>.
+*/
+/** @file Exceptions.h
+ * @author Christian <c@ethdev.com>
+ * @date 2016
+ */
+
+#pragma once
+
+#include <libdevcore/Exceptions.h>
+
+namespace dev
+{
+namespace crypto
+{
+
+/// Rare malfunction of cryptographic functions.
+DEV_SIMPLE_EXCEPTION(CryptoException);
+
+}
+}

libdevcrypto/SecretStore.cpp

@@ -29,6 +29,7 @@
 #include <libdevcore/SHA3.h>
 #include <libdevcore/FileSystem.h>
 #include <test/JsonSpiritHeaders.h>
+#include <libdevcrypto/Exceptions.h>
 using namespace std;
 using namespace dev;
 namespace js = json_spirit;
@@ -36,7 +37,8 @@ namespace fs = boost::filesystem;
 
 static const int c_keyFileVersion = 3;
 
-static js::mValue upgraded(std::string const& _s)
+/// Upgrade the json-format to the current version.
+static js::mValue upgraded(string const& _s)
 {
 	js::mValue v;
 	js::read_string(_s, v);
@@ -84,35 +86,34 @@ static js::mValue upgraded(std::string const& _s)
 	return js::mValue();
 }
 
-SecretStore::SecretStore(std::string const& _path): m_path(_path)
+SecretStore::SecretStore(string const& _path): m_path(_path)
 {
 	load();
 }
 
-SecretStore::~SecretStore()
+bytes SecretStore::secret(h128 const& _uuid, function<string()> const& _pass, bool _useCache) const
 {
-}
-
-bytes SecretStore::secret(h128 const& _uuid, function<std::string()> const& _pass, bool _useCache) const
-{
-	(void)_pass;
 	auto rit = m_cached.find(_uuid);
 	if (_useCache && rit != m_cached.end())
 		return rit->second;
 	auto it = m_keys.find(_uuid);
-	if (it == m_keys.end())
-		return bytes();
-	bytes key = decrypt(it->second.first, _pass());
-	if (!key.empty())
-		m_cached[_uuid] = key;
+	bytes key;
+	if (it != m_keys.end())
+	{
+		key = decrypt(it->second.encryptedKey, _pass());
+		if (!key.empty())
+			m_cached[_uuid] = key;
+	}
 	return key;
 }
 
-h128 SecretStore::importSecret(bytes const& _s, std::string const& _pass)
+h128 SecretStore::importSecret(bytes const& _s, string const& _pass)
 {
-	h128 r = h128::random();
+	h128 r;
+	EncryptedKey key{encrypt(_s, _pass), string()};
+	r = h128::random();
 	m_cached[r] = _s;
-	m_keys[r] = make_pair(encrypt(_s, _pass), std::string());
+	m_keys[r] = move(key);
 	save();
 	return r;
 }
@@ -122,7 +123,7 @@ void SecretStore::kill(h128 const& _uuid)
 	m_cached.erase(_uuid);
 	if (m_keys.count(_uuid))
 	{
-		boost::filesystem::remove(m_keys[_uuid].second);
+		fs::remove(m_keys[_uuid].filename);
 		m_keys.erase(_uuid);
 	}
 }
@@ -132,50 +133,50 @@ void SecretStore::clearCache() const
 	m_cached.clear();
 }
 
-void SecretStore::save(std::string const& _keysPath)
+void SecretStore::save(string const& _keysPath)
 {
 	fs::path p(_keysPath);
-	boost::filesystem::create_directories(p);
+	fs::create_directories(p);
 	for (auto& k: m_keys)
 	{
-		std::string uuid = toUUID(k.first);
-		std::string filename = (p / uuid).string() + ".json";
+		string uuid = toUUID(k.first);
+		string filename = (p / uuid).string() + ".json";
 		js::mObject v;
 		js::mValue crypto;
-		js::read_string(k.second.first, crypto);
+		js::read_string(k.second.encryptedKey, crypto);
 		v["crypto"] = crypto;
 		v["id"] = uuid;
 		v["version"] = c_keyFileVersion;
 		writeFile(filename, js::write_string(js::mValue(v), true));
-		if (!k.second.second.empty() && k.second.second != filename)
-			boost::filesystem::remove(k.second.second);
-		k.second.second = filename;
+		swap(k.second.filename, filename);
+		if (!filename.empty() && !fs::equivalent(filename, k.second.filename))
+			fs::remove(filename);
 	}
 }
 
-void SecretStore::load(std::string const& _keysPath)
+void SecretStore::load(string const& _keysPath)
 {
 	fs::path p(_keysPath);
-	boost::filesystem::create_directories(p);
+	fs::create_directories(p);
 	for (fs::directory_iterator it(p); it != fs::directory_iterator(); ++it)
-		if (is_regular_file(it->path()))
+		if (fs::is_regular_file(it->path()))
 			readKey(it->path().string(), true);
 }
 
-h128 SecretStore::readKey(std::string const& _file, bool _deleteFile)
+h128 SecretStore::readKey(string const& _file, bool _takeFileOwnership)
 {
 	cnote << "Reading" << _file;
-	return readKeyContent(contentsString(_file), _deleteFile ? _file : string());
+	return readKeyContent(contentsString(_file), _takeFileOwnership ? _file : string());
 }
 
-h128 SecretStore::readKeyContent(std::string const& _content, std::string const& _file)
+h128 SecretStore::readKeyContent(string const& _content, string const& _file)
 {
 	js::mValue u = upgraded(_content);
 	if (u.type() == js::obj_type)
 	{
 		js::mObject& o = u.get_obj();
 		auto uuid = fromUUID(o["id"].get_str());
-		m_keys[uuid] = make_pair(js::write_string(o["crypto"], false), _file);
+		m_keys[uuid] = EncryptedKey{js::write_string(o["crypto"], false), _file};
 		return uuid;
 	}
 	else
@@ -183,62 +184,63 @@ h128 SecretStore::readKeyContent(std::string const& _content, std::string const&
 	return h128();
 }
 
-bool SecretStore::recode(h128 const& _uuid, string const& _newPass, std::function<std::string()> const& _pass, KDF _kdf)
+bool SecretStore::recode(h128 const& _uuid, string const& _newPass, function<string()> const& _pass, KDF _kdf)
 {
-//	cdebug << "recode:" << toUUID(_uuid);
 	bytes s = secret(_uuid, _pass, true);
 	if (s.empty())
 		return false;
-	m_keys[_uuid].first = encrypt(s, _newPass, _kdf);
+	m_cached.erase(_uuid);
+	m_keys[_uuid].encryptedKey = encrypt(s, _newPass, _kdf);
 	save();
 	return true;
 }
 
-std::string SecretStore::encrypt(bytes const& _v, std::string const& _pass, KDF _kdf)
+static bytes deriveNewKey(string const& _pass, KDF _kdf, js::mObject& o_ret)
 {
-	js::mObject ret;
-
-	// KDF info
 	unsigned dklen = 32;
+	unsigned iterations = 1 << 18;
 	bytes salt = h256::random().asBytes();
-	bytes derivedKey;
 	if (_kdf == KDF::Scrypt)
 	{
-		unsigned iterations = 262144;
 		unsigned p = 1;
 		unsigned r = 8;
-		ret["kdf"] = "scrypt";
+		o_ret["kdf"] = "scrypt";
 		{
 			js::mObject params;
-			params["n"] = (int64_t)iterations;
-			params["r"] = (int)r;
-			params["p"] = (int)p;
-			params["dklen"] = (int)dklen;
+			params["n"] = int64_t(iterations);
+			params["r"] = int(r);
+			params["p"] = int(p);
+			params["dklen"] = int(dklen);
 			params["salt"] = toHex(salt);
-			ret["kdfparams"] = params;
+			o_ret["kdfparams"] = params;
 		}
-		derivedKey = scrypt(_pass, salt, iterations, r, p, dklen);
+		return scrypt(_pass, salt, iterations, r, p, dklen);
 	}
 	else
 	{
-		unsigned iterations = 262144;
-		ret["kdf"] = "pbkdf2";
+		o_ret["kdf"] = "pbkdf2";
 		{
 			js::mObject params;
 			params["prf"] = "hmac-sha256";
-			params["c"] = (int)iterations;
+			params["c"] = int(iterations);
 			params["salt"] = toHex(salt);
-			params["dklen"] = (int)dklen;
-			ret["kdfparams"] = params;
+			params["dklen"] = int(dklen);
+			o_ret["kdfparams"] = params;
 		}
-		derivedKey = pbkdf2(_pass, salt, iterations, dklen);
+		return pbkdf2(_pass, salt, iterations, dklen);
 	}
-//	cdebug << "derivedKey" << toHex(derivedKey);
+}
+
+string SecretStore::encrypt(bytes const& _v, string const& _pass, KDF _kdf)
+{
+	js::mObject ret;
+
+	bytes derivedKey = deriveNewKey(_pass, _kdf, ret);
+	if (derivedKey.empty())
+		BOOST_THROW_EXCEPTION(crypto::CryptoException() << errinfo_comment("Key derivation failed."));
 
-	// cipher info
 	ret["cipher"] = "aes-128-ctr";
 	h128 key(derivedKey, h128::AlignLeft);
-//	cdebug << "cipherKey" << key.hex();
 	h128 iv = h128::random();
 	{
 		js::mObject params;
@@ -248,18 +250,18 @@ std::string SecretStore::encrypt(bytes const& _v, std::string const& _pass, KDF
 
 	// cipher text
 	bytes cipherText = encryptSymNoAuth(key, iv, &_v);
+	if (cipherText.empty())
+		BOOST_THROW_EXCEPTION(crypto::CryptoException() << errinfo_comment("Key encryption failed."));
 	ret["ciphertext"] = toHex(cipherText);
 
 	// and mac.
 	h256 mac = sha3(ref(derivedKey).cropped(16, 16).toBytes() + cipherText);
-//	cdebug << "macBody" << toHex(ref(derivedKey).cropped(16, 16).toBytes() + cipherText);
-//	cdebug << "mac" << mac.hex();
 	ret["mac"] = toHex(mac.ref());
 
-	return js::write_string((js::mValue)ret, true);
+	return js::write_string(js::mValue(ret), true);
 }
 
-bytes SecretStore::decrypt(std::string const& _v, std::string const& _pass)
+bytes SecretStore::decrypt(string const& _v, string const& _pass)
 {
 	js::mObject o;
 	{

libdevcrypto/SecretStore.h

@@ -35,41 +35,81 @@ enum class KDF {
 	Scrypt,
 };
 
+/**
+ * Manages encrypted keys stored in a certain directory on disk. The keys are read into memory
+ * and changes to the keys are automatically synced to the directory.
+ * Each file stores exactly one key in a specific JSON format whose file name is derived from the
+ * UUID of the key.
+ * @note that most of the functions here affect the filesystem and throw exceptions on failure,
+ * and they also throw exceptions upon rare malfunction in the cryptographic functions.
+ */
 class SecretStore
 {
 public:
+	/// Construct a new SecretStore and read all keys in the given directory.
 	SecretStore(std::string const& _path = defaultPath());
-	~SecretStore();
 
+	/// @returns the secret key stored by the given @a _uuid.
+	/// @param _pass function that returns the password for the key.
+	/// @param _useCache if true, allow previously decrypted keys to be returned directly.
 	bytes secret(h128 const& _uuid, std::function<std::string()> const& _pass, bool _useCache = true) const;
+	/// Imports the (encrypted) key stored in the file @a _file and copies it to the managed directory.
 	h128 importKey(std::string const& _file) { auto ret = readKey(_file, false); if (ret) save(); return ret; }
+	/// Imports the (encrypted) key contained in the json formatted @a _content and stores it in
+	/// the managed directory.
 	h128 importKeyContent(std::string const& _content) { auto ret = readKeyContent(_content, std::string()); if (ret) save(); return ret; }
+	/// Imports the decrypted key given by @a _s and stores it, encrypted with
+	/// (a key derived from) the password @a _pass.
 	h128 importSecret(bytes const& _s, std::string const& _pass);
+	/// Decrypts and re-encrypts the key identified by @a _uuid.
 	bool recode(h128 const& _uuid, std::string const& _newPass, std::function<std::string()> const& _pass, KDF _kdf = KDF::Scrypt);
+	/// Removes the key specified by @a _uuid from both memory and disk.
 	void kill(h128 const& _uuid);
 
+	/// Returns the uuids of all stored keys.
 	std::vector<h128> keys() const { return keysOf(m_keys); }
 
-	// Clear any cached keys.
+	/// Clears all cached decrypted keys. The passwords have to be supplied in order to retrieve
+	/// secrets again after calling this function.
 	void clearCache() const;
 
-	// Doesn't save().
-	h128 readKey(std::string const& _file, bool _deleteFile);
+	/// Import the key from the file @a _file, but do not copy it to the managed directory yet.
+	/// @param _takeFileOwnership if true, deletes the file if it is not the canonical file for the
+	/// key (derived from its uuid).
+	h128 readKey(std::string const& _file, bool _takeFileOwnership);
+	/// Import the key contained in the json-encoded @a _content, but do not store it in the
+	/// managed directory.
+	/// @param _file if given, assume this file contains @a _content and delete it later, if it is
+	/// not the canonical file for the key (derived from the uuid).
 	h128 readKeyContent(std::string const& _content, std::string const& _file = std::string());
 
+	/// Store all keys in the directory @a _keysPath.
 	void save(std::string const& _keysPath);
+	/// Store all keys in the managed directory.
 	void save() { save(m_path); }
 
+	/// @returns the default path for the managed directory.
 	static std::string defaultPath() { return getDataDir("web3") + "/keys"; }
 
 private:
+	struct EncryptedKey
+	{
+		std::string encryptedKey;
+		std::string filename;
+	};
+
+	/// Loads all keys in the given directory.
 	void load(std::string const& _keysPath);
 	void load() { load(m_path); }
+	/// Encrypts @a _v with a key derived from @a _pass or the empty string on error.
 	static std::string encrypt(bytes const& _v, std::string const& _pass, KDF _kdf = KDF::Scrypt);
+	/// Decrypts @a _v with a key derived from @a _pass or the empty byte array on error.
 	static bytes decrypt(std::string const& _v, std::string const& _pass);
 
+	/// Stores decrypted keys by uuid.
 	mutable std::unordered_map<h128, bytes> m_cached;
-	std::unordered_map<h128, std::pair<std::string, std::string>> m_keys;
+	/// Stores encrypted keys together with the file they were loaded from by uuid.
+	std::unordered_map<h128, EncryptedKey> m_keys;
 
 	std::string m_path;
 };

test/TestUtils.cpp

@@ -22,6 +22,7 @@
 #include <thread>
 #include <boost/test/unit_test.hpp>
 #include <boost/filesystem.hpp>
+#include <libdevcrypto/Common.h>
 #include <libtestutils/Common.h>
 #include <libtestutils/BlockChainLoader.h>
 #include <libtestutils/FixedClient.h>
@@ -116,3 +117,13 @@ void ParallelClientBaseFixture::enumerateClients(std::function<void(Json::Value
 		});
 	});
 }
+
+MoveNonceToTempDir::MoveNonceToTempDir()
+{
+	crypto::Nonce::setSeedFilePath(m_dir.path() + "/seed");
+}
+
+MoveNonceToTempDir::~MoveNonceToTempDir()
+{
+	crypto::Nonce::reset();
+}

test/TestUtils.h

@@ -24,6 +24,7 @@
 #include <functional>
 #include <string>
 #include <json/json.h>
+#include <libdevcore/TransientDirectory.h>
 #include <libethereum/BlockChain.h>
 #include <libethereum/ClientBase.h>
 
@@ -78,5 +79,13 @@ struct JsonRpcFixture: public ClientBaseFixture
 	
 };
 
+struct MoveNonceToTempDir
+{
+	MoveNonceToTempDir();
+	~MoveNonceToTempDir();
+private:
+	TransientDirectory m_dir;
+};
+
 }
 }

test/libdevcrypto/SecretStore.cpp

@@ -29,11 +29,16 @@
 #include <libdevcore/TrieDB.h>
 #include <libdevcore/TrieHash.h>
 #include "MemTrie.h"
-#include "../TestHelper.h"
+#include <test/TestHelper.h>
+#include <test/TestUtils.h>
 using namespace std;
 using namespace dev;
+using namespace dev::test;
 
 namespace js = json_spirit;
+namespace fs = boost::filesystem;
+
+BOOST_GLOBAL_FIXTURE( MoveNonceToTempDir )
 
 BOOST_AUTO_TEST_SUITE(KeyStore)
 
@@ -52,7 +57,8 @@ BOOST_AUTO_TEST_CASE(basic_tests)
 	{
 		cnote << i.first;
 		js::mObject& o = i.second.get_obj();
-		SecretStore store(".");
+		TransientDirectory tmpDir;
+		SecretStore store(tmpDir.path());
 		h128 u = store.readKeyContent(js::write_string(o["json"], false));
 		cdebug << "read uuid" << u;
 		bytes s = store.secret(u, [&](){ return o["password"].get_str(); });
@@ -61,4 +67,132 @@ BOOST_AUTO_TEST_CASE(basic_tests)
 	}
 }
 
+BOOST_AUTO_TEST_CASE(import_key_from_file)
+{
+	// Imports a key from an external file. Tests that the imported key is there
+	// and that the external file is not deleted.
+	TransientDirectory importDir;
+	string importFile = importDir.path() + "/import.json";
+	TransientDirectory storeDir;
+	string keyData = R"({
+		"version": 3,
+		"crypto": {
+			"ciphertext": "d69313b6470ac1942f75d72ebf8818a0d484ac78478a132ee081cd954d6bd7a9",
+			"cipherparams": { "iv": "ffffffffffffffffffffffffffffffff" },
+			"kdf": "pbkdf2",
+			"kdfparams": { "dklen": 32,  "c": 262144,  "prf": "hmac-sha256",  "salt": "c82ef14476014cbf438081a42709e2ed" },
+			"mac": "cf6bfbcc77142a22c4a908784b4a16f1023a1d0e2aff404c20158fa4f1587177",
+			"cipher": "aes-128-ctr",
+			"version": 1
+		},
+		"id": "abb67040-8dbe-0dad-fc39-2b082ef0ee5f"
+	})";
+	string password = "bar";
+	string priv = "0202020202020202020202020202020202020202020202020202020202020202";
+	writeFile(importFile, keyData);
+
+	h128 uuid;
+	{
+		SecretStore store(storeDir.path());
+		BOOST_CHECK_EQUAL(store.keys().size(), 0);
+		uuid = store.importKey(importFile);
+		BOOST_CHECK(!!uuid);
+		BOOST_CHECK(contentsString(importFile) == keyData);
+		BOOST_CHECK_EQUAL(priv, toHex(store.secret(uuid, [&](){ return password; })));
+		BOOST_CHECK_EQUAL(store.keys().size(), 1);
+	}
+	fs::remove(importFile);
+	// now do it again to check whether SecretStore properly stored it on disk
+	{
+		SecretStore store(storeDir.path());
+		BOOST_CHECK_EQUAL(store.keys().size(), 1);
+		BOOST_CHECK_EQUAL(priv, toHex(store.secret(uuid, [&](){ return password; })));
+	}
+}
+
+BOOST_AUTO_TEST_CASE(import_secret)
+{
+	for (string const& password: {"foobar", ""})
+	{
+		TransientDirectory storeDir;
+		string priv = "0202020202020202020202020202020202020202020202020202020202020202";
+
+		h128 uuid;
+		{
+			SecretStore store(storeDir.path());
+			BOOST_CHECK_EQUAL(store.keys().size(), 0);
+			uuid = store.importSecret(fromHex(priv), password);
+			BOOST_CHECK(!!uuid);
+			BOOST_CHECK_EQUAL(priv, toHex(store.secret(uuid, [&](){ return password; })));
+			BOOST_CHECK_EQUAL(store.keys().size(), 1);
+		}
+		{
+			SecretStore store(storeDir.path());
+			BOOST_CHECK_EQUAL(store.keys().size(), 1);
+			BOOST_CHECK_EQUAL(priv, toHex(store.secret(uuid, [&](){ return password; })));
+		}
+	}
+}
+
+BOOST_AUTO_TEST_CASE(wrong_password)
+{
+	TransientDirectory storeDir;
+	SecretStore store(storeDir.path());
+	string password = "foobar";
+	string priv = "0202020202020202020202020202020202020202020202020202020202020202";
+
+	h128 uuid;
+	{
+		SecretStore store(storeDir.path());
+		BOOST_CHECK_EQUAL(store.keys().size(), 0);
+		uuid = store.importSecret(fromHex(priv), password);
+		BOOST_CHECK(!!uuid);
+		BOOST_CHECK_EQUAL(priv, toHex(store.secret(uuid, [&](){ return password; })));
+		BOOST_CHECK_EQUAL(store.keys().size(), 1);
+		// password will not be queried
+		BOOST_CHECK_EQUAL(priv, toHex(store.secret(uuid, [&](){ return "abcdefg"; })));
+	}
+	{
+		SecretStore store(storeDir.path());
+		BOOST_CHECK_EQUAL(store.keys().size(), 1);
+		BOOST_CHECK(store.secret(uuid, [&](){ return "abcdefg"; }).empty());
+	}
+}
+
+BOOST_AUTO_TEST_CASE(recode)
+{
+	TransientDirectory storeDir;
+	SecretStore store(storeDir.path());
+	string password = "foobar";
+	string changedPassword = "abcdefg";
+	string priv = "0202020202020202020202020202020202020202020202020202020202020202";
+
+	h128 uuid;
+	{
+		SecretStore store(storeDir.path());
+		BOOST_CHECK_EQUAL(store.keys().size(), 0);
+		uuid = store.importSecret(fromHex(priv), password);
+		BOOST_CHECK(!!uuid);
+		BOOST_CHECK_EQUAL(priv, toHex(store.secret(uuid, [&](){ return password; })));
+		BOOST_CHECK_EQUAL(store.keys().size(), 1);
+	}
+	{
+		SecretStore store(storeDir.path());
+		BOOST_CHECK_EQUAL(store.keys().size(), 1);
+		BOOST_CHECK(store.secret(uuid, [&](){ return "abcdefg"; }).empty());
+		BOOST_CHECK(store.recode(uuid, changedPassword, [&](){ return password; }));
+		BOOST_CHECK_EQUAL(store.keys().size(), 1);
+		BOOST_CHECK_EQUAL(priv, toHex(store.secret(uuid, [&](){ return changedPassword; })));
+		store.clearCache();
+		BOOST_CHECK(store.secret(uuid, [&](){ return password; }).empty());
+		BOOST_CHECK_EQUAL(priv, toHex(store.secret(uuid, [&](){ return changedPassword; })));
+	}
+	{
+		SecretStore store(storeDir.path());
+		BOOST_CHECK_EQUAL(store.keys().size(), 1);
+		BOOST_CHECK(store.secret(uuid, [&](){ return password; }).empty());
+		BOOST_CHECK_EQUAL(priv, toHex(store.secret(uuid, [&](){ return changedPassword; })));
+	}
+}
+
 BOOST_AUTO_TEST_SUITE_END()

test/libdevcrypto/crypto.cpp

@@ -31,12 +31,16 @@
 #include <libdevcore/SHA3.h>
 #include <libdevcrypto/ECDHE.h>
 #include <libdevcrypto/CryptoPP.h>
+#include <test/TestUtils.h>
 
 using namespace std;
 using namespace dev;
+using namespace dev::test;
 using namespace dev::crypto;
 using namespace CryptoPP;
 
+BOOST_GLOBAL_FIXTURE( MoveNonceToTempDir )
+
 BOOST_AUTO_TEST_SUITE(devcrypto)
 
 static Secp256k1 s_secp256k1;