Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The second TCP connection will be directly dropped by ipstack on the Windows platform #27

Closed
xmh0511 opened this issue Mar 27, 2024 · 6 comments

Comments

@xmh0511
Copy link
Contributor

xmh0511 commented Mar 27, 2024

The minimum reproductive example is https://github.com/xmh0511/ipstack-issue. This issue can only be reproduced on the Windows platform(I tested it on Windows 10). see #27 (comment)

1. Directly drop connection in the next time when using user side timeout

The key point to reproduce the issue is adding the user timeout for the connection

async fn my_bidirection_copy<L, R>(lhs: L, rhs: R,addr:String)
where
    L: AsyncRead + AsyncWrite + Send + Sync + 'static,
    R: AsyncRead + AsyncWrite + Send + Sync + 'static,
{
    let (mut l_reader, mut l_writer) = tokio::io::split(lhs);
    let (mut r_reader, mut r_writer) = tokio::io::split(rhs);
    let mut join_set = tokio::task::JoinSet::new();
    let _addr1 = addr.clone();
    join_set.spawn(async move {
        let mut buf = [0u8; 1500];
        loop {
            let size = match tokio::time::timeout(std::time::Duration::from_secs(3), l_reader.read(&mut buf)).await{
                Ok(v)=>v?,
                Err(_e)=>{
                   // r_writer.shutdown().await.unwrap();
                   // println!("left close ok for {addr1}");
                    return anyhow::Ok(());
                }
            };
            if size == 0 {
                //println!("left 0 {addr1}");
                //r_writer.shutdown().await.unwrap();
                //println!("left close ok for {addr1}");
                return anyhow::Ok(());
            }
            //println!("outbound {}",String::from_utf8_lossy(&buf[..size]));
            r_writer.write_all(&buf[..size]).await?;
        }
    });
    let _addr2 = addr.clone();
    join_set.spawn(async move {
        let mut buf = [0u8; 1500];
        loop {
            let size = match tokio::time::timeout(std::time::Duration::from_secs(3), r_reader.read(&mut buf)).await{
                Ok(v) => {
                    v?
                },
                Err(_e) =>{
                    //l_writer.shutdown().await.unwrap();
                    return anyhow::Ok(());
                },
            };
            if size == 0 {
                //println!("right read 0 {addr2}");
                //l_writer.shutdown().await.unwrap();
                //println!("right close ok for {addr2}");
                return anyhow::Ok(());
            }
            //println!("inbound {}", String::from_utf8_lossy(&buf[..size]));
            l_writer.write_all(&buf[..size]).await?;
        }
    });
    while let Some(_) = join_set.join_next().await{
        //break;
    }
    println!("====== end tcp connection ====== {addr}");
}

The default timeout of IpstackTcpStream is 60s while the setting timeout at the user side is 3s. In PowerShell, then running the command curl http://10.0.0.6:6, the first time connection can be sent to IpStackStream::Tcp(tcp), however, the second time IpStackStream::Tcp(tcp) received nothing, that is, tcpstream does not deliver to the user side. I debug ipstack and find that ipstack directly drops the new connection.

2. IpStackStream::shutdown().await always be pending

The second issue is that when the user side timeout, invoking IpStackStream::shutdown().await will always be pending. The complete reproduced video can be seen complete video link

截屏2024-03-27 13 45 08

snippet

2024-03-27.13.43.06-1.mov
@xmh0511
Copy link
Contributor Author

xmh0511 commented Mar 27, 2024

After digging into tcp.rs, the second request emitted by curl is determined by InvalidTcpPacket

        println!("tcp.inner().syn = {}",tcp.inner().syn);
        if !tcp.inner().syn {
            _ = pkt_sender.send(stream.create_rev_packet(RST | ACK, TTL, None, Vec::new())?);
            Err(IpStackError::InvalidTcpPacket)
        } else {
            Ok(stream)
        }
截屏2024-03-27 14 39 55

However, tcp.rs has sent DROP_TTL to the previous connection. However, for ttl = 0, the lib.rs didn't write anything to tun device

  if packet.ttl() == 0{
      println!("ipstack sending to tun {:?} since ttl = {}", packet.reverse_network_tuple(),packet.ttl());
      streams.remove(&packet.reverse_network_tuple());
      continue;
  }

So, I suspect that the second running of curl http://10.0.0.6:6 re-uses the previous TCP connection?

截屏2024-03-27 14 59 35

In this screenshot, we can see the first and the second both have source_port 56253, the first syn = true while the second syn = false.

@xmh0511
Copy link
Contributor Author

xmh0511 commented Mar 27, 2024

Find out the way that can reproduce the behavior on other systems

  1. Start the main.rs
  2. running the client.rs in src/bin.

Then, the same error would be printed out.

截屏2024-03-27 16 07 01

@xmh0511
Copy link
Contributor Author

xmh0511 commented Mar 27, 2024

The interpretation of this issue is, that when ipstack drops or shutdown the IpstackTcpStream, it does not write any TCP packet denoting "shutdown" to the tunnel device, which results in the client in the other side of tun device does not receive any close signal, and when the TCP of the client writes some packet to tun and ipstack read the packet from the tun, ipstack thought it was a new connection since ipstack has removed the connection from the map streams, hence if !tcp.inner().syn is invoked to return Err(IpStackError::InvalidTcpPacket).

I think when an instance of IpstackTcpStream is shut down or dropped, we should send the corresponding packet to the tunnel device to tell the peer we close the connection.

@SajjadPourali
Copy link
Collaborator

Thank you for the report. I was able to reproduce the timeout issue on shutdown. Let me investigate the Windows issue to make the best decision on how to fix these issues.

@SajjadPourali
Copy link
Collaborator

Both issues has the same root cause.
We need to implement shutdown in the Drop function, but since Drop cannot be async in Rust and shutdown is async, it is challenging to address the issue properly.

SajjadPourali added a commit that referenced this issue Mar 27, 2024
@SajjadPourali
Copy link
Collaborator

Both issues has the same root cause. We need to implement shutdown in the Drop function, but since Drop cannot be async in Rust and shutdown is async, it is challenging to address the issue properly.

Currently, you need to shut down the stream manually due to the lack of async drop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants