Move exit location logic to daemon #5542

Serock3 · 2023-12-01T14:24:24Z

Fixes: DES-514

Currently, the daemon exposes an API for fetching current location, which in turn contacts am.i.mullvad.net. If this fails, each front end needs its own logic for retrying the call to the daemon, which is not ideal. We want the daemon itself handle retries in the REST API and notify frontends when the out IP is available.

This PR removes the GetCurrentLocation rpc. The frontends already listen for updates to the tunnel state, and TunnelState::Connected contains GeoIpLocation. The ipv4 and ipv6 fields of this GeoIpLocation are currently null, so we should fill them with information from am.i.mullvad.net. The daemon will leave the fields empty at first before issuing the am.i.mullvad.net request, when the response arrives it sends another TunnelState::Connected even to the frontends. To get location information after disconnecting, we add GeoIpLocation to TunnelState::Disconnected

This change is

dlon

Reviewed 8 of 9 files at r1, all commit messages.
Reviewable status: 8 of 9 files reviewed, 5 unresolved discussions (waiting on @Serock3)

mullvad-cli/src/cmds/status.rs line 41 at r1 (raw file):

                    }

                    let location = match new_state {

What do you think of simply doing this?

  if args.location {
      match new_state {
          TunnelState::Connected {
              location: Some(location),
              ..
          }
          | TunnelState::Disconnected(Some(location)) => {
              print_location(&location)
          }
          _ => (),
      }
  }

IMO, not finding a location is not an error, but opinions may vary.

mullvad-daemon/src/lib.rs line 1073 at r1 (raw file):

                        .await
                        .ok();
                even_listener.notify_new_state(make_tunnel_state(location));

We're not saving the TunnelState to self.tunnel_state here. Would you agree that we should keep them in sync?

One "reasonable" way of doing this might be to send InternalDaemonEvent::TunnelStateTransition instead of notifying event_listenerdirectly. Or a new InternalDaemonEvent variant if reusing this one is a bad idea.

mullvad-daemon/src/lib.rs line 1073 at r1 (raw file):

                        .await
                        .ok();
                even_listener.notify_new_state(make_tunnel_state(location));

This differs from the previous code, in that we only previously appended ipv4 and ipv6 to the known location info in the connected state. Have you thought about whether it makes sense to change this?

mullvad-daemon/src/lib.rs line 1073 at r1 (raw file):

                        .await
                        .ok();
                even_listener.notify_new_state(make_tunnel_state(location));

Why notify when location.is_none()? Would it not make more sense to do nothing in this case?

mullvad-daemon/src/lib.rs line 1075 at r1 (raw file):

                even_listener.notify_new_state(make_tunnel_state(location));
            })
            .abort_handle(),

Nit: You can call abort directly on JoinHandle<()>

Serock3

Reviewable status: 8 of 9 files reviewed, 5 unresolved discussions (waiting on @dlon)

mullvad-cli/src/cmds/status.rs line 41 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

What do you think of simply doing this?

  if args.location {
      match new_state {
          TunnelState::Connected {
              location: Some(location),
              ..
          }
          | TunnelState::Disconnected(Some(location)) => {
              print_location(&location)
          }
          _ => (),
      }
  }

IMO, not finding a location is not an error, but opinions may vary.

I agree that it's not an error in general, but maybe when you explicitly provide the '--location' flag?

mullvad-daemon/src/lib.rs line 1073 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

We're not saving the TunnelState to self.tunnel_state here. Would you agree that we should keep them in sync?

One "reasonable" way of doing this might be to send InternalDaemonEvent::TunnelStateTransition instead of notifying event_listenerdirectly. Or a new InternalDaemonEvent variant if reusing this one is a bad idea.

You definitely have a point there. We don't currently use this field in self.tunnel_state, but seems very error prone to not sync them properly. I think adding a new InternalDaemonEvent is better than reusing the TunnelStateTransition, since we would get stuck in a recursive loop.

mullvad-daemon/src/lib.rs line 1073 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

Why notify when location.is_none()? Would it not make more sense to do nothing in this case?

Good point.

mullvad-daemon/src/lib.rs line 1073 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

This differs from the previous code, in that we only previously appended ipv4 and ipv6 to the known location info in the connected state. Have you thought about whether it makes sense to change this?

I am discussing this with Oskar. Since I removed the get_current_location rpc, I needed to add this information to the disconnected state too. Calling self.parameters_generator.get_last_location().await for the disconnected case is not right though. The disconnected location should either be fetched from its own cache, or just set to None.

mullvad-daemon/src/lib.rs line 1075 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

Nit: You can call abort directly on JoinHandle<()>

Is that preferred? What is the point of an abort handle?

Serock3

Reviewable status: 8 of 9 files reviewed, 5 unresolved discussions (waiting on @dlon)

mullvad-cli/src/cmds/status.rs line 41 at r1 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

I agree that it's not an error in general, but maybe when you explicitly provide the '--location' flag?

Though now that I think about it, since we never supply the IP the first time around this will always produce an error. Not finding the location cannot be an error for the listen command.

What do you think about always sending a TunnelStatewith no IP before contacting am.i.mullvad.net? The alternative would be to try to contact am.i.mullvad.net immediately, and if it takes more than, say, 0.5 sec send the TunnelState with no IP and move the future into a task?

dlon

Reviewed 1 of 2 files at r2, 3 of 4 files at r3, 2 of 2 files at r8, all commit messages.
Reviewable status: 9 of 16 files reviewed, 2 unresolved discussions (waiting on @Serock3)

mullvad-daemon/src/lib.rs line 1047 at r8 (raw file):

    ///
    /// See [`Daemon::handle_location_event()`]
    async fn fetch_am_i_mullvad(&mut self) {

Nit: This does not need to be async.

mullvad-daemon/src/lib.rs line 1057 at r8 (raw file):

        }

        let use_ipv6 = self.settings.tunnel_options.generic.enable_ipv6;

This setting is independent of whether the "real" network has IPv6 connectivity. I'm not sure what the right approach is. Maybe this could always be true when disconnected?

Serock3

Reviewable status: 7 of 16 files reviewed, all discussions resolved (waiting on @dlon)

mullvad-daemon/src/lib.rs line 1047 at r8 (raw file):

Previously, dlon (David Lönnhager) wrote…

Nit: This does not need to be async.

Thanks, it's fixed now. Why isn't that a clippy warning?

mullvad-daemon/src/lib.rs line 1057 at r8 (raw file):

Previously, dlon (David Lönnhager) wrote…

This setting is independent of whether the "real" network has IPv6 connectivity. I'm not sure what the right approach is. Maybe this could always be true when disconnected?

I made the disconnected state use the wireguard setting as we discussed

linear · 2023-12-12T12:47:44Z

DES-514 Assist android implementation of new location logic

Rawa

Reviewable status: 5 of 49 files reviewed, 1 unresolved discussion (waiting on @dlon and @Serock3)

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModel.kt line 100 at r19 (raw file):

                                is TunnelState.Connected -> tunnelRealState.location
                                is TunnelState.Disconnecting -> lastKnownDisconnectedLocation
                                is TunnelState.Error -> lastKnownDisconnectedLocation

What location do we show on desktop if we have TunnelState error @raksooo? Would it be different if they are blocking or not blocking? Does connecting also display tunnel state or is this something we cache?

Rawa

Reviewed 1 of 42 files at r16.
Reviewable status: 6 of 49 files reviewed, all discussions resolved (waiting on @dlon)

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModel.kt line 100 at r19 (raw file):

Previously, Rawa (David Göransson) wrote…

What location do we show on desktop if we have TunnelState error @raksooo? Would it be different if they are blocking or not blocking? Does connecting also display tunnel state or is this something we cache?

I've talked with @raksooo about this and aligned so it is the same behaviour as on desktop.

Pururun

Reviewed 1 of 3 files at r12, 3 of 42 files at r16, 7 of 7 files at r17, 28 of 32 files at r18, 3 of 3 files at r19, 1 of 1 files at r20, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Serock3)

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/test/ComposeTestTagConstants.kt line 27 at r20 (raw file):

const val CONNECT_BUTTON_TEST_TAG = "connect_button_test_tag"
const val RECONNECT_BUTTON_TEST_TAG = "reconnect_button_test_tag"
const val LOCATION_INFO_TEST_TAG = "location_info_test_tag"

Why was this change made? It is no longer aligned with the other test tags.

Rawa

Reviewable status: 46 of 49 files reviewed, 1 unresolved discussion (waiting on @Pururun)

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/test/ComposeTestTagConstants.kt line 27 at r20 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

Why was this change made? It is no longer aligned with the other test tags.

Fixed

Serock3

Reviewable status: 10 of 52 files reviewed, 1 unresolved discussion (waiting on @Pururun and @Rawa)

android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/test/ComposeTestTagConstants.kt line 27 at r20 (raw file):

Previously, Rawa (David Göransson) wrote…

Fixed

Done.

albin-mullvad

Reviewed 27 of 30 files at r27.
Reviewable status: 37 of 52 files reviewed, 2 unresolved discussions (waiting on @Pururun, @Rawa, and @Serock3)

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModel.kt line 159 at r27 (raw file):

            .filter { it.location == null }
            .map { it.location }
            .onStart { emit(null) }

Is this aligned with how this case is handled in the desktop app? Also, is it working reliably?

Code quote:

    private fun ConnectionProxy.lastKnownLocation(): Flow<GeoIpLocation?> =
        tunnelRealStateFlow()
            .filterIsInstance<TunnelState.Disconnected>()
            .filter { it.location == null }
            .map { it.location }
            .onStart { emit(null) }

Pururun

Reviewed 1 of 3 files at r21, 3 of 41 files at r22, 1 of 38 files at r25, 7 of 7 files at r26, 3 of 30 files at r27, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Serock3)

MarkusPettersson98

Reviewed all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @Serock3)

mullvad-cli/src/format.rs line 54 at r27 (raw file):

    };
    if let Some(location) = location {
        print!("Your connection appears from: {}", location.country);

Nit: This phrasing seems a bit cut short to me. Would it make sense to extend it a little bit to make it grammatically correct? 😊

Your connection appears to be from:

Code quote:

Your connection appears from:

mullvad-cli/src/cmds/status.rs line 21 at r27 (raw file):

    verbose: bool,

    // TODO: changelog about removing location flag

Has this been added to the changelog yet? If so, remove this comment 😊

Code quote:

// TODO: changelog about removing location flag

mullvad-cli/src/cmds/status.rs line 39 at r27 (raw file):

                        // When we enter the connected or disconnected state, am.i.mullvad.net will
                        // be polled to get IP information. When it arrives, we will get another
                        // tunnel state of the same enum type, but with the IP  filled in. This

Nit: Some extra whitespace snuck into this comment. Could you trim it a bit? 😊

mullvad-cli/src/cmds/status.rs line 95 at r27 (raw file):

        println!("Tunnel state: {state:#?}");
    } else {
        // TODO: respect location arg?

Should this TODO be handled in this PR, or is the fix blocked on something that is not yet implemented? 😊

Code quote:

// TODO: respect location arg?

mullvad-daemon/src/geoip.rs line 75 at r27 (raw file):

        // Increment request ID
        self.request_id = self.request_id.wrapping_add(1);
        let request_id_copy = self.request_id;

Nit: Could this (implicit) copy be grouped with the other clones just before invoking tokio::spawn? 😊

let request_id_copy = self.request_id;
let rest_service = self.rest_service.clone();
let location_sender = self.location_sender.clone();
tokio::spawn(async move {
    if let Ok(merged_location) = get_geo_location_with_retry(use_ipv6, rest_service).await {
        let _ = location_sender.send(InternalDaemonEvent::LocationEvent((
            request_id_copy,
            merged_location,
        )));
    }
});

Code quote:

let request_id_copy = self.request_id;

mullvad-daemon/src/lib.rs line 374 at r27 (raw file):

    DeviceMigrationEvent(Result<PrivateAccountAndDevice, device::Error>),
    /// A geographical location has has been received from am.i.mullvad.net
    LocationEvent((usize, GeoIpLocation)),

Would it make sense to define a dedicated type for this InternalDaemonEvent to give usize and GeoIpLocation descriptive names?
I could imagine the type to look something like:

struct LocationEventData {
    /// Keep track of which request led to this event being triggered
    request_id: usize,
    /// New location information
    location: GeoIpLocation,
}

...

pub(crate) enum InternalDaemonEvent {
...
    LocationEvent(LocationEventData),
...
}

It would automate the naming of these variables for any caller pattern matching on LocationEvent, which I think is a huge win 🏆

match event {
    LocationEvent(LocationEventData { request_id, location }) => {
        self.handle_location_event(request_id, location)
    }
    ...
}

Code quote:

    /// A geographical location has has been received from am.i.mullvad.net
    LocationEvent((usize, GeoIpLocation)),

mullvad-daemon/src/lib.rs line 882 at r27 (raw file):

        } else {
            self.fetch_am_i_mullvad()
        }

It would be nice to have some comment explaining why fetch_am_i_mullvad is invoked in this else-branch. At the moment, I'm a bit puzzled as to why this is not the case for both branches 😊

Code quote:

        } else {
            self.fetch_am_i_mullvad()
        }

Jontified

Reviewed 2 of 9 files at r1, 3 of 4 files at r3, 1 of 3 files at r12, 4 of 42 files at r16, 7 of 7 files at r17, 32 of 32 files at r18, 1 of 3 files at r21, 3 of 41 files at r22, 1 of 38 files at r25, 7 of 7 files at r29, 30 of 30 files at r30, all commit messages.
Reviewable status: all files reviewed, 13 unresolved discussions (waiting on @MarkusPettersson98 and @Serock3)

CHANGELOG.md line 38 at r18 (raw file):

### Changed
- Remove `--location` flag from `mullvad status` CLI. Location and IP will now always
  be printed (if available). `mullvad status listen` does no longer print location info.

does no longer print => no longer prints

mullvad-cli/src/cmds/status.rs line 21 at r18 (raw file):

    verbose: bool,

    // TODO: changelog about removing location flag

TODO

mullvad-cli/src/cmds/status.rs line 95 at r18 (raw file):

        println!("Tunnel state: {state:#?}");
    } else {
        // TODO: respect location arg?

TODO
I don't think there is a good reason to keep the flag. We are moving this information into state and will be printing that regardless.
We could avoid printing location in state and keep the flag, but that seems needlessly complex.
We could print only the location if the user provides the flag but then we're breaking the CLI regardless and it seems unnecessary.

mullvad-daemon/src/geoip.rs line 106 at r18 (raw file):

        move || send_location_request(rest_service.clone(), use_ipv6),
        move |result| match result {
            Err(error) => error.is_network_error(),

What is the reason for this change? Is it because we don't have access to API availability or for some other reason?

mullvad-daemon/src/lib.rs line 1038 at r18 (raw file):

        self.tunnel_state = tunnel_state.clone();
        self.event_listener.notify_new_state(tunnel_state);
        self.fetch_am_i_mullvad();

I am wondering what the privacy implications are of pinging this every time we do a state transition, it would seem like this isn't a problem in the Connected state but when we're disconnected do we want the app to ping this?
Not sure what the current state of the app is with regards to this. Perhaps all the frontends will ping am.i.mullvad when you're disconnected?

mullvad-daemon/src/lib.rs line 374 at r27 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

Would it make sense to define a dedicated type for this InternalDaemonEvent to give usize and GeoIpLocation descriptive names?
I could imagine the type to look something like:
struct LocationEventData {
    /// Keep track of which request led to this event being triggered
    request_id: usize,
    /// New location information
    location: GeoIpLocation,
}

...

pub(crate) enum InternalDaemonEvent {
...
    LocationEvent(LocationEventData),
...
}
It would automate the naming of these variables for any caller pattern matching on LocationEvent, which I think is a huge win 🏆
match event {
    LocationEvent(LocationEventData { request_id, location }) => {
        self.handle_location_event(request_id, location)
    }
    ...
}

Dedicated type seems reasonable

Serock3

Reviewable status: 11 of 53 files reviewed, 10 unresolved discussions (waiting on @dlon, @Jontified, @MarkusPettersson98, and @Pururun)

CHANGELOG.md line 38 at r18 (raw file):

Previously, Jontified wrote…

does no longer print => no longer prints

Done.

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModel.kt line 159 at r27 (raw file):

Previously, albin-mullvad wrote…

Is this aligned with how this case is handled in the desktop app? Also, is it working reliably?

Looks good with the change to !=

mullvad-cli/src/format.rs line 54 at r27 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

Nit: This phrasing seems a bit cut short to me. Would it make sense to extend it a little bit to make it grammatically correct? 😊

Your connection appears to be from:

My reasoning was that it would make sense with the location filled in, e.g. "Your connection appears to be from: Sweden, Gothenburg, IPv4 x.x.x.x". Do you have any suggestions for better phrasing?

mullvad-cli/src/cmds/status.rs line 21 at r18 (raw file):

Previously, Jontified wrote…

TODO

Done.

mullvad-cli/src/cmds/status.rs line 95 at r18 (raw file):

Previously, Jontified wrote…

TODO
I don't think there is a good reason to keep the flag. We are moving this information into state and will be printing that regardless.
We could avoid printing location in state and keep the flag, but that seems needlessly complex.
We could print only the location if the user provides the flag but then we're breaking the CLI regardless and it seems unnecessary.

Yup, the --location flag is removed in this PR for that reason, I just missed to clean up this comment.

mullvad-cli/src/cmds/status.rs line 21 at r27 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

Has this been added to the changelog yet? If so, remove this comment 😊

It has, good catch! Fixed.

mullvad-cli/src/cmds/status.rs line 95 at r27 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

Should this TODO be handled in this PR, or is the fix blocked on something that is not yet implemented? 😊

Yup, the location flag is removed, forgot about the comment. Thanks.

mullvad-daemon/src/geoip.rs line 106 at r18 (raw file):

Previously, Jontified wrote…

What is the reason for this change? Is it because we don't have access to API availability or for some other reason?

I implemented the previous version of this retry-strategy which checks for API availability and @dlon pointed out that it isn't needed in this case (I can't quite remember the reasoning, perhaps because it isn't a user initiated action? You would have to ask him).

mullvad-daemon/src/geoip.rs line 75 at r27 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

Nit: Could this (implicit) copy be grouped with the other clones just before invoking tokio::spawn? 😊

let request_id_copy = self.request_id;
let rest_service = self.rest_service.clone();
let location_sender = self.location_sender.clone();
tokio::spawn(async move {
    if let Ok(merged_location) = get_geo_location_with_retry(use_ipv6, rest_service).await {
        let _ = location_sender.send(InternalDaemonEvent::LocationEvent((
            request_id_copy,
            merged_location,
        )));
    }
});

Sure. Fixed.

mullvad-daemon/src/lib.rs line 1038 at r18 (raw file):

Previously, Jontified wrote…

I am wondering what the privacy implications are of pinging this every time we do a state transition, it would seem like this isn't a problem in the Connected state but when we're disconnected do we want the app to ping this?
Not sure what the current state of the app is with regards to this. Perhaps all the frontends will ping am.i.mullvad when you're disconnected?

Yes, that is the implication, and we have discussed it. The frontends already poll your geoip every time you disconnect as far as I am aware, so fixing that was not deemed to block this PR. Probably a better solution would be to cache your disconnected location and only update it, say, every 30 minutes.

mullvad-daemon/src/lib.rs line 374 at r27 (raw file):

Previously, Jontified wrote…

Dedicated type seems reasonable

Very good suggestion. Implemented it.

mullvad-daemon/src/lib.rs line 882 at r27 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

It would be nice to have some comment explaining why fetch_am_i_mullvad is invoked in this else-branch. At the moment, I'm a bit puzzled as to why this is not the case for both branches 😊

Fixed

MarkusPettersson98

Reviewed 5 of 42 files at r31, 7 of 7 files at r32, 30 of 30 files at r33, 1 of 38 files at r34, 7 of 7 files at r35, 30 of 30 files at r36, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @dlon and @Jontified)

mullvad-daemon/src/lib.rs line 882 at r27 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

Fixed

🙌

Serock3

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

mullvad-daemon/src/geoip.rs line 106 at r18 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

I implemented the previous version of this retry-strategy which checks for API availability and @dlon pointed out that it isn't needed in this case (I can't quite remember the reasoning, perhaps because it isn't a user initiated action? You would have to ask him).

am.i.mullvad.net is a separate rest API (not entirely sure why), so using using the normal API availability handle was a bug.

Make the daemon send two tunnel state updates, one with out IP being empty, and another with it being filled when am.i.mullvad.net responds. Update CLI for this change. Other front ends are left out.

Add `geoip::GeoIpHandler`, which sends an `InternalDaemonEvent::LocationEvent` when the location arrives. It also handles aborting in-flight requests and retries.

Remove `get_current_location` from jni.

Serock3 changed the title ~~Out ip delay~~ Move exit location logic to daemon Dec 1, 2023

dlon reviewed Dec 1, 2023

View reviewed changes

Serock3 commented Dec 4, 2023

View reviewed changes

Serock3 force-pushed the out-ip-delay branch 4 times, most recently from 2d3c581 to 2e3d784 Compare December 7, 2023 17:53

dlon reviewed Dec 8, 2023

View reviewed changes

Serock3 commented Dec 8, 2023

View reviewed changes

Serock3 force-pushed the out-ip-delay branch from 59b5e6e to 95bd4b8 Compare December 8, 2023 15:55

Serock3 force-pushed the out-ip-delay branch from 4578bc5 to 6af2d90 Compare December 13, 2023 09:19

Rawa requested review from Pururun and albin-mullvad December 14, 2023 14:53

Rawa force-pushed the out-ip-delay branch from e01260b to 92b6686 Compare December 15, 2023 09:41

Serock3 force-pushed the out-ip-delay branch 2 times, most recently from cec4058 to 4418907 Compare December 15, 2023 10:54

Serock3 marked this pull request as ready for review December 15, 2023 10:54

Serock3 self-assigned this Dec 15, 2023

Serock3 requested a review from Jontified December 15, 2023 10:55

Rawa force-pushed the out-ip-delay branch from 4418907 to 2d7c975 Compare December 15, 2023 11:36

Rawa requested changes Dec 15, 2023

View reviewed changes

Rawa force-pushed the out-ip-delay branch from 2d7c975 to e33c861 Compare December 18, 2023 10:49

Rawa reviewed Dec 18, 2023

View reviewed changes

Pururun requested changes Dec 18, 2023

View reviewed changes

Rawa force-pushed the out-ip-delay branch from e33c861 to c0a0d84 Compare December 18, 2023 11:03

Rawa reviewed Dec 18, 2023

View reviewed changes

Rawa force-pushed the out-ip-delay branch from c0a0d84 to 9749526 Compare December 18, 2023 11:09

Serock3 force-pushed the out-ip-delay branch from 9749526 to c2ac28b Compare December 18, 2023 11:13

Serock3 force-pushed the out-ip-delay branch 2 times, most recently from 0742ec3 to 2cafbad Compare December 18, 2023 13:04

Serock3 requested a review from MarkusPettersson98 December 18, 2023 13:08

Serock3 force-pushed the out-ip-delay branch 2 times, most recently from bd1b5cb to 9e9682f Compare December 18, 2023 13:20

Serock3 commented Dec 18, 2023

View reviewed changes

albin-mullvad reviewed Dec 18, 2023

View reviewed changes

Pururun approved these changes Dec 18, 2023

View reviewed changes

MarkusPettersson98 requested changes Dec 19, 2023

View reviewed changes

Rawa force-pushed the out-ip-delay branch from 9e9682f to 77903a0 Compare December 19, 2023 09:39

Jontified requested changes Dec 19, 2023

View reviewed changes

Rawa force-pushed the out-ip-delay branch from 77903a0 to a08a291 Compare December 19, 2023 13:49

Serock3 force-pushed the out-ip-delay branch from a08a291 to 2417049 Compare December 19, 2023 14:27

Serock3 commented Dec 19, 2023

View reviewed changes

Serock3 force-pushed the out-ip-delay branch from 2417049 to 72fb103 Compare December 19, 2023 14:47

MarkusPettersson98 approved these changes Dec 20, 2023

View reviewed changes

Serock3 commented Dec 20, 2023

View reviewed changes

Serock3 and others added 5 commits December 21, 2023 13:33

Change to exponential retry delay

98b7117

Remove GetCurrentLocation.

72a22cf

Make the daemon send two tunnel state updates, one with out IP being empty, and another with it being filled when am.i.mullvad.net responds. Update CLI for this change. Other front ends are left out.

Handle am.i.mullvad.net with internal daemon event

a9e75d4

Add `geoip::GeoIpHandler`, which sends an `InternalDaemonEvent::LocationEvent` when the location arrives. It also handles aborting in-flight requests and retries.

Remove location fetch in Electron app

c70509a

Support new tunnel state API in the Android frontend.

8ed53a0

Remove `get_current_location` from jni.

Jontified force-pushed the out-ip-delay branch from 72fb103 to 8ed53a0 Compare December 21, 2023 12:36

Jontified merged commit 9d71ecc into main Dec 21, 2023
41 of 42 checks passed

Jontified deleted the out-ip-delay branch December 21, 2023 12:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move exit location logic to daemon #5542

Move exit location logic to daemon #5542

Serock3 commented Dec 1, 2023 •

edited

Loading

dlon left a comment

Serock3 left a comment

Serock3 left a comment

dlon left a comment

Serock3 left a comment

linear bot commented Dec 12, 2023

Rawa left a comment

Rawa left a comment

Pururun left a comment

Rawa left a comment

Serock3 left a comment

albin-mullvad left a comment

Pururun left a comment

MarkusPettersson98 left a comment

Jontified left a comment

Serock3 left a comment

MarkusPettersson98 left a comment

Serock3 left a comment

Move exit location logic to daemon #5542

Move exit location logic to daemon #5542

Conversation

Serock3 commented Dec 1, 2023 • edited Loading

dlon left a comment

Choose a reason for hiding this comment

Serock3 left a comment

Choose a reason for hiding this comment

Serock3 left a comment

Choose a reason for hiding this comment

dlon left a comment

Choose a reason for hiding this comment

Serock3 left a comment

Choose a reason for hiding this comment

linear bot commented Dec 12, 2023

Rawa left a comment

Choose a reason for hiding this comment

Rawa left a comment

Choose a reason for hiding this comment

Pururun left a comment

Choose a reason for hiding this comment

Rawa left a comment

Choose a reason for hiding this comment

Serock3 left a comment

Choose a reason for hiding this comment

albin-mullvad left a comment

Choose a reason for hiding this comment

Pururun left a comment

Choose a reason for hiding this comment

MarkusPettersson98 left a comment

Choose a reason for hiding this comment

Jontified left a comment

Choose a reason for hiding this comment

Serock3 left a comment

Choose a reason for hiding this comment

MarkusPettersson98 left a comment

Choose a reason for hiding this comment

Serock3 left a comment

Choose a reason for hiding this comment

Serock3 commented Dec 1, 2023 •

edited

Loading