Use the dummy address 0.0.0.0:9 in the c= and the m= lines if the
default connection address is a hostname candidate.
Using a FQDN in the c= line has caused an inter-op issue with Firefox
when hostname candidates are the only candidates gathered when forming
the media sections. To address this issue, we use 0.0.0.0:9 when a
hostname candidate would be used to populate the c= and the m= lines.
The SDP grammar related to ICE candidates has been moved out of RFC8445,
and is currently defined in draft-ietf-mmusic-ice-sip-sdp. A FQDN
address must not be used in the connection address attribute per the
latest draft, if the ICE agent generates local candidates. Also, the
wildcard addresses (0.0.0.0 or ::) with port 9 are given the exception
as the connection address that will not result in an ICE mismatch. We
thus adopt the aforementioned solution after combining these
considerations.
Bug: chromium:927309, chromium:982108
Change-Id: I3df2db0f154276da39f99650289cf81baa677e74
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145280
Commit-Queue: Qingsi Wang <qingsi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28547}
diff --git a/pc/jsep_session_description.cc b/pc/jsep_session_description.cc
index 001aa76..4c04531 100644
--- a/pc/jsep_session_description.cc
+++ b/pc/jsep_session_description.cc
@@ -90,7 +90,23 @@
}
rtc::SocketAddress connection_addr(ip, port);
if (rtc::IPIsUnspec(connection_addr.ipaddr()) && !hostname.empty()) {
- connection_addr = rtc::SocketAddress(hostname, port);
+ // When a hostname candidate becomes the (default) connection address,
+ // we use the dummy address 0.0.0.0 and port 9 in the c= and the m= lines.
+ //
+ // We have observed in deployment that with a FQDN in a c= line, SDP parsing
+ // could fail in other JSEP implementations. We note that the wildcard
+ // addresses (0.0.0.0 or ::) with port 9 are given the exception as the
+ // connection address that will not result in an ICE mismatch
+ // (draft-ietf-mmusic-ice-sip-sdp). Also, 0.0.0.0 or :: can be used as the
+ // connection address in the initial offer or answer with trickle ICE
+ // if the offerer or answerer does not want to include the host IP address
+ // (draft-ietf-mmusic-trickle-ice-sip), and in particular 0.0.0.0 has been
+ // widely deployed for this use without outstanding compatibility issues.
+ // Combining the above considerations, we use 0.0.0.0 with port 9 to
+ // populate the c= and the m= lines. See |BuildMediaDescription| in
+ // webrtc_sdp.cc for the SDP generation with
+ // |media_desc->connection_address()|.
+ connection_addr = rtc::SocketAddress(kDummyAddress, kDummyPort);
}
media_desc->set_connection_address(connection_addr);
}
diff --git a/pc/jsep_session_description_unittest.cc b/pc/jsep_session_description_unittest.cc
index 8abb500..ef86ef4 100644
--- a/pc/jsep_session_description_unittest.cc
+++ b/pc/jsep_session_description_unittest.cc
@@ -223,11 +223,14 @@
c.set_protocol(cricket::UDP_PROTOCOL_NAME);
c.set_address(rtc::SocketAddress("example.local", 1234));
c.set_type(cricket::LOCAL_PORT_TYPE);
- JsepIceCandidate hostname_candidate("audio", 0, c);
+ const size_t audio_index = 0;
+ JsepIceCandidate hostname_candidate("audio", audio_index, c);
EXPECT_TRUE(jsep_desc_->AddCandidate(&hostname_candidate));
+
ASSERT_NE(nullptr, jsep_desc_->description());
- const auto& content = jsep_desc_->description()->contents()[0];
- EXPECT_EQ("example.local:1234",
+ ASSERT_EQ(2u, jsep_desc_->description()->contents().size());
+ const auto& content = jsep_desc_->description()->contents()[audio_index];
+ EXPECT_EQ("0.0.0.0:9",
content.media_description()->connection_address().ToString());
}
@@ -242,6 +245,39 @@
EXPECT_EQ(sdp, parsed_sdp);
}
+// Test that we can serialize a JsepSessionDescription when a hostname candidate
+// is the default destination and deserialize it again. The connection address
+// in the deserialized description should be the dummy address 0.0.0.0:9.
+TEST_F(JsepSessionDescriptionTest, SerializeDeserializeWithHostnameCandidate) {
+ cricket::Candidate c;
+ c.set_component(cricket::ICE_CANDIDATE_COMPONENT_RTP);
+ c.set_protocol(cricket::UDP_PROTOCOL_NAME);
+ c.set_address(rtc::SocketAddress("example.local", 1234));
+ c.set_type(cricket::LOCAL_PORT_TYPE);
+ const size_t audio_index = 0;
+ const size_t video_index = 1;
+ JsepIceCandidate hostname_candidate_audio("audio", audio_index, c);
+ JsepIceCandidate hostname_candidate_video("video", video_index, c);
+ EXPECT_TRUE(jsep_desc_->AddCandidate(&hostname_candidate_audio));
+ EXPECT_TRUE(jsep_desc_->AddCandidate(&hostname_candidate_video));
+
+ std::string sdp = Serialize(jsep_desc_.get());
+
+ auto parsed_jsep_desc = DeSerialize(sdp);
+ EXPECT_EQ(2u, parsed_jsep_desc->number_of_mediasections());
+
+ ASSERT_NE(nullptr, parsed_jsep_desc->description());
+ ASSERT_EQ(2u, parsed_jsep_desc->description()->contents().size());
+ const auto& audio_content =
+ parsed_jsep_desc->description()->contents()[audio_index];
+ const auto& video_content =
+ parsed_jsep_desc->description()->contents()[video_index];
+ EXPECT_EQ("0.0.0.0:9",
+ audio_content.media_description()->connection_address().ToString());
+ EXPECT_EQ("0.0.0.0:9",
+ video_content.media_description()->connection_address().ToString());
+}
+
// Tests that we can serialize and deserialize a JsepSesssionDescription
// with candidates.
TEST_F(JsepSessionDescriptionTest, SerializeDeserializeWithCandidates) {
diff --git a/pc/webrtc_sdp.cc b/pc/webrtc_sdp.cc
index 7d33a6a..a022e56 100644
--- a/pc/webrtc_sdp.cc
+++ b/pc/webrtc_sdp.cc
@@ -1468,10 +1468,6 @@
} else if (media_desc->connection_address().family() == AF_INET6) {
os << " " << kConnectionIpv6Addrtype << " "
<< media_desc->connection_address().ipaddr().ToString();
- } else if (!media_desc->connection_address().hostname().empty()) {
- // For hostname candidates, we use c=IN IP4 <hostname>.
- os << " " << kConnectionIpv4Addrtype << " "
- << media_desc->connection_address().hostname();
} else {
os << " " << kConnectionIpv4Addrtype << " " << kDummyAddress;
}
diff --git a/pc/webrtc_sdp_unittest.cc b/pc/webrtc_sdp_unittest.cc
index 2be454f..bbf28c9 100644
--- a/pc/webrtc_sdp_unittest.cc
+++ b/pc/webrtc_sdp_unittest.cc
@@ -4259,31 +4259,6 @@
video_desc->connection_address().ToString());
}
-// Test that a media description that contains a hostname connection address can
-// be correctly serialized.
-TEST_F(WebRtcSdpTest, SerializeAndDeserializeWithHostnameConnectionAddress) {
- JsepSessionDescription expected_jsep(kDummyType);
- cricket::Candidate c;
- const rtc::SocketAddress hostname_addr("example.local", 1234);
- audio_desc_->set_connection_address(hostname_addr);
- video_desc_->set_connection_address(hostname_addr);
- ASSERT_TRUE(
- expected_jsep.Initialize(desc_.Clone(), kSessionId, kSessionVersion));
- // Serialization.
- std::string message = webrtc::SdpSerialize(expected_jsep);
- // Deserialization.
- JsepSessionDescription jdesc(kDummyType);
- ASSERT_TRUE(SdpDeserialize(message, &jdesc));
- auto audio_desc = jdesc.description()
- ->GetContentByName(kAudioContentName)
- ->media_description();
- auto video_desc = jdesc.description()
- ->GetContentByName(kVideoContentName)
- ->media_description();
- EXPECT_EQ(hostname_addr, audio_desc->connection_address());
- EXPECT_EQ(hostname_addr, video_desc->connection_address());
-}
-
// RFC4566 says "If a session has no meaningful name, the value "s= " SHOULD be
// used (i.e., a single space as the session name)." So we should accept that.
TEST_F(WebRtcSdpTest, DeserializeEmptySessionName) {
