search

project-chip / connectedhomeip

Matter (formerly Project CHIP) creates more connections between more objects, simplifying development for manufacturers and increasing compatibility for consumers, guided by the Connectivity Standards Alliance.
https://buildwithmatter.com
Apache License 2.0
7.23k stars 1.92k forks source link

[BUG] Chip-lighting-app with "minimal mdns" do not adhere to MDNS specs: Sends Multicast Truncated Responses #29521

Open Apollon77 opened 9 months ago

Apollon77 commented 9 months ago

Reproduction steps

I checked via Wireshark what a chip lighting app does MDNS wise and found that a Truncated response is sent to annoucne itself because all data do not fit into the 512 byte of a single MDNS message.

According to MDSN specs this should not be used - see https://www.rfc-editor.org/rfc/rfc6762.html#section-18.5

In multicast response messages, the TC bit MUST be zero on transmission, and MUST be ignored on reception.

This is the wireshark output of the announcements sent out:

First "Truncated" message: Bildschirmfoto_2023-10-01_um_23 02 35

Second "Finalizing" message: Bildschirmfoto_2023-10-01_um_23 03 47

The chip lighting app used here was run on a Raspi with Ubuntu.

Bug prevalence

always

GitHub hash of the SDK that was being used

can not reproduce, mid August 2023

Platform

other

Platform Version(s)

No response

Anything else?

No response

bzbarsky-apple commented 9 months ago

The chip lighting app used here was run on a Raspi with Ubuntu.

Compiled how? Was this using minimal mdns or platform mdns?

Also, the issue title is talking about chip-tool, but the actual description seems to be about chip-lighting-app. Which is it @Apollon77 ?

Apollon77 commented 9 months ago

Sorry to be inaccurate here.

Ok I verified it. It is the "chip lighting app" taken from the certification binaries from Test Harness (so also not self compiled), last updated 16.9.23, so I can not tell how they are compiled ...

When I see this output I would assume "minimal mdns" ...

[1696279815.505365][54692:54692] CHIP:DIS: Updating services using commissioning mode 1
[1696279815.505984][54692:54692] CHIP:DIS: CHIP minimal mDNS started advertising.
[1696279815.507661][54692:54692] CHIP:DL: Using wifi MAC for hostname
[1696279815.507761][54692:54692] CHIP:DIS: Advertise commission parameter vendorID=65521 productID=32769 discriminator=3840/15 cm=1
[1696279815.507811][54692:54692] CHIP:DIS: Responding with _matterc._udp.local
[1696279815.507836][54692:54692] CHIP:DIS: Responding with D4FD8210EAC6F82F._matterc._udp.local
[1696279815.507858][54692:54692] CHIP:DIS: Responding with DCA6323EE5EE0000.local
[1696279815.507878][54692:54692] CHIP:DIS: Responding with DCA6323EE5EE0000.local
[1696279815.507910][54692:54692] CHIP:DIS: Responding with _V65521._sub._matterc._udp.local
[1696279815.507938][54692:54692] CHIP:DIS: Responding with _T257._sub._matterc._udp.local
[1696279815.507963][54692:54692] CHIP:DIS: Responding with _S15._sub._matterc._udp.local
[1696279815.507988][54692:54692] CHIP:DIS: Responding with _L3840._sub._matterc._udp.local
[1696279815.508013][54692:54692] CHIP:DIS: Responding with _CM._sub._matterc._udp.local
[1696279815.508049][54692:54692] CHIP:DIS: Responding with D4FD8210EAC6F82F._matterc._udp.local
[1696279815.508071][54692:54692] CHIP:DIS: CHIP minimal mDNS configured as 'Commissionable node device'; instance name: D4FD8210EAC6F82F.
[1696279815.511667][54692:54692] CHIP:DIS: mDNS service published: _matterc._udp
[1696279815.511714][54692:54692] CHIP:DIS: Updating services using commissioning mode 1
[1696279815.512284][54692:54692] CHIP:DIS: CHIP minimal mDNS started advertising.
[1696279815.519674][54692:54692] CHIP:DL: Using wifi MAC for hostname
[1696279815.519785][54692:54692] CHIP:DIS: Advertise commission parameter vendorID=65521 productID=32769 discriminator=3840/15 cm=1
[1696279815.519830][54692:54692] CHIP:DIS: Responding with _matterc._udp.local
[1696279815.519854][54692:54692] CHIP:DIS: Responding with D4FD8210EAC6F82F._matterc._udp.local
[1696279815.519875][54692:54692] CHIP:DIS: Responding with DCA6323EE5EE0000.local
[1696279815.519897][54692:54692] CHIP:DIS: Responding with DCA6323EE5EE0000.local
[1696279815.519919][54692:54692] CHIP:DIS: Responding with _V65521._sub._matterc._udp.local
[1696279815.519942][54692:54692] CHIP:DIS: Responding with _T257._sub._matterc._udp.local
[1696279815.519965][54692:54692] CHIP:DIS: Responding with _S15._sub._matterc._udp.local
[1696279815.519990][54692:54692] CHIP:DIS: Responding with _L3840._sub._matterc._udp.local
[1696279815.520012][54692:54692] CHIP:DIS: Responding with _CM._sub._matterc._udp.local
[1696279815.520046][54692:54692] CHIP:DIS: Responding with D4FD8210EAC6F82F._matterc._udp.local
[1696279815.520066][54692:54692] CHIP:DIS: CHIP minimal mDNS configured as 'Commissionable node device'; instance name: D4FD8210EAC6F82F.
[1696279815.523128][54692:54692] CHIP:DIS: mDNS service published: _matterc._udp

If it helps I can play around with these and macos "self build" ones and see what they report.

Apollon77 commented 9 months ago

PS: A chip lighing app compiled on my macos seems to use platform mdns and behaves different

bzbarsky-apple commented 9 months ago

When I see this output I would assume "minimal mdns" ...

Yes, that looks like minimal mdns.

@andy31415 could you take a look, please?

andy31415 commented 9 months ago

I think truncation bit was on purpose, maybe I had mis-read the spec. What does platformdns do in those circumstances? Since the packets do not fit in 512 bytes, we have to send several packets

andy31415 commented 9 months ago

The setting is here: https://github.com/project-chip/connectedhomeip/blob/master/src/lib/dnssd/minimal_mdns/ResponseSender.cpp#L310

//....
 if (!mResponseBuilder.Ok())
    {
        mResponseBuilder.Header().SetFlags(mResponseBuilder.Header().GetFlags().SetTruncated(true));

        ReturnOnFailure(mSendState.SetError(FlushReply()));
        //...

And can be easily removed, however since we had a few iterations on this I am concerned not to introduce some odd incompatibility issues. It seems we probably have to track unicast vs multicast replies here.

Apollon77 commented 9 months ago

I think one discussion is the 512 bytes because this is the "very old interprettion" ... there are also infos that 1500 - or when looking at IPv6 1280 are completely ok values ... when you then substract udp and ipv6 headers you end up in 1232 bytes pure payload that should work these days without any issue. I honestly do not know in which cases this is not acceptable.

macos platform implementation is splitting it into several announcments: 1.) one with the Hostname and the IPs for ipv4 and ipv6

Bildschirmfoto 2023-10-03 um 16 11 03

2.) one with the matter specific records Bildschirmfoto 2023-10-03 um 16 11 34

3.) one again with hostname but only ipv6 ips

4.) one "big" (516 bytes) with all data together "lead" by TXT record Bildschirmfoto 2023-10-03 um 16 13 54

5.) And then again "anything" but without the TXT record :) Bildschirmfoto 2023-10-03 um 16 14 51

So from that honestly ... no idea whats a best practice :-))

I stumbled over the sizing issue mainly because a device paired with 4 ecosystems ( I think when you use 512 bytes you already start to get those issues with the second or third device) went over size and so all throw away.

SO when you decide for a "best practice" on how to split into packages it qwould be cool to get the infos and I will adjust matter.js implementation the same.

andy31415 commented 9 months ago

Generally I assume mac does the right thing, however we probably have to test backwards compatibility with avahi and maybe others.

Probably should do it right after a branch point to allow things to settle down. Things that we seem to want to adjust: usage of truncated, maximum payload size.

Apollon77 commented 9 months ago

I agree. Announcing host and rest separately makes sense - but why multiple times each a bit differently? ;-) (and that apple implementation always announce a hostname with just 0 is still strange but most likely different topic).

If I can support in that topic just poke me ... whatever you implement I would also add then in matter.js the same way for our "own implementation" - but we also plan to add more native options like avahi via dbus or such platform Specific solutions.

bzbarsky-apple commented 9 months ago

apple implementation always announce a hostname with just 0

That's because we never get a useful MAC address. Filed https://github.com/project-chip/connectedhomeip/issues/29556