Skip to content

Commit be49c38

Browse files
authored
Feature/socks improvements (#423)
* Changed AvailableReader from Future to Stream * comment formatting * WIP * Maximum number of bytes read per poll * More detailed socks5 printing * Split up socks5 service provider runner * Fixed closing proxy too soon on one side * Additional log information * Printing connection id in log target * Adjusted some constants * Target field in log * Removed dead code * Decreased logging level * Explicitly seperated 'Send' and 'Connect' request + data buffering * Temporarily commented out test that fails due to not understanding limitations of tokio test io builder * Fixed socks5 tests
1 parent bb04961 commit be49c38

File tree

10 files changed

+415
-397
lines changed

10 files changed

+415
-397
lines changed

clients/socks5/src/socks/client.rs

+31-47
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,13 @@ use futures::channel::mpsc;
1010
use futures::task::{Context, Poll};
1111
use log::*;
1212
use nymsphinx::addressing::clients::Recipient;
13-
use ordered_buffer::{OrderedMessageBuffer, OrderedMessageSender};
1413
use pin_project::pin_project;
15-
use proxy_helpers::available_reader::AvailableReader;
1614
use proxy_helpers::connection_controller::{
1715
ConnectionReceiver, ControllerCommand, ControllerSender,
1816
};
1917
use proxy_helpers::proxy_runner::ProxyRunner;
2018
use rand::RngCore;
21-
use socks5_requests::{ConnectionId, Request};
19+
use socks5_requests::{ConnectionId, RemoteAddress, Request};
2220
use std::net::{Shutdown, SocketAddr};
2321
use std::pin::Pin;
2422
use tokio::prelude::*;
@@ -145,8 +143,7 @@ pub(crate) struct SocksClient {
145143
impl Drop for SocksClient {
146144
fn drop(&mut self) {
147145
// TODO: decrease to debug/trace
148-
info!("socksclient is going out of scope - the stream is getting dropped!");
149-
info!("Connection {} is getting closed", self.connection_id);
146+
debug!("Connection {} is getting closed", self.connection_id);
150147
self.controller_sender
151148
.unbounded_send(ControllerCommand::Remove(self.connection_id))
152149
.unwrap();
@@ -222,26 +219,38 @@ impl SocksClient {
222219
Ok(())
223220
}
224221

225-
async fn send_request_to_mixnet(&mut self, request: Request) {
226-
self.send_to_mixnet(request.into_bytes()).await;
222+
async fn send_connect_to_mixnet(&mut self, remote_address: RemoteAddress) {
223+
let req = Request::new_connect(
224+
self.connection_id,
225+
remote_address.clone(),
226+
self.self_address.clone(),
227+
);
228+
229+
let input_message =
230+
InputMessage::new_fresh(self.service_provider.clone(), req.into_bytes(), false);
231+
self.input_sender.unbounded_send(input_message).unwrap();
227232
}
228233

229-
async fn run_proxy(
230-
&mut self,
231-
conn_receiver: ConnectionReceiver,
232-
message_sender: OrderedMessageSender,
233-
) {
234+
async fn run_proxy(&mut self, conn_receiver: ConnectionReceiver, remote_proxy_target: String) {
235+
self.send_connect_to_mixnet(remote_proxy_target.clone())
236+
.await;
237+
234238
let stream = self.stream.run_proxy();
239+
let local_stream_remote = stream
240+
.peer_addr()
241+
.expect("failed to extract peer address")
242+
.to_string();
235243
let connection_id = self.connection_id;
236244
let input_sender = self.input_sender.clone();
237245

238246
let recipient = self.service_provider.clone();
239247
let (stream, _) = ProxyRunner::new(
240248
stream,
249+
local_stream_remote,
250+
remote_proxy_target,
241251
conn_receiver,
242252
input_sender,
243253
connection_id,
244-
message_sender,
245254
)
246255
.run(move |conn_id, read_data, socket_closed| {
247256
let provider_request = Request::new_send(conn_id, read_data, socket_closed);
@@ -262,14 +271,9 @@ impl SocksClient {
262271

263272
// setup for receiving from the mixnet
264273
let (mix_sender, mix_receiver) = mpsc::unbounded();
265-
let ordered_buffer = OrderedMessageBuffer::new();
266274

267275
self.controller_sender
268-
.unbounded_send(ControllerCommand::Insert(
269-
self.connection_id,
270-
mix_sender,
271-
ordered_buffer,
272-
))
276+
.unbounded_send(ControllerCommand::Insert(self.connection_id, mix_sender))
273277
.unwrap();
274278

275279
match request.command {
@@ -278,26 +282,16 @@ impl SocksClient {
278282
trace!("Connecting to: {:?}", remote_address.clone());
279283
self.acknowledge_socks5().await;
280284

281-
let mut message_sender = OrderedMessageSender::new();
282-
// 'connect' needs to be handled manually due to different structure,
283-
// but still needs to have correct sequence number on it!
284-
285-
// read whatever we can
286-
let available_reader = AvailableReader::new(&mut self.stream);
287-
let (request_data_bytes, _) = available_reader.await?;
288-
let ordered_message = message_sender.wrap_message(request_data_bytes.to_vec());
289-
290-
let socks_provider_request = Request::new_connect(
291-
self.connection_id,
285+
info!(
286+
"Starting proxy for {} (id: {})",
292287
remote_address.clone(),
293-
ordered_message,
294-
self.self_address.clone(),
288+
self.connection_id
289+
);
290+
self.run_proxy(mix_receiver, remote_address.clone()).await;
291+
info!(
292+
"Proxy for {} is finished (id: {})",
293+
remote_address, self.connection_id
295294
);
296-
297-
self.send_request_to_mixnet(socks_provider_request).await;
298-
info!("Starting proxy for {}", remote_address.clone());
299-
self.run_proxy(mix_receiver, message_sender).await;
300-
info!("Proxy for {} is finished", remote_address);
301295
}
302296

303297
SocksCommand::Bind => unimplemented!(), // not handled
@@ -307,16 +301,6 @@ impl SocksClient {
307301
Ok(())
308302
}
309303

310-
/// Send serialized Socks5 request bytes to the mixnet. The request stream
311-
/// will be chunked up into a series of one or more Sphinx packets and
312-
/// reassembled at the destination service provider at the other end, then
313-
/// sent onwards anonymously.
314-
async fn send_to_mixnet(&self, request_bytes: Vec<u8>) {
315-
let input_message =
316-
InputMessage::new_fresh(self.service_provider.clone(), request_bytes, false);
317-
self.input_sender.unbounded_send(input_message).unwrap();
318-
}
319-
320304
/// Writes a Socks5 header back to the requesting client's TCP stream,
321305
/// basically saying "I acknowledge your request and am dealing with it".
322306
async fn acknowledge_socks5(&mut self) {

common/nymsphinx/params/src/packet_sizes.rs

+6-3
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,14 @@ pub struct InvalidPacketSize;
3434
#[repr(u8)]
3535
#[derive(Clone, Copy, Debug, PartialEq)]
3636
pub enum PacketSize {
37-
RegularPacket = 1,
3837
// for example instant messaging use case
39-
ACKPacket = 2,
38+
RegularPacket = 1,
39+
4040
// for sending SURB-ACKs
41-
ExtendedPacket = 3, // for example for streaming fast and furious in uncompressed 10bit 4K HDR quality
41+
ACKPacket = 2,
42+
43+
// for example for streaming fast and furious in uncompressed 10bit 4K HDR quality
44+
ExtendedPacket = 3,
4245
}
4346

4447
impl TryFrom<u8> for PacketSize {

common/socks5/proxy-helpers/src/available_reader.rs

+84-27
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,18 @@ use std::ops::DerefMut;
2020
use std::pin::Pin;
2121
use std::task::{Context, Poll};
2222
use tokio::io::AsyncRead;
23+
use tokio::stream::Stream;
24+
use tokio::time::{delay_for, Delay, Duration, Instant};
25+
26+
const MAX_READ_AMOUNT: usize = 500 * 1000; // 0.5MB
27+
const GRACE_DURATION: Duration = Duration::from_millis(1);
2328

2429
pub struct AvailableReader<'a, R: AsyncRead + Unpin> {
25-
// TODO: come up with a way to avoid using RefCell (not sure if possible though)
30+
// TODO: come up with a way to avoid using RefCell (not sure if possible though due to having to
31+
// mutably borrow both inner reader and buffer at the same time)
2632
buf: RefCell<BytesMut>,
2733
inner: RefCell<&'a mut R>,
28-
// idea for the future: tiny delay that allows to prevent unnecessary extra fragmentation
29-
// grace_period: Option<Delay>,
34+
grace_period: Option<Delay>,
3035
}
3136

3237
impl<'a, R> AvailableReader<'a, R>
@@ -39,20 +44,15 @@ where
3944
AvailableReader {
4045
buf: RefCell::new(BytesMut::with_capacity(Self::BUF_INCREMENT)),
4146
inner: RefCell::new(reader),
42-
// grace_period: None,
47+
grace_period: Some(delay_for(GRACE_DURATION)),
4348
}
4449
}
4550
}
4651

47-
// TODO: change this guy to a stream? Seems waaay more appropriate considering
48-
// we're getting new Bytes items regularly rather than calling it once.
49-
50-
impl<'a, R: AsyncRead + Unpin> Future for AvailableReader<'a, R> {
51-
type Output = io::Result<(Bytes, bool)>;
52+
impl<'a, R: AsyncRead + Unpin> Stream for AvailableReader<'a, R> {
53+
type Item = io::Result<Bytes>;
5254

53-
// this SHOULD stay mutable, because we rely on runtime checks inside the method
54-
#[allow(unused_mut)]
55-
fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
55+
fn poll_next(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Option<Self::Item>> {
5656
// if we have no space in buffer left - expand it
5757
if !self.buf.borrow().has_remaining_mut() {
5858
self.buf.borrow_mut().reserve(Self::BUF_INCREMENT);
@@ -68,19 +68,43 @@ impl<'a, R: AsyncRead + Unpin> Future for AvailableReader<'a, R> {
6868
if self.buf.borrow().is_empty() {
6969
Poll::Pending
7070
} else {
71+
// if exists - check grace period
72+
if let Some(grace_period) = self.grace_period.as_mut() {
73+
if Pin::new(grace_period).poll(cx).is_pending() {
74+
return Poll::Pending;
75+
}
76+
}
77+
7178
let buf = self.buf.replace(BytesMut::new());
72-
Poll::Ready(Ok((buf.freeze(), false)))
79+
Poll::Ready(Some(Ok(buf.freeze())))
7380
}
7481
}
75-
Poll::Ready(Err(err)) => Poll::Ready(Err(err)),
82+
Poll::Ready(Err(err)) => Poll::Ready(Some(Err(err))),
7683
Poll::Ready(Ok(n)) => {
84+
// if exists - reset grace period
85+
if let Some(grace_period) = self.grace_period.as_mut() {
86+
let now = Instant::now();
87+
grace_period.reset(now + GRACE_DURATION);
88+
}
89+
7790
// if we read a non-0 amount, we're not done yet!
7891
if n == 0 {
7992
let buf = self.buf.replace(BytesMut::new());
80-
Poll::Ready(Ok((buf.freeze(), true)))
93+
if buf.len() > 0 {
94+
Poll::Ready(Some(Ok(buf.freeze())))
95+
} else {
96+
Poll::Ready(None)
97+
}
8198
} else {
8299
// tell the waker we should be polled again!
83100
cx.waker().wake_by_ref();
101+
102+
// if we reached our maximum amount - return it
103+
let read_bytes_len = self.buf.borrow().len();
104+
if read_bytes_len >= MAX_READ_AMOUNT {
105+
let buf = self.buf.replace(BytesMut::new());
106+
return Poll::Ready(Some(Ok(buf.freeze())));
107+
}
84108
Poll::Pending
85109
}
86110
}
@@ -91,31 +115,34 @@ impl<'a, R: AsyncRead + Unpin> Future for AvailableReader<'a, R> {
91115
#[cfg(test)]
92116
mod tests {
93117
use super::*;
118+
use futures::poll;
94119
use std::io::Cursor;
95120
use std::time::Duration;
121+
use tokio::stream::StreamExt;
122+
use tokio_test::assert_pending;
96123

97124
#[tokio::test]
98125
async fn available_reader_reads_all_available_data_smaller_than_its_buf() {
99126
let data = vec![42u8; 100];
100127
let mut reader = Cursor::new(data.clone());
101128

102-
let available_reader = AvailableReader::new(&mut reader);
103-
let (read_data, is_finished) = available_reader.await.unwrap();
129+
let mut available_reader = AvailableReader::new(&mut reader);
130+
let read_data = available_reader.next().await.unwrap().unwrap();
104131

105132
assert_eq!(read_data, data);
106-
assert!(is_finished)
133+
assert!(available_reader.next().await.is_none())
107134
}
108135

109136
#[tokio::test]
110137
async fn available_reader_reads_all_available_data_bigger_than_its_buf() {
111138
let data = vec![42u8; AvailableReader::<Cursor<Vec<u8>>>::BUF_INCREMENT + 100];
112139
let mut reader = Cursor::new(data.clone());
113140

114-
let available_reader = AvailableReader::new(&mut reader);
115-
let (read_data, is_finished) = available_reader.await.unwrap();
141+
let mut available_reader = AvailableReader::new(&mut reader);
142+
let read_data = available_reader.next().await.unwrap().unwrap();
116143

117144
assert_eq!(read_data, data);
118-
assert!(is_finished)
145+
assert!(available_reader.next().await.is_none())
119146
}
120147

121148
#[tokio::test]
@@ -129,11 +156,11 @@ mod tests {
129156
.read(&second_data_chunk)
130157
.build();
131158

132-
let available_reader = AvailableReader::new(&mut reader_mock);
133-
let (read_data, is_finished) = available_reader.await.unwrap();
159+
let mut available_reader = AvailableReader::new(&mut reader_mock);
160+
let read_data = available_reader.next().await.unwrap().unwrap();
134161

135162
assert_eq!(read_data, first_data_chunk);
136-
assert!(!is_finished)
163+
assert_pending!(poll!(available_reader.next()));
137164
}
138165

139166
#[tokio::test]
@@ -145,10 +172,40 @@ mod tests {
145172
.read(&data)
146173
.build();
147174

148-
let available_reader = AvailableReader::new(&mut reader_mock);
149-
let (read_data, is_finished) = available_reader.await.unwrap();
175+
let mut available_reader = AvailableReader::new(&mut reader_mock);
176+
let read_data = available_reader.next().await.unwrap().unwrap();
150177

151178
assert_eq!(read_data, data);
152-
assert!(is_finished)
179+
assert!(available_reader.next().await.is_none())
153180
}
181+
182+
// perhaps the issue of tokio io builder will be resolved in tokio 0.3?
183+
// #[tokio::test]
184+
// async fn available_reader_will_wait_for_more_data_if_its_within_grace_period() {
185+
// let first_data_chunk = vec![42u8; 100];
186+
// let second_data_chunk = vec![123u8; 100];
187+
//
188+
// let combined_chunks: Vec<_> = first_data_chunk
189+
// .iter()
190+
// .cloned()
191+
// .chain(second_data_chunk.iter().cloned())
192+
// .collect();
193+
//
194+
// let mut reader_mock = tokio_test::io::Builder::new()
195+
// .read(&first_data_chunk)
196+
// .wait(Duration::from_millis(2))
197+
// .read(&second_data_chunk)
198+
// .build();
199+
//
200+
// let mut available_reader = AvailableReader {
201+
// buf: RefCell::new(BytesMut::with_capacity(4096)),
202+
// inner: RefCell::new(&mut reader_mock),
203+
// grace_period: Some(delay_for(Duration::from_millis(5))),
204+
// };
205+
//
206+
// let read_data = available_reader.next().await.unwrap().unwrap();
207+
//
208+
// assert_eq!(read_data, combined_chunks);
209+
// assert!(available_reader.next().await.is_none())
210+
// }
154211
}

0 commit comments

Comments
 (0)