Apply the obfuscation port to the entry configuration only

[rablador]

Currently, the obfuscation port is applied to the exit configuration at all times. Instead, it should be applied to the entry configuration when using multihop.

---

This change is 

[linear]

IOS-936 Apply the shadowsocks port to the entry configuration.

[buggmagnet]

Reviewed 8 of 8 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @rablador)

---

ios/MullvadREST/Relay/RelayPicking.swift line 25 at r1 (raw file):

        from candidates: [RelayWithLocation<REST.ServerRelay>],
        closeTo location: Location? = nil,
        obfuscate: Bool

I think this conveys the wrong information, and is misleading.
It's effectively saying "use obfuscation" when instead it should be saying "use the obfuscated port if present" since this doesn't decide whether obfuscation is ultimately used or not.

How about we rename it shouldUseObfuscatedPort ?

[rablador]

Added fixes for issue https://linear.app/mullvad/issue/IOS-948/fix-port-selection-for-shadowsocks as well.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @buggmagnet)

---

ios/MullvadREST/Relay/RelayPicking.swift line 25 at r1 (raw file):

Previously, buggmagnet wrote…

I think this conveys the wrong information, and is misleading.
It's effectively saying "use obfuscation" when instead it should be saying "use the obfuscated port if present" since this doesn't decide whether obfuscation is ultimately used or not.

How about we rename it shouldUseObfuscatedPort ?

Done.

[buggmagnet]

Looks good, but Unit Tests are broken now 🫠

Reviewed 4 of 5 files at r2, all commit messages.
Reviewable status: 9 of 10 files reviewed, all discussions resolved

[rablador]

Fixed!

Reviewable status: 9 of 10 files reviewed, all discussions resolved (waiting on @buggmagnet)

[buggmagnet]

Reviewed 1 of 5 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

[acb-mv]

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

[mojganii]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @rablador)

---

ios/MullvadREST/Relay/RelayPicking.swift line 14 at r2 (raw file):

protocol RelayPicking {
    var obfuscation: ObfuscatorPortSelection { get }

nit : that wouldn't be better to integrate all properties in another structure. since the it's growing up.

---

ios/MullvadREST/Relay/RelayPicking.swift line 15 at r2 (raw file):

protocol RelayPicking {
    var obfuscation: ObfuscatorPortSelection { get }
    var relays: REST.ServerRelaysResponse { get }

there no need to make it an exposed computed property out of the object. instead of using that property I suggest to use obfuscation.relays directly

---

ios/MullvadREST/Relay/RelayPicking.swift line 54 at r2 (raw file):

        var endpoint = match.endpoint

        if !portIsWithinRange {

I suggest to add some comment here about the process we are doing when the port is out of the range.

---

ios/MullvadREST/Relay/RelayPicking.swift line 89 at r2 (raw file):

            )

            let match = try findBestMatch(from: exitCandidates, useObfuscatedPort: true)

what is the reason behind setting that true in single hop?

[rablador]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @mojganii)

---

ios/MullvadREST/Relay/RelayPicking.swift line 14 at r2 (raw file):

Previously, mojganii wrote…

nit : that wouldn't be better to integrate all properties in another structure. since the it's growing up.

I think it's fine at this point, but maybe in the future. Adding a wrapper adds an extra layer that might make things harder to follow.

---

ios/MullvadREST/Relay/RelayPicking.swift line 15 at r2 (raw file):

Previously, mojganii wrote…

there no need to make it an exposed computed property out of the object. instead of using that property I suggest to use obfuscation.relays directly

I thought it would be clearer to specify a generic relays variable. It might not be obvious for a reader that the filtered relays from the obfuscation result is the thing you want.

---

ios/MullvadREST/Relay/RelayPicking.swift line 54 at r2 (raw file):

Previously, mojganii wrote…

I suggest to add some comment here about the process we are doing when the port is out of the range.

Done.

---

ios/MullvadREST/Relay/RelayPicking.swift line 89 at r2 (raw file):

Previously, mojganii wrote…

what is the reason behind setting that true in single hop?

It's not very obvious here, I agree, but useObfuscatedPort means that we should use the obfuscated port if there's one available. In singlehop there's only one (exit) relay, so obfuscation should always be applied in that case. In multihop we only set useObfuscatedPort to true if it's the entry relay.

[buggmagnet]

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @mojganii)

[mojganii]

Reviewable status: complete! all files reviewed, all discussions resolved

[github-actions]

🚨 End to end tests failed. Please check the failed workflow run.