Commit Graph

29 Commits

Author SHA1 Message Date
89824f6fe0 Relanding: 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

Review-Url: https://codereview.webrtc.org/2163683003
Cr-Commit-Position: refs/heads/master@{#14461}
2016-09-30 18:55:49 +00:00
81f6f4fc56 Revert of Allow the DTLS fingerprint verification to occur after the handshake. (patchset #11 id:200001 of https://codereview.webrtc.org/2163683003/ )
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}
2016-09-20 00:21:00 +00:00
042041bf95 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

Review URL: https://codereview.webrtc.org/2163683003 .

Cr-Commit-Position: refs/heads/master@{#14296}
2016-09-19 23:02:35 +00:00
d82eee0675 Log how often DTLS negotiation failed because of incompatible ciphersuites.
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}
2016-08-26 18:25:09 +00:00
c8762a838f Remove StartSSLWithServer from SSLStreamAdapter.
It's not used by anything any more. We only use SSLStreamAdapter in
the mode where it verifies the peer's certificate using a signaled
digest.

R=pthatcher@webrtc.org, zhihuang@webrtc.org

Review URL: https://codereview.webrtc.org/2204883004 .

Cr-Commit-Position: refs/heads/master@{#13730}
2016-08-11 19:01:58 +00:00
cb56065c62 Add support for GCM cipher suites from RFC 7714.
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}
2016-08-04 12:20:38 +00:00
4f0dfbd213 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=pthatcher@webrtc.org, juberti@chromium.org, juberti@webrtc.org

Review URL: https://codereview.webrtc.org/1981463002 .

Committed: https://crrev.com/1e435628366fb9fed71632369f05928ed857d8ef
Cr-Original-Commit-Position: refs/heads/master@{#12853}
Cr-Commit-Position: refs/heads/master@{#13159}
2016-06-16 00:15:35 +00:00
555604a746 Replace scoped_ptr with unique_ptr in webrtc/base/
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}
2016-04-26 10:13:28 +00:00
4b9d1dec5a OpenSSL/BoringSSL compatibility fixes.
With this CL, legacy OpenSSL should work again.

BUG=webrtc:5714

Review URL: https://codereview.webrtc.org/1868033005

Cr-Commit-Position: refs/heads/master@{#12300}
2016-04-09 18:35:35 +00:00
b4d01c4ded A bunch of interfaces: Return scoped_ptr<SSLCertificate>
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}
2016-04-06 12:15:11 +00:00
43166b8adf Add IsAcceptableCipher, use instead of GetDefaultCipher.
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}
2016-03-11 08:06:55 +00:00
521ed7bf02 Reland Convert internal representation of Srtp cryptos from string to int
TBR=pthatcher@webrtc.org
BUG=webrtc:5043

Review URL: https://codereview.webrtc.org/1458023002 .

Cr-Commit-Position: refs/heads/master@{#10703}
2015-11-19 03:42:00 +00:00
318166bed7 Revert of Convert internal representation of Srtp cryptos from string to int. (patchset #10 id:180001 of https://codereview.webrtc.org/1416673006/ )
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}
2015-11-19 03:03:46 +00:00
2764e1027a 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

Review URL: https://codereview.webrtc.org/1416673006

Cr-Commit-Position: refs/heads/master@{#10701}
2015-11-19 02:02:40 +00:00
0c4e06b4c6 Use suffixed {uint,int}{8,16,32,64}_t types.
Removes the use of uint8, etc. in favor of uint8_t.

BUG=webrtc:5024
R=henrik.lundin@webrtc.org, henrikg@webrtc.org, perkj@webrtc.org, solenberg@webrtc.org, stefan@webrtc.org, tina.legrand@webrtc.org

Review URL: https://codereview.webrtc.org/1362503003 .

Cr-Commit-Position: refs/heads/master@{#10196}
2015-10-07 10:23:32 +00:00
6caafbe5b6 Convert uint16_t to int for WebRTC cipher/crypto suite.
This is a follow up CL on https://codereview.webrtc.org/1337673002

BUG=
R=pthatcher@webrtc.org

Review URL: https://codereview.webrtc.org/1377733004 .

Cr-Commit-Position: refs/heads/master@{#10175}
2015-10-05 19:43:33 +00:00
456696a9c1 Reland Change WebRTC SslCipher to be exposed as number only
This is to revert the change of https://codereview.webrtc.org/1380603005/

TBR=pthatcher@webrtc.org
BUG=523033

Review URL: https://codereview.webrtc.org/1375543003 .

Cr-Commit-Position: refs/heads/master@{#10126}
2015-10-01 04:49:02 +00:00
27dc29b0df Revert of Change WebRTC SslCipher to be exposed as number only. (patchset #20 id:750001 of https://codereview.webrtc.org/1337673002/ )
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}
2015-10-01 02:23:15 +00:00
4fe3c9a773 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

Review URL: https://codereview.webrtc.org/1337673002

Cr-Commit-Position: refs/heads/master@{#10124}
2015-10-01 01:49:17 +00:00
b6d4ec4185 Support generation of EC keys using P256 curve and support ECDSA certs.
This CL started life here: https://webrtc-codereview.appspot.com/51189004

BUG=webrtc:4685, webrtc:4686
R=hbos@webrtc.org, juberti@webrtc.org

Review URL: https://codereview.webrtc.org/1189583002 .

Cr-Commit-Position: refs/heads/master@{#9718}
2015-08-17 12:09:10 +00:00
831c5585c7 Allow setting maximum protocol version for SSL stream adapters.
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}
2015-05-20 10:48:24 +00:00
67186fe00c Fix clang style warnings in webrtc/base
Mostly this consists of marking functions with override when
applicable, and moving function bodies from .h to .cc files.

Not inlining virtual functions with simple bodies such as

  { return false; }

strikes me as probably losing more in readability than we gain in
binary size and compilation time, but I guess it's just like any other
case where enabling a generally good warning forces us to write
slightly worse code in a couple of places.

BUG=163
R=kjellander@webrtc.org, tommi@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/47429004

Cr-Commit-Position: refs/heads/master@{#8656}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8656 4adac7df-926f-26a2-2b94-8c16560cd09d
2015-03-09 22:24:25 +00:00
3ee4fe5a94 Re-land: Add API to get negotiated SSL ciphers
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.

The previously approved CL https://webrtc-codereview.appspot.com/26009004/ was reverted in https://webrtc-codereview.appspot.com/40689004/ due to compilation issues while rolling into Chromium.
As the new method has landed in Chromium in https://crrev.com/bc321c76ace6e1d5a03440e554ccb207159802ec, this should be safe to land here now.

BUG=3976
R=pthatcher@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/37209004

Cr-Commit-Position: refs/heads/master@{#8343}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8343 4adac7df-926f-26a2-2b94-8c16560cd09d
2015-02-11 22:35:30 +00:00
2bf0e90c9d Revert 8275 "This CL adds an API to the SSL stream adapters and ..."
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/26009004

TBR=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
2015-02-07 11:13:18 +00:00
1d11c8202b 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/26009004

Cr-Commit-Position: refs/heads/master@{#8275}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8275 4adac7df-926f-26a2-2b94-8c16560cd09d
2015-02-06 19:47:39 +00:00
c569a49a3d Unit tests for SSLAdapter
R=juberti@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/17309004

Patch from Manish Jethani <manish.jethani@gmail.com>.

git-svn-id: http://webrtc.googlecode.com/svn/trunk@7269 4adac7df-926f-26a2-2b94-8c16560cd09d
2014-09-23 05:56:44 +00:00
f048872e91 Adds a modified copy of talk/base to webrtc/base. It is the first step in
migrating talk/base to webrtc/base.

BUG=N/A
R=niklas.enbom@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/17479005

git-svn-id: http://webrtc.googlecode.com/svn/trunk@6129 4adac7df-926f-26a2-2b94-8c16560cd09d
2014-05-13 18:00:26 +00:00
e9a604accd Revert 6107 "Adds a modified copy of talk/base to webrtc/base. I..."
This breaks Chromium FYI builds and prevent roll of webrtc/libjingle to Chrome.

http://chromegw.corp.google.com/i/chromium.webrtc.fyi/builders/Win%20Builder/builds/457


> Adds a modified copy of talk/base to webrtc/base. It is the first step in migrating talk/base to webrtc/base.
> 
> BUG=N/A
> R=andrew@webrtc.org, wu@webrtc.org
> 
> Review URL: https://webrtc-codereview.appspot.com/12199004

TBR=henrike@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/14479004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@6116 4adac7df-926f-26a2-2b94-8c16560cd09d
2014-05-13 08:15:48 +00:00
2c7d1b39b9 Adds a modified copy of talk/base to webrtc/base. It is the first step in migrating talk/base to webrtc/base.
BUG=N/A
R=andrew@webrtc.org, wu@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/12199004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@6107 4adac7df-926f-26a2-2b94-8c16560cd09d
2014-05-12 18:03:09 +00:00