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

IPv6 AAAA records don't connect, while raw [::] addresses do #15

Open
rubdos opened this issue Nov 13, 2019 · 9 comments · May be fixed by #25
Open

IPv6 AAAA records don't connect, while raw [::] addresses do #15

rubdos opened this issue Nov 13, 2019 · 9 comments · May be fixed by #25
Labels

Comments

@rubdos
Copy link

rubdos commented Nov 13, 2019

What works: coap://[fd01::dead:beef]/abc/def, what doesn't work: coap://lamp01/abc/def, where lamp01 resolves to fd01::dead:beef on a AAAA record.

I had to manually add

reqOpts.agent = coap.globalAgentIPv6;

to function _makeRequest(msg) for the DNS record to resolve.

I fear this is a limitation in the coap library though. If so, I'll direct my issue there. However I think it would imply a major interface redesign there, and maybe the node-red interface should expose a workaround -- somehow.

@JKRhb JKRhb added the bug label Feb 1, 2021
@JKRhb
Copy link
Owner

JKRhb commented Feb 5, 2021

Have you been able to further investigate this issue, @rubdos? I updated the package's dependencies, maybe this solved the problem. If not, let me know :)

@rubdos
Copy link
Author

rubdos commented Feb 10, 2021

Hey @JKRhb, we couldn't have this lab session at the uni this year because of COVID... We did remote Zigbee stuff instead. I hope to have it back next year though, which would be November 2021.

@thielemans, do you know by any chance whether the dependency bump would have helped here?

@JKRhb
Copy link
Owner

JKRhb commented Feb 10, 2021

Hey @JKRhb, we couldn't have this lab session at the uni this year because of COVID... We did remote Zigbee stuff instead. I hope to have it back next year though, which would be November 2021.

Fingers crossed! :)

Regarding the issue, I noticed that there have been similar problems in other packages that have been using the default udp4 agent. Similar to the addition in #17, I would add a checkbox in the request node that lets you choose between the two types of agent. If the checkbox is ticked (which should be the default, I guess) then the line

reqOpts.agent = coap.globalAgentIPv6;

will be included. Do you think this could be a working solution?

@rubdos
Copy link
Author

rubdos commented Feb 11, 2021

Do you think this could be a working solution?

I'm not 100% sure; if the globalAgentIPV6 has no A-record fall back, then that change will probably break the plugin for all the V4 users out there that use DNS.

Besides, this issue should probably be resolved at https://github.com/mcollina/node-coap and not here, now I think about it. DNS and IP is a lower layer than what this package should provide, but I'll have to dig deeper in the source code for that.

By the way (re #17): binding on [::1] doesn't guarantee a hybrid/multi protocol socket. I think that binding on [::] yields a v4/v6 hybrid socket on some Linux systems, but that's distro-dependent iirc.

Thanks for coming back to this, sorry that I'm currently not very helpful!

@JKRhb
Copy link
Owner

JKRhb commented Feb 20, 2021

Thanks for coming back to this, sorry that I'm currently not very helpful!

No worries, thank you for your helpful feedback and for raising this issue in the first place! I will try to investigate this problem further in the next days, especially when it comes to the potential breaking change the switch to globalAgentIPV6 might be. I've seen a similar issues in other packages where a distinction between V4 and V6 URIs was added to ensure continued support for the former. Maybe this could be a solution (or rather: a workaround) for this package as well.

By the way (re #17): binding on [::1] doesn't guarantee a hybrid/multi protocol socket. I think that binding on [::] yields a v4/v6 hybrid socket on some Linux systems, but that's distro-dependent iirc.

You are right – maybe it would be more accurate to label the option as "Use UDP6 socket" and turn it off by default?

@JKRhb JKRhb linked a pull request Feb 20, 2021 that will close this issue
@JKRhb
Copy link
Owner

JKRhb commented Feb 20, 2021

I just opened #25 as a potential fix/workaround.

@JKRhb
Copy link
Owner

JKRhb commented Feb 21, 2021

Hmm, I just noticed that a distinction between IPv4 and IPv6 hostnames is already happening in node-coap (see here), making the changes in #25 unnecessary. Therefore, this issue should actually be resolved there if it still persists, I guess.

JKRhb added a commit that referenced this issue Mar 18, 2021
Avoid issues on operating systems that do not use
 v4/v6 hybrid sockets (see #15).
JKRhb added a commit that referenced this issue Mar 18, 2021
Avoid issues on operating systems that do not use
 v4/v6 hybrid sockets (see #15).
@rubdos
Copy link
Author

rubdos commented Nov 25, 2021

The COVID situation finally allows us to give this lab again on-campus, which means that I'm again bouncing on this.

So, it appears that the way node-coap exposes the API, i.e., via an Agent that creates a socket before sending a packet, this issue needs to be worked around. node-coap should provide an API that only creates the UDP socket after resolving the DNS name. The reason for the API being exposed as it currently stands, is because node-coap seemingly wants to (incorrectly, IMO) unify a listening socket and a sending socket under the Agent abstraction. I have filed coapjs/node-coap#320 to discuss this. I find this extra painful, because NodeJS is a very dynamic thingy where I would expect that connecting to a udp://-style socket with hostname already has all these things built-in.

I suggest we merge #25;, I can confirm after tomorrow 3pm CEST whether that patch is sufficient as workaround.

@JKRhb
Copy link
Owner

JKRhb commented Nov 25, 2021

Thank you for the update on this issue and filing coapjs/node-coap#320! I updated the workaround in #25 and hope that works for now. Otherwise I totally agree with you that this should be handled on a lower level and I am looking forward to the discussion over at node-coap!

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

Successfully merging a pull request may close this issue.

2 participants