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

feat: support setting ICE ufrag and pwd, disabling fingerprint validation and specifying certificates #256

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

achingbrain
Copy link
Contributor

@achingbrain achingbrain commented Jun 6, 2024

paullouisageneau/libjuice#243 allows setting the ICE ufrag and pwd fields instead of generating random ones every time.

paullouisageneau/libdatachannel#1207 adds an init arg to setLocalDescription to allow setting the fields in libjuice from libdatachannel.

paullouisageneau/libdatachannel#1206 allows reading the fingerprint of the cert being used by the remote peer.

This PR adds the init arg to setLocalDescription, and the remoteFingerprint method, plus the missing config keys.

This is required to implement libp2p WebRTC-Direct.

It will require the PRs above being merged and shipped before this is ready so I've opened it as a draft for now.

Refs: paullouisageneau/libdatachannel#1166

@achingbrain achingbrain changed the title feat: support setting ICE ufrag and pwd feat: support setting ICE ufrag and pwd, disabling fingerprint validation and specifying certificates Jun 7, 2024
achingbrain added a commit to libp2p/js-libp2p that referenced this pull request Jun 7, 2024
achingbrain added a commit to libp2p/js-libp2p that referenced this pull request Jun 7, 2024
@murat-dogan
Copy link
Owner

Thanks for the PR.
It would be good if you also add these changes here;

@achingbrain achingbrain force-pushed the feat/support-setting-ufrag-and-pwd branch from 2e41beb to 4cb6ad1 Compare June 12, 2024 08:39
Updates the implementation to match the latest libdatachannel with
features for setting ICE ufrag/pwd and reading the remote cert
fingerprint.

Also adds pass-through for missing config values.
@achingbrain achingbrain force-pushed the feat/support-setting-ufrag-and-pwd branch from 4c8b9b6 to 10051a8 Compare June 12, 2024 08:49
@murat-dogan
Copy link
Owner

Hello,
Thanks for the work!

When that could be ready for merging?

@unicomp21
Copy link
Contributor

@murat-dogan @achingbrain what remains before we can merge?

@achingbrain
Copy link
Contributor Author

For the functionality to be usable this PR needs to be merged/released as well - paullouisageneau/libjuice#248 - then this one: paullouisageneau/libdatachannel#1211 - then we can expose it here.

@unicomp21
Copy link
Contributor

given the delay, would it make sense to consider forking in the meantime?

@unicomp21
Copy link
Contributor

unicomp21 commented Nov 18, 2024

disabling fingerprint validation

if ^ was a separate PR, do we have anything blocking us on the libdatachannel side? My reason for asking, I'm using a homegrown turn: server w/ local short circuit, and don't need the other parts, afaik.

@unicomp21
Copy link
Contributor

#308

@achingbrain
Copy link
Contributor Author

given the delay, would it make sense to consider forking in the meantime?

@paullouisageneau what do you think, will you have some time to let us merge the final couple of outstanding PRs (paullouisageneau/libjuice#248 and paullouisageneau/libdatachannel#1211) or should we do our own thing?

@murat-dogan
Copy link
Owner

Hi all,

I think forking the lib is not feasible.
Let’s ping @paullouisageneau to see if there’s a problem to be solved for PRs @achingbrain mentioned.

To merge only fingerprint feature is something @achingbrain should decide, while he worked on this PR already. If this will be the case also this PR must be re-worked.

@paullouisageneau
Copy link
Contributor

I'll do another pass on paullouisageneau/libjuice#248

This issue was closed but as I understand it is still blocking, as the ICE ufrag and pwd can't be applied from the polyfill: paullouisageneau/libdatachannel#1218

@murat-dogan
Copy link
Owner

Is there something that I can do @paullouisageneau ?

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

Successfully merging this pull request may close these issues.

4 participants