Filter IPv6 addresses not suitable for DNS updates by thalman · Pull Request #8142 · SSSD/sssd

thalman · 2025-10-14T17:52:45Z

Exclude IP addresses not suitable for DNS updates.

alexey-tikhonov · 2025-10-15T06:34:47Z

Could you please add 'Resolves: #8089' line to the commit message?

thalman · 2025-10-16T08:13:52Z

Could you please add 'Resolves: #8089' line to the commit message?

Sure I will, so far it is not-working prototype

justin-stephenson · 2025-10-23T13:52:49Z

@thalman are you able to write a test for this? or maybe QE?

thalman · 2025-10-23T14:18:29Z

@thalman are you able to write a test for this? or maybe QE?

So far I did not figured out how to mark IPv6 address temporary or deprecated. This can't be done from user space by using some tool. It might be possible to write some TCP client that will use/create temporary address and run it.

Also I was not able to figure out why DNS updates are not working in our ci containers.

Any Ideas are welcomed. Once we have such an address in CI the test is simple to write.

justin-stephenson · 2025-10-23T14:27:53Z

@thalman are you able to write a test for this? or maybe QE?

So far I did not figured out how to mark IPv6 address temporary or deprecated. This can't be done from user space by using some tool. It might be possible to write some TCP client that will use/create temporary address and run it.

Also I was not able to figure out why DNS updates are not working in our ci containers.

Any Ideas are welcomed. Once we have such an address in CI the test is simple to write.

In that case a unit test would suffice, IMO.

thalman · 2025-10-23T15:52:42Z

So far I fixed the linking issue and @justin-stephenson's comments. I will think of some unit-test to be on the safe side.

thalman · 2025-10-31T17:58:22Z

@justin-stephenson Thanks for the suggestions. It took me some time to figure out the unit test. It is there as next commit.

justin-stephenson

Thanks! Ack

alexey-tikhonov · 2025-11-05T09:38:37Z

    $(CMOCKA_LIBS) \
    $(SSSD_LIBS) \
    $(SSSD_INTERNAL_LTLIBS) \
+    $(LIBNL_LIBS) \


Why is this needed in this patch?

This is for netlink library. I added unit test for it and there the dependency is introduced.

This line is added in a first commit - "Filter IPv6 addresses not suitable for DNS updates" - that doesn't add a test. That's why I automatically asked.

Ahh good catch, thanks

alexey-tikhonov · 2025-11-05T09:42:10Z

+}
+
+struct sss_iface_addr *
+sss_iface_addr_list_filter_addresses (struct sss_iface_addr *list,


Could this be static?

This one can, I will do it

alexey-tikhonov · 2025-11-05T09:56:59Z

    struct sss_iface_addr *prev;

    struct sockaddr *addr;
+    int ifa_flags;


Do you foresee any use of this flag outside of sss_iface_addr_list_get()?

If not then I propose to apply a filter function right in the for (ifa = ifaces; ifa != NULL; ifa = ifa->ifa_next) loop, before address = talloc_zero(mem_ctx, struct sss_iface_addr);.

Imo, this would simplify the patch quite a bit.

Ok, I have read actual implementation and see that it's not so straightforward.

It feels weird that code now builds two lists - getifaddrs() and rtnl_addr_alloc_cache() - and then loops over to find matches just to call rtnl_addr_get_flags()

Would it be possible to get rid of getifaddrs() completely and rely on rtnl_addr_alloc_cache() to build (filtered on the fly) list? (Sorry in advance if question is silly.)

We can't get rid of it completely we have to keep it for platforms that do not have netlink library. But I will think of better alternative.

we have to keep it for platforms that do not have netlink library

If not (Free)BSD I would vote to make it unconditional requirement...

But I will think of better alternative.

I added 'changes requested' based on this ^^
But we can go with current solution as well, thought it feels a bit awkward.

alexey-tikhonov · 2025-11-05T10:33:57Z

@arrowd, do you build SSSD with libnl3 on FreeBSD?

arrowd · 2025-11-05T15:21:01Z

We do not have libnl3. I'll look into porting it.

thalman · 2025-11-05T21:16:39Z

We do not have libnl3. I'll look into porting it.

When I was looking into this I have seen some documents about how this is done in BSD and IIRC it was much simpler and quite different from Linux. I doubt that there is the same netlink interface, but if there is then it will be worth to migrate to it completely. @arrowd please let us know.

arrowd · 2025-11-14T11:13:51Z

You're right, it is much different to Linux. I made an attempt for porting libnl3 to FreeBSD, and while it might be possible at least to some extent, it clearly requires a lot of work and netlink knowledge.

alexey-tikhonov · 2025-11-14T11:30:30Z

You're right, it is much different to Linux. I made an attempt for porting libnl3 to FreeBSD, and while it might be possible at least to some extent, it clearly requires a lot of work and netlink knowledge.

Ok, so we need to keep support building without libnl

thalman · 2025-11-27T14:10:53Z

@alexey-tikhonov, After an attempt to rewrite this completely with libnl (keeping freebsd in mind) I found out that this will be a bigger work, particularly in unit tests. Let's merge it as it is and I will work on better implementation later.

IPv6 brings a concept of address states defined in several RFCs (RFC 2462, RFC 3041, RFC 4862). Adresses that are marked as temporary, deprecated or tentative should not be used in DNS updates. This PR reads those flags via netlink interface and it excludes addresses with those flags. Resolves: SSSD#8089 Reviewed-by: Alexey Tikhonov <[email protected]> Reviewed-by: Justin Stephenson <[email protected]>

IPv6 address can be marked as temporary, deprecated or tentative. We should exclude them from DNS updates. Reviewed-by: Alexey Tikhonov <[email protected]> Reviewed-by: Justin Stephenson <[email protected]>

sssd-bot · 2025-11-27T14:20:41Z

The pull request was accepted by @alexey-tikhonov with the following PR CI status:

There are unsuccessful or unfinished checks. Make sure that the failures are not related to this pull request before merging.

thalman mentioned this pull request Oct 14, 2025

Including innapropriate IPv6 addresses in dyndns_update #8089

Closed

alexey-tikhonov added the backport-to-sssd-2-9 label Oct 15, 2025

thalman changed the title ~~Filter IPv6 addresses not suitable for DNS updates~~ Draft: Filter IPv6 addresses not suitable for DNS updates Oct 16, 2025

thalman force-pushed the dyndns_update_ipv6_temporary branch 2 times, most recently from 8be59bc to f54194d Compare October 21, 2025 15:23

thalman changed the title ~~Draft: Filter IPv6 addresses not suitable for DNS updates~~ Filter IPv6 addresses not suitable for DNS updates Oct 21, 2025

thalman marked this pull request as ready for review October 21, 2025 15:27

alexey-tikhonov self-assigned this Oct 23, 2025

alexey-tikhonov requested review from alexey-tikhonov and justin-stephenson October 23, 2025 12:43

alexey-tikhonov assigned justin-stephenson Oct 23, 2025

justin-stephenson reviewed Oct 23, 2025

View reviewed changes

Comment thread src/providers/be_dyndns.c Outdated

justin-stephenson reviewed Oct 23, 2025

View reviewed changes

Comment thread src/providers/be_dyndns.c Outdated

thalman force-pushed the dyndns_update_ipv6_temporary branch from f54194d to fac0f1f Compare October 23, 2025 15:50

thalman added the Waiting for review label Oct 31, 2025

justin-stephenson approved these changes Nov 3, 2025

View reviewed changes

pbrezina force-pushed the master branch 2 times, most recently from f5d64b3 to b854636 Compare November 4, 2025 14:27

alexey-tikhonov reviewed Nov 5, 2025

View reviewed changes

Comment thread src/providers/be_dyndns.c

alexey-tikhonov reviewed Nov 5, 2025

View reviewed changes

alexey-tikhonov added Changes requested and removed Waiting for review labels Nov 14, 2025

alexey-tikhonov approved these changes Nov 27, 2025

View reviewed changes

alexey-tikhonov added Accepted and removed Changes requested labels Nov 27, 2025

thalman added 2 commits November 27, 2025 14:20

test: check temporary address exclusion

6272ced

IPv6 address can be marked as temporary, deprecated or tentative. We should exclude them from DNS updates. Reviewed-by: Alexey Tikhonov <[email protected]> Reviewed-by: Justin Stephenson <[email protected]>

sssd-bot force-pushed the dyndns_update_ipv6_temporary branch from 7bd3c43 to 6272ced Compare November 27, 2025 14:20

alexey-tikhonov merged commit 637b7bc into SSSD:master Nov 27, 2025
11 of 16 checks passed

sssd-bot mentioned this pull request Nov 27, 2025

[autobackport: sssd-2-9] Filter IPv6 addresses not suitable for DNS updates #8249

Merged

Uh oh!

Conversation

thalman commented Oct 14, 2025

Uh oh!

alexey-tikhonov commented Oct 15, 2025

Uh oh!

thalman commented Oct 16, 2025

Uh oh!

justin-stephenson commented Oct 23, 2025

Uh oh!

thalman commented Oct 23, 2025

Uh oh!

justin-stephenson commented Oct 23, 2025

Uh oh!

Uh oh!

Uh oh!

thalman commented Oct 23, 2025

Uh oh!

thalman commented Oct 31, 2025

Uh oh!

justin-stephenson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexey-tikhonov Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexey-tikhonov commented Nov 5, 2025

Uh oh!

arrowd commented Nov 5, 2025

Uh oh!

thalman commented Nov 5, 2025

Uh oh!

arrowd commented Nov 14, 2025

Uh oh!

alexey-tikhonov commented Nov 14, 2025

Uh oh!

thalman commented Nov 27, 2025

Uh oh!

sssd-bot commented Nov 27, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

alexey-tikhonov Nov 5, 2025 •

edited

Loading