Skip to content

Commit a37b78e

Browse files
ReenigneArcherReenigneArcher
authored
fix(network): pairing timeout (#197)
1 parent c024b2d commit a37b78e

3 files changed

Lines changed: 70 additions & 17 deletions

File tree

‎src/network/host_pairing.cpp‎

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ namespace {
116116
constexpr std::size_t CLIENT_CHALLENGE_BYTE_COUNT = 16;
117117
constexpr std::size_t CLIENT_SECRET_BYTE_COUNT = 16;
118118
constexpr int SOCKET_TIMEOUT_MILLISECONDS = 5000;
119+
constexpr int PIN_ENTRY_SOCKET_TIMEOUT_MILLISECONDS = 90000;
119120
constexpr uint16_t DEFAULT_SERVERINFO_HTTP_PORT = 47989;
120121
constexpr uint16_t FALLBACK_SERVERINFO_HTTP_PORT = 47984;
121122
constexpr uint16_t DEFAULT_SERVERINFO_HTTPS_PORT = 47990;
@@ -217,7 +218,7 @@ namespace {
217218

218219
bool is_timeout_error(int errorCode) {
219220
#if defined(NXDK) || !defined(_WIN32)
220-
return errorCode == ETIMEDOUT;
221+
return errorCode == ETIMEDOUT || errorCode == EWOULDBLOCK || errorCode == EAGAIN;
221222
#else
222223
return errorCode == WSAETIMEDOUT;
223224
#endif
@@ -249,18 +250,18 @@ namespace {
249250
return true;
250251
}
251252

252-
void set_socket_timeouts(SOCKET socketHandle) {
253+
void set_socket_timeouts(SOCKET socketHandle, int timeoutMilliseconds) {
253254
#if defined(NXDK) || !defined(_WIN32)
254255
timeval timeout {
255-
SOCKET_TIMEOUT_MILLISECONDS / 1000,
256-
(SOCKET_TIMEOUT_MILLISECONDS % 1000) * 1000,
256+
timeoutMilliseconds / 1000,
257+
(timeoutMilliseconds % 1000) * 1000,
257258
};
258259
setsockopt(socketHandle, SOL_SOCKET, SO_RCVTIMEO, reinterpret_cast<const char *>(&timeout), sizeof(timeout));
259260
setsockopt(socketHandle, SOL_SOCKET, SO_SNDTIMEO, reinterpret_cast<const char *>(&timeout), sizeof(timeout));
260261
#else
261-
const DWORD timeoutMilliseconds = SOCKET_TIMEOUT_MILLISECONDS;
262-
setsockopt(socketHandle, SOL_SOCKET, SO_RCVTIMEO, reinterpret_cast<const char *>(&timeoutMilliseconds), sizeof(timeoutMilliseconds));
263-
setsockopt(socketHandle, SOL_SOCKET, SO_SNDTIMEO, reinterpret_cast<const char *>(&timeoutMilliseconds), sizeof(timeoutMilliseconds));
262+
const DWORD platformTimeoutMilliseconds = static_cast<DWORD>(timeoutMilliseconds);
263+
setsockopt(socketHandle, SOL_SOCKET, SO_RCVTIMEO, reinterpret_cast<const char *>(&platformTimeoutMilliseconds), sizeof(platformTimeoutMilliseconds));
264+
setsockopt(socketHandle, SOL_SOCKET, SO_SNDTIMEO, reinterpret_cast<const char *>(&platformTimeoutMilliseconds), sizeof(platformTimeoutMilliseconds));
264265
#endif
265266
}
266267

@@ -351,7 +352,8 @@ namespace {
351352
std::string_view expectedTlsCertificatePem,
352353
HttpResponse *response,
353354
std::string *errorMessage,
354-
const std::atomic<bool> *cancelRequested = nullptr
355+
const std::atomic<bool> *cancelRequested = nullptr,
356+
int socketIoTimeoutMilliseconds = SOCKET_TIMEOUT_MILLISECONDS
355357
);
356358

357359
std::string summarize_http_payload_preview(std::string_view text) {
@@ -1496,18 +1498,25 @@ namespace {
14961498
return true;
14971499
}
14981500

1499-
bool finalize_connected_socket(SOCKET socketHandle, std::string *errorMessage) {
1501+
bool finalize_connected_socket(SOCKET socketHandle, int socketIoTimeoutMilliseconds, std::string *errorMessage) {
15001502
trace_pairing_phase("restoring blocking mode after connect");
15011503
if (!set_socket_non_blocking(socketHandle, false, errorMessage)) {
15021504
return false;
15031505
}
15041506

1505-
set_socket_timeouts(socketHandle);
1507+
set_socket_timeouts(socketHandle, socketIoTimeoutMilliseconds);
15061508
trace_pairing_phase("socket connected");
15071509
return true;
15081510
}
15091511

1510-
bool connect_socket(const std::string &address, uint16_t port, SocketGuard *socketGuard, std::string *errorMessage, const std::atomic<bool> *cancelRequested = nullptr) {
1512+
bool connect_socket(
1513+
const std::string &address,
1514+
uint16_t port,
1515+
SocketGuard *socketGuard,
1516+
int socketIoTimeoutMilliseconds,
1517+
std::string *errorMessage,
1518+
const std::atomic<bool> *cancelRequested = nullptr
1519+
) {
15111520
if (socketGuard == nullptr) {
15121521
return append_error(errorMessage, "Internal pairing error while preparing the host connection");
15131522
}
@@ -1547,7 +1556,7 @@ namespace {
15471556
}
15481557
}
15491558

1550-
return finalize_connected_socket(socketGuard->handle, errorMessage);
1559+
return finalize_connected_socket(socketGuard->handle, socketIoTimeoutMilliseconds, errorMessage);
15511560
}
15521561

15531562
bool recv_all_plain(SOCKET socketHandle, std::string *response, std::string *errorMessage, const std::atomic<bool> *cancelRequested = nullptr) {
@@ -1855,7 +1864,8 @@ namespace {
18551864
std::string_view expectedTlsCertificatePem,
18561865
HttpResponse *response,
18571866
std::string *errorMessage,
1858-
const std::atomic<bool> *cancelRequested
1867+
const std::atomic<bool> *cancelRequested,
1868+
int socketIoTimeoutMilliseconds
18591869
) {
18601870
if (pairing_cancel_requested(cancelRequested)) {
18611871
return append_cancelled_pairing_error(errorMessage);
@@ -1869,6 +1879,7 @@ namespace {
18691879
useTls,
18701880
tlsClientIdentity,
18711881
std::string(expectedTlsCertificatePem),
1882+
socketIoTimeoutMilliseconds,
18721883
};
18731884
network::testing::HostPairingHttpTestResponse testResponse {};
18741885
if (std::string testError; !testHandler(testRequest, &testResponse, &testError, cancelRequested)) {
@@ -1891,7 +1902,7 @@ namespace {
18911902

18921903
SocketGuard socketGuard;
18931904
trace_pairing_phase("http_get: connect_socket");
1894-
if (!connect_socket(address, port, &socketGuard, errorMessage, cancelRequested)) {
1905+
if (!connect_socket(address, port, &socketGuard, socketIoTimeoutMilliseconds, errorMessage, cancelRequested)) {
18951906
return false;
18961907
}
18971908

@@ -2149,12 +2160,18 @@ namespace {
21492160
return true;
21502161
}
21512162

2152-
bool execute_pairing_phase_request(PairingSessionState *session, const std::string &path, bool useTls, std::string_view expectedTlsCertificatePem = {}) {
2163+
bool execute_pairing_phase_request(
2164+
PairingSessionState *session,
2165+
const std::string &path,
2166+
bool useTls,
2167+
std::string_view expectedTlsCertificatePem = {},
2168+
int socketIoTimeoutMilliseconds = SOCKET_TIMEOUT_MILLISECONDS
2169+
) {
21532170
if (session == nullptr) {
21542171
return false;
21552172
}
21562173

2157-
if (!http_get(session->request.address, useTls ? session->serverInfo.httpsPort : session->serverInfo.httpPort, path, useTls, useTls ? &session->request.identity : nullptr, expectedTlsCertificatePem, &session->response, &session->errorMessage, session->cancelRequested)) {
2174+
if (!http_get(session->request.address, useTls ? session->serverInfo.httpsPort : session->serverInfo.httpPort, path, useTls, useTls ? &session->request.identity : nullptr, expectedTlsCertificatePem, &session->response, &session->errorMessage, session->cancelRequested, socketIoTimeoutMilliseconds)) {
21582175
return false;
21592176
}
21602177
return parse_pairing_tag(session->response, "paired", &session->phaseValue, &session->errorMessage);
@@ -2187,7 +2204,7 @@ namespace {
21872204

21882205
const std::string phasePath = "/pair?uniqueid=" + session->uniqueId + "&uuid=" + session->requestUuid + "&devicename=" + session->deviceName + "&updateState=1&phrase=getservercert&salt=" + session->saltHex + "&clientcert=" + certHex;
21892206
trace_pairing_phase("phase 1 getservercert request");
2190-
if (!execute_pairing_phase_request(session, phasePath, false)) {
2207+
if (!execute_pairing_phase_request(session, phasePath, false, {}, PIN_ENTRY_SOCKET_TIMEOUT_MILLISECONDS)) {
21912208
return false;
21922209
}
21932210
if (session->phaseValue != "1") {

‎src/network/host_pairing.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,7 @@ namespace network {
277277
bool useTls = false; ///< True when the request would normally use TLS.
278278
const PairingIdentity *tlsClientIdentity = nullptr; ///< Optional client identity attached to TLS requests.
279279
std::string expectedTlsCertificatePem; ///< Optional pinned host certificate expected by the request.
280+
int socketIoTimeoutMilliseconds = 0; ///< Socket read/write timeout that the real transport would apply.
280281
};
281282

282283
/**

‎tests/unit/network/host_pairing_test.cpp‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ namespace {
3333
using network::testing::HostPairingHttpTestRequest;
3434
using network::testing::HostPairingHttpTestResponse;
3535

36+
constexpr int kDefaultPairingSocketTimeoutMilliseconds = 5000;
37+
constexpr int kPinEntrySocketTimeoutMilliseconds = 90000;
3638
constexpr std::string_view kUnpairedClientErrorMessage = "The host reports that this client is no longer paired. Pair the host again.";
3739

3840
class ScopedHostPairingHttpTestHandler {
@@ -1309,6 +1311,39 @@ namespace {
13091311
EXPECT_EQ(result.message, "Pairing failed during phase 1 (getservercert): The host rejected the initial pairing request");
13101312
}
13111313

1314+
TEST(HostPairingTest, PairHostUsesExtendedTimeoutForPinEntryResponse) {
1315+
const network::PairingIdentity identity = network::create_pairing_identity();
1316+
ASSERT_TRUE(network::is_valid_pairing_identity(identity));
1317+
1318+
std::size_t callCount = 0U;
1319+
ScopedHostPairingHttpTestHandler guard([&callCount](const HostPairingHttpTestRequest &request, HostPairingHttpTestResponse *response, std::string *, const std::atomic<bool> *) {
1320+
if (callCount++ == 0U) {
1321+
EXPECT_EQ(request.socketIoTimeoutMilliseconds, kDefaultPairingSocketTimeoutMilliseconds);
1322+
response->statusCode = 200;
1323+
response->body = make_server_info_xml(false, 47989U, 47990U, "Pair Host", "pair-host");
1324+
return true;
1325+
}
1326+
1327+
EXPECT_NE(request.pathAndQuery.find("phrase=getservercert"), std::string::npos);
1328+
EXPECT_EQ(request.socketIoTimeoutMilliseconds, kPinEntrySocketTimeoutMilliseconds);
1329+
response->statusCode = 200;
1330+
response->body = make_pair_phase_response("0");
1331+
return true;
1332+
});
1333+
1334+
const network::HostPairingResult result = network::pair_host({
1335+
test_support::kTestIpv4Addresses[test_support::kIpLivingRoom],
1336+
47989U,
1337+
"1234",
1338+
"MoonlightXboxOG",
1339+
identity,
1340+
});
1341+
1342+
EXPECT_EQ(callCount, 2U);
1343+
EXPECT_FALSE(result.success);
1344+
EXPECT_EQ(result.message, "Pairing failed during phase 1 (getservercert): The host rejected the initial pairing request");
1345+
}
1346+
13121347
TEST(HostPairingTest, PairHostFailsWhenTheChallengeResponseIsTooShort) {
13131348
const network::PairingIdentity clientIdentity = network::create_pairing_identity();
13141349
const network::PairingIdentity serverIdentity = network::create_pairing_identity();

0 commit comments

Comments
 (0)