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

Socket: Fix some non-blocking connect edge cases #10700

Merged
merged 4 commits into from Jun 28, 2022

Conversation

sepalani
Copy link
Contributor

Some Capcom games (namely, Monster Hunter 3 and Monster Hunter G) use non-blocking connect then directly call some network/ssl functions. Unfortunately, it seems that mbedtls doesn't support SSL handshake when the connection is in progress (i.e. not fully completed yet). The same happens with network calls returning ENOTCONN instead of EAGAIN. In sum, the games will abort the connection if Dolphin isn't quick enough to connect before the network function is called. Obviously, it doesn't happen if you host locally the servers that the games are connecting to. However, it will happen if they are on the Internet and the network is too slow.

This PR refactors parts of the network code and fix some error codes. I'm unaware of other games affected by this issues. I wrote 2 hardware tests, one for the SSL handshake (wii-ssl-connect.zip), the other one for the send/recv (wii-net-connect.zip).

HWTEST - SSL Connect (naswii.nintendowifi.net:443)

Test case Block Wii value Dolphin value PR value
Invalid connect context (-1) N/A -8 -8 -8
Invalid handshake context (-1) N/A -8 -8 -8
Invalid connect context (3) N/A 0 -8 -8
Invalid socket (-1) N/A 0 0 0
Invalid socket (12) N/A 0 0 0
Disconnected connect true 0 0 0
Disconnected handshake true -5 -1 -5
Disconnected connect false 0 0 0
Disconnected handshake false -5 -1 -5
SSL connect true 0 0 0
SSL connect false 0 0 0
Socket flags after SSL connect true 0 0 0
Socket flags after SSL connect false 4 4 4
SSL handshake true -1 -1 -1
SSL handshake false -1 -1 -1
SSL handshake (CLI) true 0 0 0
SSL handshake (CLI) false 0 -1 0
SSL handshake (CA) true -1 -1 -1
SSL handshake (CA) false -1 -1 -1
SSL handshake (CLI+CA) true 0 0 0
SSL handshake (CLI+CA) false 0 -1 0

HWTEST - Non-blocking NET Connect (8.8.8.8:53)

Test case Type Connect value Wii value Dolphin value PR value
Connect/Recv TCP -119 -11 -128 -11
Connect/Recv UDP 0 -11 -11 -11
Connect/Send TCP -119 5 -128 -11
Connect/Send UDP 0 5 5 5
Connect/RecvFrom TCP -119 -11 -128 -11
Connect/RecvFrom UDP 0 -11 -11 -11
Connect/SendTo TCP -119 5 -128 -11
Connect/SendTo UDP 0 5 5 5

Ready to be reviewed & tested.

@AdmiralCurtiss
Copy link
Contributor

Please rebase this so the Android build passes.

@JMC47
Copy link
Contributor

JMC47 commented May 31, 2022

Is there a Monster Hunter Tri server I can connect to in order to test this?

@sepalani
Copy link
Contributor Author

sepalani commented Jun 2, 2022

@JMC47
I have a test server on 162.19.75.89 running MH3SP's master_server.py and dns_server.py, altwfc's nas_server.py.

You will need this Riivolution patch to allow the game to connect to custom servers. You should also try this with a hunter profile which doesn't have online data (or you should backup your save) as it might alter some of its online properties.

On real hardware, you will only need to setup the Internet connection with the server IP as primary DNS and launch the game with the Riivolution patch. On Dolphin you will need to add the following lines in your hosts file and launch the game with Riivolution:

162.19.75.89 mh.capcom.co.jp
162.19.75.89 mmh-t1-opn01.mmh-service.capcom.co.jp
162.19.75.89 mmh-t1-opn02.mmh-service.capcom.co.jp
162.19.75.89 mmh-t1-opn03.mmh-service.capcom.co.jp
162.19.75.89 mmh-t1-opn04.mmh-service.capcom.co.jp
162.19.75.89 naswii.nintendowifi.net

You should also delete said lines when done testing to remove the redirections.

The test server should allow you to pass the login process on real hardware but will fail on Dolphin (without this PR) with the error code 11602. BTW, my test server might not allow you to connect to online lobbies and might display python error messages. Regardless, it should be good enough to illustrate this connection issue related to the login process.

@Leseratte10
Copy link
Contributor

For the HWTEST, you have Connect/Send and Connect/SendTo Wii value 5, Dolphin value -128, and PR value -11. Is that just a copy-paste error in that table, or is there a reason you're making Dolphin return -11 even though the Wii returns 5?

@sepalani
Copy link
Contributor Author

sepalani commented Jun 5, 2022

For the HWTEST, you have Connect/Send and Connect/SendTo Wii value 5, Dolphin value -128, and PR value -11. Is that just a copy-paste error in that table, or is there a reason you're making Dolphin return -11 even though the Wii returns 5?

It's not a copy-paste error. The Wii takes enough time to connect and send the packet successfully. However, Dolphin is too fast, so instead of returning ENOTCONN, it returns EAGAIN.

Though, it does fix the issue because Send/SendTo are non-blocking here. I don't think delaying the IPC reply of non-blocking syscalls to match the hwtest is a good idea since they are supposed to be non-blocking. Moreover, adding yet another manual timeout to our current socket emulation doesn't sound good either. I also really don't know how I can measure accurately such timing from software.

I assume this difference might be caused by a timing issue which might not be entirely network related.

@sepalani
Copy link
Contributor Author

sepalani commented Jun 7, 2022

@iwubcode
Done.

@iwubcode
Copy link
Contributor

iwubcode commented Jun 7, 2022

@sepalani - thanks so much. One minor comment, otherwise LGTM.

Copy link
Contributor

@iwubcode iwubcode left a comment

Choose a reason for hiding this comment

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

Untested. LGTM

@JMC47
Copy link
Contributor

JMC47 commented Jun 8, 2022

I don't know exactly how much functionality is on the servers, but I'd be willing to merge this. I haven't had a chance to test it yet, but I'll try to get to it tonight.

@sepalani
Copy link
Contributor Author

I was wondering why during the wii-network branch development, the issue wasn't detected when the original servers for MH3 were still up.

Then I realised that it wasn't using mbedTLS (known as PolarSSL back then) but GnuTLS/OpenSSL instead and was supporting non-blocking handshakes. The western MH3 servers closed on April 2013 and around August 2013 Dolphin changed its SSL library with the one that we're currently using today.

I think that's why this issue went unnoticed for so long, especially since the few games having it weren't playable online anymore.

@JMC47
Copy link
Contributor

JMC47 commented Jun 28, 2022

Ignore previous comment, I did not follow instructions properly because I didn't read through them thoroughly and missed a step. I got pretty far in before encountering an error - much further than master will get.

image

Is this as much as I'm supposed to see?

Edit: Looks like it based on your previous post. I never got a chance to play this on the actual servers, so I wasn't sure how many steps were are, but this appears to be the part where you said it might not load. So it LGTM in terms of what is expected.

@JMC47 JMC47 merged commit e50e45f into dolphin-emu:master Jun 28, 2022
@sepalani sepalani deleted the ssl-handshake branch June 28, 2022 04:59
@sepalani
Copy link
Contributor Author

@JMC47
Yeah, that's a known issue of the custom server implementation. However, as mentioned, you get far enough to illustrate the connection issue which differs from real hardware.

If you want to go further, you should be able to circumvent this error message by choosing an empty city gate. The recommended way is to self host the custom server, atm. My test server being for testing purpose, it's not the latest stable version. Multiplayer support isn't (officially) supported by the custom server yet. I wouldn't recommend playing it unless you're really fond of the game as it's more difficult, especially while doing it solo.

By the way, don't forget to remove these entries from your hosts file, otherwise, you'll have issues to play online with other games.

dvessel pushed a commit to dvessel/dolphin that referenced this pull request Jun 28, 2022
Socket: Fix some non-blocking connect edge cases
InusualZ added a commit to sepalani/MH3SP that referenced this pull request Aug 16, 2022
sepalani pushed a commit to sepalani/MH3SP that referenced this pull request Sep 2, 2022
kbarkevich pushed a commit to kbarkevich/MH3SP that referenced this pull request Sep 10, 2022
sepalani pushed a commit to sepalani/MH3SP that referenced this pull request Oct 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants