network interface selector, eliminate localhost mDNS advertisements #258

vves · 2023-02-20T14:53:35Z

@Apollon77 - updated the old osx branch to allow for selection of network interface in CLI at startup of Device.js. Additionally -

The Matter Core Spec claims that only Site-Local IPv6 should be used so I updated the AAAA record to only be set for addresses prefixed with 0xF#.

Minor logging updates for debugging the Linux mDNS issues. From my observations, I believe the mDSN issue that I am observing occurs in the  Home App as other network sniffers on my iOS device are able to ping the mDNS.local address but the Home App is unable to initiate commissioning from either the QR or from selecting the Matter Device icon in the Add Device UI. At this point I am waiting for 16.4 before looking deeper into the mDNS issue as I can properly test the cluster I am working on in OSX even though the final implementation will be on Linux.

Feel free to update and use what's useful.

Apollon77

Hi @vves

Thanks a lot for your contribution. I agree with limiting to local IPv6 interfaces but that should be done in the existing classes that already implement getting network interfaces.
We already have platforms abstracted.

Also while a network interface selection is convenient for interaction usage it is not for scripted usage. So a cli Parameter should be enough in my eyes. I would simply get that like the others in the Device class.

For logging please rethink if logging needed for you to figure out things makes the same sense for the long run.

I myself basically spent the whole weekend also with that topic and have this and a bit more already locally waiting to bring in as PR. Maybe an option if you review mine? Then you see the things i mean.

package.json

src/Device.ts

Apollon77 · 2023-02-20T22:16:07Z

src/matter/mdns/MdnsBroadcaster.ts

@@ -110,7 +112,9 @@ export class MdnsBroadcaster implements Broadcaster {
                if (isIPv4(ip)) {
                    records.push(ARecord(hostname, ip));
                } else {
-                    records.push(AAAARecord(hostname, ip));
+                    if(ip.startsWith('fe80::')){ // only add the local address


Is this the right place? What about Interfaces with just non local IPv6? They would still add the other records, or?! Makes that sense?

It makes sense, but my reading of the spec - which might be incorrect - is that only local IPv6 should be used for the IPv6 Unicast & Multicast address per section 2.5.6.2 and 2.5.6.1 in the Core Spec.

I agree with the change. it is just the wrong code place in my eyes. We shoukd sort it out directly when getting the networtk interfaces and maybe then cut out whole interfaces if we removed all adresses

There could be twoplaces where this better matches.
1.) In https://github.com/mfucci/node-matter/blob/main/src/net/node/NetworkNode.ts#L72 we prepare mac and ip list for a network interface ... that is currently also only used when broadcasting when I digged through code correctly
2.) In https://github.com/mfucci/node-matter/blob/main/src/net/MdnsServer.ts#L80 we prepare the list of network interfaces for announce. Here we shuld only return those with an IP in it that is announceable

So maybe idea is to sort out all "non local ipv6" in place #1 and check that this has no effects for receiving, but should not. Then sort out networkinterfaces without any IP left in #2and throw error when nothing is left?!

Thanks for the pointers - I'll do some testing with it in MdnsServer as that is closer than where I originally put the logic. cheers.

Looks like NetworkNode makes the most sense. Note that I have only tested against HomeApp, but I validated that the IPAddrs for two TP-Link production Matter devices only have the fe, fd prefixed IPv6 addrs in the _matter._tcp entries AND in re-reading the 2.5.6.2 section of the Core Spec believe this is correct. Please sanity check this.

"fe" is link.local, fc/fc is site local ...(and yes fc deprecated from what I read, but could still be out there)

ooookkkk ... I had a longer chat with a Network administrator about the whole topic and ... well ... it's not that easy :-)

IF we assume that all users have only one "network segment"/"VLAN" then fe would be best because we would know that all devices are "Local" and so in the same network. BUT I think we can not assume this safely because alone I know enough people that separate "iot stuff" into an own network and all other stuff into s separate one. Yes they will then have fun that UDP flows correctly, but thats another story.

But annoucement wise it makes no sense, from what I understood, to limit network interfaces at all because it is still valid to use global adresses for better routing ...

Yes maybe we need "site local only" for group multicsts later ... but thats later".

Do anyone know opther people to get expertises? I will also try to find the codeplace in chip tool later ... lets see what they do

src/util/Platform.ts

Apollon77 · 2023-02-20T22:27:38Z

Ps: I also work on macOS and for me anything is fine with iOS and also HomePod. What exact issues you have?

package.json

src/util/Platform.ts

src/Device.ts

vves · 2023-02-21T01:01:40Z

@Apollon77 @mfucci thanks for the review. One thing that would help is understanding who the current audience is for this library. Expect updates to the branch tomorrow.

vves · 2023-02-21T01:07:33Z

@Apollon77 The issues I am having around Home App at this point only occur when setting up node-matter on a Parallels Linux (any flavor) VM. My final usage of the library will be on a Linux instance, so it's important to me that I sort out the IPv6 discovery issues for the Home App. That said - while the mDNS configuration seems correct when running on Linux, the Home App does not seem to find the instance or start the commissioning process (unlike when I run node-matter on OSX). In a deeper dive, I found that my iOS devices can resolve the XXXXXXXXXXXX.local address in apps other than the Home App. Have you found the Home App to be sensitive to mDNS lookup and do you know how the Home App WiFi Matter device lookup works?
Cheers!

Apollon77 · 2023-02-21T06:13:14Z

One thing that would help is understanding who the current audience is for this library.

In m eyes it is a library and the current device/controller are kind of example scripts. We prepare to splitup further and on the long run I personally would see the device/controller files more go into a searate repository that then offers a "cli"-approach. There we can then go fancy to séasiely allow to "do things with CLI", and this here stays the "library to integrate e.g. into node-red and such".

Thats a bit my personal opinion because not deeply discussed in the group.

Apollon77 · 2023-02-21T06:14:04Z

Hm ... maybe check how your VM is setup (host mode on network?) so that UDP flows correctly. I run chip tool in a linux VM and this also works fine with my iOS Home app...

src/Device.ts

Apollon77 · 2023-02-21T08:28:37Z

src/matter/mdns/MdnsBroadcaster.ts

@@ -110,7 +112,9 @@ export class MdnsBroadcaster implements Broadcaster {
                if (isIPv4(ip)) {
                    records.push(ARecord(hostname, ip));
                } else {
-                    records.push(AAAARecord(hostname, ip));
+                    if(ip.startsWith('fe80::')){ // only add the local address


There could be twoplaces where this better matches.
1.) In https://github.com/mfucci/node-matter/blob/main/src/net/node/NetworkNode.ts#L72 we prepare mac and ip list for a network interface ... that is currently also only used when broadcasting when I digged through code correctly
2.) In https://github.com/mfucci/node-matter/blob/main/src/net/MdnsServer.ts#L80 we prepare the list of network interfaces for announce. Here we shuld only return those with an IP in it that is announceable

So maybe idea is to sort out all "non local ipv6" in place #1 and check that this has no effects for receiving, but should not. Then sort out networkinterfaces without any IP left in #2and throw error when nothing is left?!

Apollon77

Thank you for the corrections.

Can you point me to the part of the specs where you found that "Site local" info? I lso founnd it but just/mainly for "group multicasts" and these are not yet implemented.

In the whole capture about IPv6 (4.2) and Discovery (4.3) I can not find this.
In fact there link-local is mentioned, which makes a lot of sense because also mdns is mainly designed for link-local usage:

When all Matter Nodes are attached to the same Wi-Fi / Ethernet network, link-local IPv6 addressing is sufficient - no addi tional IPv6 network infrastructure is required.

and in 4.2.2 (Matter Node behaviour)

Matter Nodes SHALL configure a link-local IPv6 address.

Would (and I'm far away from being an IPv6 expert) that not mean that also announcements should mainly be with "Link local" interfaces?

But in fact also site-local makes sense when thinking, so maybe add "fe" also as allowed?

@mfucci @turon maybe you also have an opinion on this?

Apollon77 · 2023-02-24T11:13:14Z

src/net/node/NetworkNode.ts

@@ -72,7 +72,9 @@ export class NetworkNode extends Network {
    getIpMac(netInterface: string): { mac: string; ips: string[]; } | undefined {
        const netInterfaceInfo = networkInterfaces()[netInterface];
        if (netInterfaceInfo === undefined) return undefined;
-        return { mac: netInterfaceInfo[0].mac, ips: netInterfaceInfo.map(({address}) => address) };
+        // only use local IPv6 address
+        const ips = netInterfaceInfo.map(({address}) => address).filter(ip => isLocalIPv6Address(ip))


This would also filter out all IPV4 adresses, so there should be another check?

2.5.6.4. IPv4 Coexistence
Matter devices SHALL be tolerant of IPv4 addresses and MAY ignore those addresses for the purposes of Matter operations.

I interpret this as there is NO need to advertise on the IPv4 network as Matter requires an IPv6 network to function.

In hindsite - It might be useful to provide an error upon using the library on a network interface without IPv6.

I agree, according to specs we could kill ipv4 in announcement. But honestly it also do not hurt to still announce there.

Previously when advertising the global IPv6 address as well as the ULA, the Home App was unable to 'find' the virtual device to commission when using a Linux instance. Now that node-matter only advertises the ULA as defined in the spec, Home App had no issue commissioning. I am going to strongly recommend we NOT advertise the global IPv6 address and adhere to the spec.

I will discuss this with some more devs tonight. I can only say that for me all works with Apple, Google and Alexa, also when they use the global adress because it seesm to be correctly routed for me ... So the interesting question is why not for you?
How your network looks like? Where is the ios device, where node-matter ... ?!

can you provide an example of a 'global' address on your system?

Sure, here two network interfaces as ifconfig shows them

bridge0: flags=8863<UP,BROADCAST,SMART,RUNNING,SIMPLEX,MULTICAST> mtu 1500 options=3<RXCSUM,TXCSUM> ether c8:2a:14:59:a1:8e inet6 fe80::83a:a16e:cdc2:bbd5%bridge0 prefixlen 64 secured scopeid 0xd inet 192.168.178.57 netmask 0xffffff00 broadcast 192.168.178.255 inet6 2a02:8071:2c87:399c:1cc5:8756:a18d:3ecf prefixlen 64 autoconf secured inet6 2a02:8071:2c87:399c:d90a:8773:6b1c:915a prefixlen 64 autoconf temporary Configuration: id 0:0:0:0:0:0 priority 0 hellotime 0 fwddelay 0 maxage 0 holdcnt 0 proto stp maxaddr 100 timeout 1200 root id 0:0:0:0:0:0 priority 0 ifcost 0 port 0 ipfilter disabled flags 0x0 member: en11 flags=3<LEARNING,DISCOVER> ifmaxaddr 0 port 4 priority 0 path cost 0 member: en1 flags=3<LEARNING,DISCOVER> ifmaxaddr 0 port 11 priority 0 path cost 0 member: en2 flags=3<LEARNING,DISCOVER> ifmaxaddr 0 port 10 priority 0 path cost 0 member: en3 flags=3<LEARNING,DISCOVER> ifmaxaddr 0 port 9 priority 0 path cost 0 member: en4 flags=3<LEARNING,DISCOVER> ifmaxaddr 0 port 12 priority 0 path cost 0 nd6 options=201<PERFORMNUD,DAD> media: autoselect status: active en0: flags=8863<UP,BROADCAST,SMART,RUNNING,SIMPLEX,MULTICAST> mtu 1500 options=6463<RXCSUM,TXCSUM,TSO4,TSO6,CHANNEL_IO,PARTIAL_CSUM,ZEROINVERT_CSUM> ether 88:66:5a:44:3c:4c inet6 fe80::142e:c5db:53f:cd10%en0 prefixlen 64 secured scopeid 0x7 inet 192.168.178.159 netmask 0xffffff00 broadcast 192.168.178.255 inet6 2a02:8071:2c87:399c:8a0:8914:4763:faad prefixlen 64 autoconf secured inet6 2a02:8071:2c87:399c:cdfc:1c49:5275:8b9c prefixlen 64 autoconf temporary nd6 options=201<PERFORMNUD,DAD> media: autoselect status: active

src/Device.ts

src/matter/mdns/MdnsBroadcaster.ts

Apollon77 · 2023-02-28T21:44:46Z

@vves Hi, we (@mfucci @turon and myself) had a a talk how we should best handle this topic and situation and we came to the following conclusion:

Yes according to specs IPv4 is completely optional, but because of the fact that it works we decided that we want ti stay with IPv4 announcements but allow a "parameter/setting" to do not announce them
For many users so far the IPv6 "global address announcement" works fine because also sometimes they are better routeable then the local ones. So we also decided that we also want to stay with the "announce all" as default but add a parameter/setting to only announce locally

I think with this we can adress all cases best for now. We will then see how many users have issues and need the parameters and based on this we can adjust the defaults.

What do you think about that? If you like it would be cool if you adjust the PR for that ... if not also fine then I would offer to take it over and adjust your PR and you review. Just tell me what you like.

Ingo

Apollon77 · 2023-03-03T05:24:07Z

@vves i had one idea that could be worth trying. We could sort the interface addresses by "local IPv6 first, ipv4 last" ... at least should have an effect on the order of the records returned by the mdns server. Maybe then your device chooses also a local one over the globals?? WDYT?

vves · 2023-03-03T17:05:49Z

@vves i had one idea that could be worth trying. We could sort the interface addresses by "local IPv6 first, ipv4 last" ... tragbar least should have an effect on the order of the records returned by the mdns server. Maybe then your device chooses also a local one over the globals?? WDYT?

This is the way. I'll make this happen monday.

Apollon77 · 2023-03-03T19:41:10Z

I think the best place would be in https://github.com/mfucci/node-matter/blob/main/src/net/node/NetworkNode.ts#L75

Apollon77 · 2023-03-03T23:28:50Z

@vves FYI I added one commit to solve a merge conflict.

Additionally for your WindowCovering CLuster you work on: We prepare a move of this repo into matter.js repo ... If all went well could be next week-ish. So please decide if you bring it in before or afterwards :-)

vves · 2023-03-07T01:14:20Z

@Apollon77 Today got away from me. Expect the IPv6 changes Tuesday and let's wait on the WC Cluster. Watch for questions in the WC issue.

vves · 2023-03-07T14:06:14Z

@Apollon77 I am going to filter out 'fe80::/10', '::1' as well. There is no reason to advertise on localhost.

vves · 2023-03-10T16:56:23Z

@Apollon77 After much time with wireshark between iOS and Parallels:Ubuntu I am 100% sure that Parallels does NOT correctly handle IPv6 traffic. I should be able to finish this issue today.

vves · 2023-03-10T19:15:39Z

@Apollon77 At this point - I recommend culling just the ipv6 localhost addresses and close this out. That reasonable?

Apollon77 · 2023-03-10T19:23:58Z

Go ahead

Apollon77

Please have a look at my comments ...

src/matter/mdns/MdnsBroadcaster.ts

src/Device.ts

src/util/Ip.ts

src/net/node/NetworkNode.ts

…oadcaster

Apollon77 · 2023-03-15T08:50:57Z

src/util/Ip.ts

+ */
+export function isMatterAddressableIPv6Address(targetIpAddr: string): boolean {
+    return !(
+        targetIpAddr.toLowerCase().startsWith("fe80") ||


@vves Hm ... why you now sort out link local too? fe80 are all link locals ... why not announce them? In my eyes they are valid

I have come to the conclusion based on empirical testing and the spec that while fe80 is valid, it is NOT valid when it's the ONLY address on an interface. The larger issue that I have come across is that if an interface only provides on they IPV6 fe80 address the HomeApp and SmartThings will not resolve the endpoint. The solution is to validate a network interface provides at least ONE valid IPv6 address that is not a loopback address or IPv4 Address, but NOT remove the loopback and IPv4 Addresses. If there is not at least one Matter addressable IPv6 address, the library should not advertise that interface.

Below is the list from my test machine. Only the en0 interface is addressable from HomeApp or SmartThings.

This function is valid for as named, but the use of it needs to be in conjuntion with a network interface validator.

2023-03-15 09:39:46.336 DEBUG MdnsBroadcaster anpi0 on IP Addrs: fe80::5082:3eff:fe47:1cdc host: 52823E471CDC0000.local 2023-03-15 09:39:46.336 DEBUG MdnsBroadcaster anpi2 on IP Addrs: fe80::5082:3eff:fe47:1cde host: 52823E471CDE0000.local 2023-03-15 09:39:46.337 DEBUG MdnsBroadcaster en0 on IP Addrs: fe80::85e:cc32:9f1:c265, fd69:48a4:185f:1:10eb:e991:a821:617e, 2601:285:500:bf9:14a5:cb4d:2f38:873f, 2601:285:500:bf9:3885:23da:3b72:902d, fd68:9b7f:6836:44cd:25:de98:15d:375c, 10.0.10.145 host: F02F4B09EDFA0000.local 2023-03-15 09:39:46.337 DEBUG MdnsBroadcaster awdl0 on IP Addrs: fe80::80ab:7dff:fe1a:4df host: 82AB7D1A04DF0000.local 2023-03-15 09:39:46.338 DEBUG MdnsBroadcaster llw0 on IP Addrs: fe80::80ab:7dff:fe1a:4df host: 82AB7D1A04DF0000.local 2023-03-15 09:39:46.338 DEBUG MdnsBroadcaster utun0 on IP Addrs: fe80::9ae3:6419:5975:cc9 host: 0000000000000000.local 2023-03-15 09:39:46.339 DEBUG MdnsBroadcaster utun1 on IP Addrs: fe80::97e2:cfd3:445a:8763 host: 0000000000000000.local 2023-03-15 09:39:46.339 DEBUG MdnsBroadcaster utun2 on IP Addrs: fe80::ce81:b1c:bd2c:69e host: 0000000000000000.local 2023-03-15 09:39:46.340 DEBUG MdnsBroadcaster utun3 on IP Addrs: fe80::e81a:e66d:611a:b465 host: 0000000000000000.local 2023-03-15 09:39:46.340 DEBUG MdnsBroadcaster utun4 on IP Addrs: fe80::5120:d18f:4b0e:ed81 host: 0000000000000000.local 2023-03-15 09:39:46.341 DEBUG MdnsBroadcaster utun5 on IP Addrs: fe80::1c2a:4bb3:bf2:39b4 host: 0000000000000000.local 2023-03-15 09:39:46.341 DEBUG MdnsBroadcaster utun6 on IP Addrs: fe80::557c:6550:d754:c9c host: 0000000000000000.local 2023-03-15 09:39:46.341 DEBUG MdnsBroadcaster utun7 on IP Addrs: fe80::f4d0:e8d5:2bde:f628 host: 0000000000000000.local 2023-03-15 09:39:46.342 DEBUG MdnsBroadcaster utun8 on IP Addrs: fe80::b396:3357:d6aa:fd8e host: 0000000000000000.local```

Sorry but then I'm back to: How does your network structure looks like? Completely fine that you tested it on your side, but I can not reproduce this at all behaving that way and "Link local" adresses are valid and used when devices are on the same switch. please check your firewall settings not that you have any kind of firewall active that blpocks connections on the link local adress?

Apollon77 · 2023-03-19T17:21:21Z

The announcement interface is incorporated in project-chip/matter.js#64 together with more changes

vves · 2023-03-27T18:44:46Z

Closing due to duplicate functionality in the new project-chip library.

vves and others added 6 commits February 15, 2023 14:11

Fix for OSX network interface. set to en0

1c82f8d

better logging for mdns issue

799a7de

ease of use package scripts for library consumers

ac96697

mdns should only provide the AAAA record for local IPv6 addresses

b0f7750

explicitly select the network interface

9d361c6

Merge branch 'mfucci:main' into osx-virtual-device-fix

00b605e

Apollon77 requested changes Feb 20, 2023

View reviewed changes

mfucci reviewed Feb 20, 2023

View reviewed changes

code review updates

c354a11

vves requested a review from Apollon77 February 21, 2023 01:35

Apollon77 requested changes Feb 21, 2023

View reviewed changes

Filter Multicast Addresses for local IPv6 only.

3cbc0bd

vves requested a review from Apollon77 February 23, 2023 01:26

maintaining consistency with other 'demo' code in library

ecb81c1

Apollon77 requested changes Feb 24, 2023

View reviewed changes

Merge branch 'main' into osx-virtual-device-fix

01fc701

vves marked this pull request as draft March 7, 2023 01:12

rename for clarity

f67363c

vves added 3 commits March 10, 2023 12:44

exclude IPv6 localhost address from mDNS Ad

333832b

better localhost matching w/ v4 addition

96b151c

Merge branch 'mfucci:main' into osx-virtual-device-fix

ae150c0

vves changed the title ~~Network interface selector, IPV6 local ads only~~ network interface selector, eliminate localhost mDNS advertisements Mar 10, 2023

vves requested a review from Apollon77 March 10, 2023 20:18

vves marked this pull request as ready for review March 10, 2023 20:19

Apollon77 requested changes Mar 11, 2023

View reviewed changes

src/matter/mdns/MdnsBroadcaster.ts Outdated Show resolved Hide resolved

src/Device.ts Outdated Show resolved Hide resolved

src/util/Ip.ts Show resolved Hide resolved

src/net/node/NetworkNode.ts Show resolved Hide resolved

vves added 3 commits March 13, 2023 08:47

naming preference change

88b1584

Merge branch 'mfucci:main' into osx-virtual-device-fix

cbd84bf

filter network interfaces for valid networks before setting up MdnsBr…

f26d95e

…oadcaster

vves requested a review from Apollon77 March 13, 2023 20:06

Apollon77 added 2 commits March 15, 2023 09:43

codestyle

f0597aa

consistent naming

17bafcf

Apollon77 reviewed Mar 15, 2023

View reviewed changes

vves closed this Mar 27, 2023

network interface selector, eliminate localhost mDNS advertisements #258

network interface selector, eliminate localhost mDNS advertisements #258

Conversation

vves commented Feb 20, 2023

Apollon77 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Apollon77 commented Feb 20, 2023

vves commented Feb 21, 2023

vves commented Feb 21, 2023

Apollon77 commented Feb 21, 2023

Apollon77 commented Feb 21, 2023

Choose a reason for hiding this comment

Apollon77 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Apollon77 Feb 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Apollon77 commented Feb 28, 2023

Apollon77 commented Mar 3, 2023 • edited Loading

vves commented Mar 3, 2023

Apollon77 commented Mar 3, 2023

Apollon77 commented Mar 3, 2023

vves commented Mar 7, 2023

vves commented Mar 7, 2023 • edited Loading

vves commented Mar 10, 2023

vves commented Mar 10, 2023

Apollon77 commented Mar 10, 2023

Apollon77 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vves Mar 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Apollon77 commented Mar 19, 2023

vves commented Mar 27, 2023

Apollon77 Feb 28, 2023 •

edited

Loading

Apollon77 commented Mar 3, 2023 •

edited

Loading

vves commented Mar 7, 2023 •

edited

Loading

vves Mar 15, 2023 •

edited

Loading