Bug 169681 - [WebRTC] SDP sess-id in the "o=" line should be a value between 0 and LLONG_MAX.
Summary: [WebRTC] SDP sess-id in the "o=" line should be a value between 0 and LLONG_MAX.
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Carlos Alberto Lopez Perez
Depends on:
Reported: 2017-03-15 09:36 PDT by Carlos Alberto Lopez Perez
Modified: 2017-03-16 04:59 PDT (History)
8 users (show)

See Also:

Patch (4.84 KB, patch)
2017-03-15 12:04 PDT, Carlos Alberto Lopez Perez
no flags Details | Formatted Diff | Diff
Patch (7.00 KB, patch)
2017-03-15 15:53 PDT, Carlos Alberto Lopez Perez
alex: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Alberto Lopez Perez 2017-03-15 09:36:24 PDT
For the origin ("o=") line of the SDP
    o=<username> <sess-id> <sess-version> <nettype> <addrtype> <unicast-address>

RFC 3264 says:
    The numeric value of the session id and version in the o line MUST be representable with a 64 bit signed integer. 
    The initial value of the version MUST be less than (2**62)-1, to avoid rollovers.
RFC 4566 defines both values as:
    sess-id =             1*DIGIT
                         ;should be unique for this username/host
    sess-version =        1*DIGIT

    According to the ABNF grammar defined in RFC 5234:
        1*DIGIT means at least one ([1-inf]) numeric characters [0-9].
    So, this means this values should be an integer value >= 0.
The combination says only values from 0 - 2^63-1 pass both tests for the session-id and values from 0 - 2^62-1 for the version.
Therefore I understand the following:

 - We should use int64_t for representing both values (sess-id and sess-version).
 - The session-id value generated must be an integer inside [0 - 2^63-1]
 - The version value generated must be an integer inside [0 - 2^62-1]

Right now we are using uint_64t for the session id, and for the initial session id value we generate a random number that sometimes exceeds LLONG_MAX.

And when that happens firefox rejects our SDP offer.

I observed that firefox gives this fatal error on AppRTC when we generate a session-id bigger than LLONG_MAX
    setRemoteDescription: InvalidSessionDescriptionError: Failed to parse SDP: SDP Parse Error on line 2:  Invalid owner session id specified for o=.

Comment 1 Carlos Alberto Lopez Perez 2017-03-15 12:04:04 PDT
Created attachment 304523 [details]
Comment 2 Michael Catanzaro 2017-03-15 13:21:51 PDT
This seems like something that should be possible to test for, right?
Comment 3 Carlos Alberto Lopez Perez 2017-03-15 15:53:25 PDT
Created attachment 304570 [details]

Add a check for the sess-id value on the tests
Comment 4 Alejandro G. Castro 2017-03-16 03:30:01 PDT
Comment on attachment 304570 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=304570&action=review

Good catch! LGTM

> LayoutTests/fast/mediastream/resources/sdp-utils.js:32
> +                    if (sessid > 0 && sessid <= 9223372036854775807)

Can we comment what that number is or add it to a constant somewhere that explains?
Comment 5 Carlos Alberto Lopez Perez 2017-03-16 04:59:35 PDT
Committed r214037: <http://trac.webkit.org/changeset/214037>