tladesignz / IPtProxy

Obfs4proxy and Snowflake Pluggable Transports for iOS
MIT License
45 stars 11 forks source link

StopSnowflakeProxy() doesn't work #19

Closed bitmold closed 2 years ago

bitmold commented 2 years ago

I noticed that calling this function in (at lesat) IPtProxy 1.4.0 doesn't do anything, you still get events:

2022-02-10 00:24:13.218 5890-5962/com.example.testsnowflakeproxy E/GoLog: 2022/02/10 05:24:13 NAT type: restricted
2022-02-10 00:24:16.078 5890-5962/com.example.testsnowflakeproxy E/GoLog: 2022/02/10 05:24:16 NAT type: restricted
func StopSnowflakeProxy() {
    if snowflakeProxy == nil {
        return
    }

    go func() {
        snowflakeProxy.Stop()
        snowflakeProxy = nil
    }()
}

The null assignment here does work, since you can then invoke StartSnowflakeProxy()

however this loop never seems to end:

for ; true; <-ticker.C {
        select {
        case <-sf.shutdown:
            return nil
        default:
            tokens.get()
            sessionID := genSessionID()
            sf.runSession(sessionID)
        }
    }
    return nil
bitmold commented 2 years ago

I'm not sure if this is an issue in IPtProxy or snowflake itself

bitmold commented 2 years ago

If you update snowflake to 2.1.0 https://github.com/tladesignz/IPtProxy/pull/20 this bug still exists too

bitmold commented 2 years ago

This was an issue in IPtProxy, by nil'ing the SnowflakeProxy pointer outside and after the goroutine IPtProxy behaves correctly.

The latest commit in that pull request fixes this bug. In the PR I made the patch to 2.1.0 as a means of testing to see if the bug goes away and for my own experimentation. I've only tested the proxy side of things with that patch too, not the snowflake client. So if you just want this bug to go away and do the patching your own way, you can cherry-pick the last commit from that branch.

For simple testing with an IPtProxy.aar file and what's hosted on jitpack.io you can use https://github.com/bitmold/TestSnowflakeProxy

bitmold commented 2 years ago

@n8fr8 This effects Orbot too, we need to bump Orbot once this gets fixed

tladesignz commented 2 years ago

Fixed with bd61f76a5e234be08f49a648f7ce76cb0dfe07d0. Thanks a lot for catching this!