Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an end to end test for UDP-over-TCP on port 80 #7668

Merged

Conversation

buggmagnet
Copy link
Contributor

@buggmagnet buggmagnet commented Feb 13, 2025

This PR adds an end to end test that connects to a relay using UDP-over-TCP obfuscation on port80.
The test also verifies using packet capture that the traffic sent to the relay during its course is happening exclusively on port 80 using a TCP connection.

The PR also fixes an innocent mistake in DetailsView.swift and makes the chevron icon accessibility tap action actually expand the connection details.


This change is Reviewable

@buggmagnet buggmagnet added the iOS Issues related to iOS label Feb 13, 2025
@buggmagnet buggmagnet self-assigned this Feb 13, 2025
Copy link

linear bot commented Feb 13, 2025

Copy link
Contributor

@SteffenErn SteffenErn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion


ios/MullvadVPNUITests/RelayTests.swift line 199 at r1 (raw file):

        let streamFromPeeerToRelay = try XCTUnwrap(
            capturedStreams
                .filter { $0.destinationAddress == connectedToIPAddress }.first

Is it enough to check that one stream on port 80 exists or does it make sense to verify that no other stream on a different port exists?

Copy link
Contributor Author

@buggmagnet buggmagnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


ios/MullvadVPNUITests/RelayTests.swift line 199 at r1 (raw file):

Previously, SteffenErn (Steffen Ernst) wrote…

Is it enough to check that one stream on port 80 exists or does it make sense to verify that no other stream on a different port exists?

The way the packet capture API was designed is not immediately obvious.
There will be 1 Stream object exactly, that we capture here by filtering by connectedToIPAddress

As the router that does the capture is used by multiple teams, we only want to bother about the relay our device has connected to.
In a nutshell, this is only stream we care about here.

@buggmagnet buggmagnet force-pushed the add-end-to-end-test-for-udp-to-tcp-port-selection-ios-942 branch from 7268479 to 1dada3e Compare February 18, 2025 10:04
Copy link
Contributor

@SteffenErn SteffenErn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

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


ios/MullvadVPNUITests/RelayTests.swift line 199 at r1 (raw file):

Previously, buggmagnet wrote…

The way the packet capture API was designed is not immediately obvious.
There will be 1 Stream object exactly, that we capture here by filtering by connectedToIPAddress

As the router that does the capture is used by multiple teams, we only want to bother about the relay our device has connected to.
In a nutshell, this is only stream we care about here.

Ok, thanks for the clarification :)

@buggmagnet buggmagnet force-pushed the add-end-to-end-test-for-udp-to-tcp-port-selection-ios-942 branch from 1dada3e to 58157b9 Compare February 18, 2025 10:23
@buggmagnet buggmagnet merged commit ff1427d into main Feb 18, 2025
10 of 11 checks passed
@buggmagnet buggmagnet deleted the add-end-to-end-test-for-udp-to-tcp-port-selection-ios-942 branch February 18, 2025 10:26
Copy link

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

Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iOS Issues related to iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants