Commit f4237031 authored by Harald Alvestrand's avatar Harald Alvestrand Committed by Commit Bot

Add memory of last SDP offer/answer created

This adds internal slots "LastOffer" and "LastAnswer", and uses
those to check that SetLocalDescription uses an unchanged SDP offer/answer.

Because modification of SDP is suspected to be used in a number of places,
this change only rejects SDP with modified fingerprints (which would fail
anyway), but merely counts the usage for other modifications.

Bug: chromium:823036
Change-Id: I0c978d5ff3e63b0afab3ec02334c57a5aaa94cdd
Reviewed-on: https://chromium-review.googlesource.com/966441
Commit-Queue: Harald Alvestrand <hta@chromium.org>
Reviewed-by: default avatarHenrik Boström <hbos@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarTommi <tommi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545175}
parent 12761027
......@@ -238,10 +238,10 @@
createConnections(null);
setOfferSdpTransform(removeCrypto);
setOnLocalDescriptionError(function(error) {
var expectedMsg = 'Failed to set local offer sdp:' +
' Called with SDP without DTLS fingerprint.';
assertEquals(expectedMsg, error.message);
// Note: Per spec, error should be 'IllegalModificationError'
// https://crbug.com/823036
var expectedError = 'OperationError';
assertEquals(expectedError, error.name);
reportTestSuccess();
});
navigator.mediaDevices.getUserMedia({audio: true, video: true})
......
This is a testharness.js-based test.
FAIL setLocalDescription() with valid answer should succeed assert_not_equals: Expect session description to be defined got disallowed value undefined
FAIL setLocalDescription() with type answer and null sdp should use lastAnswer generated from createAnswer promise_test: Unhandled rejection with value: object "OperationError: Failed to parse SessionDescription. Expect line: v="
FAIL setLocalDescription() with answer not created by own createAnswer() should reject with InvalidModificationError assert_throws: function "function () { throw e }" threw object "OperationError: Failed to set local answer sdp: Failed to push down transport description: Local fingerprint does not match identity. Expected: sha-256 B5:81:44:BA:F9:E4:DB:24:6C:1C:7E:FF:52:82:7A:67:3C:52:69:F6:5C:D8:BF:B8:FC:34:A2:23:E5:B2:5D:0D Got: sha-256 9D:21:20:4B:66:BC:27:C1:C5:C1:3F:48:63:4A:A9:AC:ED:9F:1C:18:CF:BC:D5:88:9D:19:4F:A3:FC:D2:13:23" that is not a DOMException InvalidModificationError: property "code" is equal to 0, expected 13
FAIL Calling setLocalDescription(answer) from stable state should reject with InvalidStateError assert_throws: function "function () { throw e }" threw object "OperationError: Failed to set local answer sdp: Called in wrong state: STATE_INIT" that is not a DOMException InvalidStateError: property "code" is equal to 0, expected 11
FAIL Calling setLocalDescription(answer) from have-local-offer state should reject with InvalidStateError assert_throws: function "function () { throw e }" threw object "OperationError: Failed to set local answer sdp: Called in wrong state: STATE_SENTOFFER" that is not a DOMException InvalidStateError: property "code" is equal to 0, expected 11
FAIL setLocalDescription() with type answer and null sdp should use lastAnswer generated from createAnswer assert_not_equals: Expect session description to be defined got disallowed value undefined
PASS setLocalDescription() with answer not created by own createAnswer() should reject with InvalidModificationError
PASS Calling setLocalDescription(answer) from stable state should reject with InvalidStateError
PASS Calling setLocalDescription(answer) from have-local-offer state should reject with InvalidStateError
PASS Test onsignalingstatechange event for setLocalDescription() with valid answer should succeed
Harness: the test ran to completion.
This is a testharness.js-based test.
FAIL setLocalDescription with valid offer should succeed assert_not_equals: Expect session description to be defined got disallowed value undefined
FAIL setLocalDescription with type offer and null sdp should use lastOffer generated from createOffer promise_test: Unhandled rejection with value: object "OperationError: Failed to parse SessionDescription. Expect line: v="
FAIL setLocalDescription() with offer not created by own createOffer() should reject with InvalidModificationError assert_unreached: Should have rejected: undefined Reached unreachable code
FAIL setLocalDescription with type offer and null sdp should use lastOffer generated from createOffer assert_not_equals: Expect session description to be defined got disallowed value undefined
PASS setLocalDescription() with offer not created by own createOffer() should reject with InvalidModificationError
FAIL Set created offer other than last offer should reject with InvalidModificationError assert_unreached: Should have rejected: undefined Reached unreachable code
FAIL Creating and setting offer multiple times should succeed assert_not_equals: Expect session description to be defined got disallowed value undefined
PASS Test onsignalingstatechange event for setLocalDescription with valid offer should succeed
......
......@@ -99,11 +99,12 @@
*/
promise_test(t => {
const pc = new RTCPeerConnection();
const pc2 = new RTCPeerConnection();
return generateOffer({ pc, data: true })
.then(offer =>
promise_rejects(t, 'InvalidModificationError',
pc.setLocalDescription(offer)));
pc2.setLocalDescription(offer)));
}, 'setLocalDescription() with offer not created by own createOffer() should reject with InvalidModificationError');
promise_test(t => {
......
This is a testharness.js-based test.
FAIL setLocalDescription(pranswer) from stable state should reject with InvalidStateError assert_throws: function "function() { throw e }" threw object "OperationError: Failed to set local pranswer sdp: Called in wrong state: kStable" that is not a DOMException InvalidStateError: property "code" is equal to 0, expected 11
PASS setLocalDescription(pranswer) from stable state should reject with InvalidStateError
FAIL setLocalDescription(pranswer) should succeed assert_not_equals: Expect session description to be defined got disallowed value undefined
PASS setLocalDescription(pranswer) can be applied multiple times while still in have-local-pranswer
FAIL setLocalDescription(answer) from have-local-pranswer state should succeed assert_not_equals: Expect session description to be defined got disallowed value undefined
......
......@@ -118,6 +118,14 @@ namespace {
const char kSignalingStateClosedMessage[] =
"The RTCPeerConnection's signalingState is 'closed'.";
const char kSignalingStatePreventsOfferMessage[] =
"The RTCPeerConnection's signalingState must be 'stable' or "
"'have-local-offer' to apply an offer";
const char kSignalingStatePreventsAnswerMessage[] =
"The RTCPeerConnection's signalingState must be 'have-remote-offer' or "
"'have-local-pranswer' to apply an answer";
const char kModifiedSdpMessage[] =
"The SDP does not match the previously generated SDP for this type";
// The maximum number of PeerConnections that can exist simultaneously.
const long kMaxPeerConnections = 500;
......@@ -437,6 +445,30 @@ class WebRTCStatsReportCallbackResolver : public WebRTCStatsReportCallback {
Persistent<ScriptPromiseResolver> resolver_;
};
bool FingerprintMismatch(String old_sdp, String new_sdp) {
// Check special case of externally generated SDP without fingerprints.
// It's impossible to generate a valid fingerprint without createOffer
// or createAnswer, so this only applies when there are no fingerprints.
// This is allowed.
const size_t new_fingerprint_pos = new_sdp.Find("\na=fingerprint:");
if (new_fingerprint_pos == kNotFound)
return false;
// Look for fingerprint having been added. Not allowed.
const size_t old_fingerprint_pos = old_sdp.Find("\na=fingerprint:");
if (old_fingerprint_pos == kNotFound) {
return true;
}
// Look for fingerprint being modified. Not allowed.
const size_t old_fingerprint_end =
old_sdp.Find("\n", old_fingerprint_pos + 1);
const size_t new_fingerprint_end =
new_sdp.Find("\n", new_fingerprint_pos + 1);
return old_sdp.Substring(old_fingerprint_pos,
old_fingerprint_end - old_fingerprint_pos) !=
new_sdp.Substring(new_fingerprint_pos,
new_fingerprint_end - new_fingerprint_pos);
}
} // namespace
RTCPeerConnection::EventWrapper::EventWrapper(Event* event,
......@@ -735,20 +767,71 @@ ScriptPromise RTCPeerConnection::createAnswer(
return ScriptPromise::CastUndefined(script_state);
}
DOMException* RTCPeerConnection::checkSdpForStateErrors(
ExecutionContext* context,
const RTCSessionDescriptionInit& session_description_init,
String* sdp) {
if (signaling_state_ == kSignalingStateClosed) {
return DOMException::Create(kInvalidStateError,
kSignalingStateClosedMessage);
}
*sdp = session_description_init.sdp();
if (session_description_init.type() == "offer") {
if (signaling_state_ != kSignalingStateStable &&
signaling_state_ != kSignalingStateHaveLocalOffer) {
return DOMException::Create(kInvalidStateError,
kSignalingStatePreventsOfferMessage);
}
if (sdp->IsNull() || sdp->IsEmpty()) {
*sdp = last_offer_;
} else if (session_description_init.sdp() != last_offer_) {
if (FingerprintMismatch(last_offer_, *sdp)) {
return DOMException::Create(kInvalidModificationError,
kModifiedSdpMessage);
} else {
UseCounter::Count(context, WebFeature::kRTCLocalSdpModification);
return nullptr;
// TODO(https://crbug.com/823036): Return failure for all modification.
}
}
} else if (session_description_init.type() == "answer" ||
session_description_init.type() == "pranswer") {
if (signaling_state_ != kSignalingStateHaveRemoteOffer &&
signaling_state_ != kSignalingStateHaveLocalPrAnswer) {
return DOMException::Create(kInvalidStateError,
kSignalingStatePreventsAnswerMessage);
}
if (sdp->IsNull() || sdp->IsEmpty()) {
*sdp = last_answer_;
} else if (session_description_init.sdp() != last_answer_) {
if (FingerprintMismatch(last_answer_, *sdp)) {
return DOMException::Create(kInvalidModificationError,
kModifiedSdpMessage);
} else {
UseCounter::Count(context, WebFeature::kRTCLocalSdpModification);
return nullptr;
// TODO(https://crbug.com/823036): Return failure for all modification.
}
}
}
return nullptr;
}
ScriptPromise RTCPeerConnection::setLocalDescription(
ScriptState* script_state,
const RTCSessionDescriptionInit& session_description_init) {
if (signaling_state_ == kSignalingStateClosed)
return ScriptPromise::RejectWithDOMException(
script_state,
DOMException::Create(kInvalidStateError, kSignalingStateClosedMessage));
String sdp;
DOMException* exception = checkSdpForStateErrors(
ExecutionContext::From(script_state), session_description_init, &sdp);
if (exception) {
return ScriptPromise::RejectWithDOMException(script_state, exception);
}
ScriptPromiseResolver* resolver = ScriptPromiseResolver::Create(script_state);
ScriptPromise promise = resolver->Promise();
RTCVoidRequest* request = RTCVoidRequestPromiseImpl::Create(this, resolver);
peer_handler_->SetLocalDescription(
request, WebRTCSessionDescription(session_description_init.type(),
session_description_init.sdp()));
request, WebRTCSessionDescription(session_description_init.type(), sdp));
return promise;
}
......@@ -775,8 +858,14 @@ ScriptPromise RTCPeerConnection::setLocalDescription(
kRTCPeerConnectionSetLocalDescriptionLegacyNoFailureCallback);
}
if (CallErrorCallbackIfSignalingStateClosed(signaling_state_, error_callback))
String sdp;
DOMException* exception =
checkSdpForStateErrors(context, session_description_init, &sdp);
if (exception) {
if (error_callback)
AsyncCallErrorCallback(error_callback, exception);
return ScriptPromise::CastUndefined(script_state);
}
RTCVoidRequest* request = RTCVoidRequestImpl::Create(
GetExecutionContext(), this, success_callback, error_callback);
......@@ -1454,6 +1543,14 @@ void RTCPeerConnection::close() {
CloseInternal();
}
void RTCPeerConnection::NoteSdpCreated(const RTCSessionDescription& desc) {
if (desc.type() == "offer") {
last_offer_ = desc.sdp();
} else if (desc.type() == "answer") {
last_answer_ = desc.sdp();
}
}
void RTCPeerConnection::OnStreamAddTrack(MediaStream* stream,
MediaStreamTrack* track) {
ExceptionState exception_state(nullptr, ExceptionState::kUnknownContext,
......
......@@ -190,6 +190,9 @@ class MODULES_EXPORT RTCPeerConnection final
DEFINE_ATTRIBUTE_EVENT_LISTENER(icegatheringstatechange);
DEFINE_ATTRIBUTE_EVENT_LISTENER(datachannel);
// Utility to note result of CreateOffer / CreateAnswer
void NoteSdpCreated(const RTCSessionDescription&);
// MediaStreamObserver
void OnStreamAddTrack(MediaStream*, MediaStreamTrack*) override;
void OnStreamRemoveTrack(MediaStream*, MediaStreamTrack*) override;
......@@ -304,6 +307,10 @@ class MODULES_EXPORT RTCPeerConnection final
void RecordRapporMetrics();
DOMException* checkSdpForStateErrors(ExecutionContext*,
const RTCSessionDescriptionInit&,
String* sdp);
SignalingState signaling_state_;
ICEGatheringState ice_gathering_state_;
ICEConnectionState ice_connection_state_;
......@@ -329,6 +336,10 @@ class MODULES_EXPORT RTCPeerConnection final
bool stopped_;
bool closed_;
// Internal state [[LastOffer]] and [[LastAnswer]]
String last_offer_;
String last_answer_;
bool has_data_channels_; // For RAPPOR metrics
};
......
......@@ -65,7 +65,7 @@ RTCSessionDescription::RTCSessionDescription(
WebRTCSessionDescription web_session_description)
: web_session_description_(web_session_description) {}
String RTCSessionDescription::type() {
String RTCSessionDescription::type() const {
return web_session_description_.GetType();
}
......@@ -73,7 +73,7 @@ void RTCSessionDescription::setType(const String& type) {
web_session_description_.SetType(type);
}
String RTCSessionDescription::sdp() {
String RTCSessionDescription::sdp() const {
return web_session_description_.Sdp();
}
......
......@@ -50,10 +50,10 @@ class RTCSessionDescription final : public ScriptWrappable {
const RTCSessionDescriptionInit&);
static RTCSessionDescription* Create(WebRTCSessionDescription);
String type();
String type() const;
void setType(const String&);
String sdp();
String sdp() const;
void setSdp(const String&);
ScriptValue toJSONForBinding(ScriptState*);
......
......@@ -67,8 +67,9 @@ void RTCSessionDescriptionRequestImpl::RequestSucceeded(
bool should_fire_callback =
requester_ ? requester_->ShouldFireDefaultCallbacks() : false;
if (should_fire_callback && success_callback_) {
success_callback_->InvokeAndReportException(
nullptr, RTCSessionDescription::Create(web_session_description));
auto description = RTCSessionDescription::Create(web_session_description);
requester_->NoteSdpCreated(*description);
success_callback_->InvokeAndReportException(nullptr, description);
}
Clear();
}
......
......@@ -34,7 +34,9 @@ RTCSessionDescriptionRequestPromiseImpl::
void RTCSessionDescriptionRequestPromiseImpl::RequestSucceeded(
const WebRTCSessionDescription& web_session_description) {
if (requester_ && requester_->ShouldFireDefaultCallbacks()) {
resolver_->Resolve(RTCSessionDescription::Create(web_session_description));
auto description = RTCSessionDescription::Create(web_session_description);
requester_->NoteSdpCreated(*description);
resolver_->Resolve(description);
} else {
// This is needed to have the resolver release its internal resources
// while leaving the associated promise pending as specified.
......
......@@ -1882,6 +1882,7 @@ enum WebFeature {
kVRDisplayGetFrameData = 2390,
kXMLHttpRequestResponseXML = 2391,
kMessagePortTransferClosedPort = 2392,
kRTCLocalSdpModification = 2393,
// Add new features immediately above this line. Don't change assigned
// numbers of any item, and don't reuse removed slots.
......
......@@ -17976,6 +17976,7 @@ Called by update_net_error_codes.py.-->
<int value="2390" label="VRDisplayGetFrameData"/>
<int value="2391" label="XMLHttpRequestResponseXML"/>
<int value="2392" label="MessagePortTransferClosedPort"/>
<int value="2393" label="RTCLocalSdpModification"/>
</enum>
<enum name="FeedbackSource">
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment