diff --git a/mediaengine_test.go b/mediaengine_test.go index d598c03cb89..fa3cc2d8994 100644 --- a/mediaengine_test.go +++ b/mediaengine_test.go @@ -645,7 +645,7 @@ v=0 o=- 8448668841136641781 4 IN IP4 127.0.0.1 s=- t=0 0 -a=group:BUNDLE 0 1 2 +a=group:BUNDLE 1 a=extmap-allow-mixed a=msid-semantic: WMS 4beea6b0-cf95-449c-a1ec-78e16b247426 m=video 9 UDP/TLS/RTP/SAVPF 96 127 diff --git a/peerconnection_go_test.go b/peerconnection_go_test.go index d7e88da4131..3de9acf5f2a 100644 --- a/peerconnection_go_test.go +++ b/peerconnection_go_test.go @@ -1398,6 +1398,7 @@ func TestTransceiverCreatedByRemoteSdpHasSameCodecOrderAsRemote(t *testing.T) { o=- 4596489990601351948 2 IN IP4 127.0.0.1 s=- t=0 0 +a=group:BUNDLE 0 1 m=video 60323 UDP/TLS/RTP/SAVPF 98 94 106 a=ice-ufrag:1/MvHwjAyVf27aLu a=ice-pwd:3dBU7cFOBl120v33cynDvN1E @@ -1458,6 +1459,7 @@ a=fmtp:125 level-asymmetry-allowed=1;packetization-mode=0;profile-level-id=42e01 o=- 4596489990601351948 2 IN IP4 127.0.0.1 s=- t=0 0 +a=group:BUNDLE 0 1 m=video 60323 UDP/TLS/RTP/SAVPF 98 106 a=ice-ufrag:1/MvHwjAyVf27aLu a=ice-pwd:3dBU7cFOBl120v33cynDvN1E @@ -1548,7 +1550,7 @@ o=- 4596489990601351948 2 IN IP4 127.0.0.1 s=- t=0 0 a=fingerprint:sha-256 F7:BF:B4:42:5B:44:C0:B9:49:70:6D:26:D7:3E:E6:08:B1:5B:25:2E:32:88:50:B6:3C:BE:4E:18:A7:2C:85:7C -a=group:BUNDLE 0 1 +a=group:BUNDLE 0 a=msid-semantic:WMS * m=video 9 UDP/TLS/RTP/SAVPF 97 c=IN IP4 0.0.0.0 diff --git a/peerconnection_media_test.go b/peerconnection_media_test.go index 74863148a53..a9450096334 100644 --- a/peerconnection_media_test.go +++ b/peerconnection_media_test.go @@ -1072,6 +1072,9 @@ func TestPeerConnection_Simulcast_Probe(t *testing.T) { for scanner.Scan() { if strings.HasPrefix(scanner.Text(), "m=video") { shouldDiscard = !shouldDiscard + } else if strings.HasPrefix(scanner.Text(), "a=group:BUNDLE") { + filtered += "a=group:BUNDLE 1 2\r\n" + continue } if !shouldDiscard { diff --git a/peerconnection_test.go b/peerconnection_test.go index 12811ebce22..a87ba1fbb81 100644 --- a/peerconnection_test.go +++ b/peerconnection_test.go @@ -282,6 +282,7 @@ const minimalOffer = `v=0 o=- 4596489990601351948 2 IN IP4 127.0.0.1 s=- t=0 0 +a=group:BUNDLE data a=msid-semantic: WMS m=application 47299 DTLS/SCTP 5000 c=IN IP4 192.168.20.129 diff --git a/sdp.go b/sdp.go index 3dd79b50a03..10a24edc405 100644 --- a/sdp.go +++ b/sdp.go @@ -719,96 +719,156 @@ func getPeerDirection(media *sdp.MediaDescription) RTPTransceiverDirection { return RTPTransceiverDirectionUnknown } -func extractFingerprint(desc *sdp.SessionDescription) (string, string, error) { - fingerprints := []string{} +func extractBundleID(desc *sdp.SessionDescription) string { + groupAttribute, _ := desc.Attribute(sdp.AttrKeyGroup) - if fingerprint, haveFingerprint := desc.Attribute("fingerprint"); haveFingerprint { - fingerprints = append(fingerprints, fingerprint) + isBundled := strings.Contains(groupAttribute, "BUNDLE") + + if !isBundled { + return "" } - for _, m := range desc.MediaDescriptions { - if fingerprint, haveFingerprint := m.Attribute("fingerprint"); haveFingerprint { - fingerprints = append(fingerprints, fingerprint) - } + bundleIDs := strings.Split(groupAttribute, " ") + + if len(bundleIDs) < 2 { + return "" } - if len(fingerprints) < 1 { - return "", "", ErrSessionDescriptionNoFingerprint + return bundleIDs[1] +} + +func extractFingerprint(desc *sdp.SessionDescription) (string, string, error) { //nolint: gocognit + fingerprint := "" + + // Fingerprint on session level has highest priority + if sessionFingerprint, haveFingerprint := desc.Attribute("fingerprint"); haveFingerprint { + fingerprint = sessionFingerprint } - for _, m := range fingerprints { - if m != fingerprints[0] { - return "", "", ErrSessionDescriptionConflictingFingerprints + if fingerprint == "" { + bundleID := extractBundleID(desc) + if bundleID != "" { + // Locate the fingerprint of the bundled media section + for _, m := range desc.MediaDescriptions { + if mid, haveMid := m.Attribute("mid"); haveMid { + if mid == bundleID && fingerprint == "" { + if mediaFingerprint, haveFingerprint := m.Attribute("fingerprint"); haveFingerprint { + fingerprint = mediaFingerprint + } + } + } + } + } else { + // Take the fingerprint from the first media section which has one. + // Note: According to Bundle spec each media section would have it's own transport + // with it's own cert and fingerprint each, so we would need to return a list. + for _, m := range desc.MediaDescriptions { + mediaFingerprint, haveFingerprint := m.Attribute("fingerprint") + if haveFingerprint && fingerprint == "" { + fingerprint = mediaFingerprint + } + } } } - parts := strings.Split(fingerprints[0], " ") + if fingerprint == "" { + return "", "", ErrSessionDescriptionNoFingerprint + } + + parts := strings.Split(fingerprint, " ") if len(parts) != 2 { return "", "", ErrSessionDescriptionInvalidFingerprint } return parts[1], parts[0], nil } -func extractICEDetails(desc *sdp.SessionDescription, log logging.LeveledLogger) (string, string, []ICECandidate, error) { // nolint:gocognit +func extractICEDetailsFromMedia(media *sdp.MediaDescription, log logging.LeveledLogger) (string, string, []ICECandidate, error) { + remoteUfrag := "" + remotePwd := "" candidates := []ICECandidate{} - remotePwds := []string{} - remoteUfrags := []string{} + if ufrag, haveUfrag := media.Attribute("ice-ufrag"); haveUfrag { + remoteUfrag = ufrag + } + if pwd, havePwd := media.Attribute("ice-pwd"); havePwd { + remotePwd = pwd + } + for _, a := range media.Attributes { + if a.IsICECandidate() { + c, err := ice.UnmarshalCandidate(a.Value) + if err != nil { + if errors.Is(err, ice.ErrUnknownCandidateTyp) || errors.Is(err, ice.ErrDetermineNetworkType) { + log.Warnf("Discarding remote candidate: %s", err) + continue + } + return "", "", nil, err + } + + candidate, err := newICECandidateFromICE(c) + if err != nil { + return "", "", nil, err + } + + candidates = append(candidates, candidate) + } + } + + return remoteUfrag, remotePwd, candidates, nil +} + +func extractICEDetails(desc *sdp.SessionDescription, log logging.LeveledLogger) (string, string, []ICECandidate, error) { // nolint:gocognit + remoteCandidates := []ICECandidate{} + remotePwd := "" + remoteUfrag := "" + + // Ufrag and Pw are allow at session level and thus have highest prio if ufrag, haveUfrag := desc.Attribute("ice-ufrag"); haveUfrag { - remoteUfrags = append(remoteUfrags, ufrag) + remoteUfrag = ufrag } if pwd, havePwd := desc.Attribute("ice-pwd"); havePwd { - remotePwds = append(remotePwds, pwd) + remotePwd = pwd } - for _, m := range desc.MediaDescriptions { - if ufrag, haveUfrag := m.Attribute("ice-ufrag"); haveUfrag { - remoteUfrags = append(remoteUfrags, ufrag) - } - if pwd, havePwd := m.Attribute("ice-pwd"); havePwd { - remotePwds = append(remotePwds, pwd) - } + bundleID := extractBundleID(desc) + missing := true - for _, a := range m.Attributes { - if a.IsICECandidate() { - c, err := ice.UnmarshalCandidate(a.Value) + for _, m := range desc.MediaDescriptions { + mid := getMidValue(m) + // If bundled, only take ICE detail from bundle master section + if bundleID != "" { + if mid == bundleID { + ufrag, pwd, candidates, err := extractICEDetailsFromMedia(m, log) if err != nil { - if errors.Is(err, ice.ErrUnknownCandidateTyp) || errors.Is(err, ice.ErrDetermineNetworkType) { - log.Warnf("Discarding remote candidate: %s", err) - continue - } return "", "", nil, err } - - candidate, err := newICECandidateFromICE(c) - if err != nil { - return "", "", nil, err + if remoteUfrag == "" && ufrag != "" { + remoteUfrag = ufrag + remotePwd = pwd } - - candidates = append(candidates, candidate) + remoteCandidates = candidates + } + } else if missing { + // For not-bundled, take ICE details from the first media section + ufrag, pwd, candidates, err := extractICEDetailsFromMedia(m, log) + if err != nil { + return "", "", nil, err + } + if remoteUfrag == "" && ufrag != "" { + remoteUfrag = ufrag + remotePwd = pwd } + remoteCandidates = candidates + missing = false } } - if len(remoteUfrags) == 0 { + if remoteUfrag == "" { return "", "", nil, ErrSessionDescriptionMissingIceUfrag - } else if len(remotePwds) == 0 { + } else if remotePwd == "" { return "", "", nil, ErrSessionDescriptionMissingIcePwd } - for _, m := range remoteUfrags { - if m != remoteUfrags[0] { - return "", "", nil, ErrSessionDescriptionConflictingIceUfrag - } - } - - for _, m := range remotePwds { - if m != remotePwds[0] { - return "", "", nil, ErrSessionDescriptionConflictingIcePwd - } - } - - return remoteUfrags[0], remotePwds[0], candidates, nil + return remoteUfrag, remotePwd, remoteCandidates, nil } func haveApplicationMediaSection(desc *sdp.SessionDescription) bool { diff --git a/sdp_test.go b/sdp_test.go index e3b9bf9bfda..46a3b8de504 100644 --- a/sdp_test.go +++ b/sdp_test.go @@ -59,22 +59,69 @@ func TestExtractFingerprint(t *testing.T) { assert.Equal(t, ErrSessionDescriptionInvalidFingerprint, err) }) - t.Run("Conflicting Fingerprint", func(t *testing.T) { + t.Run("Session fingerprint wins over media", func(t *testing.T) { s := &sdp.SessionDescription{ - Attributes: []sdp.Attribute{{Key: "fingerprint", Value: "foo"}}, + Attributes: []sdp.Attribute{{Key: "fingerprint", Value: "foo bar"}}, MediaDescriptions: []*sdp.MediaDescription{ - {Attributes: []sdp.Attribute{{Key: "fingerprint", Value: "foo blah"}}}, + {Attributes: []sdp.Attribute{{Key: "fingerprint", Value: "zoo boo"}}}, }, } - _, _, err := extractFingerprint(s) - assert.Equal(t, ErrSessionDescriptionConflictingFingerprints, err) + fingerprint, hash, err := extractFingerprint(s) + assert.NoError(t, err) + assert.Equal(t, fingerprint, "bar") + assert.Equal(t, hash, "foo") + }) + + t.Run("Fingerprint from master bundle section", func(t *testing.T) { + s := &sdp.SessionDescription{ + Attributes: []sdp.Attribute{ + {Key: "group", Value: "BUNDLE 1 0"}, + }, + MediaDescriptions: []*sdp.MediaDescription{ + {Attributes: []sdp.Attribute{ + {Key: "mid", Value: "0"}, + {Key: "fingerprint", Value: "zoo boo"}, + }}, + {Attributes: []sdp.Attribute{ + {Key: "mid", Value: "1"}, + {Key: "fingerprint", Value: "bar foo"}, + }}, + }, + } + + fingerprint, hash, err := extractFingerprint(s) + assert.NoError(t, err) + assert.Equal(t, fingerprint, "foo") + assert.Equal(t, hash, "bar") + }) + + t.Run("Fingerprint from first media section", func(t *testing.T) { + s := &sdp.SessionDescription{ + MediaDescriptions: []*sdp.MediaDescription{ + {Attributes: []sdp.Attribute{ + {Key: "mid", Value: "0"}, + {Key: "fingerprint", Value: "zoo boo"}, + }}, + {Attributes: []sdp.Attribute{ + {Key: "mid", Value: "1"}, + {Key: "fingerprint", Value: "bar foo"}, + }}, + }, + } + + fingerprint, hash, err := extractFingerprint(s) + assert.NoError(t, err) + assert.Equal(t, fingerprint, "boo") + assert.Equal(t, hash, "zoo") }) } func TestExtractICEDetails(t *testing.T) { - const defaultUfrag = "defaultPwd" - const defaultPwd = "defaultUfrag" + const defaultUfrag = "defaultUfrag" + const defaultPwd = "defaultPwd" + const invalidUfrag = "invalidUfrag" + const invalidPwd = "invalidPwd" t.Run("Missing ice-pwd", func(t *testing.T) { s := &sdp.SessionDescription{ @@ -131,28 +178,91 @@ func TestExtractICEDetails(t *testing.T) { assert.NoError(t, err) }) - t.Run("Conflict ufrag", func(t *testing.T) { + t.Run("ice details at session preferred over media", func(t *testing.T) { s := &sdp.SessionDescription{ - Attributes: []sdp.Attribute{{Key: "ice-ufrag", Value: "invalidUfrag"}}, + Attributes: []sdp.Attribute{ + {Key: "ice-ufrag", Value: defaultUfrag}, + {Key: "ice-pwd", Value: defaultPwd}, + }, MediaDescriptions: []*sdp.MediaDescription{ - {Attributes: []sdp.Attribute{{Key: "ice-ufrag", Value: defaultUfrag}, {Key: "ice-pwd", Value: defaultPwd}}}, + { + Attributes: []sdp.Attribute{ + {Key: "ice-ufrag", Value: invalidUfrag}, + {Key: "ice-pwd", Value: invalidPwd}, + }, + }, }, } - _, _, _, err := extractICEDetails(s, nil) - assert.Equal(t, err, ErrSessionDescriptionConflictingIceUfrag) + ufrag, pwd, _, err := extractICEDetails(s, nil) + assert.Equal(t, ufrag, defaultUfrag) + assert.Equal(t, pwd, defaultPwd) + assert.NoError(t, err) }) - t.Run("Conflict pwd", func(t *testing.T) { + t.Run("ice details from bundle media section", func(t *testing.T) { s := &sdp.SessionDescription{ - Attributes: []sdp.Attribute{{Key: "ice-pwd", Value: "invalidPwd"}}, + Attributes: []sdp.Attribute{ + {Key: "group", Value: "BUNDLE 5 2"}, + }, + MediaDescriptions: []*sdp.MediaDescription{ + { + Attributes: []sdp.Attribute{ + {Key: "mid", Value: "2"}, + {Key: "ice-ufrag", Value: invalidUfrag}, + {Key: "ice-pwd", Value: invalidPwd}, + }, + }, + { + Attributes: []sdp.Attribute{ + {Key: "mid", Value: "5"}, + {Key: "ice-ufrag", Value: defaultUfrag}, + {Key: "ice-pwd", Value: defaultPwd}, + }, + }, + }, + } + + ufrag, pwd, _, err := extractICEDetails(s, nil) + assert.Equal(t, ufrag, defaultUfrag) + assert.Equal(t, pwd, defaultPwd) + assert.NoError(t, err) + }) + + t.Run("ice details from first media section", func(t *testing.T) { + s := &sdp.SessionDescription{ + MediaDescriptions: []*sdp.MediaDescription{ + { + Attributes: []sdp.Attribute{ + {Key: "ice-ufrag", Value: defaultUfrag}, + {Key: "ice-pwd", Value: defaultPwd}, + }, + }, + { + Attributes: []sdp.Attribute{ + {Key: "ice-ufrag", Value: invalidUfrag}, + {Key: "ice-pwd", Value: invalidPwd}, + }, + }, + }, + } + + ufrag, pwd, _, err := extractICEDetails(s, nil) + assert.Equal(t, ufrag, defaultUfrag) + assert.Equal(t, pwd, defaultPwd) + assert.NoError(t, err) + }) + + t.Run("Missing pwd at session level", func(t *testing.T) { + s := &sdp.SessionDescription{ + Attributes: []sdp.Attribute{{Key: "ice-ufrag", Value: "invalidUfrag"}}, MediaDescriptions: []*sdp.MediaDescription{ {Attributes: []sdp.Attribute{{Key: "ice-ufrag", Value: defaultUfrag}, {Key: "ice-pwd", Value: defaultPwd}}}, }, } _, _, _, err := extractICEDetails(s, nil) - assert.Equal(t, err, ErrSessionDescriptionConflictingIcePwd) + assert.Equal(t, err, ErrSessionDescriptionMissingIcePwd) }) }