Fix off-by-one error when choosing access method candidates

[MarkusPettersson98]

As the code comment says: get_next_inner will skip the start position, which means that by starting from the back of the available access methods we effectively look for candidates starting from index 0, i.e. the position of the direct access method. It's probably a good idea to not default users to bridges/proxies to reach the API.

Edit: The solution was found elsewhere. As it turns out, mullvad-api always forced a rotation of access methods whenever MullvadRestHandle was initialized, which means that get_next_inner/select_next_active was called twice at startup. This was subsequently solved by passing along an initial access method to the constructor of MullvadRestHandle, as is done with the tunnel state machine.

---

This change is 

[dlon]

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

---

mullvad-daemon/src/api.rs line 250 at r1 (raw file):

            // by starting from the back of the available access methods we
            // effectively look for candidates starting from index 0.
            access_method_settings.cardinality() - 1,

Correct me if I'm wrong, but this seems to end up selecting the last method in the list if a method should be added before get_next is called.

I think a less clever solution might be better. Maybe by not setting an initial method at all, or not relying on get_next to select the "first" method.

But we could use this as a hotfix if we need one.

[MarkusPettersson98]

Reviewable status: 0 of 9 files reviewed, 1 unresolved discussion (waiting on @dlon)

---

mullvad-daemon/src/api.rs line 250 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

Correct me if I'm wrong, but this seems to end up selecting the last method in the list if a method should be added before get_next is called.

I think a less clever solution might be better. Maybe by not setting an initial method at all, or not relying on get_next to select the "first" method.

But we could use this as a hotfix if we need one.

Turns out that the solution in this module was very straightforward - the turned out to be somewhere else! See my updated PR comment for context 😊

Refactored get_next_inner (now select_next_active) to be more declarative and start looking for candidates starting from the start position, otherwise not much else has changed here.

[dlon]

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

[dlon]

Reviewed 1 of 2 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved