ice restart fail due to empty credential on libnice master

[michelepra]

I've do some test to discover the reason why ice restart fail in janus 0.11.2 (nosip plugin) with libnice master on centos7.

Libnice log, after ice restart , always report that have empty credential (ufrag/pwd) when start connectivity check and the reason is on method nice_agent_restart that reset credential

The reason that libnice have empty credential is janus call nice_agent_set_remote_credentials with new credential before nice_agent_restart

By reverse the order when calling this methods ice restart always goes on

[lminiero]

It seems to be working just fine for me (echotest demo).

[michelepra]

do you think that can be issue related to plugin?
Here ther's log before and after this patch.

Before patch when libnice try to send stun message to new candidate it log (line 1348)

Agent 0x7f69dc00a690: no credentials found, cancelling conncheck

After patch [STUN-CC REQ]/[STUN-CC RESP] with new pair and credential SUCCEEDED

[michelepra]

So sorry, i reported the wrong version of libnice, this behaviour was with libnice master.
I try janus 0.11.2 without patch with libnice 0.1.18 and icerestart work

As you can see here master is different from 0.1.18 so i think that or libnice introduce bug/regression in master or latest janus is not compatible with libnice master (because with proposed patch ice restart work)

[lminiero]

Got it, thanks for the heads up! We'll need to check what changes they may have made. We still don't advise using a version later than 0.1.18 as to my knowledge there are some issues with keepalives that may cause sessions to be torn out sooner than needed (I think @alexamirante or @atoppi know the details), so for the time being we're fine.

[michelepra]

and here the commits of change of the functionality of ice restart in libnice master.
Interesting, from this commit i can think that previous version do wrong thing in ice restart phase, but it work and nobody complains 😅

Happy to help if i can.

[lminiero]

Yeah, it looks like this is the commit that started resetting remote credentials on a restart. In that case I agree with you we should somehow ensure remote credentials are set after we call nice_agent_restart, which in our code though depends on how the negotiation is taking place.

When you say you had issues with NoSIP, who were ICE restarts not working for? Caller or callee? That's important because in the former it's the browser sending an offer to Janus and Janus answering, while it's the other way around for the latter, and the two cases are obviously handled differently as far as ICE restarts are concerned.

[michelepra]

All test that i do are in same condition of network and direction. A(caller - jsep) B(callee - sdp)
After all webrtc stuff are done (ice/dtls) rtp transit from A to B and around. At this time A switching network and do ice restart phase. So, for answer your question, my test are always browser that sending offer to Janus ans Janus answering

[lminiero]

@michelepra I prepared a small PR that you can find linked above. If possible, please test with both an older and newer version of libnice (especially the ones that broke for you), and possibly testing restarts originated both by NoSIP caller and callee to cover both perspectives. In case you make any test and have feedback to share, please add it to the PR and not here.

[michelepra]

Nice, your pr is exactly the same that i do
With this change i've tested older and newer libnice version and icerestart works, good to know that is become official.

Thanks