Skip to content

Commit

Permalink
Only collect single fingerprints and ICE credentials
Browse files Browse the repository at this point in the history
  • Loading branch information
Nils Ohlmeier committed Nov 13, 2024
1 parent 363e017 commit faf969d
Show file tree
Hide file tree
Showing 7 changed files with 250 additions and 84 deletions.
12 changes: 0 additions & 12 deletions errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,6 @@ var (
// has an invalid fingerprint
ErrSessionDescriptionInvalidFingerprint = errors.New("SetRemoteDescription called with an invalid fingerprint")

// ErrSessionDescriptionConflictingFingerprints indicates SetRemoteDescription was called with a SessionDescription that
// has an conflicting fingerprints
ErrSessionDescriptionConflictingFingerprints = errors.New("SetRemoteDescription called with multiple conflicting fingerprint")

// ErrSessionDescriptionMissingIceUfrag indicates SetRemoteDescription was called with a SessionDescription that
// is missing an ice-ufrag value
ErrSessionDescriptionMissingIceUfrag = errors.New("SetRemoteDescription called with no ice-ufrag")
Expand All @@ -118,14 +114,6 @@ var (
// is missing an ice-pwd value
ErrSessionDescriptionMissingIcePwd = errors.New("SetRemoteDescription called with no ice-pwd")

// ErrSessionDescriptionConflictingIceUfrag indicates SetRemoteDescription was called with a SessionDescription that
// contains multiple conflicting ice-ufrag values
ErrSessionDescriptionConflictingIceUfrag = errors.New("SetRemoteDescription called with multiple conflicting ice-ufrag values")

// ErrSessionDescriptionConflictingIcePwd indicates SetRemoteDescription was called with a SessionDescription that
// contains multiple conflicting ice-pwd values
ErrSessionDescriptionConflictingIcePwd = errors.New("SetRemoteDescription called with multiple conflicting ice-pwd values")

// ErrNoSRTPProtectionProfile indicates that the DTLS handshake completed and no SRTP Protection Profile was chosen
ErrNoSRTPProtectionProfile = errors.New("DTLS Handshake completed and no SRTP Protection Profile was chosen")

Expand Down
2 changes: 1 addition & 1 deletion mediaengine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion peerconnection_go_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions peerconnection_media_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions peerconnection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
172 changes: 117 additions & 55 deletions sdp.go
Original file line number Diff line number Diff line change
Expand Up @@ -719,96 +719,158 @@ func getPeerDirection(media *sdp.MediaDescription) RTPTransceiverDirection {
return RTPTransceiverDirectionUnknown
}

func extractFingerprint(desc *sdp.SessionDescription) (string, string, error) {
fingerprints := []string{}
func extractBundleId(desc *sdp.SessionDescription) string {

Check failure on line 722 in sdp.go

View workflow job for this annotation

GitHub Actions / lint / Go

var-naming: func extractBundleId should be extractBundleID (revive)
groupAttribute, _ := desc.Attribute(sdp.AttrKeyGroup)

if fingerprint, haveFingerprint := desc.Attribute("fingerprint"); haveFingerprint {
fingerprints = append(fingerprints, fingerprint)
}
isBundled := strings.Contains(groupAttribute, "BUNDLE")

for _, m := range desc.MediaDescriptions {
if fingerprint, haveFingerprint := m.Attribute("fingerprint"); haveFingerprint {
fingerprints = append(fingerprints, fingerprint)
}
if !isBundled {
return ""
}

if len(fingerprints) < 1 {
return "", "", ErrSessionDescriptionNoFingerprint
bundleIds := strings.Split(groupAttribute, " ")

Check failure on line 731 in sdp.go

View workflow job for this annotation

GitHub Actions / lint / Go

var-naming: var bundleIds should be bundleIDs (revive)

if len(bundleIds) < 2 {
return ""
}

for _, m := range fingerprints {
if m != fingerprints[0] {
return "", "", ErrSessionDescriptionConflictingFingerprints
return bundleIds[1]
}

func extractFingerprint(desc *sdp.SessionDescription) (string, string, error) {

Check failure on line 740 in sdp.go

View workflow job for this annotation

GitHub Actions / lint / Go

cognitive complexity 32 of func `extractFingerprint` is high (> 30) (gocognit)
fingerprint := ""

// Fingerprint on session level has highest priority
if sessionFingerprint, haveFingerprint := desc.Attribute("fingerprint"); haveFingerprint {
fingerprint = sessionFingerprint
}

if fingerprint == "" {
bundleId := extractBundleId(desc)

Check failure on line 749 in sdp.go

View workflow job for this annotation

GitHub Actions / lint / Go

var-naming: var bundleId should be bundleID (revive)
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

}

Check failure on line 818 in sdp.go

View workflow job for this annotation

GitHub Actions / lint / Go

unnecessary trailing newline (whitespace)

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)

Check failure on line 833 in sdp.go

View workflow job for this annotation

GitHub Actions / lint / Go

var-naming: var bundleId should be bundleID (revive)
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

}

Check failure on line 864 in sdp.go

View workflow job for this annotation

GitHub Actions / lint / Go

unnecessary trailing newline (whitespace)
}

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 {
Expand Down
Loading

0 comments on commit faf969d

Please sign in to comment.