Merge pull request #2731 from subtly/cryptoFix

Simplify/remove unused methods in Nonce

libdevcrypto/Common.cpp

@@ -28,7 +28,6 @@
 #include <libscrypt/libscrypt.h>
 #include <libdevcore/Guards.h>
 #include <libdevcore/SHA3.h>
-#include <libdevcore/FileSystem.h>
 #include <libdevcore/RLP.h>
 #if ETH_HAVE_SECP256K1
 #include <secp256k1/include/secp256k1.h>
@@ -309,48 +308,23 @@ h256 crypto::kdf(Secret const& _priv, h256 const& _hash)
 	return s;
 }
 
-mutex Nonce::s_x;
-static string s_seedFile;
-
-Secret Nonce::get()
-{
-	// todo: atomic efface bit, periodic save, kdf, rr, rng
-	// todo: encrypt
-	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)
+string const& Nonce::seedFilePath(string const& _filePath)
 {
-	s_seedFile = _filePath;
-}
+	static mutex x_seedFile;
+	static string s_seedFile;
 	
-Nonce::~Nonce()
-{
-	Guard l(Nonce::s_x);
-	if (m_value)
-		// this might throw
-		resetInternal();
+	Guard l(x_seedFile);
+	if (s_seedFile.empty())
+		s_seedFile = _filePath.empty() ? getDataDir() + "/seed" : _filePath;
+	return s_seedFile;
 }
 
-Nonce& Nonce::singleton()
+Secret Nonce::next()
 {
-	static Nonce s;
-	return s;
-}
-
-void Nonce::initialiseIfNeeded()
+	Guard l(x_value);
+	if (!m_value)
 	{
-	if (m_value)
-		return;
-
-	bytesSec b = contentsSec(seedFile());
+		bytesSec b = contentsSec(seedFilePath());
 		if (b.size() == 32)
 			b.ref().populate(m_value.writable().ref());
 		else
@@ -360,27 +334,8 @@ void Nonce::initialiseIfNeeded()
 		
 		// prevent seed reuse if process terminates abnormally
 		// this might throw
-	writeFile(seedFile(), bytes());
+		writeFile(seedFilePath(), bytes());
 	}
-
-Secret Nonce::next()
-{
-	initialiseIfNeeded();
-	m_value = sha3(m_value);
+	m_value = sha3Secure(m_value.ref());
 	return sha3(~m_value);
 }
-
-void Nonce::resetInternal()
-{
-	// this might throw
-	next();
-	writeFile(seedFile(), m_value.ref());
-	m_value.clear();
-}
-
-string const& Nonce::seedFile()
-{
-	if (s_seedFile.empty())
-		s_seedFile = getDataDir() + "/seed";
-	return s_seedFile;
-}

libdevcrypto/Common.h

@@ -28,6 +28,7 @@
 #include <libdevcore/Common.h>
 #include <libdevcore/FixedHash.h>
 #include <libdevcore/Exceptions.h>
+#include <libdevcore/FileSystem.h>
 
 namespace dev
 {
@@ -188,7 +189,7 @@ DEV_SIMPLE_EXCEPTION(InvalidState);
 h256 kdf(Secret const& _priv, h256 const& _hash);
 
 /**
- * @brief Generator for nonce material.
+ * @brief Generator for non-repeating nonce material.
  * The Nonce class should only be used when a non-repeating nonce
  * is required and, in its current form, not recommended for signatures.
  * This is primarily because the key-material for signatures is 
@@ -200,38 +201,22 @@ class Nonce
 {
 public:
 	/// Returns the next nonce (might be read from a file).
-	static Secret get();
+	static Secret get() { static Nonce s; return s.next(); }
 
-	/// 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);
+	/// @returns path of the seed file. FOR TESTS ONLY: optionally set path to @_filePath.
+	static std::string const& seedFilePath(std::string const& _filePath = std::string());
 
 private:
 	Nonce() = default;
-	~Nonce();
-
-	/// @returns the singleton instance.
-	static Nonce& singleton();
 	
-	/// Reads the last seed from the seed file.
-	void initialiseIfNeeded();
+	/// Destructor. IO operation may throw.
+	~Nonce() { if (m_value && next()) dev::writeFile(seedFilePath(), m_value.ref()); }
 
 	/// @returns the next nonce.
 	Secret next();
 
-	/// Stores the current seed in the seed file.
-	void resetInternal();
-
-	/// @returns the path of the seed file.
-	static std::string const& seedFile();
-
+	std::mutex x_value;
 	Secret m_value;
-
-	/// Mutex for the singleton object.
-	/// @note Every access to any private function has to be guarded by this mutex.
-	static std::mutex s_x;
 };
 
 }

test/TestUtils.cpp

@@ -122,10 +122,6 @@ void ParallelClientBaseFixture::enumerateClients(std::function<void(Json::Value
 
 MoveNonceToTempDir::MoveNonceToTempDir()
 {
-	crypto::Nonce::setSeedFilePath(m_dir.path() + "/seed");
+	crypto::Nonce::seedFilePath(m_dir.path() + "/seed");
 }
 
-MoveNonceToTempDir::~MoveNonceToTempDir()
-{
-	crypto::Nonce::reset();
-}

test/TestUtils.h

@@ -82,7 +82,7 @@ struct JsonRpcFixture: public ClientBaseFixture
 struct MoveNonceToTempDir
 {
 	MoveNonceToTempDir();
-	~MoveNonceToTempDir();
+	~MoveNonceToTempDir() {}
 private:
 	TransientDirectory m_dir;
 };