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.

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

As a client, the handling of DTLS exceptions leads to a failure in connecting to the server #2629

Closed
minlp opened this issue Dec 1, 2023 · 5 comments

Comments

@minlp
Copy link

minlp commented Dec 1, 2023

Your environment.

  • Version: pion/webrtc/v3 v3.2.21、pion/dtls/v2 v2.2.7

What happened?

When connecting to the WebRTC media server, there are occasional issues with streaming failure. Upon analyzing the captured packets, it appears that the failure is due to DTLS handshake failure.

cap
Based on the highlighted section in the image, the packets for "server hello," "certificate," "server key exchange," "certificate request," and "server hello done" are split into two packets (packet sequence numbers 10766 and 10830). In a normal network scenario, packet 10830 should be received before packet 10766. However, due to network latency, packet 10766 arrived first, causing the client to handle it abnormally and return an internal error. This abnormal handling resulted in an overall disruption in the process.

go_code1 Based on the analysis of the captured packets, when packet sequence number 10766 is received and there is no "server hello" and "hello verify request" in the current cache, it results in an internal error being returned. In this case, can the fullPullMap method be modified as follows?
func (h *handshakeCache) fullPullMap(startSeq int, cipherSuite CipherSuite, rules ...handshakeCachePullRule) (int, map[handshake.Type]handshake.Message, bool) {
   ...
out := make(map[handshake.Type]handshake.Message)
	seq := startSeq
        // change the meaning of the second return parameter to whether the result is obtained from the cache
	ok := false
	for _, r := range rules {
		t := r.typ
		i := ci[t]
		if i == nil {
			continue
		}
		var keyExchangeAlgorithm CipherSuiteKeyExchangeAlgorithm
		if cipherSuite != nil {
			keyExchangeAlgorithm = cipherSuite.KeyExchangeAlgorithm()
		}
		rawHandshake := &handshake.Handshake{
			KeyExchangeAlgorithm: keyExchangeAlgorithm,
		}
		if err := rawHandshake.Unmarshal(i.data); err != nil {
			return startSeq, nil, false
		}
		if uint16(seq) != rawHandshake.Header.MessageSequence {
			// There is a gap. Some messages are not arrived.
			return startSeq, nil, false
		}
		seq++
		ok = true
		out[t] = rawHandshake.Message
	}
	return seq, out, ok
}

What did you expect?

In this situation, can successfully establish a connection with the server.

@adriancable
Copy link
Contributor

Hi @minlp - I haven't tested the revised code but it looks plausible. Can you confirm it fixes the issue in your case? If so, can you kindly open a PR on pion/dtls so we can review in more detail and run the CI tests?

@minlp
Copy link
Author

minlp commented Dec 5, 2023

Hi @minlp - I haven't tested the revised code but it looks plausible. Can you confirm it fixes the issue in your case? If so, can you kindly open a PR on pion/dtls so we can review in more detail and run the CI tests?

It can solve my problem, as I haven't encountered any DTLS handshake failures after making local modifications. I will open a pull request on pion/dtls.

@minlp
Copy link
Author

minlp commented Dec 5, 2023

Hi @minlp - I haven't tested the revised code but it looks plausible. Can you confirm it fixes the issue in your case? If so, can you kindly open a PR on pion/dtls so we can review in more detail and run the CI tests?

but I can't push my branch to pion/dtls, can you give me the relevant permissions? @adriancable
push_error

@adriancable
Copy link
Contributor

Hi @minlp - you don’t/can’t push to this repo. You fork pion/dtls, push changes to your fork, then create a PR on pion/dtls repo based on the fork. Then me (or one of the other pion maintainers) will take it from there.

@Sean-Der
Copy link
Member

This was fixed with the merging of pion/dtls#600

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants