The P2PTransportChannel will eventually inherit from IceTransportInternal instead of TransportChannelImpl.
However, the Chromium/remoting depends on TransportChannel and TransportChannelImpl.
The solution to work around this:
Step1:
Make a WebRTC CL to introduce IceTransportInternal and IceTransportInternal2 by type-defining
TransportChannel and TransportChannelImpl.
Step2:
Make a Chromium CL to replace the TransportChannel and TransportChannelImpl with
IceTransportInternal and IceTransportInternal2.
Step3:
Make a WebRTC to redefine IceTransportInternal2 to be IceTransportInternal and switch the base
class of P2PTransportChannel with IceTransportInternal.
Step4"
Make a Chromium CL to remove the IceTransportInternal2.
This CL is the Step1. The real IceTransportInternal implementation
is commented out temporarily.
BUG=none
Review-Url: https://codereview.webrtc.org/2598103003
Cr-Commit-Position: refs/heads/master@{#15824}
Reason for revert:
Breaks Chromium WebRTC FYI bots:
https://build.chromium.org/p/chromium.webrtc.fyi/builders/Linux%20Builder/builds/12337
The error was masked by another breaking change that was committer earlier. This is the first build showing the error.
Original issue's description:
> Make P2PTransportChannel inherit from IceTransportInternal.
>
> Make P2PTransportChannel inherit from IceTransportInternal instead of
> TransportChannelImpl and TransportChannel, so that the DTLS-related methods can
> be separated from P2PTransportChannel.
>
> BUG=none
>
> Review-Url: https://codereview.webrtc.org/2590063002
> Cr-Commit-Position: refs/heads/master@{#15743}
> Committed: 12749d89d9TBR=deadbeef@webrtc.org,zhihuang@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=none
Review-Url: https://codereview.webrtc.org/2594343002
Cr-Commit-Position: refs/heads/master@{#15751}
Make P2PTransportChannel inherit from IceTransportInternal instead of
TransportChannelImpl and TransportChannel, so that the DTLS-related methods can
be separated from P2PTransportChannel.
BUG=none
Review-Url: https://codereview.webrtc.org/2590063002
Cr-Commit-Position: refs/heads/master@{#15743}
Reason for revert:
This CL broke all Chromium WebRTC FYI bots. A roll+fix was attempted here: https://codereview.chromium.org/2590783003/, but failed to land. I'm reverting this CL now to make the tree green again. Make the API change gradual when you reland so that we can update Chromium between.
Original issue's description:
> Add disabled certificate check support to IceServer PeerConnection API.
>
> Refactor "OPT_SSLTCP" renaming it to "OPT_TLS_FAKE", making it clear
> that it's not actually some kind of SSL over TCP. Also making it clear
> that it's mutually exclusive with OPT_TLS.
>
> Add "OPT_TLS_INSECURE" that implements the new certificate-check
> disabled TLS mode, which is also mutually exclusive with the other
> TLS options.
>
> PortAllocator: Add a new TLS policy enum TlsCertPolicy which defines
> the new insecure mode and added it as a RelayCredentials member.
>
> TurnPort: Add new TLS policy member with appropriate getter and setter
> to avoid constructor bloat. Initialize it from the RelayCredentials
> after the TurnPort is created.
>
> Expose the new feature in the PeerConnection API via
> IceServer.tls_certificate_policy as well as via the Android JNI
> PeerConnection API.
>
> For security reasons we ensure that:
>
> 1) The policy is always explicitly initialized to secure.
> 2) API users have to explicitly integrate with the feature to
> use it, and will otherwise get no change in behavior.
> 3) The feature is not immediately exposed in non-native
> contexts. For example, disabling of certificate validation
> is not implemented via URI parsing since this would
> immediately allow it to be used from a web page.
>
> BUG=webrtc:6840
>
> Review-Url: https://codereview.webrtc.org/2557803002
> Cr-Commit-Position: refs/heads/master@{#15670}
> Committed: b0f04fdb9eTBR=pthatcher@webrtc.org,deadbeef@webrtc.org,hnsl@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6840
Review-Url: https://codereview.webrtc.org/2590153002
Cr-Commit-Position: refs/heads/master@{#15703}
(This is a re-upload of https://codereview.webrtc.org/2567243003/, the
CQ stopped working there.)
The previously used WebRtcSession::GetTransportStats did a synchronous
invoke per channel (voice, video, data) on the signaling thread to the
network thread - e.g. 3 blocking invokes.
It is replaced by WebRtcSession::GetStats[_s] which can be invoked on
the signaling thread or on any thread if a ChannelNamePairs argument is
present (provided by WebRtcSession::GetChannelNamePairs on the signaling
thread).
With these changes, and changes allowing the getting of certificates
from any thread, the RTCStatsCollector can turn the 3 blocking thread
invokes into 1 non-blocking invoke.
BUG=webrtc:6875, chromium:627816
Review-Url: https://codereview.webrtc.org/2583883002
Cr-Commit-Position: refs/heads/master@{#15672}
Refactor "OPT_SSLTCP" renaming it to "OPT_TLS_FAKE", making it clear
that it's not actually some kind of SSL over TCP. Also making it clear
that it's mutually exclusive with OPT_TLS.
Add "OPT_TLS_INSECURE" that implements the new certificate-check
disabled TLS mode, which is also mutually exclusive with the other
TLS options.
PortAllocator: Add a new TLS policy enum TlsCertPolicy which defines
the new insecure mode and added it as a RelayCredentials member.
TurnPort: Add new TLS policy member with appropriate getter and setter
to avoid constructor bloat. Initialize it from the RelayCredentials
after the TurnPort is created.
Expose the new feature in the PeerConnection API via
IceServer.tls_certificate_policy as well as via the Android JNI
PeerConnection API.
For security reasons we ensure that:
1) The policy is always explicitly initialized to secure.
2) API users have to explicitly integrate with the feature to
use it, and will otherwise get no change in behavior.
3) The feature is not immediately exposed in non-native
contexts. For example, disabling of certificate validation
is not implemented via URI parsing since this would
immediately allow it to be used from a web page.
BUG=webrtc:6840
Review-Url: https://codereview.webrtc.org/2557803002
Cr-Commit-Position: refs/heads/master@{#15670}
We relied on the default destructor of RefCountedChannel to destroy its
members in reverse initialization order (deleting the DTLS wrapper
before the underlying ICE channel).
However, std::vector also may use the default assignment operator, which
performs a member-wise copy in initialization order. Which results in
deleting the ICE channel before the DTLS one. This CL fixes this by
using a vector of pointers instead of structures, and uses RefCountedObject
to handle ref-counting.
BUG=chromium:672951
Review-Url: https://codereview.webrtc.org/2571683004
Cr-Commit-Position: refs/heads/master@{#15583}
"Crypto required" is a property of the PeerConnection of construction
time; it has nothing to do with SDP. So I'm moving it out of
MediaContentDescription and putting it in the BaseChannel constructor
instead. This is more intuitive, and provides the added assurance that
"secure_required_" can't be flipped from "true" to "false".
BUG=None
Review-Url: https://codereview.webrtc.org/2537343003
Cr-Commit-Position: refs/heads/master@{#15579}
Also make supported protocols explicit in check.
Fix inconsistency where TLS_PROTOCOL_NAME was not exported.
BUG=webrtc:6885
Review-Url: https://codereview.webrtc.org/2570803003
Cr-Commit-Position: refs/heads/master@{#15577}
This CL doesn't start *using* a=bundle-only; it just adds support for
parsing it. We need to do this first, because otherwise old versions of
WebRTC will interpret a zero port value as a rejected m= section.
BUG=webrtc:4674
Review-Url: https://codereview.webrtc.org/2562183002
Cr-Commit-Position: refs/heads/master@{#15558}
Changing the configuration will cause subsequently generated offers to change
the ufrag/pwd as necessary, so that a new round of gathering is started that
uses the new configuration.
This CL also makes some minor unrelated changes: changing the reference SDP in
the PC tests to more match what we generate, and relaxing the network thread
requirement for JsepTransport (since there's no reason the "needs-ice-restart"
flag can't be accessed from the signaling thread).
BUG=webrtc:6714
Review-Url: https://codereview.webrtc.org/2563153002
Cr-Commit-Position: refs/heads/master@{#15527}
Their base class, Transport, still exists, but it now has a more specific
role: a helper class that applies TransportDescriptions. And is renamed
to JsepTransport as a result.
TransportController is now the entity primarily responsible for managing
TransportChannels. It also starts storing pointers to the DTLS and ICE
chanels separately, which will make it easier to remove
TransportChannel/TransportChannelImpl in a subsequent CL.
BUG=None
Review-Url: https://codereview.webrtc.org/2517883002
Cr-Commit-Position: refs/heads/master@{#15453}
Reason for revert:
Deletion of transport.h broke downstream builds.
Going to reland with transport.h containing enums/etc.
Original issue's description:
> Refactoring that removes P2PTransport and DtlsTransport classes.
>
> Their base class, Transport, still exists, but it now has a more specific
> role: a helper class that applies TransportDescriptions. And is renamed
> to JsepTransport as a result.
>
> TransportController is now the entity primarily responsible for managing
> TransportChannels. It also starts storing pointers to the DTLS and ICE
> chanels separately, which will make it easier to remove
> TransportChannel/TransportChannelImpl in a subsequent CL.
>
> BUG=None
>
> Committed: https://crrev.com/bd28681d02dee8c185aeb39207e8154f0ad14a37
> Cr-Commit-Position: refs/heads/master@{#15450}
TBR=pthatcher@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=None
Review-Url: https://codereview.webrtc.org/2553043004
Cr-Commit-Position: refs/heads/master@{#15452}
Their base class, Transport, still exists, but it now has a more specific
role: a helper class that applies TransportDescriptions. And is renamed
to JsepTransport as a result.
TransportController is now the entity primarily responsible for managing
TransportChannels. It also starts storing pointers to the DTLS and ICE
chanels separately, which will make it easier to remove
TransportChannel/TransportChannelImpl in a subsequent CL.
BUG=None
Review-Url: https://codereview.webrtc.org/2517883002
Cr-Commit-Position: refs/heads/master@{#15450}
Some applications explicitly require RFC3550 style RTP without ICE.
Port number requirement of RFC3550 section 11 will be addressed in a follow-up CL.
BUG=webrtc:6436
Review-Url: https://codereview.webrtc.org/2377883003
Cr-Commit-Position: refs/heads/master@{#15005}
When the selected connection becomes not receiving and there are many connections,
If we use a round-robin fashion to ping all connections, none of the connections will
be in receiving state for sufficient long time to ensure switching connections promptly.
Triggered check will help in this situation to some extent but it may still fail to switch promptly when there are a lot of connections.
With this CL, if the selected connection is weak, once we find a writable connection on a network we start to ping it with a higher priority to keep it in receiving state.
Plus, if the selected connection is weak, we choose a shorter ping interval (900ms) for all writable connections.
BUG=b/32022719
Review-Url: https://codereview.webrtc.org/2369963004
Cr-Commit-Position: refs/heads/master@{#14991}
Writable() and the related signal are already part of rtc::PacketTransportInterface. Sense of code symmetry aesthetics dictates that receiving() and the related signal should be declared in the same place.
BUG=webrtc:6531
Review-Url: https://codereview.webrtc.org/2444793003
Cr-Commit-Position: refs/heads/master@{#14865}
The loopback range is 127.0.0.0/8, which is everything from 127.0.0.0 to
127.255.255.255.
BUG=chromium:649118
Review-Url: https://codereview.webrtc.org/2445933003
Cr-Commit-Position: refs/heads/master@{#14807}
Introduce rtc::PacketTransportInterface. Refactor cricket::TransportChannel.
Fix signal slots parameter types in all related code.
BUG=webrtc:6531
Review-Url: https://codereview.webrtc.org/2416023002
Cr-Commit-Position: refs/heads/master@{#14778}
In the swarming test, the machines sometimes were blocked for 1-2 seconds without processing anything.
This CL makes sure that 1 second timeout is only used with fake clock.
BUG=webrtc:6500
Review-Url: https://codereview.webrtc.org/2442813002
Cr-Commit-Position: refs/heads/master@{#14756}
This can be used for a certain security exploit, and doesn't have any
other practical applications we know of.
BUG=chromium:649118
Review-Url: https://codereview.webrtc.org/2440043004
Cr-Commit-Position: refs/heads/master@{#14751}
A rtc::PacketTransportInterface typedef is introduced to allow preparing
downstream projects for the upcoming refactoring of
cricket::Transport. This refactoring will introduce
rtc::PacketTransportInterface in https://codereview.webrtc.org/2416023002/ .
BUG=webrtc:6531
Review-Url: https://codereview.webrtc.org/2429803002
Cr-Commit-Position: refs/heads/master@{#14672}
Reason for revert:
Breaks upstream code.
Original issue's description:
> Prune connections based on network name.
> Previously we prune connections on the same network pointer.
> So if an IPv6 and an IPv4 network are on the same network interface, IPv4 connection won't be pruned even if an IPv6 connection with higher priority becomes writable.
>
> With this change, as long as one connection becomes writable, all connections having lower priority with the same network name will be pruned.
>
> Also simplify the implementation.
>
> BUG=webrtc:6512
>
> Committed: https://crrev.com/aae2784c1fab9d1510393dec15d76caa574e2da8
> Cr-Commit-Position: refs/heads/master@{#14593}
TBR=skvlad@webrtc.org,honghaiz@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6512
Review-Url: https://codereview.webrtc.org/2412433003
Cr-Commit-Position: refs/heads/master@{#14601}
Previously we prune connections on the same network pointer.
So if an IPv6 and an IPv4 network are on the same network interface, IPv4 connection won't be pruned even if an IPv6 connection with higher priority becomes writable.
With this change, as long as one connection becomes writable, all connections having lower priority with the same network name will be pruned.
Also simplify the implementation.
BUG=webrtc:6512
Review-Url: https://codereview.webrtc.org/2395243005
Cr-Commit-Position: refs/heads/master@{#14593}
The Connection class will now blindly forward SignalReadyToSend, and
P2PTransportChannel will decide whether to forward it further (which
it was already doing).
BUG=webrtc:6448
Review-Url: https://codereview.webrtc.org/2374183005
Cr-Commit-Position: refs/heads/master@{#14462}
This means the DTLS handshake can make progress while the SDP answer
containing the fingerprint is still in transit. If the signaling path
if significantly slower than the media path, this can have a moderate
impact on call setup time.
Of course, until the fingerprint is verified no media can be sent. Any
attempted write will result in SR_BLOCK.
This essentially fulfills the requirements of RFC 4572, Section 6.2:
Note that when the offer/answer model is being used, it is possible
for a media connection to outrace the answer back to the offerer.
Thus, if the offerer has offered a 'setup:passive' or 'setup:actpass'
role, it MUST (as specified in RFC 4145 [2]) begin listening for an
incoming connection as soon as it sends its offer. However, it MUST
NOT assume that the data transmitted over the TLS connection is valid
until it has received a matching fingerprint in an SDP answer. If
the fingerprint, once it arrives, does not match the client's
certificate, the server endpoint MUST terminate the media connection
with a bad_certificate error, as stated in the previous paragraph.
BUG=webrtc:6387
Review-Url: https://codereview.webrtc.org/2163683003
Cr-Commit-Position: refs/heads/master@{#14461}
Reason for revert:
It caused some tests in p2ptransportchannel flaky.
Original issue's description:
> Replace RelayPort with TurnPort in p2ptransportchannel tests.
>
> Also remove the relay servers in the tests.
> Most of the code and the downstream apps are using TurnPort, not RelayPort. Most of the tests in this file are not using RelayPort anyway.
>
> BUG=None
> R=deadbeef@webrtc.org
>
> Committed: https://crrev.com/c8d21712dde64c7d613d1ea56c840438505a909f
> Cr-Commit-Position: refs/heads/master@{#14441}
TBR=deadbeef@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=None
Review-Url: https://codereview.webrtc.org/2385563002
Cr-Commit-Position: refs/heads/master@{#14443}
Also remove the relay servers in the tests.
Most of the code and the downstream apps are using TurnPort, not RelayPort. Most of the tests in this file are not using RelayPort anyway.
BUG=None
R=deadbeef@webrtc.org
Review URL: https://codereview.webrtc.org/2380923002 .
Cr-Commit-Position: refs/heads/master@{#14441}
Reason for revert:
Broke a downstream user of SSLStreamAdapter. Need to add the new interface (returning error code instead of bool) in a backwards compatible way.
Original issue's description:
> Allow the DTLS fingerprint verification to occur after the handshake.
>
> This means the DTLS handshake can make progress while the SDP answer
> containing the fingerprint is still in transit. If the signaling path
> if significantly slower than the media path, this can have a moderate
> impact on call setup time.
>
> Of course, until the fingerprint is verified no media can be sent. Any
> attempted write will result in SR_BLOCK.
>
> This essentially fulfills the requirements of RFC 4572, Section 6.2:
>
> Note that when the offer/answer model is being used, it is possible
> for a media connection to outrace the answer back to the offerer.
> Thus, if the offerer has offered a 'setup:passive' or 'setup:actpass'
> role, it MUST (as specified in RFC 4145 [2]) begin listening for an
> incoming connection as soon as it sends its offer. However, it MUST
> NOT assume that the data transmitted over the TLS connection is valid
> until it has received a matching fingerprint in an SDP answer. If
> the fingerprint, once it arrives, does not match the client's
> certificate, the server endpoint MUST terminate the media connection
> with a bad_certificate error, as stated in the previous paragraph.
>
> BUG=webrtc:6387
> R=mattdr@webrtc.org, pthatcher@webrtc.org
>
> Committed: https://crrev.com/042041bf9585f92e962387c59ca805f1218338f9
> Cr-Commit-Position: refs/heads/master@{#14296}
TBR=pthatcher@webrtc.org,mattdr@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6387
Review-Url: https://codereview.webrtc.org/2352863003
Cr-Commit-Position: refs/heads/master@{#14298}
This means the DTLS handshake can make progress while the SDP answer
containing the fingerprint is still in transit. If the signaling path
if significantly slower than the media path, this can have a moderate
impact on call setup time.
Of course, until the fingerprint is verified no media can be sent. Any
attempted write will result in SR_BLOCK.
This essentially fulfills the requirements of RFC 4572, Section 6.2:
Note that when the offer/answer model is being used, it is possible
for a media connection to outrace the answer back to the offerer.
Thus, if the offerer has offered a 'setup:passive' or 'setup:actpass'
role, it MUST (as specified in RFC 4145 [2]) begin listening for an
incoming connection as soon as it sends its offer. However, it MUST
NOT assume that the data transmitted over the TLS connection is valid
until it has received a matching fingerprint in an SDP answer. If
the fingerprint, once it arrives, does not match the client's
certificate, the server endpoint MUST terminate the media connection
with a bad_certificate error, as stated in the previous paragraph.
BUG=webrtc:6387
R=mattdr@webrtc.org, pthatcher@webrtc.org
Review URL: https://codereview.webrtc.org/2163683003 .
Cr-Commit-Position: refs/heads/master@{#14296}
It may happen in some legitimate scenarios.
For example a turn port may have had a refresh request timeout, so it won't create a new connection for a ping from an unknown address.
R=deadbeef@webrtc.org
Review URL: https://codereview.webrtc.org/2327233002 .
Cr-Commit-Position: refs/heads/master@{#14173}
