Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Copy scope id when binding to IPv6 Link-Local address #2969

Merged
merged 2 commits into from
Nov 1, 2024

Conversation

ThymoNL
Copy link

@ThymoNL ThymoNL commented Dec 18, 2023

Thank you for contributing your time to the Mosquitto project!

Before you go any further, please note that we cannot accept contributions if
you haven't signed the Eclipse Contributor Agreement.
If you aren't able to do that, or just don't want to, please describe your bug
fix/feature change in an issue. For simple bug fixes it is can be just as easy
for us to be told about the problem and then go fix it directly.

Then please check the following list of things we ask for in your pull request:

  • Have you signed the Eclipse Contributor Agreement, using the same email address as you used in your commits?
  • Do each of your commits have a "Signed-off-by" line, with the correct email address? Use "git commit -s" to generate this line for you.
  • If you are contributing a bugfix, is your work based off the fixes branch?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you successfully run make test with your changes locally?

Configuring a listener with bind_interface on an interface with an ipv6 link-local address causes Mosquitto to exit with "Error: invalid argument". This happens due to the the IPv6 scope ID not being copied to the returned addrinfo struct when resolving interfaces matching bind_interface.

This PR adds a commit that also copies the sin6_scope_id when a matching interface is found.

Fixes #2696

Signed-off-by: Thymo van Beers <tvanbeers@bauwatch.com>
Binding to an IPv6 Link-Local address requires the scope ID to be set.
If this is not set bind will return EINVAL.

Signed-off-by: Thymo van Beers <tvanbeers@bauwatch.com>
@ThymoNL
Copy link
Author

ThymoNL commented Jan 24, 2024

@ralight May I gently bump this your way? Hoping you can take a look at this bugfix. 🙂

src/net.c Show resolved Hide resolved
@ThymoNL ThymoNL requested a review from ralight June 10, 2024 14:37
@ThymoNL
Copy link
Author

ThymoNL commented Jul 17, 2024

@ralight Can I do anything else to answer your questions in order to get this merged? :)

@ThymoNL
Copy link
Author

ThymoNL commented Oct 31, 2024

@ralight friendly reminder 🙂

@ralight
Copy link
Contributor

ralight commented Nov 1, 2024

@ThymoNL Thanks for the reminder. I would be happy to merge this, however I'd be much happier if there was a test to check the link local case. Any ideas on adding one?

@ralight
Copy link
Contributor

ralight commented Nov 1, 2024

Having looked into this a bit more, I think it's going to be impractical to write a general test for it.

The github runners do get a link local address:

2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP group default qlen 1000
    link/ether 60:45:bd:4a:dd:40 brd ff:ff:ff:ff:ff:ff
    inet 10.1.0.23/16 metric 100 brd 10.1.255.255 scope global eth0
       valid_lft forever preferred_lft forever
    inet6 fe80::6245:bdff:fe4a:dd40/64 scope link 
       valid_lft forever preferred_lft forever

However in other places, like my home, I get other non-link local IPv6 addresses and the bug does not manifest there in the same way.

@ralight ralight merged commit c968f5b into eclipse-mosquitto:fixes Nov 1, 2024
1 check passed
@ThymoNL
Copy link
Author

ThymoNL commented Nov 1, 2024

Yeah, I remember looking at this back then and not finding a trivial way to write a test either. Thanks for merging!

@ThymoNL ThymoNL deleted the bind-ip6-ll branch November 1, 2024 11:13
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants