Merge remote-tracking branch 'isis/bug25127' (8d142e23) · Commits · David Goulet / Tor

changes/bug25127

0 → 100644

+7 −0

Original line number	Diff line number	Diff line
		o Minor bugfixes (Rust FFI):
		- Fix a minor memory leak which would happen whenever the C code would call
		the Rust implementation of protover_get_supported_protocols(). This was
		due to the C version returning a static string, whereas the Rust version
		newly allocated a CString to pass accross the FFI boundary. Consequently,
		the C code was not expecting to need to free() what it was given. Fixes
		bug 25127; bugfix on 0.3.2.1-alpha.

src/rust/protover/ffi.rs

+44 −21

Original line number	Diff line number	Diff line
		@@ -12,6 +12,9 @@ use std::ffi::CString;
		use protover::*;
		use smartlist::*;
		use tor_allocate::allocate_and_copy_string;
		use tor_util::strings::byte_slice_is_c_like;
		use tor_util::strings::empty_static_cstr;


		/// Translate C enums to Rust Proto enums, using the integer value of the C
		/// enum to map to its associated Rust enum
		@@ -137,18 +140,24 @@ pub extern "C" fn protocol_list_supports_protocol_or_later(
		/// Provide an interface for C to translate arguments and return types for
		/// protover::get_supported_protocols
		#[no_mangle]
		pub extern "C" fn protover_get_supported_protocols() -> *mut c_char {
		// Not handling errors when unwrapping as the content is controlled
		// and is an empty string
		let empty = CString::new("").unwrap();

		let supported = get_supported_protocols();
		let c_supported = match CString::new(supported) {
		Ok(n) => n,
		Err(_) => return empty.into_raw(),
		};
		pub extern "C" fn protover_get_supported_protocols() -> *const c_char {
		let supported: &'static CStr;

		// If we're going to pass it to C, there cannot be any intermediate NUL
		// bytes. An assert is okay here, since changing the const byte slice
		// in protover.rs to contain a NUL byte somewhere in the middle would be a
		// programming error.
		assert!(byte_slice_is_c_like(SUPPORTED_PROTOCOLS));

		// It's okay to call the "unchecked" version of the function because
		// we can see that the bytes we're passing into it 1) are valid UTF-8,
		// 2) have no intermediate NUL bytes, and 3) are terminated with a NUL
		// byte.
		unsafe {
		supported = CStr::from_bytes_with_nul_unchecked(SUPPORTED_PROTOCOLS);
		}

		c_supported.into_raw()
		supported.as_ptr()
		}

		/// Provide an interface for C to translate arguments and return types for
		@@ -193,15 +202,15 @@ pub extern "C" fn protover_is_supported_here(
		/// Provide an interface for C to translate arguments and return types for
		/// protover::compute_for_old_tor
		#[no_mangle]
		pub extern "C" fn protover_compute_for_old_tor(
		version: *const c_char,
		) -> *mut c_char {
		// Not handling errors when unwrapping as the content is controlled
		// and is an empty string
		let empty = String::new();
		pub extern "C" fn protover_compute_for_old_tor(version: const c_char) -> const c_char {
		let supported: &'static CStr;
		let elder_protocols: &'static [u8];
		let empty: &'static CStr;

		empty = empty_static_cstr();

		if version.is_null() {
		return allocate_and_copy_string(&empty);
		return empty.as_ptr();
		}

		// Require an unsafe block to read the version from a C string. The pointer
		@@ -210,10 +219,24 @@ pub extern "C" fn protover_compute_for_old_tor(

		let version = match c_str.to_str() {
		Ok(n) => n,
		Err(_) => return allocate_and_copy_string(&empty),
		Err(_) => return empty.as_ptr(),
		};

		let supported = compute_for_old_tor(&version);
		elder_protocols = compute_for_old_tor(&version);

		// If we're going to pass it to C, there cannot be any intermediate NUL
		// bytes. An assert is okay here, since changing the const byte slice
		// in protover.rs to contain a NUL byte somewhere in the middle would be a
		// programming error.
		assert!(byte_slice_is_c_like(elder_protocols));

		// It's okay to call the "unchecked" version of the function because
		// we can see that the bytes we're passing into it 1) are valid UTF-8,
		// 2) have no intermediate NUL bytes, and 3) are terminated with a NUL
		// byte.
		unsafe {
		supported = CStr::from_bytes_with_nul_unchecked(elder_protocols);
		}

		allocate_and_copy_string(&supported)
		supported.as_ptr()
		}

src/rust/protover/lib.rs

+1 −0

Original line number	Diff line number	Diff line
		@@ -26,6 +26,7 @@ extern crate libc;
		extern crate smartlist;
		extern crate external;
		extern crate tor_allocate;
		extern crate tor_util;

		mod protover;
		pub mod ffi;

src/rust/protover/protover.rs

+44 −36

Original line number	Diff line number	Diff line
		@@ -3,12 +3,15 @@

		use external::c_tor_version_as_new_as;

		use std::str;
		use std::str::FromStr;
		use std::fmt;
		use std::collections::{HashMap, HashSet};
		use std::ops::Range;
		use std::string::String;

		use tor_util::strings::NUL_BYTE;

		/// The first version of Tor that included "proto" entries in its descriptors.
		/// Authorities should use this to decide whether to guess proto lines.
		///
		@@ -22,21 +25,29 @@ const FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS: &'static str = "0.2.9.3-alpha";
		/// C_RUST_COUPLED: src/or/protover.c `MAX_PROTOCOLS_TO_EXPAND`
		const MAX_PROTOCOLS_TO_EXPAND: u32 = 500;

		/// Currently supported protocols and their versions
		/// Currently supported protocols and their versions, as a byte-slice.
		///
		/// # Warning
		///
		/// This byte-slice ends in a NUL byte. This is so that we can directly convert
		/// it to an `&'static CStr` in the FFI code, in order to hand the static string
		/// to C in a way that is compatible with C static strings.
		///
		/// Rust code which wishes to accesses this string should use
		/// `protover::get_supported_protocols()` instead.
		///
		/// C_RUST_COUPLED: src/or/protover.c `protover_get_supported_protocols`
		const SUPPORTED_PROTOCOLS: &'static [&'static str] = &[
		"Cons=1-2",
		"Desc=1-2",
		"DirCache=1-2",
		"HSDir=1-2",
		"HSIntro=3-4",
		"HSRend=1-2",
		"Link=1-5",
		"LinkAuth=1,3",
		"Microdesc=1-2",
		"Relay=1-2",
		];
		pub(crate) const SUPPORTED_PROTOCOLS: &'static [u8] =
		b"Cons=1-2 \
		Desc=1-2 \
		DirCache=1-2 \
		HSDir=1-2 \
		HSIntro=3-4 \
		HSRend=1-2 \
		Link=1-5 \
		LinkAuth=1,3 \
		Microdesc=1-2 \
		Relay=1-2\0";

		/// Known subprotocols in Tor. Indicates which subprotocol a relay supports.
		///
		@@ -94,8 +105,11 @@ impl FromStr for Proto {
		///
		/// "HSDir=1-1 LinkAuth=1"
		///
		pub fn get_supported_protocols() -> String {
		SUPPORTED_PROTOCOLS.join(" ")
		pub fn get_supported_protocols() -> &'static str {
		unsafe {
		// The `len() - 1` is to remove the NUL byte.
		str::from_utf8_unchecked(&SUPPORTED_PROTOCOLS[..SUPPORTED_PROTOCOLS.len() - 1])
		}
		}

		/// Translates a vector representation of a protocol list into a HashMap
		@@ -134,7 +148,7 @@ fn parse_protocols_from_string<'a>(
		/// of the error.
		///
		fn tor_supported() -> Result<HashMap<Proto, HashSet<u32>>, &'static str> {
		parse_protocols(SUPPORTED_PROTOCOLS.iter())
		parse_protocols(get_supported_protocols().split(" "))
		}

		/// Get the unique version numbers supported by a subprotocol.
		@@ -625,7 +639,7 @@ pub fn compute_vote(
		}

		let mut final_output: HashMap<String, String> =
		HashMap::with_capacity(SUPPORTED_PROTOCOLS.len());
		HashMap::with_capacity(get_supported_protocols().split(" ").count());

		// Go through and remove verstions that are less than the threshold
		for (protocol, versions) in all_count {
		@@ -711,11 +725,11 @@ pub fn is_supported_here(proto: Proto, vers: u32) -> bool {
		///
		/// # Inputs
		///
		/// * `version`, a string comprised of "[0-9,-]"
		/// * `version`, a string comprised of "[0-9a-z.-]"
		///
		/// # Returns
		///
		/// A `String` whose value is series of pairs, comprising of the protocol name
		/// A `&'static [u8]` whose value is series of pairs, comprising of the protocol name
		/// and versions that it supports. The string takes the following format:
		///
		/// "HSDir=1-1 LinkAuth=1"
		@@ -724,33 +738,27 @@ pub fn is_supported_here(proto: Proto, vers: u32) -> bool {
		/// only for tor versions older than FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS.
		///
		/// C_RUST_COUPLED: src/rust/protover.c `compute_for_old_tor`
		pub fn compute_for_old_tor(version: &str) -> String {
		if c_tor_version_as_new_as(
		version,
		FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS,
		)
		{
		return String::new();
		pub fn compute_for_old_tor(version: &str) -> &'static [u8] {
		if c_tor_version_as_new_as(version, FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS) {
		return NUL_BYTE;
		}

		if c_tor_version_as_new_as(version, "0.2.9.1-alpha") {
		let ret = "Cons=1-2 Desc=1-2 DirCache=1 HSDir=1 HSIntro=3 HSRend=1-2 \
		Link=1-4 LinkAuth=1 Microdesc=1-2 Relay=1-2";
		return String::from(ret);
		return b"Cons=1-2 Desc=1-2 DirCache=1 HSDir=1 HSIntro=3 HSRend=1-2 \
		Link=1-4 LinkAuth=1 Microdesc=1-2 Relay=1-2\0";
		}

		if c_tor_version_as_new_as(version, "0.2.7.5") {
		let ret = "Cons=1-2 Desc=1-2 DirCache=1 HSDir=1 HSIntro=3 HSRend=1 \
		Link=1-4 LinkAuth=1 Microdesc=1-2 Relay=1-2";
		return String::from(ret);
		return b"Cons=1-2 Desc=1-2 DirCache=1 HSDir=1 HSIntro=3 HSRend=1 \
		Link=1-4 LinkAuth=1 Microdesc=1-2 Relay=1-2\0";
		}

		if c_tor_version_as_new_as(version, "0.2.4.19") {
		let ret = "Cons=1 Desc=1 DirCache=1 HSDir=1 HSIntro=3 HSRend=1 \
		Link=1-4 LinkAuth=1 Microdesc=1 Relay=1-2";
		return String::from(ret);
		return b"Cons=1 Desc=1 DirCache=1 HSDir=1 HSIntro=3 HSRend=1 \
		Link=1-4 LinkAuth=1 Microdesc=1 Relay=1-2\0";
		}
		String::new()

		NUL_BYTE
		}

		#[cfg(test)]

src/rust/tor_util/lib.rs

+1 −1

Original line number	Diff line number	Diff line
		@@ -3,9 +3,9 @@

		//! Small module to announce Rust support during startup for demonstration
		//! purposes.
		//!

		extern crate libc;
		extern crate tor_allocate;

		pub mod ffi;
		pub mod strings;