Bug 159207

Summary: WebRTC: ice-char can not contain '=' characters for credentials
Product: WebKit Reporter: Alejandro G. Castro <alex>
Component: WebCore Misc.Assignee: Alejandro G. Castro <alex>
Status: RESOLVED FIXED    
Severity: Normal CC: adam.bergkvist, commit-queue, eric.carlson
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 143211    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Alejandro G. Castro
Reported 2016-06-28 03:20:14 PDT
We are generating a wrong pass string because we are using base64 random string with padding, and paddings are filled with '=' characters, it is a problem because the sdp parser does not work corrently in this cases and we are not sending good credentials. The parser in the test we have was modified to avoid this issue.
Attachments
Patch (4.43 KB, patch)
2016-06-28 04:13 PDT, Alejandro G. Castro 		no flags
DetailsFormatted DiffDiff
Patch (4.96 KB, patch)
2016-06-29 02:02 PDT, Alejandro G. Castro 		no flags
DetailsFormatted DiffDiff
Patch for landing (4.91 KB, patch)
2016-06-29 10:06 PDT, Alejandro G. Castro 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Alejandro G. Castro
Comment 1 2016-06-28 04:13:31 PDT
Created attachment 282242 [details] Patch
Adam Bergkvist
Comment 2 2016-06-28 11:10:43 PDT
Comment on attachment 282242 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=282242&action=review > Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.cpp:75 > + , m_icePassword(randomString(24)) // From 22 to 256 ice-chars, we use base64 to generate we need to avoid padding. Feels like a word is missing to make these sentences complete
Alejandro G. Castro
Comment 3 2016-06-29 01:42:55 PDT
(In reply to comment #2) > Comment on attachment 282242 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=282242&action=review > > > Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.cpp:75 > > + , m_icePassword(randomString(24)) // From 22 to 256 ice-chars, we use base64 to generate we need to avoid padding. > > Feels like a word is missing to make these sentences complete Right, I'll fix it, thanks for the review!
Alejandro G. Castro
Comment 4 2016-06-29 02:02:57 PDT
Created attachment 282333 [details] Patch
Eric Carlson
Comment 5 2016-06-29 08:17:55 PDT
Comment on attachment 282333 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=282333&action=review > Source/WebCore/ChangeLog:8 > + Avoided the general calculation to get a base64 without padding Nit: "Avoided the general calculation..." -> "Avoid a general calculation..." > Source/WebCore/ChangeLog:9 > + which was wrong in the randomString function. Each parameter using Nit: "Each parameter using..." -> "Because each parameter using..." > Source/WebCore/ChangeLog:16 > + Covered by existing test modified to match the correct behavior. Nit: "Covered by existing test modified to..." -> "Existing test modified to..."
Alejandro G. Castro
Comment 6 2016-06-29 10:06:47 PDT
Created attachment 282355 [details] Patch for landing
WebKit Commit Bot
Comment 7 2016-06-29 10:36:55 PDT
Comment on attachment 282355 [details] Patch for landing Clearing flags on attachment: 282355 Committed r202628: <http://trac.webkit.org/changeset/202628>
WebKit Commit Bot
Comment 8 2016-06-29 10:36:59 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.