New issue
Advanced search Search tips

Issue 830974 link

Starred by 1 user
Status: Fixed
Owner:
lambroslambrou@chromium.org
Closed: Apr 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

[remoting WebRTC] Ensure that SDP has \r\n line-endings

Project Member Reported by lambroslambrou@chromium.org, Apr 10 2018 
The SDP spec requires that each line of SDP messages ends with CRLF:
https://tools.ietf.org/html/rfc4566#section-9

Both host and client (Web site) should be updated to comply with this (currently, we generate SDP with just \n endings). Parsing of SDP (on client and host) is already tolerant of \r\n versus \n - we just need to update the SDP messages that we generate.

Note that we apply signature verification to SDP messages, which is based on a normalized form of the SDP with just \n endings. This behavior needs to be preserved for backwards-compatibility. (Alternatively, we could migrate the signature check to a \r\n-based normalization)

Note that this CL:
https://chromium-review.googlesource.com/c/chromium/src/+/966441
recently broke us, because of the line-ending mismatch. Addressing this will avoid similar breakages in future.
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 10 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/34aaa2b2693e30a3d22bbeaa8887b318a36ad132

commit 34aaa2b2693e30a3d22bbeaa8887b318a36ad132
Author: Lambros Lambrou <lambroslambrou@chromium.org>
Date: Tue Apr 10 18:03:21 2018

[remoting WebRTC] Ensure SDP messages end with \r\n

This modifies the host to generate SDP messages with CRLF line-endings.
Both host and client already accept incoming SDP with CRLF or just LF.
However, the normalized form of SDP for signature-verification uses
just LF endings, and we need to maintain this for compatibilty.
So this CL introduces two normalized forms, one with CRLF endings for
sending SDP, and one with LF endings for signature calculation.

Bug:  830974 
Test: Unittests, and manual end-to-end test using Chrome beta (M65).
Change-Id: I061637cf639af97184295e4a1b23aa4b663e246d
Reviewed-on: https://chromium-review.googlesource.com/1003773
Reviewed-by: Joe Downing <joedow@chromium.org>
Commit-Queue: Lambros Lambrou <lambroslambrou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549587}
[modify] https://crrev.com/34aaa2b2693e30a3d22bbeaa8887b318a36ad132/remoting/protocol/sdp_message.cc
[modify] https://crrev.com/34aaa2b2693e30a3d22bbeaa8887b318a36ad132/remoting/protocol/sdp_message.h
[modify] https://crrev.com/34aaa2b2693e30a3d22bbeaa8887b318a36ad132/remoting/protocol/sdp_message_unittest.cc
[modify] https://crrev.com/34aaa2b2693e30a3d22bbeaa8887b318a36ad132/remoting/protocol/webrtc_transport.cc

Comment 2 by lambroslambrou@chromium.org, Apr 23 2018

Status: Fixed (was: Started)
►

Sign in to add a comment