Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WIP] DTLS: Switch to native DTLS #225

Merged
merged 12 commits into from
Dec 4, 2018
Merged

[WIP] DTLS: Switch to native DTLS #225

merged 12 commits into from
Dec 4, 2018

Conversation

backkem
Copy link
Member

@backkem backkem commented Nov 14, 2018 •
edited by Sean-Der
Loading

WIP switch to native DTLS using pions/dtls.

TODO:

  • Fix broken tests

  • Cache SRTP so we don't lose data before keying material is available (a small cache would be huge for UX)

  • Test FireFox and Chromium

  • Add alerts/error handling

  • Ensure that all examples work

    • data-channels: works on Windows & Linux!
    • data-channels-create: works on Windows & Linux!
    • gstreamer-receive: works on Linux. However we have to force PLI (we are losing some packets) TODO: Retest
    • gstreamer-send: works on Windows & Linux!
    • gstreamer-send-offer: works on Windows & Linux!
    • janus-gateway: works on Linux. TODO: Retest
    • pion-to-pion: works on Windows & Linux!
    • save-to-disk: works on Windows & Linux!
    • sfu: Not working TODO: Retest

  • Confirm that RTCTunnel and WebTTY still work

  • Tag pions/dtls and start using versioned libs for pion/webrtc (good first step before we split out SRTP)

  • Fix initial packet loss with SRTP (and possibly SCTP) Update: Fixed packet loss caused by DTLS. SCTP can be improved in a separate PR.

  • ...

Resolves #204

@backkem backkem force-pushed the dtls-native branch 5 times, most recently from 9c9400f to 3743174 Compare November 19, 2018 19:52
This was referenced Nov 24, 2018
Closed
Open
@coveralls
Copy link

coveralls commented Nov 25, 2018 •
edited
Loading

Pull Request Test Coverage Report for Build 1225

  • 331 of 756 (43.78%) changed or added relevant lines in 17 files are covered.
  • 18 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.6%) to 31.062%
Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/sdp/ice.go 0 1 0.0%
internal/sdp/jsep.go 7 8 87.5%
internal/sdp/media_description.go 5 6 83.33%
internal/sdp/unmarshal.go 14 16 87.5%
rtccertificate.go 12 14 85.71%
pkg/ice/util.go 3 7 42.86%
pkg/ice/candidate.go 0 6 0.0%
internal/network/receive.go 0 17 0.0%
rtcpeerconnection.go 48 66 72.73%
pkg/ice/candidatepair.go 0 32 0.0%
Files with Coverage Reduction New Missed Lines %
rtcpeerconnection.go 1 58.12%
pkg/ice/agent.go 6 38.48%
internal/network/manager.go 11 22.14%
Totals Coverage Status
Change from base Build 1223: 0.6%
Covered Lines: 2179
Relevant Lines: 7015
💛 - Coveralls

@Sean-Der Sean-Der force-pushed the dtls-native branch 2 times, most recently from 74db3cd to cb89c63 Compare December 1, 2018 10:36
backkem and others added 12 commits December 3, 2018 23:49
@backkem @Sean-Der
ICE: Transport refactor
7c4070f 
Resolves #205
@backkem @Sean-Der
DTLS: Switch to native DTLS
1b81b5a 
Resolves #204
@Sean-Der
DTLS: Implement Go native SRTP
8f89f0d 
Resolves #204
@Sean-Der
Send PLI in all recv examples
598e4cf 
DTLS Keying material is slower then OpenSSL, for now just send PLI on an
interval until we improve our DTLS implementation

Resolves #204
@Sean-Der
Remove OpenSSL instructions from README
1c356cb 
Resolves #204
@Sean-Der
Pass ICE creds directly to WithICECredentials
ccb72f7 
Instead of allocating a temporary variable, no real change. Just a tiny
bit less state to worry about.

Resolves #204
@backkem @Sean-Der
Move fingerprint logic to DTLS
4dcdbef 
Resolves #204
@Sean-Der
Add nil check to sctp, dtls and ICE on shutdown
f343276 
Resolves #204
@Sean-Der
Fix all lint and test failures
c6e2e99 
Resolves #204
@Sean-Der
Update janus-gateway README
4ca1b9e 
Janus now works everytime! Since we have DTLS retransmissions now we can
remove the note from the README about it

Resolves #204
@backkem @Sean-Der
Match updated pions/dtls API
7d5334b 
Resolves #204
@Sean-Der
DTLS is fast enough now that we don't need PLI
84501c6 
Remove temporary fix that only existed when handshaking was slow

Resolves #204
@Sean-Der Sean-Der merged commit 6bf22b0 into master Dec 4, 2018
@backkem backkem deleted the dtls-native branch December 4, 2018 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Sean-Der Sean-Der Sean-Der approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@backkem @coveralls @Sean-Der