Unify hopping and regular API

We want the client to have a unified API rather than being a socks server for "regular" mode and a transparent UDP proxy for "hopping" mode.

We chose to move to the transparent UDP proxy because socks+UDP support is missing/not particularly reliable.

On the client side it's simplest for the "regular" mode to be conceptually a "hopping" client that only has a single IP+port endpoint and doesn't bother to ever "hop" to any other endpoint. In that way we can go ahead and remove the other code path.

On the server side we do a similar thing, renaming the "HopServer" to "UDPServer" and depending on whether hopping is enabled we listen on a single port or many.

We leave the TCP server for now because our introducer code uses the obfsvpn.NewDialerFromCert method to create a dialer which depends on an obfsvpn server endpoint running on TCP mode.

Note that the client API (and FFI API) have changed so any upstream libraries need to reflect those changes.

ALSO: the openvpn client config has changed, including the protocol, the remote, and the removal of the socks parameters.

changed milestone to %2024.06 LEAP VPN Release

added Doing label

requested review from @cyberta

assigned to @maxb

changed the description

added 1 commit

• bd70cc66 - Unify hopping and regular API

Compare with previous version

added 1 commit

• 7549f56b - Unify hopping and regular API

Compare with previous version

added 1 commit

• 64e4df21 - Set docker-compose "project" in integration tests

Compare with previous version

we need to keep this in mind, that the locking was removed. It might affect the integration into the desktop client (Android uses a version a couple commits before the mutex was introduced). I assume it was there to address race-conditions when starting and stopping obfsvpn quickly.

hmmmm I can see if there's anything useful in the git log...

lol I guess I shouldn't be surprised... d24eb3bb

@cyberta should we just add the start/stop lock back?

ok gonna answer my own question here, it looks like we might as well add the mutex back in as it would be problematic for the start and stop functions to get called concurrently....

changed this line in version 5 of the diff

Before merging this breaking change it would be nice to git tag a release (and maybe start a CHANGELOG?), so that it's easier to revert the API change in integrating clients.

Do we have any prior art on this in any of our other repos so we can try and stay consistent on these kinds of patterns?

Doing a little thinking on this:

• I think we can pretty easily just tag the previous version as 0.0.1 and then this would be 1.0.0
• I can manually write up a changelog, but I think in general I'd love a tool that does some automation here. I'm not sure I necessarily want a fully automatic semantic commit -> changelog tool, but I also don't love depending on people to do this automatically every time...

I don't have a strong opinion about the versioning, in our updated code standards we thought about keeping the versioning formatted as major.minor.hotfix. In that case I would probably start with 0.1.0 and move to 1.0.0

I think we don't use any changelog creation tools and I think I never did. Which is why I would need to lookup for tools that do that. If you have proposals, please go ahead. Starting with a manual CHANGELOG would be totally fine with me.

It looks like @jkito added a pkg/version file to the bitmask-vpn repo: https://0xacab.org/leap/bitmask-vpn/-/blob/main/pkg/version?ref_type=heads

Is that standard? I don't think I've ever seen that before?

I like having the commit for the version (instead of just a git tag), so maybe just the changelog diff in that version bump commit?

Oh also the bitmask-vpn tags don't have the leading v: https://0xacab.org/leap/bitmask-vpn/-/tags

I was under the impression that for go we'd tag it like v0.1.0, v1.0.0, etc... right?

Is that standard? I don't think I've ever seen that before?

We do have sth similar on Android where we also have to define the version in a config amongst other settings (build.gradle file).

imo we can keep the v out of the version tag

We probably don't have a working standard for it though :)

Android also doesn't have the v in the git version tag

I like having the commit for the version (instead of just a git tag), so maybe just the changelog diff in that version bump commit?

sounds good.

imo we can keep the v out of the version tag

I think go module dependency resolution requires the leading v? https://go.dev/doc/modules/version-numbers https://github.com/golang/go/issues/32945 https://github.com/golang/go/issues/34512

Ok went with !38 (6336ea3e)

It looks like @jkito added a pkg/version file to the bitmask-vpn repo: https://0xacab.org/leap/bitmask-vpn/-/blob/main/pkg/version?ref_type=heads Is that standard? I don't think I've ever seen that before?

that was a quick fix to create a release, i've addressed this in bitmask-vpn!200 (merged) which is standard practice in go projects

mentioned in issue #43 (closed)

code lgtm! (although the diff is a litte bit difficult to read)

could you maybe be a little bit more explicit in the description here? It's hard to understand the difference between kcp and udp and why one would choose both flags.

Also maybe sth. similar for obfs4-KCP to point out it's the packet format on the wire / the outer packaet stream?

changed this line in version 6 of the diff

@cyberta thanks for all the feedback! I'll get on these

added 1 commit

• 524e26e2 - Add mutex to client Start/Stop functions

Compare with previous version

added 1 commit

• 6dd40745 - fixup! Unify hopping and regular API

Compare with previous version

marked this merge request as draft from 6dd40745

marked this merge request as ready

added 3 commits

• 09b1497c - Unify hopping and regular API
• af3188d8 - Set docker-compose "project" in integration tests
• ab250eab - Add mutex to client Start/Stop functions

Compare with previous version

added 1 commit

• 6336ea3e - Version v1.0.0

Compare with previous version