To reproduce the issue:
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=udp4The problem: vpn06-ams.riseup.net shows up twice in the list.
The problem does not occur when we use automatic location selection.
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 obfs4Then 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
}
}