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:
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}
Log the DTLS handshake error code in OpenSSLStreamAdapter.
Forward the error code to WebRTCSession with the Signals.
This part is only for the WebRTC native code.
To make it work, need another CL for Chromium.
BUG=webrtc:5959
Review-Url: https://codereview.webrtc.org/2167363002
Cr-Commit-Position: refs/heads/master@{#13940}
GCM cipher suites are optional (disabled by default) and can be enabled
through "PeerConnectionFactoryInterface::Options".
If compiled with Chromium (i.e. "ENABLE_EXTERNAL_AUTH" is defined), no
GCM ciphers can be used yet (see https://crbug.com/628400).
BUG=webrtc:5222, 628400
Review-Url: https://codereview.webrtc.org/1528843005
Cr-Commit-Position: refs/heads/master@{#13635}
Every message will now be traced with the location from which it was
posted, including function name, file and line number.
This CL also writes a normal LOG message when the dispatch took more
than a certain amount of time (currently 50ms).
This logging should help us identify messages that are taking
longer than expected to be dispatched.
R=pthatcher@webrtc.org, tommi@webrtc.org
Review URL: https://codereview.webrtc.org/2019423006 .
Cr-Commit-Position: refs/heads/master@{#13104}
Reason for revert:
Seems like this CL cause
DtlsTransportChannelTest.TestReceiveClientHelloBeforeRemoteFingerprint
DtlsTransportChannelTest.TestReceiveClientHelloBeforeWritable
to consistently fail on Win DrMemory Full and for
DtlsTransportChannelTest.TestReceiveClientHelloBeforeRemoteFingerprint
DtlsTransportChannelTest.TestReceiveClientHelloBeforeWritable
to consistently fail on Linux Memcheck
Original issue's description:
> Change initial DTLS retransmission timer from 1 second to 50ms.
>
> This will help ensure a timely DTLS handshake when there's packet
> loss. It will likely result in spurious retransmissions (since the
> RTT is usually > 50ms), but since exponential backoff is still used,
> there will at most be ~4 extra retransmissions. For a time-sensitive
> application like WebRTC this seems like a reasonable tradeoff.
>
> R=juberti@chromium.org, juberti@webrtc.org, pthatcher@webrtc.org
>
> Committed: https://crrev.com/1e435628366fb9fed71632369f05928ed857d8ef
> Cr-Commit-Position: refs/heads/master@{#12853}
TBR=pthatcher@webrtc.org,juberti@webrtc.org,juberti@chromium.org,deadbeef@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Review-Url: https://codereview.webrtc.org/2002403002
Cr-Commit-Position: refs/heads/master@{#12864}
This will help ensure a timely DTLS handshake when there's packet
loss. It will likely result in spurious retransmissions (since the
RTT is usually > 50ms), but since exponential backoff is still used,
there will at most be ~4 extra retransmissions. For a time-sensitive
application like WebRTC this seems like a reasonable tradeoff.
R=juberti@chromium.org, juberti@webrtc.org, pthatcher@webrtc.org
Review URL: https://codereview.webrtc.org/1981463002 .
Cr-Commit-Position: refs/heads/master@{#12853}
This propagated into various other places. Also had to #include headers that
were implicitly pulled by "scoped_ptr.h".
BUG=webrtc:5520
Review URL: https://codereview.webrtc.org/1920043002
Cr-Commit-Position: refs/heads/master@{#12501}
Instead of using a raw pointer output parameter. This affects
SSLStreamAdapter::GetPeerCertificate
Transport::GetRemoteSSLCertificate
TransportChannel::GetRemoteSSLCertificate
TransportController::GetRemoteSSLCertificate
WebRtcSession::GetRemoteSSLCertificate
This is a good idea in general, but will also be very convenient when
scoped_ptr is gone, since unique_ptr doesn't have an .accept() method.
BUG=webrtc:5520
Review URL: https://codereview.webrtc.org/1802013002
Cr-Commit-Position: refs/heads/master@{#12262}
Reason for revert:
We will make it possible to link to BoringSSL for WebRTC's usages of the crypto APIs and OpenSSL for other usages in the same binary. Once that is completed, we will reland this.
Original issue's description:
> Remove code interfacing legacy openssl.
>
> BUG=webrtc:5664
>
> Committed: https://crrev.com/4cd331beade6de16c073dcdaf89c4e038bdbf73f
> Cr-Commit-Position: refs/heads/master@{#12041}
TBR=tommi@webrtc.org,davidben@webrtc.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=webrtc:5664
Review URL: https://codereview.webrtc.org/1828773003 .
Cr-Commit-Position: refs/heads/master@{#12117}
The old code insists on exact cipher suite matches with hardwired expectations. It does this matching parameterized with key type (RSA vs ECDSA) and TLS version (DTLS vs TLS and version 1.0 vs 1.2).
This CL changes things to check against a white-list of cipher suites, with the check parameterized with key type (again RSA vs ECDSA). Then separately checks TLS version since the old implicit check of TLS version by means of resulting cipher suite was too blunt.
Using a white list for cipher suites isn't perfect, but it is safe and requires minimal maintenance. It allows compatibility with not just one exact version of underlying crypto lib, but any version with reasonable defaults.
The CL also re-enables critical tests which had to be disabled recently to allow a boringssl roll.
BUG=webrtc:5634
Review URL: https://codereview.webrtc.org/1774583002
Cr-Commit-Position: refs/heads/master@{#11951}
This copies the contents (unittest excluded) of base/numerics in
chromium to base/numerics in webrtc. Files added:
- safe_conversions.h
- safe_conversions_impl.h
- safe_math.h
- safe_math_impl.h
A really old version of safe_conversions[_impl].h previously existed in
base/, this has been deleted and sources using it have been updated
to include the new base/numerics/safe_converions.h.
This CL also adds a DEPS file to webrtc/base.
NOPRESUBMIT=True
BUG=webrtc:5548, webrtc:5623
Review URL: https://codereview.webrtc.org/1753293002
Cr-Commit-Position: refs/heads/master@{#11907}
When remote side closes, opensslstreamadapter could return SR_EOS which will not trigger upper layer to clean up what's left in the StreamInterfaceChannel. The result of this is when there are more packets coming in, the Write on the StreamInterfaceChannel will overflow the buffer.
The fix here is that when receiving the remote side close signal, we also close the underneath StreamInterfaceChannel which will clean up the queue to prevent overflow.
BUG=574524
TBR=pthatcher@webrtc.org
Review URL: https://codereview.webrtc.org/1566023002
Cr-Commit-Position: refs/heads/master@{#11751}
Reason for revert:
Broke chromium fyi build.
Original issue's description:
> Convert internal representation of Srtp cryptos from string to int.
>
> Note that the coversion from int to string happens in 3 places
> 1) SDP layer from int to external names. mediasession.cc GetSupportedSuiteNames.
> 2) for SSL_CTX_set_tlsext_use_srtp(), converting from int to internal names.
> 3) stats collection also needs external names.
>
> External names are like AES_CM_128_HMAC_SHA1_80, specified in sslstreamadapter.cc.
> Internal names are like SRTP_AES128_CM_SHA1_80, specified in opensslstreamadapter.cc.
>
> The conversion from string to int happens in one place only, SDP layer, SrtpFilter::ApplyParams().
>
> BUG=webrtc:5043
>
> Committed: https://crrev.com/2764e1027a08a5543e04b854a27a520801faf6eb
> Cr-Commit-Position: refs/heads/master@{#10701}
TBR=juberti@webrtc.org,pthatcher@webrtc.org,juberti@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5043
Review URL: https://codereview.webrtc.org/1455233005
Cr-Commit-Position: refs/heads/master@{#10702}
Note that the coversion from int to string happens in 3 places
1) SDP layer from int to external names. mediasession.cc GetSupportedSuiteNames.
2) for SSL_CTX_set_tlsext_use_srtp(), converting from int to internal names.
3) stats collection also needs external names.
External names are like AES_CM_128_HMAC_SHA1_80, specified in sslstreamadapter.cc.
Internal names are like SRTP_AES128_CM_SHA1_80, specified in opensslstreamadapter.cc.
The conversion from string to int happens in one place only, SDP layer, SrtpFilter::ApplyParams().
BUG=webrtc:5043
Review URL: https://codereview.webrtc.org/1416673006
Cr-Commit-Position: refs/heads/master@{#10701}
In preparation for implementing the standardized variant of CHACHA20_POLY1305
(it changed slightly in the standardization process),
TLS1_CK_ECDHE_RSA_CHACHA20_POLY1305 and TLS1_CK_ECDHE_ECDSA_CHACHA20_POLY1305
were renamed to have an _OLD suffix with compatibility unsuffixed #defines
temporarily available.
Update references to include the _OLD suffixed ones. Once we've cycled through
the few consumers of the unsuffixed names (just WebRTC and QUIC), the unsuffixed
names can refer to the to-be-implemented standardized variant and eventually
the draft version will be removed.
(This has no effect on upstream OpenSSL compatibility as OpenSSL never defined
these symbols to begin with. Though probably they will once standardization is
done.)
BUG=none
Review URL: https://codereview.webrtc.org/1412803010
Cr-Commit-Position: refs/heads/master@{#10681}
Reason for revert:
This broke chromium.fyi bot.
Original issue's description:
> Change WebRTC SslCipher to be exposed as number only.
>
> This makes the SSL exposed as uint16_t which is the IANA value. GetRfcSslCipherName is introduced to handle the conversion to names from ID. IANA value will be used for UMA reporting. Names will still be used for WebRTC stats reporting.
>
> For SRTP, currently it's still string internally but is reported as IANA number.
>
> This is used by the ongoing CL https://codereview.chromium.org/1335023002.
>
> BUG=523033
>
> Committed: https://crrev.com/4fe3c9a77386598db9abd1f0d6983aefee9cc943
> Cr-Commit-Position: refs/heads/master@{#10124}
TBR=juberti@webrtc.org,rsleevi@chromium.org,pthatcher@webrtc.org,davidben@chromium.org,juberti@google.com,davidben@webrtc.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=523033
Review URL: https://codereview.webrtc.org/1380603005
Cr-Commit-Position: refs/heads/master@{#10125}
This makes the SSL exposed as uint16_t which is the IANA value. GetRfcSslCipherName is introduced to handle the conversion to names from ID. IANA value will be used for UMA reporting. Names will still be used for WebRTC stats reporting.
For SRTP, currently it's still string internally but is reported as IANA number.
This is used by the ongoing CL https://codereview.chromium.org/1335023002.
BUG=523033
Review URL: https://codereview.webrtc.org/1337673002
Cr-Commit-Position: refs/heads/master@{#10124}
Reason for revert:
It looks like this broke the FYI bots. I tried updating libjingle_nacl.gyp, but the IOS build still failed because in Chrome it's configured to use NSS. See https://codereview.chromium.org/1316863012/.
Original issue's description:
> purge nss files and dependencies
>
> BUG=webrtc:4497
>
> Committed: https://crrev.com/5647a2cf3db888195c928a1259d98f72f6ecbc15
> Cr-Commit-Position: refs/heads/master@{#9862}
TBR=tommi@webrtc.org,kjellander@webrtc.org,torbjorng@webrtc.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:4497
Review URL: https://codereview.webrtc.org/1311843006
Cr-Commit-Position: refs/heads/master@{#9867}
This CL adds an API to SSL stream adapters to set the maximum allowed
protocol version and with that implements support for DTLS 1.2.
With DTLS 1.2 the default cipher changes in the unittests as follows.
BoringSSL
TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA -> TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
NSS
TLS_RSA_WITH_AES_128_CBC_SHA -> TLS_RSA_WITH_AES_128_GCM_SHA256
BUG=chromium:428343
R=juberti@google.com
Review URL: https://webrtc-codereview.appspot.com/50989004
Cr-Commit-Position: refs/heads/master@{#9232}
I'm reverting the patch due to compilation issues. It would be great if we could make sure Chromium is ready for the patch before we land it in WebRTC.
As is, we're trying to roll webrtc into Chromium and we can't (this is not the only reason though). I might reland this after the roll, depending on how that goes though.
Here's an example failure:
e:\b\build\slave\win_gn\build\src\jingle\glue\channel_socket_adapter_unittest.cc(77) : error C2259: 'jingle_glue::MockTransportChannel' : cannot instantiate abstract class
due to following members:
'bool cricket::TransportChannel::GetSslCipher(std::string *)' : is abstract
e:\b\build\slave\win_gn\build\src\third_party\webrtc\p2p\base\transportchannel.h(107) : see declaration of 'cricket::TransportChannel::GetSslCipher'
ninja: build stopped: subcommand failed.
> This CL adds an API to the SSL stream adapters and transport channels to get the SSL cipher that was negotiated with the remote peer.
>
> BUG=3976
> R=davidben@chromium.org, juberti@webrtc.org, pthatcher@webrtc.org
>
> Review URL: https://webrtc-codereview.appspot.com/26009004TBR=pthatcher@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/40689004
Cr-Commit-Position: refs/heads/master@{#8282}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8282 4adac7df-926f-26a2-2b94-8c16560cd09d
