a few more states addition proposal

[bh90210]

Hi Roger,

I found this little package extremely useful and would like to contribute with some low hanging fruits that I anyway coded for my project and PR it back :)

More specifically I added 5 more states

    // StateAccepted represents a (local) listener connection. This state gets triggered when a new incoming connection is made.
    StateAccepted

    // StateOpen represents the last part of a successful remote connection proccess.
    // This state's purpose is to signal when the connection is established and fully usable.
    StateOpen

    // StateClosed represents a finished connection cycle as an internal proccess of the package.
    // When triggered it means a successful connection has been terminated.
    StateClosed

    // StateFailed represents a remote dial to server failure, while the underline SSH connection is established.
    StateFailed

    // StateRemoteDropped represents a shut down SSH connection from server.
    // This state is the real remote server connection drop, not internal procedure of the package.
    StateRemoteDropped

I also included a check for connection drop from remote

    go func() {
        // check if connection is still alive
        err := sshConn.Wait()
        if err != nil {
            if tun.debug {
                log.Printf("SSH connection to %s has shut down: %s", server, err.Error())
            }
        }
        if tun.connState != nil {
            tun.connState(tun, StateRemoteDropped)
        }
    }()

thanks in advance, Byron

[bh90210]

hey, it's alright! I used the code for a small project and afterwards I left it be.

I have no problem to change something if you give me clear instructions but I am looking at the code again and I dont understand what you would like to be done I am afraid.

But I don't like the states of the tunnel and the tunneled connections to share the same ConnState.

I dont understand what you are saying here. What would be the difference between the states of the tunnel and the states of the tunneled connection? Isn't it a tautology? from the comments // SetConnState specifies an optional callback function that is called when a SSH tunnel changes state.

"This would cause confusion when having multiple connections through the tunnel because you would not know which one the state you are receiving refers too."

I thought that sshTun := sshtun.New(8080, "my.super.host.com", 80) could only provide one connection at a time?

Maybe adding a SetForwardedConnState that allows to set a callback receiving those new states you proposed and a counter (or something to identify the connection).

I can see how to implement SetForwardedConnState but I have no idea what you mean by something to identify the connection in our context.

I really feel I am missing something! Please advice :)

[bh90210]

yes! I do miss something! XD I;m sorry..

is this code u r referring to tunneled connections?

// Accept connections
    go func() {
        for {
            localConn, err := localList.Accept()
            if err != nil {
                tun.errStarted(fmt.Errorf("Local accept on %s failed: %s", local, err.Error()))
                break
            }
            if tun.debug {
                log.Printf("Accepted connection from %s", localConn.RemoteAddr().String())
            }
            if tun.connState != nil {
                tun.connState(tun, StateAccepted)
            }

            // Launch the forward
            go tun.forward(localConn, config)
        }
    }()

I think I get your point now, will look at it a bit more but my ssh skills are limited and if you could take the the time to explain in more detail it will be greatly appreciated :)

[bh90210]

hey, is something like this what u had in mind?

    // We set a callback to know the state of the forwarded connections
    sshTun.SetForwardedConnState(func(tun *sshtun.SSHTun, state sshtun.ForwardedConnState, forwardCounter int) {
        switch state {
        case sshtun.StateAccepted:
            log.Printf("Forward %d STATE is Accepted", forwardCounter)
        case sshtun.StateOpen:
            log.Printf("Forward %d STATE is Open", forwardCounter)
        case sshtun.StateClosed:
            log.Printf("Forward %d STATE is Closed", forwardCounter)
        case sshtun.StateFailed:
            log.Printf("Forward %d STATE is Failed", forwardCounter)
        case sshtun.StateRemoteDropped:
            log.Printf("Forward %d STATE is Dropped", forwardCounter)
        }
    })

2021/03/20 13:26:10 STATE is Starting
2021/03/20 13:26:10 STATE is Started
2021/03/20 13:26:11 Forward 0 STATE is Accepted
2021/03/20 13:26:11 Forward 1 STATE is Accepted
2021/03/20 13:26:11 Forward 2 STATE is Accepted
2021/03/20 13:26:11 Forward 3 STATE is Accepted
2021/03/20 13:26:11 Forward 4 STATE is Accepted
2021/03/20 13:26:11 Forward 5 STATE is Accepted
2021/03/20 13:26:12 Forward 0 STATE is Open
2021/03/20 13:26:12 Forward 3 STATE is Open
2021/03/20 13:26:12 Forward 4 STATE is Open
2021/03/20 13:26:12 Forward 1 STATE is Open
2021/03/20 13:26:12 Forward 5 STATE is Open
2021/03/20 13:26:12 Forward 2 STATE is Open

[rgzr]

Hi @bh90210!

Sorry for not answering for this long time, I have had no energy to dedicate to this project because of my work and other projects and left it unanswered. The situation has not changed and my priorities are others by now, but maybe I get some time to review, merge or give feedback in the next months... By now, I hope it's ok for anyone who wants this feature to just use your fork.

But reading your messages, I see you understood the point I was concerned about by yourself. :) Answering your last question, yes, something like that was exactly what I had in mind!

Thank you very much for your contribution, I hope the project has been useful to you and I hope others can benefit from your work soon.