zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Unbreak ICE renomination

Browse Source 
This patch fixes a problem in https://webrtc.googlesource.com/src/+/71ff07369837d6575c04ebff7002d07d6e0af25f
that when adding standard compliance validation of ufrag/pwd
accidentally broken ice renomination by introducing a new "constructor".

Bug: chromium:1044521
Change-Id: If1b18b1d728e55db9da385b37162a9cb5e61ac48
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/169549
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Magnus Flodman <mflodman@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30670}
This commit is contained in:
Jonas Oreland
2020-03-03 13:21:30 +01:00
committed by Commit Bot
parent 134c6996c8
commit 52aea5d3f3
5 changed files with 84 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
p2p/base/fake_ice_transport.h
View File

@ -112,10 +112,18 @@ class FakeIceTransport : public IceTransportInternal {
int component() const override { return component_; }
uint64_t IceTiebreaker() const { return tiebreaker_; }
IceMode remote_ice_mode() const { return remote_ice_mode_; }
const std::string& ice_ufrag() const { return ice_ufrag_; }
const std::string& ice_pwd() const { return ice_pwd_; }
const std::string& remote_ice_ufrag() const { return remote_ice_ufrag_; }
const std::string& remote_ice_pwd() const { return remote_ice_pwd_; }
const std::string& ice_ufrag() const { return ice_parameters_.ufrag; }
const std::string& ice_pwd() const { return ice_parameters_.pwd; }
const std::string& remote_ice_ufrag() const {
return remote_ice_parameters_.ufrag;
}
const std::string& remote_ice_pwd() const {
return remote_ice_parameters_.pwd;
}
const IceParameters& ice_parameters() const { return ice_parameters_; }
const IceParameters& remote_ice_parameters() const {
return remote_ice_parameters_;
}
IceTransportState GetState() const override {
if (legacy_transport_state_) {
@ -157,12 +165,10 @@ class FakeIceTransport : public IceTransportInternal {
tiebreaker_ = tiebreaker;
}
void SetIceParameters(const IceParameters& ice_params) override {
ice_ufrag_ = ice_params.ufrag;
ice_pwd_ = ice_params.pwd;
ice_parameters_ = ice_params;
}
void SetRemoteIceParameters(const IceParameters& params) override {
remote_ice_ufrag_ = params.ufrag;
remote_ice_pwd_ = params.pwd;
remote_ice_parameters_ = params;
}
void SetRemoteIceMode(IceMode mode) override { remote_ice_mode_ = mode; }
@ -312,10 +318,8 @@ class FakeIceTransport : public IceTransportInternal {
IceConfig ice_config_;
IceRole role_ = ICEROLE_UNKNOWN;
uint64_t tiebreaker_ = 0;
std::string ice_ufrag_;
std::string ice_pwd_;
std::string remote_ice_ufrag_;
std::string remote_ice_pwd_;
IceParameters ice_parameters_;
IceParameters remote_ice_parameters_;
IceMode remote_ice_mode_ = ICEMODE_FULL;
size_t connection_count_ = 0;
absl::optional<webrtc::IceTransportState> transport_state_;

36
p2p/base/transport_description.cc
View File

@ -38,7 +38,7 @@ bool IsIceChar(char c) {
return absl::ascii_isalnum(c) || c == '+' || c == '/';
}
RTCErrorOr<std::string> ParseIceUfrag(absl::string_view raw_ufrag) {
RTCError ValidateIceUfrag(absl::string_view raw_ufrag) {
if (!(ICE_UFRAG_MIN_LENGTH <= raw_ufrag.size() &&
raw_ufrag.size() <= ICE_UFRAG_MAX_LENGTH)) {
rtc::StringBuilder sb;
@ -53,10 +53,10 @@ RTCErrorOr<std::string> ParseIceUfrag(absl::string_view raw_ufrag) {
"ICE ufrag must contain only alphanumeric characters, '+', and '/'.");
}
return std::string(raw_ufrag);
return RTCError::OK();
}
RTCErrorOr<std::string> ParseIcePwd(absl::string_view raw_pwd) {
RTCError ValidateIcePwd(absl::string_view raw_pwd) {
if (!(ICE_PWD_MIN_LENGTH <= raw_pwd.size() &&
raw_pwd.size() <= ICE_PWD_MAX_LENGTH)) {
rtc::StringBuilder sb;
@ -71,35 +71,41 @@ RTCErrorOr<std::string> ParseIcePwd(absl::string_view raw_pwd) {
"ICE pwd must contain only alphanumeric characters, '+', and '/'.");
}
return std::string(raw_pwd);
return RTCError::OK();
}
} // namespace
// static
RTCErrorOr<IceParameters> IceParameters::Parse(absl::string_view raw_ufrag,
absl::string_view raw_pwd) {
IceParameters parameters(std::string(raw_ufrag), std::string(raw_pwd),
/* renomination= */ false);
auto result = parameters.Validate();
if (!result.ok()) {
return result;
}
return parameters;
}
RTCError IceParameters::Validate() const {
// For legacy protocols.
// TODO(zhihuang): Remove this once the legacy protocol is no longer
// supported.
if (raw_ufrag.empty() && raw_pwd.empty()) {
return IceParameters();
if (ufrag.empty() && pwd.empty()) {
return RTCError::OK();
}
auto ufrag_result = ParseIceUfrag(raw_ufrag);
auto ufrag_result = ValidateIceUfrag(ufrag);
if (!ufrag_result.ok()) {
return ufrag_result.MoveError();
return ufrag_result;
}
auto pwd_result = ParseIcePwd(raw_pwd);
auto pwd_result = ValidateIcePwd(pwd);
if (!pwd_result.ok()) {
return pwd_result.MoveError();
return pwd_result;
}
IceParameters parameters;
parameters.ufrag = ufrag_result.MoveValue();
parameters.pwd = pwd_result.MoveValue();
return parameters;
return RTCError::OK();
}
bool StringToConnectionRole(const std::string& role_str, ConnectionRole* role) {

6
p2p/base/transport_description.h
View File

@ -83,6 +83,10 @@ struct IceParameters {
bool operator!=(const IceParameters& other) const {
return !(*this == other);
}
// Validate IceParameters, returns a SyntaxError if the ufrag or pwd are
// malformed.
webrtc::RTCError Validate() const;
};
extern const char CONNECTIONROLE_ACTIVE_STR[];
@ -142,7 +146,7 @@ struct TransportDescription {
}
bool secure() const { return identity_fingerprint != nullptr; }
IceParameters GetIceParameters() {
IceParameters GetIceParameters() const {
return IceParameters(ice_ufrag, ice_pwd,
HasOption(ICE_OPTION_RENOMINATION));
}

18
pc/jsep_transport.cc
View File

@ -178,16 +178,15 @@ webrtc::RTCError JsepTransport::SetLocalJsepTransportDescription(
RTC_DCHECK_RUN_ON(network_thread_);
webrtc::RTCErrorOr<IceParameters> ice_parameters_result =
IceParameters::Parse(jsep_description.transport_desc.ice_ufrag,
jsep_description.transport_desc.ice_pwd);
IceParameters ice_parameters =
jsep_description.transport_desc.GetIceParameters();
webrtc::RTCError ice_parameters_result = ice_parameters.Validate();
if (!ice_parameters_result.ok()) {
rtc::StringBuilder sb;
sb << "Invalid ICE parameters: " << ice_parameters_result.error().message();
sb << "Invalid ICE parameters: " << ice_parameters_result.message();
return webrtc::RTCError(webrtc::RTCErrorType::INVALID_PARAMETER,
sb.Release());
}
IceParameters ice_parameters = ice_parameters_result.MoveValue();
if (!SetRtcpMux(jsep_description.rtcp_mux_enabled, type,
ContentSource::CS_LOCAL)) {
@ -273,17 +272,16 @@ webrtc::RTCError JsepTransport::SetRemoteJsepTransportDescription(
RTC_DCHECK_RUN_ON(network_thread_);
webrtc::RTCErrorOr<IceParameters> ice_parameters_result =
IceParameters::Parse(jsep_description.transport_desc.ice_ufrag,
jsep_description.transport_desc.ice_pwd);
IceParameters ice_parameters =
jsep_description.transport_desc.GetIceParameters();
webrtc::RTCError ice_parameters_result = ice_parameters.Validate();
if (!ice_parameters_result.ok()) {
remote_description_.reset();
rtc::StringBuilder sb;
sb << "Invalid ICE parameters: " << ice_parameters_result.error().message();
sb << "Invalid ICE parameters: " << ice_parameters_result.message();
return webrtc::RTCError(webrtc::RTCErrorType::INVALID_PARAMETER,
sb.Release());
}
IceParameters ice_parameters = ice_parameters_result.MoveValue();
if (!SetRtcpMux(jsep_description.rtcp_mux_enabled, type,
ContentSource::CS_REMOTE)) {

34
pc/jsep_transport_unittest.cc
View File

@ -1256,5 +1256,39 @@ INSTANTIATE_TEST_SUITE_P(
std::make_tuple(Scenario::kDtlsBeforeCallerSendOffer, false),
std::make_tuple(Scenario::kDtlsBeforeCallerSetAnswer, false),
std::make_tuple(Scenario::kDtlsAfterCallerSetAnswer, false)));
// This test verifies the ICE parameters are properly applied to the transports.
TEST_F(JsepTransport2Test, SetIceParametersWithRenomination) {
jsep_transport_ =
CreateJsepTransport2(/* rtcp_mux_enabled= */ true, SrtpMode::kDtlsSrtp);
JsepTransportDescription jsep_description;
jsep_description.transport_desc = TransportDescription(kIceUfrag1, kIcePwd1);
jsep_description.transport_desc.AddOption(ICE_OPTION_RENOMINATION);
ASSERT_TRUE(
jsep_transport_
->SetLocalJsepTransportDescription(jsep_description, SdpType::kOffer)
.ok());
auto fake_ice_transport = static_cast<FakeIceTransport*>(
jsep_transport_->rtp_dtls_transport()->ice_transport());
EXPECT_EQ(ICEMODE_FULL, fake_ice_transport->remote_ice_mode());
EXPECT_EQ(kIceUfrag1, fake_ice_transport->ice_ufrag());
EXPECT_EQ(kIcePwd1, fake_ice_transport->ice_pwd());
EXPECT_TRUE(fake_ice_transport->ice_parameters().renomination);
jsep_description.transport_desc = TransportDescription(kIceUfrag2, kIcePwd2);
jsep_description.transport_desc.AddOption(ICE_OPTION_RENOMINATION);
ASSERT_TRUE(jsep_transport_
->SetRemoteJsepTransportDescription(jsep_description,
SdpType::kAnswer)
.ok());
fake_ice_transport = static_cast<FakeIceTransport*>(
jsep_transport_->rtp_dtls_transport()->ice_transport());
EXPECT_EQ(ICEMODE_FULL, fake_ice_transport->remote_ice_mode());
EXPECT_EQ(kIceUfrag2, fake_ice_transport->remote_ice_ufrag());
EXPECT_EQ(kIcePwd2, fake_ice_transport->remote_ice_pwd());
EXPECT_TRUE(fake_ice_transport->remote_ice_parameters().renomination);
}
} // namespace
} // namespace cricket