Skip HelloVerify Verification for WebRTC connections

[xiaokangwang]

Description

This is a pull request that set InsecureSkipVerifyHello option on webrtc's underlying DTLS connection. This allow the current implementation of webrtc to match browser behavior better and reduce the time to establish a connection.

It have been observed that snowflake, a crowdsourced censorship resistant proxy has been blocked in some part of Russia by identifying the transmission of HelloVerify packet. This pull request is a series of pull request to remove this distinguisher.

Reference issue

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.11 🎉

Comparison is base (61f69be) 77.52% compared to head (98dfcad) 77.63%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2407      +/-   ##
==========================================
+ Coverage   77.52%   77.63%   +0.11%     
==========================================
  Files          87       87              
  Lines        9294     9296       +2     
==========================================
+ Hits         7205     7217      +12     
+ Misses       1654     1647       -7     
+ Partials      435      432       -3     

Flag	Coverage Δ
go	79.41% <100.00%> (+0.12%)	⬆️
wasm	70.15% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
dtlstransport.go	64.90% <ø> (+1.86%)	⬆️
settingengine.go	57.77% <100.00%> (+0.95%)	⬆️
icetransport.go	68.42% <0.00%> (-1.22%)	⬇️
sctptransport.go	77.90% <0.00%> (+0.74%)	⬆️
operations.go	92.59% <0.00%> (+1.85%)	⬆️
internal/mux/mux.go	89.13% <0.00%> (+4.34%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[Sean-Der]

Hi @xiaokangwang

thank you for the PR! Would you mind making this a SettingEngine option? It would be nice to keep the VerifyHello on by default.

[xiaokangwang]

Hi @xiaokangwang

thank you for the PR! Would you mind making this a SettingEngine option? It would be nice to keep the VerifyHello on by default.

Re: @Sean-Der

I have finished adopting your suggestions. Please let me know if you have any additional questions or suggestions.

[xiaokangwang]

I have fixed the lint by changing to US word spelling and added test case for setting engine (Its effect is not tested).

I will git rebase -i once it's approved.

[xiaokangwang]

I have seen the CI failure about commit message. I will fix it on rebase.

[xiaokangwang]

I have refactored the option into dtls struct in the settings engine (thanks: @AlexxIT -> #2433) .

[AlexxIT]

@xiaokangwang thanks for your pion/dtls#513
very helpful. I have spent two days looking for connectivity problems.

Also it can be fixed if we send two messages here:
https://github.com/pion/dtls/blob/debc4157d4842f33451c8e7e34f08377fa1c37ed/conn.go#L406

Just need to be changed to:

rawPackets = append(rawPackets, rawHandshakePackets...)
rawPackets = append(rawPackets, rawHandshakePackets...)

I don't know why, but this dummy fix also works.

[xiaokangwang]

@xiaokangwang thanks for your pion/dtls#513 very helpful. I have spent two days looking for connectivity problems.

Also it can be fixed if we send two messages here: https://github.com/pion/dtls/blob/debc4157d4842f33451c8e7e34f08377fa1c37ed/conn.go#L406

Just need to be changed to:

rawPackets = append(rawPackets, rawHandshakePackets...)
rawPackets = append(rawPackets, rawHandshakePackets...)

I don't know why, but this dummy fix also works.

A packet capture is needed to determine what actually happened. A reasonable guess is that it creates a packet that is still accepted by at least some implementation of WebRTC stack but can't be parsed by censor.

[xiaokangwang]

I will git rebase -i once it's approved.

I have seen the CI failure about commit message. I will fix it on rebase.

[Sean-Der]

Merged with 2a47c12 thank you @xiaokangwang !