I believe something like this ought to be able to resolve item #2 in samba_dnsupdate:

import array
import sys

def in6_is_addr_linklocal (addr):
    # Extract the first two octets
    idx = addr.find(':')
    if idx == -1:
        return False

    # Check for link-local IPv6. Note that
    # this comprises the first 10 bits (not 16)
    # See also RFC 2373
    octets = array.array('B', addr[:idx].decode("hex"))

    if len(octets) != 2:
        return False

    if octets[0] != 0xfe:
        return False

    if (octets[1] & 0xc0) != 0x80:
        return False;

    return True;

Created attachment 8566 [details]
Simple test of getnameinfo() and flags

If also decided to jump this train and did a bit of testing. Here I attach simple test program that queries the interfaces, get's their names and flags. In the next comments I'll show some resulting output, but the base line is that Landon is correct - we are testing flags of the interfaces, not IP addresses, so we can't rely on the interface flag to assume does it have broadcast address or not. A bit more digging around AF family and typ of the address has to be done.

The second statement isn't correct though - this code SUPPOSED to calculate broadcast address, not filter out Link Local addresses.

There is another difference between Linux and FreeBSD exposed also - the RFC 4007 compliant LL addresses in FreeBSD are not returned automagically by getnameinfo(). From the man page:

     This implementation allows numeric IPv6 address notation with scope iden-
     tifier, as documented in chapter 11 of RFC 4007.  IPv6 link-local address
     will appear as a string like ``fe80::1%ne0''.  Refer to getaddrinfo(3)
     for more information.

And from getaddrinfo():

    This implementation of getaddrinfo() allows numeric IPv6 address notation
     with scope identifier, as documented in chapter 11 of draft-ietf-
     ipv6-scoping-arch-02.txt.  By appending the percent character and scope
     identifier to addresses, one can fill the sin6_scope_id field for
     addresses.  This would make management of scoped addresses easier and
     allows cut-and-paste input of scoped addresses.

That brings the need to fill 'sin6_scope_id' filed of sockaddr_in6 structure(manually). Luckily, it's easy to do and test shows, how.

So, based on that test case this part of the _get_interfaces() has to be reworked. As a result we'll get LL addresses with %iface scope id, that can be filtered by current code.

FreeBSD9 with LL IPv6 only:

em0  address family: 18 (AF_LINK)
em0  address family: 2 (AF_INET)
        address: <10.0.0.236>
        Flags: IFF_UP IFF_BROADCAST IFF_DRV_RUNNING  IFF_SIMPLEX  IFF_MULTICAST
em0  address family: 28 (AF_INET6)
        address: <fe80:1::20c:29ff:fe28:c72b%em0>
        Flags: IFF_UP IFF_BROADCAST IFF_DRV_RUNNING  IFF_SIMPLEX  IFF_MULTICAST
plip0  address family: 18 (AF_LINK)
lo0  address family: 18 (AF_LINK)
lo0  address family: 28 (AF_INET6)
        address: <::1>
        Flags: IFF_UP IFF_LOOPBACK  IFF_DRV_RUNNING  IFF_MULTICAST
lo0  address family: 28 (AF_INET6)
        address: <fe80:3::1%lo0>
        Flags: IFF_UP IFF_LOOPBACK  IFF_DRV_RUNNING  IFF_MULTICAST
lo0  address family: 2 (AF_INET)
        address: <127.0.0.1>
        Flags: IFF_UP IFF_LOOPBACK  IFF_DRV_RUNNING  IFF_MULTICAST


Debina6 Linux with LL IPv6 only:

lo  address family: 17 (AF_PACKET)
eth0  address family: 17 (AF_PACKET)
eth1  address family: 17 (AF_PACKET)
eth2  address family: 17 (AF_PACKET)
eth3  address family: 17 (AF_PACKET)
eth4  address family: 17 (AF_PACKET)
eth5  address family: 17 (AF_PACKET)
eth6  address family: 17 (AF_PACKET)
eth7  address family: 17 (AF_PACKET)
bond0  address family: 17 (AF_PACKET)
bond1  address family: 17 (AF_PACKET)
bond2  address family: 17 (AF_PACKET)
bond3  address family: 17 (AF_PACKET)
lo  address family: 2 (AF_INET)
        address: <127.0.0.1>
        Flags: IFF_UP IFF_LOOPBACK
eth3  address family: 2 (AF_INET)
        address: <10.0.2.254>
        Flags: IFF_UP IFF_BROADCAST IFF_MULTICAST
eth3:0  address family: 2 (AF_INET)
        address: <192.168.2.1>
        Flags: IFF_UP IFF_BROADCAST IFF_MULTICAST
bond0  address family: 2 (AF_INET)
        address: <95.211.181.20>
        Flags: IFF_UP IFF_BROADCAST IFF_MULTICAST
bond1  address family: 2 (AF_INET)
        address: <10.0.0.201>
        Flags: IFF_UP IFF_BROADCAST IFF_MULTICAST
lo  address family: 10 (AF_INET6)
        address: <::1>
        Flags: IFF_UP IFF_LOOPBACK
eth3  address family: 10 (AF_INET6)
        address: <fe80::7ae3:b5ff:fe18:89f2%eth3>
        Flags: IFF_UP IFF_BROADCAST IFF_MULTICAST
bond0  address family: 10 (AF_INET6)
        address: <fe80::7ae3:b5ff:fe18:89e8%bond0>
        Flags: IFF_UP IFF_BROADCAST IFF_MULTICAST
bond1  address family: 10 (AF_INET6)
        address: <fe80::7ae3:b5ff:fe18:89ea%bond1>
        Flags: IFF_UP IFF_BROADCAST IFF_MULTICAST


FreeBSD 6.2(sic!) with real IPv6:

fwe0  address family: 18 (AF_LINK)
fwe0  address family: 28 (AF_INET6)
        address: <fe80:1::1:b7ff:fe01:b9b%fwe0>
        Flags: IFF_UP IFF_BROADCAST IFF_DRV_RUNNING  IFF_PROMISC  IFF_SIMPLEX  IFF_MULTICAST
xl0  address family: 18 (AF_LINK)
xl0  address family: 28 (AF_INET6)
        address: <fe80:2::2e0:81ff:fe21:37a6%xl0>
        Flags: IFF_UP IFF_BROADCAST IFF_DRV_RUNNING  IFF_SIMPLEX  IFF_MULTICAST
xl0  address family: 2 (AF_INET)
        address: <192.168.0.4>
        Flags: IFF_UP IFF_BROADCAST IFF_DRV_RUNNING  IFF_SIMPLEX  IFF_MULTICAST
xl0  address family: 28 (AF_INET6)
        address: <2001:888:1d8f:0:2e0:81ff:fe21:37a6>
        Flags: IFF_UP IFF_BROADCAST IFF_DRV_RUNNING  IFF_SIMPLEX  IFF_MULTICAST
plip0  address family: 18 (AF_LINK)
pflog0  address family: 18 (AF_LINK)
pfsync0  address family: 18 (AF_LINK)
lo0  address family: 18 (AF_LINK)
lo0  address family: 28 (AF_INET6)
        address: <::1>
        Flags: IFF_UP IFF_LOOPBACK  IFF_DRV_RUNNING  IFF_MULTICAST
lo0  address family: 28 (AF_INET6)
        address: <fe80:6::1%lo0>
        Flags: IFF_UP IFF_LOOPBACK  IFF_DRV_RUNNING  IFF_MULTICAST
lo0  address family: 2 (AF_INET)
        address: <127.0.0.1>
        Flags: IFF_UP IFF_LOOPBACK  IFF_DRV_RUNNING  IFF_MULTICAST
lo0  address family: 2 (AF_INET)
        address: <10.10.10.10>
        Flags: IFF_UP IFF_LOOPBACK  IFF_DRV_RUNNING  IFF_MULTICAST

Created attachment 8567 [details]
Patch on interfaces.c

This patch should address the problem with Link-Local addresses on FreeBSD and, hopefully on Linux.

Created attachment 8568 [details]
Patch on interfaces.c

URL link, seems does smth. else...

This patch should address the problem with Link Local addresses on FreeBSD and Linux(?).

A test for <net/if.h> and presence of if_nametoindex() is possibly necessary.

(In reply to comment #3)
> The second statement isn't correct though - this code SUPPOSED to calculate
> broadcast address, not filter out Link Local addresses.

If you take a look at:

http://gitweb.samba.org/?p=samba.git;a=blob;f=lib/socket/interfaces.c;h=74c642372a24323befae23534d1f525991a96fab;hb=HEAD#l198

The code in question would filter out link-local addresses, except for the fact that the IFF_BROADCAST flag preempts it. The intended behavior of the code seems to be to filter out link-local addresses, but according to git blame, it hasn't done so for some time :)

> So, based on that test case this part of the _get_interfaces() has to be
> reworked. As a result we'll get LL addresses with %iface scope id, that can be
> filtered by current code.

I believe it's also still necessary to modify any filtering code (eg, samba_dnsupdate) to check the address prefix rather than relying on finding an RFC4007 site scope; the use of % scopes appears to be optional, rather than required, when representing link-local addresses:

https://tools.ietf.org/html/rfc4007#section-11.6

(In reply to comment #8)
> (In reply to comment #3)
> > The second statement isn't correct though - this code SUPPOSED to calculate
> > broadcast address, not filter out Link Local addresses.
> 
> If you take a look at:
> 
> http://gitweb.samba.org/?p=samba.git;a=blob;f=lib/socket/interfaces.c;h=74c642372a24323befae23534d1f525991a96fab;hb=HEAD#l198
> 
> The code in question would filter out link-local addresses, except for the fact
> that the IFF_BROADCAST flag preempts it. The intended behavior of the code
> seems to be to filter out link-local addresses, but according to git blame, it
> hasn't done so for some time :)

If you read the whole function, you'll notice that it tries to build the triplet
of ip, netmask and broadcast addresses. In general, it almost works, after rearranging the order of the code it fully does(Well, I have some doubts about 
IN6_IS_ADDR_V4COMPAT there, but that can be discussed).

> > So, based on that test case this part of the _get_interfaces() has to be
> > reworked. As a result we'll get LL addresses with %iface scope id, that can be
> > filtered by current code.
> 
> I believe it's also still necessary to modify any filtering code (eg,
> samba_dnsupdate) to check the address prefix rather than relying on finding an
> RFC4007 site scope; the use of % scopes appears to be optional, rather than
> required, when representing link-local addresses:
> 
> https://tools.ietf.org/html/rfc4007#section-11.6

Well, I agree that relying on the '%' syntax may be not strict enough. Still, this works both on FreeBSD and Linux now.

There is at least one more place, where LL addresses have to be excluded, check 
provision/__init.py__

Did you test my patch by any chance?

(In reply to comment #9)
> 
> If you read the whole function, you'll notice that it tries to build the
> triplet
> of ip, netmask and broadcast addresses. In general, it almost works, after
> rearranging the order of the code it fully does(Well, I have some doubts about 
> IN6_IS_ADDR_V4COMPAT there, but that can be discussed).

I'm specifically referring to this code:

 205    if (IN6_IS_ADDR_LINKLOCAL(in6) || IN6_IS_ADDR_V4COMPAT(in6)) {
 206        continue;
 207    }

> Did you test my patch by any chance?

Not yet!

If we can get agreement that this works for FreeBSD and Linux, I can try to shepherd this into git (following the correct rules, however), unless Andrew takes it up.

I don't currently have any IPv6 setups, and will have to come up to speed, so will be relying on others doing the testing.

(In reply to comment #11)
> If we can get agreement that this works for FreeBSD and Linux, I can try to
> shepherd this into git (following the correct rules, however), unless Andrew
> takes it up.

It should work for both platforms, but as Landon stated above it's not clear what was intended policy for Link-Local and V4inV6 addresses.

Shall they be dropped entirely from the list of IP addresses or they should be filtered later on application level?

Also, in case we have only lo with 127.0.0.1 - what would be the behaviour of this function? It overrides array element with the index 'total', but in such a case there will be nothing to override with.

> I don't currently have any IPv6 setups, and will have to come up to speed, so
> will be relying on others doing the testing.

Well, as soon as you have IPv6 enabled kernel(which is the default) - you already have linked-local addresses, what is the part of the problem - they have no real use, but present in the list of valid addresses.

The bug has grown long in the tooth, and I think we're at the point where we need guidance from someone higher up the code chain, so here's my attempt at summarizing the issue for review:

Problem:

Samba 4 on FreeBSD publishes link-local IPv6 addresses in DNS. This does not happen on Linux.
Replication breaks when clients attempt to connect via the IPv6 link-local address.

Cause:

There are two issues:

- get_interfaces() is used to fetch the list of local addresses, and it has code to filter out link-local addresses. Due to a logic error, that code is never run.

- samba_dnsupdate has code to filter out link-local IPv6 addresses, but it relies on there being a %-encoded scope identifier in the address. This succeeds on Linux, and fails on FreeBSD, which is why this issue has not appeared on Linux.

Potential fixes:

There are also two, either of which would resolve this issue:

- get_interfaces() can be modified to not return link-local IPv6 addresses. This seems to be what was originally intended, but I'm not sure whether this is the now-desired behavior.

- samba_dnsupdate can be modified to filter based on the link-local IPv6 address prefix, rather than based on the display formatting of the address.

I'm happy to do both/either, depending on direction from someone responsible for the code in question.

(In reply to comment #13)
> The bug has grown long in the tooth, and I think we're at the point where we
> need guidance from someone higher up the code chain, so here's my attempt at
> summarizing the issue for review:
> 
> Problem:
> 
> Samba 4 on FreeBSD publishes link-local IPv6 addresses in DNS. This does not
> happen on Linux.
> Replication breaks when clients attempt to connect via the IPv6 link-local
> address.
> 
> Cause:
> 
> There are two issues:
> 
> - get_interfaces() is used to fetch the list of local addresses, and it has
> code to filter out link-local addresses. Due to a logic error, that code is
> never run.
> 
> - samba_dnsupdate has code to filter out link-local IPv6 addresses, but it
> relies on there being a %-encoded scope identifier in the address. This
> succeeds on Linux, and fails on FreeBSD, which is why this issue has not
> appeared on Linux.
> 
> Potential fixes:
> 
> There are also two, either of which would resolve this issue:
> 
> - get_interfaces() can be modified to not return link-local IPv6 addresses.
> This seems to be what was originally intended, but I'm not sure whether this is
> the now-desired behavior.
> 
> - samba_dnsupdate can be modified to filter based on the link-local IPv6
> address prefix, rather than based on the display formatting of the address.
> 
> I'm happy to do both/either, depending on direction from someone responsible
> for the code in question.

Off the top of my head I would prefer that samba_dnsupdate filter based on the link-local address prefix, as that has the highest likelihood of working on other platforms without us having to put in platform specific stuff all the time.

get_interfaces() is intended to filter out all link-local addresses. I'd prefer a fix that restores this behaviour.

Jeremy.

(In reply to comment #15)
> get_interfaces() is intended to filter out all link-local addresses. I'd prefer
> a fix that restores this behaviour.

In that case, it seems that the filtering done in samba_dnsupdate should be removed so we do not have two places doing the same thing.

Created attachment 8593 [details]
git-am fix for mater.

This is Timurs patch as a git-am attachment with attribution and a review. Richard, if you can review then I'll push to master and we'll get this into 4.0.next and 3.6.next.

Jeremy.

(In reply to comment #15)
> get_interfaces() is intended to filter out all link-local addresses. I'd prefer
> a fix that restores this behaviour.

[snip]

(In reply to comment #17)
> Created attachment 8593 [details]
> git-am fix for mater.
> 
> This is Timurs patch as a git-am attachment with attribution and a review.
> Richard, if you can review then I'll push to master and we'll get this into
> 4.0.next and 3.6.next.

Note that Timur's patch modifies the check to add the sin6_scope_id, rather than filtering the link-local address. This allows the check for '%' in samba_dnsupdate to succeed, as the address will be then be display-formatted with the % scope id.

Wasn't sure if that was the intention.

Ah. good point and I missed that. That change to

sin6->sin6_scope_id = if_nametoindex(ifptr->ifa_name); 

is actually irrelevant, as the next line is a 'continue' which means that address won't be returned anyway. I'll fix that up and re-attach the patch.

Jeremy.

Created attachment 8594 [details]
Updated git-am fix for master.

Removed unused element in previous patch.

Comment on attachment 8594 [details]
Updated git-am fix for master.

This looks good.

Now if only we can get rid of the special case filtering done by samba_dnsupdate.

Patch applies cleanly to both 3.6.next and 4.0.next. Re-assigning to Karolin for inclusion.

Jeremy.

Pushed to v3-6-test and autobuild-v4-0-test.

I can confirm it resolves (pun intended) replication issues on FreeBSD (although I also had to manually delete the previously created AAAA records).

Created attachment 8598 [details]
Patch for samba_dnsupdate

An additional patch to remove the now-obsolete link-local check from samba_dnsupdate, as per Richard's comment.

(In reply to comment #26)
> Created attachment 8598 [details]
> Patch for samba_dnsupdate
> 
> An additional patch to remove the now-obsolete link-local check from
> samba_dnsupdate, as per Richard's comment.

Hi Landon,

Thank you for this patch, but one problem per bug and this bug is close to being closed.

Can you create a new bug and attach the patch, and I will shepherd it through.

Hi again Landon.

Can you include a Signed-Off-By line in that patch as well?

(In reply to comment #28)
> Hi again Landon.
> 
> Can you include a Signed-Off-By line in that patch as well?

Sure. Follow-up filed as https://bugzilla.samba.org/show_bug.cgi?id=9696

Pushed to v4-0-test.
Closing out bug report.

Thanks!