Don't return gateway twice for manual location selection, fixes #823

To reproduce the issue:

1. Start desktop client with riseup provider
2. wait until gateways are loaded
3. select any manual location
4. connect VPN

The log files will show you:

2024-07-01T10:32:20+02:00 DBG Adding gateway to command line via --remote gateway=vpn06-ams.riseup.net ip4=51.158.144.32 port=80 proto=udp4
2024-07-01T10:32:20+02:00 DBG Adding gateway to command line via --remote gateway=vpn06-ams.riseup.net ip4=51.158.144.32 port=1194 proto=udp4
2024-07-01T10:32:20+02:00 DBG Adding gateway to command line via --remote gateway=vpn06-ams.riseup.net ip4=51.158.144.32 port=80 proto=udp4
2024-07-01T10:32:20+02:00 DBG Adding gateway to command line via --remote gateway=vpn06-ams.riseup.net ip4=51.158.144.32 port=1194 proto=udp4
2024-07-01T10:32:20+02:00 DBG Adding gateway to command line via --remote gateway=vpn04-ams.riseup.net ip4=51.15.9.205 port=80 proto=udp4
2024-07-01T10:32:20+02:00 DBG Adding gateway to command line via --remote gateway=vpn04-ams.riseup.net ip4=51.15.9.205 port=1194 proto=udp4

The problem: vpn06-ams.riseup.net shows up twice in the list.

The problem does not occur when we use automatic location selection.

Some background/confusing things on this:

curl https://api.black.riseup.net:9001/json -k | jq The reponse has a field gatways. This is a sorted list. If you run curl from a random server, the gateways are in a different order.

If we load the json into a go struct, it has two fields for gateways (Gateways and SortedGateways):

type geoLocation struct {
    IPAddress      string       `json:"ip"`
    Country        string       `json:"cc"`
    City           string       `json:"city"`
    Latitude       float64      `json:"lat"`
    Longitude      float64      `json:"lon"`
    Gateways       []string     `json:"gateways"`
    SortedGateways []geoGateway `json:"sortedGateways"`
}

Even if the api response serves a sorted list, the values are stored into the variable Gateways... This is very confusing but I think that's not a bug at this point.

There are two lists of gateways in the strct gatewayPool: available and recommended.

p.available = eip.getGateways()

In eip.getGateways() (eip_service.go), we add each gateway to p.available multiple times (for each Transport: each riseup gateway supports obfs4 and openvpn - at least that what's the api says). So the next confusing thing is: p.available does not have a list of Gateways. Gateways are listed multiple times, if they support different transport. That's my debug log:

2024-07-01T11:10:39+02:00 INF Adding gateway to p.available vpn01-sea.riseup.net with transport openvpn
2024-07-01T11:10:39+02:00 INF Adding gateway to p.available vpn01-sea.riseup.net with transport obfs4

Then there is *gatewayPool.recommended. It's set in setRecommendedGateways (gateways.go). If if len(geo.SortedGateways) != 0 (and it's always empty), we are basically just setting p.recommended = p.available.

Back to the original issue:

The function gatewayPool.getBest() returns the list of gateways we connect to. It calls *gatewayPool.getGatewaysFromMenshenByLocation(), where our bug lays:

var gateways []Gateway
for _, gw := range p.recommended {
    for _, locatedGw := range gws {
        if !locatedGw.isTransport(transport) {
            continue
        }
        if locatedGw.Host == gw.gateway.Host {
            gateways = append(gateways, *locatedGw)
            break
        }
    }
    if len(gateways) == maxGateways {
        break
    }

As each gateway is listed twice in p.recommended. And that's why each gateway gets returned twice here. We check the transport of locatedGw, not the transport of gw.

I fixed it by changing the logic: First ignore all gateways where the transport does not match. Then check if the hostname machtes.

for _, gw := range p.recommended {
    if !gw.gateway.isTransport(transport) {
        continue
    }
    for _, locatedGw := range gws {
        if locatedGw.Host == gw.gateway.Host {
            gateways = append(gateways, *locatedGw)
            break
        }
    }