Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(138)

Side by Side Diff: webrtc/p2p/base/jseptransport.cc

Issue 2770903003: Accept remote offers with current DTLS role, rather than "actpass". (Closed)
Patch Set: Adding link to spec and section number. Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « webrtc/p2p/base/jseptransport.h ('k') | webrtc/p2p/base/jseptransport_unittest.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 /* 1 /*
2 * Copyright 2004 The WebRTC Project Authors. All rights reserved. 2 * Copyright 2004 The WebRTC Project Authors. All rights reserved.
3 * 3 *
4 * Use of this source code is governed by a BSD-style license 4 * Use of this source code is governed by a BSD-style license
5 * that can be found in the LICENSE file in the root of the source 5 * that can be found in the LICENSE file in the root of the source
6 * tree. An additional intellectual property rights grant can be found 6 * tree. An additional intellectual property rights grant can be found
7 * in the file PATENTS. All contributing project authors may 7 * in the file PATENTS. All contributing project authors may
8 * be found in the AUTHORS file in the root of the source tree. 8 * be found in the AUTHORS file in the root of the source tree.
9 */ 9 */
10 10
(...skipping 260 matching lines...) Expand 10 before | Expand all | Expand 10 after
271 if (!needs_ice_restart_) { 271 if (!needs_ice_restart_) {
272 needs_ice_restart_ = true; 272 needs_ice_restart_ = true;
273 LOG(LS_VERBOSE) << "needs-ice-restart flag set for transport " << mid(); 273 LOG(LS_VERBOSE) << "needs-ice-restart flag set for transport " << mid();
274 } 274 }
275 } 275 }
276 276
277 bool JsepTransport::NeedsIceRestart() const { 277 bool JsepTransport::NeedsIceRestart() const {
278 return needs_ice_restart_; 278 return needs_ice_restart_;
279 } 279 }
280 280
281 void JsepTransport::GetSslRole(rtc::SSLRole* ssl_role) const { 281 rtc::Optional<rtc::SSLRole> JsepTransport::GetSslRole() const {
282 RTC_DCHECK(ssl_role); 282 return ssl_role_;
283 *ssl_role = secure_role_;
284 } 283 }
285 284
286 bool JsepTransport::GetStats(TransportStats* stats) { 285 bool JsepTransport::GetStats(TransportStats* stats) {
287 stats->transport_name = mid(); 286 stats->transport_name = mid();
288 stats->channel_stats.clear(); 287 stats->channel_stats.clear();
289 for (auto& kv : channels_) { 288 for (auto& kv : channels_) {
290 DtlsTransportInternal* dtls_transport = kv.second; 289 DtlsTransportInternal* dtls_transport = kv.second;
291 TransportChannelStats substats; 290 TransportChannelStats substats;
292 substats.component = kv.first; 291 substats.component = kv.first;
293 dtls_transport->GetSrtpCryptoSuite(&substats.srtp_crypto_suite); 292 dtls_transport->GetSrtpCryptoSuite(&substats.srtp_crypto_suite);
(...skipping 41 matching lines...) Expand 10 before | Expand all | Expand 10 after
335 if (certificate_) { 334 if (certificate_) {
336 ret = dtls_transport->SetLocalCertificate(certificate_); 335 ret = dtls_transport->SetLocalCertificate(certificate_);
337 RTC_DCHECK(ret); 336 RTC_DCHECK(ret);
338 } 337 }
339 return ret; 338 return ret;
340 } 339 }
341 340
342 bool JsepTransport::ApplyRemoteTransportDescription( 341 bool JsepTransport::ApplyRemoteTransportDescription(
343 DtlsTransportInternal* dtls_transport, 342 DtlsTransportInternal* dtls_transport,
344 std::string* error_desc) { 343 std::string* error_desc) {
345 // Currently, all ICE-related calls still go through this DTLS channel. But
346 // that will change once we get rid of TransportChannelImpl, and the DTLS
347 // channel interface no longer includes ICE-specific methods. Then this class
348 // will need to call dtls->ice()->SetIceRole(), for example, assuming the Dtls
349 // interface will expose its inner ICE channel.
350 dtls_transport->ice_transport()->SetRemoteIceParameters( 344 dtls_transport->ice_transport()->SetRemoteIceParameters(
351 remote_description_->GetIceParameters()); 345 remote_description_->GetIceParameters());
352 dtls_transport->ice_transport()->SetRemoteIceMode( 346 dtls_transport->ice_transport()->SetRemoteIceMode(
353 remote_description_->ice_mode); 347 remote_description_->ice_mode);
354 return true; 348 return true;
355 } 349 }
356 350
357 bool JsepTransport::ApplyNegotiatedTransportDescription( 351 bool JsepTransport::ApplyNegotiatedTransportDescription(
358 DtlsTransportInternal* dtls_transport, 352 DtlsTransportInternal* dtls_transport,
359 std::string* error_desc) { 353 std::string* error_desc) {
360 // Set SSL role. Role must be set before fingerprint is applied, which 354 // Set SSL role. Role must be set before fingerprint is applied, which
361 // initiates DTLS setup. 355 // initiates DTLS setup.
362 if (!dtls_transport->SetSslRole(secure_role_)) { 356 if (ssl_role_ && !dtls_transport->SetSslRole(*ssl_role_)) {
363 return BadTransportDescription("Failed to set SSL role for the channel.", 357 return BadTransportDescription("Failed to set SSL role for the channel.",
364 error_desc); 358 error_desc);
365 } 359 }
366 // Apply remote fingerprint. 360 // Apply remote fingerprint.
367 if (!dtls_transport->SetRemoteFingerprint( 361 if (!dtls_transport->SetRemoteFingerprint(
368 remote_fingerprint_->algorithm, 362 remote_fingerprint_->algorithm,
369 reinterpret_cast<const uint8_t*>(remote_fingerprint_->digest.data()), 363 reinterpret_cast<const uint8_t*>(remote_fingerprint_->digest.data()),
370 remote_fingerprint_->digest.size())) { 364 remote_fingerprint_->digest.size())) {
371 return BadTransportDescription("Failed to apply remote fingerprint.", 365 return BadTransportDescription("Failed to apply remote fingerprint.",
372 error_desc); 366 error_desc);
373 } 367 }
374 return true; 368 return true;
375 } 369 }
376 370
377 bool JsepTransport::NegotiateTransportDescription(ContentAction local_role, 371 bool JsepTransport::NegotiateTransportDescription(
378 std::string* error_desc) { 372 ContentAction local_description_type,
373 std::string* error_desc) {
379 if (!local_description_ || !remote_description_) { 374 if (!local_description_ || !remote_description_) {
380 const std::string msg = 375 const std::string msg =
381 "Applying an answer transport description " 376 "Applying an answer transport description "
382 "without applying any offer."; 377 "without applying any offer.";
383 return BadTransportDescription(msg, error_desc); 378 return BadTransportDescription(msg, error_desc);
384 } 379 }
385 rtc::SSLFingerprint* local_fp = 380 rtc::SSLFingerprint* local_fp =
386 local_description_->identity_fingerprint.get(); 381 local_description_->identity_fingerprint.get();
387 rtc::SSLFingerprint* remote_fp = 382 rtc::SSLFingerprint* remote_fp =
388 remote_description_->identity_fingerprint.get(); 383 remote_description_->identity_fingerprint.get();
389 if (remote_fp && local_fp) { 384 if (remote_fp && local_fp) {
390 remote_fingerprint_.reset(new rtc::SSLFingerprint(*remote_fp)); 385 remote_fingerprint_.reset(new rtc::SSLFingerprint(*remote_fp));
391 if (!NegotiateRole(local_role, &secure_role_, error_desc)) { 386 if (!NegotiateRole(local_description_type, error_desc)) {
392 return false; 387 return false;
393 } 388 }
394 } else if (local_fp && (local_role == CA_ANSWER)) { 389 } else if (local_fp && (local_description_type == CA_ANSWER)) {
395 return BadTransportDescription( 390 return BadTransportDescription(
396 "Local fingerprint supplied when caller didn't offer DTLS.", 391 "Local fingerprint supplied when caller didn't offer DTLS.",
397 error_desc); 392 error_desc);
398 } else { 393 } else {
399 // We are not doing DTLS 394 // We are not doing DTLS
400 remote_fingerprint_.reset(new rtc::SSLFingerprint("", nullptr, 0)); 395 remote_fingerprint_.reset(new rtc::SSLFingerprint("", nullptr, 0));
401 } 396 }
402 // Now that we have negotiated everything, push it downward. 397 // Now that we have negotiated everything, push it downward.
403 // Note that we cache the result so that if we have race conditions 398 // Note that we cache the result so that if we have race conditions
404 // between future SetRemote/SetLocal invocations and new channel 399 // between future SetRemote/SetLocal invocations and new channel
405 // creation, we have the negotiation state saved until a new 400 // creation, we have the negotiation state saved until a new
406 // negotiation happens. 401 // negotiation happens.
407 for (const auto& kv : channels_) { 402 for (const auto& kv : channels_) {
408 if (!ApplyNegotiatedTransportDescription(kv.second, error_desc)) { 403 if (!ApplyNegotiatedTransportDescription(kv.second, error_desc)) {
409 return false; 404 return false;
410 } 405 }
411 } 406 }
412 return true; 407 return true;
413 } 408 }
414 409
415 bool JsepTransport::NegotiateRole(ContentAction local_role, 410 bool JsepTransport::NegotiateRole(ContentAction local_description_type,
416 rtc::SSLRole* ssl_role, 411 std::string* error_desc) {
417 std::string* error_desc) const {
418 RTC_DCHECK(ssl_role);
419 if (!local_description_ || !remote_description_) { 412 if (!local_description_ || !remote_description_) {
420 const std::string msg = 413 const std::string msg =
421 "Local and Remote description must be set before " 414 "Local and Remote description must be set before "
422 "transport descriptions are negotiated"; 415 "transport descriptions are negotiated";
423 return BadTransportDescription(msg, error_desc); 416 return BadTransportDescription(msg, error_desc);
424 } 417 }
425 418
426 // From RFC 4145, section-4.1, The following are the values that the 419 // From RFC 4145, section-4.1, The following are the values that the
427 // 'setup' attribute can take in an offer/answer exchange: 420 // 'setup' attribute can take in an offer/answer exchange:
428 // Offer Answer 421 // Offer Answer
(...skipping 14 matching lines...) Expand all
443 // latency. setup:active allows the answer and the DTLS handshake to 436 // latency. setup:active allows the answer and the DTLS handshake to
444 // occur in parallel. Thus, setup:active is RECOMMENDED. Whichever 437 // occur in parallel. Thus, setup:active is RECOMMENDED. Whichever
445 // party is active MUST initiate a DTLS handshake by sending a 438 // party is active MUST initiate a DTLS handshake by sending a
446 // ClientHello over each flow (host/port quartet). 439 // ClientHello over each flow (host/port quartet).
447 // IOW - actpass and passive modes should be treated as server and 440 // IOW - actpass and passive modes should be treated as server and
448 // active as client. 441 // active as client.
449 ConnectionRole local_connection_role = local_description_->connection_role; 442 ConnectionRole local_connection_role = local_description_->connection_role;
450 ConnectionRole remote_connection_role = remote_description_->connection_role; 443 ConnectionRole remote_connection_role = remote_description_->connection_role;
451 444
452 bool is_remote_server = false; 445 bool is_remote_server = false;
453 if (local_role == CA_OFFER) { 446 if (local_description_type == CA_OFFER) {
454 if (local_connection_role != CONNECTIONROLE_ACTPASS) { 447 if (local_connection_role != CONNECTIONROLE_ACTPASS) {
455 return BadTransportDescription( 448 return BadTransportDescription(
456 "Offerer must use actpass value for setup attribute.", error_desc); 449 "Offerer must use actpass value for setup attribute.", error_desc);
457 } 450 }
458 451
459 if (remote_connection_role == CONNECTIONROLE_ACTIVE || 452 if (remote_connection_role == CONNECTIONROLE_ACTIVE ||
460 remote_connection_role == CONNECTIONROLE_PASSIVE || 453 remote_connection_role == CONNECTIONROLE_PASSIVE ||
461 remote_connection_role == CONNECTIONROLE_NONE) { 454 remote_connection_role == CONNECTIONROLE_NONE) {
462 is_remote_server = (remote_connection_role == CONNECTIONROLE_PASSIVE); 455 is_remote_server = (remote_connection_role == CONNECTIONROLE_PASSIVE);
463 } else { 456 } else {
464 const std::string msg = 457 const std::string msg =
465 "Answerer must use either active or passive value " 458 "Answerer must use either active or passive value "
466 "for setup attribute."; 459 "for setup attribute.";
467 return BadTransportDescription(msg, error_desc); 460 return BadTransportDescription(msg, error_desc);
468 } 461 }
469 // If remote is NONE or ACTIVE it will act as client. 462 // If remote is NONE or ACTIVE it will act as client.
470 } else { 463 } else {
471 if (remote_connection_role != CONNECTIONROLE_ACTPASS && 464 if (remote_connection_role != CONNECTIONROLE_ACTPASS &&
472 remote_connection_role != CONNECTIONROLE_NONE) { 465 remote_connection_role != CONNECTIONROLE_NONE) {
473 return BadTransportDescription( 466 // Accept a remote role attribute that's not "actpass", but matches the
474 "Offerer must use actpass value for setup attribute.", error_desc); 467 // current negotiated role. This is allowed by dtls-sdp, though our
468 // implementation will never generate such an offer as it's not
469 // recommended.
470 //
471 // See https://datatracker.ietf.org/doc/html/draft-ietf-mmusic-dtls-sdp,
472 // section 5.5.
473 if (!ssl_role_ ||
474 (*ssl_role_ == rtc::SSL_CLIENT &&
475 remote_connection_role == CONNECTIONROLE_ACTIVE) ||
476 (*ssl_role_ == rtc::SSL_SERVER &&
477 remote_connection_role == CONNECTIONROLE_PASSIVE)) {
478 return BadTransportDescription(
479 "Offerer must use actpass value or current negotiated role for "
480 "setup attribute.",
481 error_desc);
482 }
475 } 483 }
476 484
477 if (local_connection_role == CONNECTIONROLE_ACTIVE || 485 if (local_connection_role == CONNECTIONROLE_ACTIVE ||
478 local_connection_role == CONNECTIONROLE_PASSIVE) { 486 local_connection_role == CONNECTIONROLE_PASSIVE) {
479 is_remote_server = (local_connection_role == CONNECTIONROLE_ACTIVE); 487 is_remote_server = (local_connection_role == CONNECTIONROLE_ACTIVE);
480 } else { 488 } else {
481 const std::string msg = 489 const std::string msg =
482 "Answerer must use either active or passive value " 490 "Answerer must use either active or passive value "
483 "for setup attribute."; 491 "for setup attribute.";
484 return BadTransportDescription(msg, error_desc); 492 return BadTransportDescription(msg, error_desc);
485 } 493 }
486 494
487 // If local is passive, local will act as server. 495 // If local is passive, local will act as server.
488 } 496 }
489 497
490 *ssl_role = is_remote_server ? rtc::SSL_CLIENT : rtc::SSL_SERVER; 498 ssl_role_.emplace(is_remote_server ? rtc::SSL_CLIENT : rtc::SSL_SERVER);
491 return true; 499 return true;
492 } 500 }
493 501
494 } // namespace cricket 502 } // namespace cricket
OLDNEW
« no previous file with comments | « webrtc/p2p/base/jseptransport.h ('k') | webrtc/p2p/base/jseptransport_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698