Stun server should return XOR-MAPPED-ADDRESS/MAPPED-ADDRESS correctly
* Return xor mapped address for RFC5389 compatible client
* fix a typo in function name
* update stunserver unitest case
* update author
Bug: webrtc:10764
Change-Id: I466799744a343508233c18b7c477d2212680392a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/143841
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28421}
diff --git a/AUTHORS b/AUTHORS
index bdd24f3..fbf9353 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -78,6 +78,7 @@
Jan Grulich <grulja@gmail.com>
Eike Rathke <erathke@redhat.com>
Michel Promonet <michel.promonet.1@gmail.com>
+Min Wang <mingewang@gmail.com>
&yet LLC <*@andyet.com>
Agora IO <*@agora.io>
diff --git a/p2p/base/stun_server.cc b/p2p/base/stun_server.cc
index 38ccc3f..382b787 100644
--- a/p2p/base/stun_server.cc
+++ b/p2p/base/stun_server.cc
@@ -54,7 +54,7 @@
void StunServer::OnBindingRequest(StunMessage* msg,
const rtc::SocketAddress& remote_addr) {
StunMessage response;
- GetStunBindReqponse(msg, remote_addr, &response);
+ GetStunBindResponse(msg, remote_addr, &response);
SendResponse(response, remote_addr);
}
@@ -83,7 +83,7 @@
RTC_LOG_ERR(LS_ERROR) << "sendto";
}
-void StunServer::GetStunBindReqponse(StunMessage* request,
+void StunServer::GetStunBindResponse(StunMessage* request,
const rtc::SocketAddress& remote_addr,
StunMessage* response) const {
response->SetType(STUN_BINDING_RESPONSE);
@@ -91,7 +91,7 @@
// Tell the user the address that we received their request from.
std::unique_ptr<StunAddressAttribute> mapped_addr;
- if (!request->IsLegacy()) {
+ if (request->IsLegacy()) {
mapped_addr = StunAttribute::CreateAddress(STUN_ATTR_MAPPED_ADDRESS);
} else {
mapped_addr = StunAttribute::CreateXorAddress(STUN_ATTR_XOR_MAPPED_ADDRESS);
diff --git a/p2p/base/stun_server.h b/p2p/base/stun_server.h
index 7ddc5c1..01d74e2 100644
--- a/p2p/base/stun_server.h
+++ b/p2p/base/stun_server.h
@@ -57,7 +57,7 @@
void SendResponse(const StunMessage& msg, const rtc::SocketAddress& addr);
// A helper method to compose a STUN binding response.
- void GetStunBindReqponse(StunMessage* request,
+ void GetStunBindResponse(StunMessage* request,
const rtc::SocketAddress& remote_addr,
StunMessage* response) const;
diff --git a/p2p/base/stun_server_unittest.cc b/p2p/base/stun_server_unittest.cc
index efc1cd2..7b11d6f 100644
--- a/p2p/base/stun_server_unittest.cc
+++ b/p2p/base/stun_server_unittest.cc
@@ -74,7 +74,8 @@
TEST_F(StunServerTest, TestGood) {
StunMessage req;
- std::string transaction_id = "0123456789ab";
+ // kStunLegacyTransactionIdLength = 16 for legacy RFC 3489 request
+ std::string transaction_id = "0123456789abcdef";
req.SetType(STUN_BINDING_REQUEST);
req.SetTransactionID(transaction_id);
Send(req);
@@ -89,11 +90,50 @@
EXPECT_TRUE(mapped_addr != NULL);
EXPECT_EQ(1, mapped_addr->family());
EXPECT_EQ(client_addr.port(), mapped_addr->port());
- if (mapped_addr->ipaddr() != client_addr.ipaddr()) {
- RTC_LOG(LS_WARNING) << "Warning: mapped IP ("
- << mapped_addr->ipaddr().ToString() << ") != local IP ("
- << client_addr.ipaddr().ToString() << ")";
- }
+
+ delete msg;
+}
+
+TEST_F(StunServerTest, TestGoodXorMappedAddr) {
+ StunMessage req;
+ // kStunTransactionIdLength = 12 for RFC 5389 request
+ // StunMessage::Write will automatically insert magic cookie (0x2112A442)
+ std::string transaction_id = "0123456789ab";
+ req.SetType(STUN_BINDING_REQUEST);
+ req.SetTransactionID(transaction_id);
+ Send(req);
+
+ StunMessage* msg = Receive();
+ ASSERT_TRUE(msg != NULL);
+ EXPECT_EQ(STUN_BINDING_RESPONSE, msg->type());
+ EXPECT_EQ(req.transaction_id(), msg->transaction_id());
+
+ const StunAddressAttribute* mapped_addr =
+ msg->GetAddress(STUN_ATTR_XOR_MAPPED_ADDRESS);
+ EXPECT_TRUE(mapped_addr != NULL);
+ EXPECT_EQ(1, mapped_addr->family());
+ EXPECT_EQ(client_addr.port(), mapped_addr->port());
+
+ delete msg;
+}
+
+// Send legacy RFC 3489 request, should not get xor mapped addr
+TEST_F(StunServerTest, TestNoXorMappedAddr) {
+ StunMessage req;
+ // kStunLegacyTransactionIdLength = 16 for legacy RFC 3489 request
+ std::string transaction_id = "0123456789abcdef";
+ req.SetType(STUN_BINDING_REQUEST);
+ req.SetTransactionID(transaction_id);
+ Send(req);
+
+ StunMessage* msg = Receive();
+ ASSERT_TRUE(msg != NULL);
+ EXPECT_EQ(STUN_BINDING_RESPONSE, msg->type());
+ EXPECT_EQ(req.transaction_id(), msg->transaction_id());
+
+ const StunAddressAttribute* mapped_addr =
+ msg->GetAddress(STUN_ATTR_XOR_MAPPED_ADDRESS);
+ EXPECT_TRUE(mapped_addr == NULL);
delete msg;
}
diff --git a/p2p/base/test_stun_server.cc b/p2p/base/test_stun_server.cc
index 3c98cd8..9330a00 100644
--- a/p2p/base/test_stun_server.cc
+++ b/p2p/base/test_stun_server.cc
@@ -30,7 +30,7 @@
StunServer::OnBindingRequest(msg, remote_addr);
} else {
StunMessage response;
- GetStunBindReqponse(msg, fake_stun_addr_, &response);
+ GetStunBindResponse(msg, fake_stun_addr_, &response);
SendResponse(response, remote_addr);
}
}
