Merge pull request #1003 from aldenml/crypto_plugin-refactor-1.2

refactor in crypto_plugin to use span

appveyor.yml

@@ -99,7 +99,7 @@ build_script:
 
 test_script:
 - cd %ROOT_DIRECTORY%\test
-- b2.exe -l400 --hash warnings-as-errors=on -j2 %compiler% address-model=%model% debug-iterators=on picker-debugging=on invariant-checks=full variant=%variant% %linkflags% %include% link=shared ssl=%ssl% crypto=%crypto% win-tests
+- appveyor-retry b2.exe -l400 --hash warnings-as-errors=on -j2 %compiler% address-model=%model% debug-iterators=on picker-debugging=on invariant-checks=full variant=%variant% %linkflags% %include% link=shared ssl=%ssl% crypto=%crypto% win-tests
 
 - cd %ROOT_DIRECTORY%\bindings\python
 # we use 64 bit python build

include/libtorrent/extensions.hpp

@@ -259,7 +259,7 @@ namespace libtorrent
 		// ``optimistic_unchoke_feature`` in the return value from implemented_features().
 		// If multiple plugins implement this function the lowest return value
 		// (i.e. the highest priority) is used.
-		virtual uint64_t get_unchoke_priority(peer_connection_handle /* peer */)
+		virtual uint64_t get_unchoke_priority(peer_connection_handle const& /* peer */)
 		{ return std::numeric_limits<uint64_t>::max(); }
 
 		// called when saving settings state
@@ -482,8 +482,8 @@ namespace libtorrent
 		// hidden
 		virtual ~crypto_plugin() {}
 
-		virtual void set_incoming_key(unsigned char const* key, int len) = 0;
-		virtual void set_outgoing_key(unsigned char const* key, int len) = 0;
+		virtual void set_incoming_key(span<char const> key) = 0;
+		virtual void set_outgoing_key(span<char const> key) = 0;
 
 		// encrypted the provided buffers and returns the number of bytes which
 		// are now ready to be sent to the lower layer. This must be at least
@@ -498,6 +498,9 @@ namespace libtorrent
 		encrypt(span<span<char>> /*send_vec*/) = 0;
 
 		// decrypt the provided buffers.
+		// returns is a tuple representing the values
+		// (consume, produce, packet_size)
+		//
 		// consume is set to the number of bytes which should be trimmed from the
 		// head of the buffers, default is 0
 		//
@@ -506,8 +509,7 @@ namespace libtorrent
 		//
 		// packet_size is set to the minimum number of bytes which must be read to
 		// advance the next step of decryption. default is 0
-		virtual void decrypt(span<span<char>> /*receive_vec*/
-			, int& /* consume */, int& /*produce*/, int& /*packet_size*/) = 0;
+		virtual std::tuple<int, int, int> decrypt(span<span<char>> /*receive_vec*/) = 0;
 	};
 }
 

include/libtorrent/pe_crypto.hpp

@@ -58,7 +58,7 @@ namespace libtorrent
 
 	using key_t = mp::number<mp::cpp_int_backend<768, 768, mp::unsigned_magnitude, mp::unchecked, void>>;
 
-	std::array<char, 96> export_key(key_t const& k);
+	TORRENT_EXTRA_EXPORT std::array<char, 96> export_key(key_t const& k);
 
 	// RC4 state from libtomcrypt
 	struct rc4 {
@@ -66,9 +66,6 @@ namespace libtorrent
 		std::array<std::uint8_t, 256> buf;
 	};
 
-	void TORRENT_EXTRA_EXPORT rc4_init(const unsigned char* in, unsigned long len, rc4 *state);
-	unsigned long TORRENT_EXTRA_EXPORT rc4_encrypt(unsigned char *out, unsigned long outlen, rc4 *state);
-
 	// TODO: 3 dh_key_exchange should probably move into its own file
 	class TORRENT_EXTRA_EXPORT dh_key_exchange
 	{
@@ -142,17 +139,13 @@ namespace libtorrent
 		rc4_handler();
 
 		// Input keys must be 20 bytes
-		// TODO: 4 use uint768_t here instead of pointer + length
-		void set_incoming_key(unsigned char const* key, int len) override;
-		void set_outgoing_key(unsigned char const* key, int len) override;
+		void set_incoming_key(span<char const> key) override;
+		void set_outgoing_key(span<char const> key) override;
 
 		std::tuple<int, span<span<char const>>>
 		encrypt(span<span<char>> buf) override;
 
-		void decrypt(span<span<char>> buf
-			, int& consume
-			, int& produce
-			, int& packet_size) override;
+		std::tuple<int, int, int> decrypt(span<span<char>> buf) override;
 
 	private:
 		rc4 m_rc4_incoming;

src/bt_peer_connection.cpp

@@ -122,8 +122,8 @@ namespace libtorrent
 
 		boost::shared_ptr<rc4_handler> ret = boost::make_shared<rc4_handler>();
 
-		ret->set_incoming_key(&remote_key[0], 20);
-		ret->set_outgoing_key(&local_key[0], 20);
+		ret->set_incoming_key(remote_key);
+		ret->set_outgoing_key(local_key);
 
 		return ret;
 	}
@@ -542,7 +542,7 @@ namespace libtorrent
 		int const buf_size = dh_key_len + pad_size;
 
 		std::array<char, dh_key_len> const local_key = export_key(m_dh_key_exchange->get_local_key());
-		memcpy(ptr, local_key.data(), dh_key_len);
+		std::memcpy(ptr, local_key.data(), dh_key_len);
 		ptr += dh_key_len;
 
 		std::generate(ptr, ptr + pad_size, random_byte);
@@ -727,10 +727,7 @@ namespace libtorrent
 
 	void bt_peer_connection::rc4_decrypt(span<char> buf)
 	{
-		int consume = 0;
-		int produce = int(buf.size());
-		int packet_size = 0;
-		m_rc4->decrypt(buf, consume, produce, packet_size);
+		m_rc4->decrypt(buf);
 	}
 
 	namespace {

src/pe_crypto.cpp

@@ -77,12 +77,15 @@ namespace libtorrent
 		if (end < begin + 96)
 		{
 			int const len = end - begin;
-			memmove(begin + 96 - len, begin, len);
-			memset(begin, 0, 96 - len);
+			std::memmove(begin + 96 - len, begin, len);
+			std::memset(begin, 0, 96 - len);
 		}
 		return ret;
 	}
 
+	void rc4_init(const unsigned char* in, unsigned long len, rc4 *state);
+	unsigned long rc4_encrypt(unsigned char *out, unsigned long outlen, rc4 *state);
+
 	// Set the prime P and the generator, generate local public key
 	dh_key_exchange::dh_key_exchange()
 	{
@@ -216,9 +219,9 @@ namespace libtorrent
 		if (recv_buffer.crypto_packet_finished())
 		{
 			span<char> wr_buf = recv_buffer.mutable_buffer(bytes_transferred);
+			int produce = 0;
 			int packet_size = 0;
-			int produce = int(bytes_transferred);
-			m_dec_handler->decrypt(wr_buf, consume, produce, packet_size);
+			std::tie(consume, produce, packet_size) = m_dec_handler->decrypt(wr_buf);
 			TORRENT_ASSERT(packet_size || produce);
 			TORRENT_ASSERT(packet_size >= 0);
 			bytes_transferred = produce;
@@ -262,7 +265,7 @@ namespace libtorrent
 			int consume = 0;
 			int produce = 0;
 			std::vector<span<char>> wr_buf;
-			crypto->decrypt(wr_buf, consume, produce, packet_size);
+			std::tie(consume, produce, packet_size) = crypto->decrypt(wr_buf);
 			TORRENT_ASSERT(wr_buf.empty());
 			TORRENT_ASSERT(consume == 0);
 			TORRENT_ASSERT(produce == 0);
@@ -280,23 +283,22 @@ namespace libtorrent
 		m_rc4_outgoing.y = 0;
 	}
 
-	void rc4_handler::set_incoming_key(unsigned char const* key, int len)
+	void rc4_handler::set_incoming_key(span<char const> key)
 	{
 		m_decrypt = true;
-		rc4_init(key, len, &m_rc4_incoming);
+		rc4_init(reinterpret_cast<unsigned char const*>(key.data())
+			, key.size(), &m_rc4_incoming);
 		// Discard first 1024 bytes
-		int consume = 0;
-		int produce = 0;
-		int packet_size = 0;
 		char buf[1024];
 		span<char> vec(buf, sizeof(buf));
-		decrypt(vec, consume, produce, packet_size);
+		decrypt(vec);
 	}
 
-	void rc4_handler::set_outgoing_key(unsigned char const* key, int len)
+	void rc4_handler::set_outgoing_key(span<char const> key)
 	{
 		m_encrypt = true;
-		rc4_init(key, len, &m_rc4_outgoing);
+		rc4_init(reinterpret_cast<unsigned char const*>(key.data())
+			, key.size(), &m_rc4_outgoing);
 		// Discard first 1024 bytes
 		char buf[1024];
 		span<char> vec(buf, sizeof(buf));
@@ -325,16 +327,9 @@ namespace libtorrent
 		return std::make_tuple(bytes_processed, empty);
 	}
 
-	void rc4_handler::decrypt(span<span<char>> bufs
-		, int& consume
-		, int& produce
-		, int& packet_size)
+	std::tuple<int, int, int> rc4_handler::decrypt(span<span<char>> bufs)
 	{
-		// these are out-parameters that are not set
-		TORRENT_UNUSED(consume);
-		TORRENT_UNUSED(packet_size);
-
-		if (!m_decrypt) return;
+		if (!m_decrypt) std::make_tuple(0, 0, 0);
 
 		int bytes_processed = 0;
 		for (auto& buf : bufs)
@@ -348,7 +343,7 @@ namespace libtorrent
 			bytes_processed += len;
 			rc4_encrypt(pos, len, &m_rc4_incoming);
 		}
-		produce = bytes_processed;
+		return std::make_tuple(0, bytes_processed, 0);
 	}
 
 // All this code is based on libTomCrypt (http://www.libtomcrypt.com/)

src/ut_pex.cpp

@@ -31,26 +31,18 @@ POSSIBILITY OF SUCH DAMAGE.
 */
 
 #include "libtorrent/config.hpp"
-#include "libtorrent/peer_connection.hpp"
 #include "libtorrent/bt_peer_connection.hpp"
 #include "libtorrent/peer_connection_handle.hpp"
 #include "libtorrent/bencode.hpp"
 #include "libtorrent/torrent.hpp"
-#include "libtorrent/torrent_handle.hpp"
 #include "libtorrent/extensions.hpp"
 #include "libtorrent/broadcast_socket.hpp"
 #include "libtorrent/socket_io.hpp"
 #include "libtorrent/peer_info.hpp"
-#include "libtorrent/random.hpp"
 #include "libtorrent/socket_type.hpp" // for is_utp
 #include "libtorrent/performance_counters.hpp" // for counters
-
 #include "libtorrent/extensions/ut_pex.hpp"
 
-#ifndef TORRENT_DISABLE_LOGGING
-#include "libtorrent/lazy_entry.hpp"
-#endif
-
 #ifndef TORRENT_DISABLE_EXTENSIONS
 
 namespace libtorrent { namespace

test/test_pe_crypto.cpp

@@ -73,10 +73,10 @@ void test_enc_handler(libtorrent::crypto_plugin& a, libtorrent::crypto_plugin& b
 
 		{
 			int consume = 0;
-			int produce = buf_len;
+			int produce = 0;
 			int packet_size = 0;
 			lt::span<char> iovec(&buf[0], buf_len);
-			b.decrypt(iovec, consume, produce, packet_size);
+			std::tie(consume, produce, packet_size) = b.decrypt(iovec);
 			TEST_CHECK(buf == cmp_buf);
 			TEST_EQUAL(consume, 0);
 			TEST_EQUAL(produce, buf_len);
@@ -93,10 +93,10 @@ void test_enc_handler(libtorrent::crypto_plugin& a, libtorrent::crypto_plugin& b
 			TEST_EQUAL(next_barrier, buf_len);
 
 			int consume = 0;
-			int produce = buf_len;
+			int produce = 0;
 			int packet_size = 0;
 			lt::span<char> iovec2(&buf[0], buf_len);
-			a.decrypt(iovec2, consume, produce, packet_size);
+			std::tie(consume, produce, packet_size) = a.decrypt(iovec2);
 			TEST_CHECK(buf == cmp_buf);
 			TEST_EQUAL(consume, 0);
 			TEST_EQUAL(produce, buf_len);
@@ -145,11 +145,11 @@ TORRENT_TEST(rc4)
 
 	std::fprintf(stderr, "testing RC4 handler\n");
 	rc4_handler rc41;
-	rc41.set_incoming_key(&test2_key[0], 20);
-	rc41.set_outgoing_key(&test1_key[0], 20);
+	rc41.set_incoming_key(test2_key);
+	rc41.set_outgoing_key(test1_key);
 	rc4_handler rc42;
-	rc42.set_incoming_key(&test1_key[0], 20);
-	rc42.set_outgoing_key(&test2_key[0], 20);
+	rc42.set_incoming_key(test1_key);
+	rc42.set_outgoing_key(test2_key);
 	test_enc_handler(rc41, rc42);
 }