Merge pull request #2630 from ethereum/securenonce

Make Nonce use Secret rather than plain h256.

libdevcore/CommonIO.cpp

@@ -91,6 +91,14 @@ bytes dev::contents(string const& _file)
 	return contentsGeneric<bytes>(_file);
 }
 
+bytesSec dev::contentsSec(string const& _file)
+{
+	bytes b = contentsGeneric<bytes>(_file);
+	bytesSec ret(b);
+	bytesRef(&b).cleanse();
+	return ret;
+}
+
 string dev::contentsString(string const& _file)
 {
 	return contentsGeneric<string>(_file);

libdevcore/CommonIO.h

@@ -48,6 +48,8 @@ std::string getPassword(std::string const& _prompt);
 /// Retrieve and returns the contents of the given file.
 /// If the file doesn't exist or isn't readable, returns an empty container / bytes.
 bytes contents(std::string const& _file);
+/// Secure variation.
+bytesSec contentsSec(std::string const& _file);
 /// Retrieve and returns the contents of the given file as a std::string.
 /// If the file doesn't exist or isn't readable, returns an empty container / bytes.
 std::string contentsString(std::string const& _file);

libdevcrypto/Common.cpp

@@ -150,7 +150,7 @@ bool dev::decryptSym(Secret const& _k, bytesConstRef _cipher, bytes& o_plain)
 
 std::pair<bytes, h128> dev::encryptSymNoAuth(SecureFixedHash<16> const& _k, bytesConstRef _plain)
 {
-	h128 iv(Nonce::get());
+	h128 iv(Nonce::get().makeInsecure());
 	return make_pair(encryptSymNoAuth(_k, iv, _plain), iv);
 }
 
@@ -312,7 +312,7 @@ h256 crypto::kdf(Secret const& _priv, h256 const& _hash)
 mutex Nonce::s_x;
 static string s_seedFile;
 
-h256 Nonce::get()
+Secret Nonce::get()
 {
 	// todo: atomic efface bit, periodic save, kdf, rr, rng
 	// todo: encrypt
@@ -350,11 +350,11 @@ void Nonce::initialiseIfNeeded()
 	if (m_value)
 		return;
 
-	bytes b = contents(seedFile());
+	bytesSec b = contentsSec(seedFile());
 	if (b.size() == 32)
-		memcpy(m_value.data(), b.data(), 32);
+		b.ref().populate(m_value.writable().ref());
 	else
-		m_value = h256::random();
+		m_value = Secret::random();
 	if (!m_value)
 		BOOST_THROW_EXCEPTION(InvalidState());
 
@@ -363,7 +363,7 @@ void Nonce::initialiseIfNeeded()
 	writeFile(seedFile(), bytes());
 }
 
-h256 Nonce::next()
+Secret Nonce::next()
 {
 	initialiseIfNeeded();
 	m_value = sha3(m_value);
@@ -374,8 +374,8 @@ void Nonce::resetInternal()
 {
 	// this might throw
 	next();
-	writeFile(seedFile(), m_value.asBytes());
-	m_value = h256();
+	writeFile(seedFile(), m_value.ref());
+	m_value.clear();
 }
 
 string const& Nonce::seedFile()

libdevcrypto/Common.h

@@ -181,7 +181,8 @@ private:
 
 namespace crypto
 {
-struct InvalidState: public dev::Exception {};
+
+DEV_SIMPLE_EXCEPTION(InvalidState);
 
 /// Key derivation
 h256 kdf(Secret const& _priv, h256 const& _hash);
@@ -189,35 +190,44 @@ h256 kdf(Secret const& _priv, h256 const& _hash);
 /**
  * @brief Generator for nonce material.
  */
-struct Nonce
+class Nonce
 {
+public:
 	/// Returns the next nonce (might be read from a file).
-	static h256 get();
+	static Secret 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() = default;
 	~Nonce();
+
 	/// @returns the singleton instance.
 	static Nonce& singleton();
+
 	/// Reads the last seed from the seed file.
 	void initialiseIfNeeded();
+
 	/// @returns the next nonce.
-	h256 next();
+	Secret next();
+
 	/// Stores the current seed in the seed file.
 	void resetInternal();
+
 	/// @returns the path of the seed file.
 	static std::string const& seedFile();
 
+	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;
-
-	h256 m_value;
 };
+
 }
 
 }

test/libp2p/rlpx.cpp

@@ -455,12 +455,12 @@ BOOST_AUTO_TEST_CASE(ecies_interop_test_primitives)
 BOOST_AUTO_TEST_CASE(segmentedPacketFlush)
 {
 	ECDHE localEph;
-	h256 localNonce = Nonce::get();
+	Secret localNonce = Nonce::get();
 	ECDHE remoteEph;
-	h256 remoteNonce = Nonce::get();
+	Secret remoteNonce = Nonce::get();
 	bytes ackCipher{0};
 	bytes authCipher{1};
-	RLPXFrameCoder encoder(true, remoteEph.pubkey(), remoteNonce, localEph, localNonce, &ackCipher, &authCipher);
+	RLPXFrameCoder encoder(true, remoteEph.pubkey(), remoteNonce.makeInsecure(), localEph, localNonce.makeInsecure(), &ackCipher, &authCipher);
 	
 	/// Test writing a 64byte RLPStream and drain with frame size that
 	/// forces packet to be pieced into 4 frames.
@@ -506,7 +506,7 @@ BOOST_AUTO_TEST_CASE(segmentedPacketFlush)
 	}
 	
 	// read and assemble dequed encframes
-	RLPXFrameCoder decoder(false, localEph.pubkey(), localNonce, remoteEph, remoteNonce, &ackCipher, &authCipher);
+	RLPXFrameCoder decoder(false, localEph.pubkey(), localNonce.makeInsecure(), remoteEph, remoteNonce.makeInsecure(), &ackCipher, &authCipher);
 	vector<RLPXPacket> packets;
 	RLPXFrameReader r(0);
 	for (size_t i = 0; i < encframes.size(); i++)
@@ -529,12 +529,12 @@ BOOST_AUTO_TEST_CASE(segmentedPacketFlush)
 BOOST_AUTO_TEST_CASE(coalescedPacketsPadded)
 {
 	ECDHE localEph;
-	h256 localNonce = Nonce::get();
+	Secret localNonce = Nonce::get();
 	ECDHE remoteEph;
-	h256 remoteNonce = Nonce::get();
+	Secret remoteNonce = Nonce::get();
 	bytes ackCipher{0};
 	bytes authCipher{1};
-	RLPXFrameCoder encoder(true, remoteEph.pubkey(), remoteNonce, localEph, localNonce, &ackCipher, &authCipher);
+	RLPXFrameCoder encoder(true, remoteEph.pubkey(), remoteNonce.makeInsecure(), localEph, localNonce.makeInsecure(), &ackCipher, &authCipher);
 	
 	/// Test writing four 32 byte RLPStream packets such that
 	/// a single 1KB frame will incldue all four packets.
@@ -559,7 +559,7 @@ BOOST_AUTO_TEST_CASE(coalescedPacketsPadded)
 	BOOST_REQUIRE_EQUAL(expectedFrameSize, encframes[0].size());
 	
 	// read and assemble dequed encframes
-	RLPXFrameCoder decoder(false, localEph.pubkey(), localNonce, remoteEph, remoteNonce, &ackCipher, &authCipher);
+	RLPXFrameCoder decoder(false, localEph.pubkey(), localNonce.makeInsecure(), remoteEph, remoteNonce.makeInsecure(), &ackCipher, &authCipher);
 	vector<RLPXPacket> packets;
 	RLPXFrameReader r(0);
 	bytesRef frameWithHeader(encframes[0].data(), encframes[0].size());
@@ -587,12 +587,12 @@ BOOST_AUTO_TEST_CASE(coalescedPacketsPadded)
 BOOST_AUTO_TEST_CASE(singleFramePacketFlush)
 {
 	ECDHE localEph;
-	h256 localNonce = Nonce::get();
+	Secret localNonce = Nonce::get();
 	ECDHE remoteEph;
-	h256 remoteNonce = Nonce::get();
+	Secret remoteNonce = Nonce::get();
 	bytes ackCipher{0};
 	bytes authCipher{1};
-	RLPXFrameCoder encoder(true, remoteEph.pubkey(), remoteNonce, localEph, localNonce, &ackCipher, &authCipher);
+	RLPXFrameCoder encoder(true, remoteEph.pubkey(), remoteNonce.makeInsecure(), localEph, localNonce.makeInsecure(), &ackCipher, &authCipher);
 	
 	/// Test writing four 32 byte RLPStream packets such that
 	/// a single 1KB frame will incldue all four packets.
@@ -611,7 +611,7 @@ BOOST_AUTO_TEST_CASE(singleFramePacketFlush)
 	BOOST_REQUIRE_EQUAL(dequeLen, encframes[0].size());
 	
 	// read and assemble dequed encframes
-	RLPXFrameCoder decoder(false, localEph.pubkey(), localNonce, remoteEph, remoteNonce, &ackCipher, &authCipher);
+	RLPXFrameCoder decoder(false, localEph.pubkey(), localNonce.makeInsecure(), remoteEph, remoteNonce.makeInsecure(), &ackCipher, &authCipher);
 	vector<RLPXPacket> packets;
 	RLPXFrameReader r(0);
 	bytesRef frameWithHeader(encframes[0].data(), encframes[0].size());