Feature/vpn 4192 fix domain fronting #3669

trojanfoe · 2025-10-10T14:58:47Z

Ticket

JIRA-VPN-4192

Description

Complete the integration of Domain Fronting into the VPN client.

Checklist:

Changelog

Screenshots (optional, if UI related)

This change is

….com/nymtech/nym-vpn-client into feature/vpn-4192-fix-domain-fronting

pronebird

Please rebase.

Reviewable status: 0 of 59 files reviewed, all discussions resolved

…ix-domain-fronting

pronebird

@pronebird reviewed 1 of 59 files at r1, 1 of 49 files at r2.
Reviewable status: 2 of 59 files reviewed, 1 unresolved discussion

nym-vpn-core/crates/nym-vpn-account-controller/src/command_sender.rs line 149 at r2 (raw file):

    pub async fn set_static_api_addresses(
        &self,
        static_api_addresses: Option<Vec<SocketAddr>>,

Probably has to be ResolverOverrides

pronebird

Reviewable status: 2 of 59 files reviewed, 3 unresolved discussions (waiting on @trojanfoe)

nym-vpn-core/crates/nym-vpn-api-client/src/client.rs line 163 at r2 (raw file):

        // TODO: from_network is buggy! it uses nym_api_urls instead of nym_vpn_api_urls!
        // nym_http_api_client::ClientBuilder::from_network(network)

Can be removed too I assume

nym-vpn-core/crates/nym-vpn-api-client/src/client.rs line 165 at r2 (raw file):

        // nym_http_api_client::ClientBuilder::from_network(network)
        let mut builder = nym_http_api_client::ClientBuilder::new_with_urls(nym_vpn_api_urls)
            // .map_err(|e| {

You can probably remove these comments

pronebird

Reviewable status: 2 of 59 files reviewed, 4 unresolved discussions (waiting on @trojanfoe)

nym-vpn-core/crates/nym-vpn-api-client/src/client.rs line 226 at r2 (raw file):

    }

    pub fn override_resolver_addresses(

Is it done for some compatibility? Because override_resolver should be enough I believe

pronebird

@pronebird reviewed 2 of 59 files at r1.
Reviewable status: 4 of 59 files reviewed, 5 unresolved discussions (waiting on @trojanfoe)

nym-vpn-core/crates/nym-vpn-account-controller/src/commands/dispatch.rs line 83 at r2 (raw file):

                    return_sender.send(Err(error))
                }
                CommonCommand::SetResolverOverrides(return_sender, _) => {

That's what SetStaticApiAddresses was for

pronebird

@pronebird reviewed 4 of 59 files at r1.
Reviewable status: 8 of 59 files reviewed, 5 unresolved discussions (waiting on @trojanfoe)

pronebird

@pronebird reviewed 1 of 49 files at r2.
Reviewable status: 9 of 59 files reviewed, 5 unresolved discussions (waiting on @trojanfoe)

trojanfoe

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

nym-vpn-core/crates/nym-vpn-api-client/src/client.rs line 226 at r2 (raw file):

Previously, pronebird (Andrej Mihajlov) wrote…

Is it done for some compatibility? Because override_resolver should be enough I believe

Yes. I expect the set_static_api_addresses stuff to go once we have transitioned to set_resolved_overrides.

…ix-domain-fronting

trojanfoe

Done.

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

trojanfoe

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

nym-vpn-core/crates/nym-vpn-api-client/src/client.rs line 163 at r2 (raw file):

Previously, pronebird (Andrej Mihajlov) wrote…

Can be removed too I assume

Done.

nym-vpn-core/crates/nym-vpn-api-client/src/client.rs line 165 at r2 (raw file):

Previously, pronebird (Andrej Mihajlov) wrote…

You can probably remove these comments

Done.

pronebird

@pronebird reviewed 3 of 22 files at r10, 4 of 8 files at r11, 1 of 3 files at r12, 1 of 5 files at r14.
Reviewable status: 77 of 96 files reviewed, 5 unresolved discussions (waiting on @doums, @mrkiwi, @neacsu, @rokas-ambrazevicius, and @trojanfoe)

nym-vpn-core/crates/nym-vpn-network-config/default/mainnet_discovery.json line 26 at r13 (raw file):

    {
      "url": "https://nymvpn.com/api/"
    },

Don't do this. Tests will fail because this won't match what https://nymvpn.com/api/public/v1/.wellknown/mainnet/discovery.json returns

trojanfoe

Reviewable status: 77 of 96 files reviewed, 5 unresolved discussions (waiting on @doums, @mrkiwi, @neacsu, @pronebird, and @rokas-ambrazevicius)

nym-vpn-core/Cargo.toml line 236 at r13 (raw file):

Previously, pronebird (Andrej Mihajlov) wrote…

Missing patch section

# For local development
# [patch."https://github.com/nymtech/nym"]

Done.

nym-vpn-core/crates/nym-vpn-api-client/src/client.rs line 148 at r13 (raw file):

Previously, pronebird (Andrej Mihajlov) wrote…

Resolved addresses should already be stored in Option<&ResolverOverrides>. The same addresses should be used for firewall rules and in here when provided to ensure that Dynamic DNS does not yield different different IPs.

OK I will need to think how this will work.

nym-vpn-core/crates/nym-vpn-api-client/src/fronted_http_client.rs line 12 at r13 (raw file):

Previously, pronebird (Andrej Mihajlov) wrote…

You probably want to pass resolved addresses here and not perform resolution on spot.

OK I need to think about this will work.

nym-vpn-core/crates/nym-vpn-api-client/src/fronted_http_client.rs line 50 at r13 (raw file):

Previously, pronebird (Andrej Mihajlov) wrote…

Consider using simple iterator, no need for indirection via i

I need to get the domain from urls_and_domains and the fronts from api_urls, so I need an index. As per the comments, this is because the Url type doesn't reveal the front_hosts and I have raised a Jira to change that. Once that's done we can simplify this.

nym-vpn-core/crates/nym-vpn-network-config/default/mainnet_discovery.json line 26 at r13 (raw file):

Previously, pronebird (Andrej Mihajlov) wrote…

Don't do this. Tests will fail because this won't match what https://nymvpn.com/api/public/v1/.wellknown/mainnet/discovery.json returns

This was front Tommy. I'll revert.

pronebird

@pronebird reviewed 1 of 22 files at r10, 1 of 8 files at r11.
Reviewable status: 79 of 96 files reviewed, 6 unresolved discussions (waiting on @doums, @mrkiwi, @neacsu, and @rokas-ambrazevicius)

nym-vpn-core/crates/nym-vpn-lib-uniffi/src/gateway_cache.rs line 73 at r13 (raw file):

        Err(e) => {
            tracing::error!("Failed to create gateway directory config: {e:#?}");
            panic!("Inconsistent network environment");

Wow that's some ugly panic

trojanfoe

Reviewable status: 79 of 96 files reviewed, 6 unresolved discussions (waiting on @doums, @mrkiwi, @neacsu, @pronebird, and @rokas-ambrazevicius)

nym-vpn-core/crates/nym-vpn-lib-uniffi/src/gateway_cache.rs line 73 at r13 (raw file):

Previously, pronebird (Andrej Mihajlov) wrote…

Wow that's some ugly panic

OK I'll return a Result.

trojanfoe

Reviewable status: 78 of 96 files reviewed, 6 unresolved discussions (waiting on @doums, @mrkiwi, @neacsu, @pronebird, and @rokas-ambrazevicius)

nym-vpn-core/crates/nym-vpn-lib-uniffi/src/gateway_cache.rs line 73 at r13 (raw file):

Previously, trojanfoe (Andy Duplain) wrote…

OK I'll return a Result.

Done.

trojanfoe

Reviewable status: 78 of 96 files reviewed, 6 unresolved discussions (waiting on @doums, @mrkiwi, @neacsu, @pronebird, and @rokas-ambrazevicius)

nym-vpn-core/crates/nym-vpn-network-config/default/mainnet_discovery.json line 26 at r13 (raw file):

Previously, trojanfoe (Andy Duplain) wrote…

This was front Tommy. I'll revert.

Which tests are these? I run the tests regularly and nothing has failed in this area.

pronebird

@pronebird reviewed 1 of 22 files at r10, 1 of 8 files at r11.
Reviewable status: 79 of 96 files reviewed, 5 unresolved discussions (waiting on @doums, @mrkiwi, @neacsu, @rokas-ambrazevicius, and @trojanfoe)

nym-vpn-core/crates/nym-vpn-network-config/default/mainnet_discovery.json line 26 at r13 (raw file):

Previously, trojanfoe (Andy Duplain) wrote…

Which tests are these? I run the tests regularly and nothing has failed in this area.

Look for test_mainnet_discovery_same_as_fetched()

pronebird

@pronebird reviewed 1 of 1 files at r16.
Reviewable status: 80 of 96 files reviewed, 3 unresolved discussions (waiting on @doums, @mrkiwi, @neacsu, @rokas-ambrazevicius, and @trojanfoe)

pronebird

@pronebird reviewed 3 of 23 files at r8, 1 of 22 files at r10, 1 of 4 files at r13.
Reviewable status: 85 of 96 files reviewed, 2 unresolved discussions (waiting on @doums, @mrkiwi, @neacsu, and @rokas-ambrazevicius)

pronebird

@pronebird reviewed 1 of 5 files at r14.
Reviewable status: 86 of 96 files reviewed, 2 unresolved discussions (waiting on @doums, @mrkiwi, @neacsu, and @rokas-ambrazevicius)

pronebird

@pronebird reviewed 1 of 23 files at r8, 1 of 22 files at r10, 1 of 8 files at r11.
Reviewable status: 89 of 96 files reviewed, 2 unresolved discussions (waiting on @doums, @mrkiwi, @neacsu, and @rokas-ambrazevicius)

nym-vpn-core/crates/nym-vpn-proto/proto/nym_vpn_service.proto line 168 at r2 (raw file):

message NymVpnNetworkDetails {
  string nym_vpn_api_url = 1;

Very nice!

pronebird

@pronebird reviewed 2 of 22 files at r10.
Reviewable status: 91 of 96 files reviewed, 2 unresolved discussions (waiting on @doums, @mrkiwi, @neacsu, and @rokas-ambrazevicius)

pronebird

@pronebird reviewed 1 of 23 files at r8, 1 of 22 files at r10.
Reviewable status: 92 of 96 files reviewed, 2 unresolved discussions (waiting on @doums, @mrkiwi, @neacsu, and @rokas-ambrazevicius)

pronebird

@pronebird reviewed 1 of 1 files at r15.
Reviewable status: 93 of 96 files reviewed, 2 unresolved discussions (waiting on @doums, @mrkiwi, @neacsu, and @rokas-ambrazevicius)

pronebird

@pronebird reviewed 1 of 5 files at r14.
Reviewable status: 94 of 96 files reviewed, 2 unresolved discussions (waiting on @doums, @mrkiwi, @neacsu, and @rokas-ambrazevicius)

!!!WIP!!!

…ix-domain-fronting

pronebird

@pronebird reviewed 3 of 13 files at r17.
Reviewable status: 88 of 98 files reviewed, 1 unresolved discussion (waiting on @doums, @mrkiwi, @neacsu, and @rokas-ambrazevicius)

nym-vpn-core/crates/nym-vpn-network-config/src/discovery.rs line 104 at r17 (raw file):

        let urls = api_urls_to_urls(api_urls).map_err(Error::CreateVpnApiClient)?;

        let resolver_overrides = ResolverOverrides::from_urls(&urls)

If I understand it correctly, ResolverOverrides performs in-place DNS resolution. This is likely to fail at some point because of missing coordination with state machine/firewall.

pronebird

Reviewable status: 88 of 98 files reviewed, 2 unresolved discussions (waiting on @doums, @mrkiwi, @neacsu, @rokas-ambrazevicius, and @trojanfoe)

nym-vpn-core/crates/nym-vpn-network-config/src/discovery.rs line 363 at r17 (raw file):

    tracing::debug!("Fetching nym vpn network details");
    let urls = api_urls_to_urls(nym_vpn_api_urls).map_err(Error::CreateVpnApiClient)?;
    let resolver_overrides = ResolverOverrides::from_urls(&urls)

Same here. Pointless to run DNS resolver when there is no coordination with the state machine/firewall.

pronebird

Reviewable status: 88 of 98 files reviewed, 3 unresolved discussions (waiting on @doums, @mrkiwi, @neacsu, @rokas-ambrazevicius, and @trojanfoe)

nym-vpn-core/crates/nym-vpn-network-config/src/envs.rs line 71 at r17 (raw file):

        let urls = api_urls_to_urls(api_urls).map_err(Error::CreateVpnApiClient)?;

        let resolver_overrides = ResolverOverrides::from_urls(&urls)

Same here too. Absent coordination with state machine/firewall. Let's talk about that on Monday and see what we can do in the short and long term to remedy this.

pronebird

@pronebird reviewed 2 of 13 files at r17.
Reviewable status: 90 of 98 files reviewed, 4 unresolved discussions (waiting on @doums, @mrkiwi, @neacsu, @rokas-ambrazevicius, and @trojanfoe)

nym-vpn-core/crates/nym-vpn-account-controller/tests/integrations/common/mod.rs line 225 at r17 (raw file):

        let urls = api_urls_to_urls(&[api_url])?;

        let resolver_overrides = ResolverOverrides::from_urls(&urls).await?;

Is it needed in the test bench for account controller?

pronebird

@pronebird reviewed 4 of 13 files at r17.
Reviewable status: 94 of 98 files reviewed, 5 unresolved discussions (waiting on @doums, @mrkiwi, @neacsu, @rokas-ambrazevicius, and @trojanfoe)

nym-vpn-core/crates/nym-vpn-api-client/src/resolve_host.rs line 117 at r17 (raw file):

        str_to_socket_addr(domain, limit).await
    } else {
        str_to_socket_addr(&format!("https://{domain}"), None).await

limit is not passed

pronebird

@pronebird reviewed 4 of 13 files at r17, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @doums, @mrkiwi, @neacsu, @rokas-ambrazevicius, and @trojanfoe)

…ique.

trojanfoe added 5 commits October 10, 2025 12:10

Set resolver overrides using the new ResolverOverrides type.

68701ae

Set resolver overrides using the new ResolverOverrides type.

5cdf6d7

Merge branch 'feature/vpn-4192-fix-domain-fronting' of https://github…

95deef6

….com/nymtech/nym-vpn-client into feature/vpn-4192-fix-domain-fronting

The old behaviour of from_network was use_nym_api_urls=false.

a2a32b1

Update firewall when we receive a new discovery.

43b2ade

trojanfoe requested a review from pronebird October 10, 2025 14:58

trojanfoe added 8 commits October 10, 2025 16:07

Remove unnecessary variable renames.

a4f5e76

Set resolver overrides using the new ResolverOverrides type.

59920ab

The old behaviour of from_network was use_nym_api_urls=false.

7e5b7ba

Update firewall when we receive a new discovery.

42d541a

Remove unnecessary variable renames.

90ab2a4

Support CommonCommand to set ResolverOverrides.

f4f2629

Merge branch 'feature/vpn-4192-fix-domain-fronting' of https://github…

c2c693d

….com/nymtech/nym-vpn-client into feature/vpn-4192-fix-domain-fronting

You cannot set the firewall on iOS/Android.

41578f6

pronebird reviewed Oct 13, 2025

View reviewed changes

trojanfoe added 4 commits October 13, 2025 10:31

Try again with iOS/Android fix.

8406b46

Minor change.

394fcc4

Merge remote-tracking branch 'origin/develop' into feature/vpn-4192-f…

679758a

…ix-domain-fronting

Remove some duplication.

5ca75ac

pronebird reviewed Oct 13, 2025

View reviewed changes

trojanfoe commented Oct 13, 2025

View reviewed changes

trojanfoe added 2 commits October 13, 2025 14:53

Use url_to_socket_addrs to resolve addresses.

7686ec9

Merge remote-tracking branch 'origin/develop' into feature/vpn-4192-f…

f25835b

…ix-domain-fronting

trojanfoe commented Oct 13, 2025

View reviewed changes

pronebird requested changes Oct 17, 2025

View reviewed changes

trojanfoe commented Oct 17, 2025

View reviewed changes

pronebird requested changes Oct 17, 2025

View reviewed changes

trojanfoe commented Oct 17, 2025

View reviewed changes

github-actions bot requested a review from pronebird October 17, 2025 12:14

make_gateway_config() now returns Result.

98613fd

trojanfoe commented Oct 17, 2025

View reviewed changes

pronebird requested changes Oct 17, 2025

View reviewed changes

github-actions bot requested a review from pronebird October 17, 2025 13:41

pronebird reviewed Oct 17, 2025

View reviewed changes

Move to using Urls to create HTTP clients.

352329a

!!!WIP!!!

github-actions bot requested a review from pronebird October 17, 2025 16:01

trojanfoe added 3 commits October 18, 2025 05:51

Merge remote-tracking branch 'origin/develop' into feature/vpn-4192-f…

8529121

…ix-domain-fronting

Complete move to using Urls to create HTTP clients.

9154c83

More work around gateway client.

f126b79

pronebird requested changes Oct 18, 2025

View reviewed changes

pronebird reviewed Oct 18, 2025

View reviewed changes

Use a HashSet to ensure all addresses in ResolverOverrides are un…

908e4d2

…ique.

Feature/vpn 4192 fix domain fronting #3669

Are you sure you want to change the base?

Feature/vpn 4192 fix domain fronting #3669

Conversation

trojanfoe commented Oct 10, 2025 • edited by mmsinclair Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ticket

Description

Checklist:

Screenshots (optional, if UI related)

Uh oh!

pronebird left a comment

Choose a reason for hiding this comment

Uh oh!

pronebird left a comment

Choose a reason for hiding this comment

Uh oh!

pronebird left a comment

Choose a reason for hiding this comment

Uh oh!

pronebird left a comment

Choose a reason for hiding this comment

Uh oh!

pronebird left a comment

Choose a reason for hiding this comment

Uh oh!

pronebird left a comment

Choose a reason for hiding this comment

Uh oh!

pronebird left a comment

Choose a reason for hiding this comment

Uh oh!

trojanfoe left a comment

Choose a reason for hiding this comment

Uh oh!

trojanfoe left a comment

Choose a reason for hiding this comment

Uh oh!

trojanfoe left a comment

Choose a reason for hiding this comment

Uh oh!

pronebird left a comment

Choose a reason for hiding this comment

Uh oh!

trojanfoe left a comment

Choose a reason for hiding this comment

Uh oh!

pronebird left a comment

Choose a reason for hiding this comment

Uh oh!

trojanfoe left a comment

Choose a reason for hiding this comment

Uh oh!

trojanfoe left a comment

Choose a reason for hiding this comment

Uh oh!

trojanfoe left a comment

Choose a reason for hiding this comment

Uh oh!

pronebird left a comment

Choose a reason for hiding this comment

Uh oh!

pronebird left a comment

Choose a reason for hiding this comment

Uh oh!

pronebird left a comment

Choose a reason for hiding this comment

Uh oh!

pronebird left a comment

Choose a reason for hiding this comment

Uh oh!

pronebird left a comment

Choose a reason for hiding this comment

Uh oh!

pronebird left a comment

Choose a reason for hiding this comment

Uh oh!

pronebird left a comment

Choose a reason for hiding this comment

Uh oh!

trojanfoe commented Oct 10, 2025 •

edited by mmsinclair

Loading