ssh icon indicating copy to clipboard operation
ssh copied to clipboard

Add ConnectionClosedCallback

Open SimoneDutto opened this issue 11 months ago • 3 comments

Hello, I am trying to implement a MaxConcurrentConnections for the ssh server. I've notice the library already offers a ConnCallback and ConnectionFailedCallback. However that seems not to be enough to create a guard around the number of maximum connection because we are missing the ConnectionClosedCallback.

With something like ConnectionClosedCallback I could simply implement my rate limiter by doing:

ssh.Server{
  ConnCallback: func(ctx ssh.Context, conn net.Conn) net.Conn {
	mux.Lock()
        defer mux.Unlock()
        if n > maxConnections {
		conn.Close()
		return conn
	}
	n ++
        return conn
  },
  ConnectionFailedCallback: func(conn net.Conn, err error) {
        mux.Lock()
        defer mux.Unlock()
  	n--
  },
  ConnectionClosedCallback: func(){
        mux.Lock()
        defer mux.Unlock()
  	n--
  }
}

I'll put a PR up just to show you how I've tried it to implement the ConnectionClosedCallback, but I am of course open to suggestions!

SimoneDutto avatar Jan 17 '25 14:01 SimoneDutto

@SimoneDutto you can get around this by listening to the session context. Just spin off a goroutine in ConnCallback and wait for <- ctx.Done(), then do what you need to do.

E.g.:

ConnCallback: func(ctx ssh.Context, conn net.Conn) net.Conn {
	go func() {
		<-ctx.Done()
		fmt.Println("Connection concluded")
	}()

	return conn
},

wolveix avatar Mar 15 '25 13:03 wolveix

@wolveix i didn't know that, thank you so much! I have another question: From my test ctx.Done() channel is filled even in case of errors returned by one of the Handlers. Can you confirm that?

SimoneDutto avatar Mar 18 '25 12:03 SimoneDutto

@SimoneDutto you're very welcome :)

Yeah, context should always conclude. Looking at this library's source code, they don't set a timeout for the context itself; however, they initialize it with a cancel function and always call it. So unless a panic occurs within the library, the context will conclude :)

wolveix avatar Mar 18 '25 13:03 wolveix