Bug 169681 – [WebRTC] SDP sess-id in the "o=" line should be a value between 0 and LLONG_MAX.

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Carlos Alberto Lopez Perez]

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=.

    
References:
https://bugzilla.mozilla.org/show_bug.cgi?id=861895#c5
https://bugs.chromium.org/p/webrtc/issues/detail?id=1628#c8

[Carlos Alberto Lopez Perez]

Created attachment 304523 [details]
Patch

[Michael Catanzaro]

This seems like something that should be possible to test for, right?

[Carlos Alberto Lopez Perez]

Created attachment 304570 [details]
Patch

Add a check for the sess-id value on the tests

[Alejandro G. Castro]

Comment on attachment 304570 [details]
Patch

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?

[Carlos Alberto Lopez Perez]

Committed r214037: <http://trac.webkit.org/changeset/214037>