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.39k stars 1.98k forks source link

[Android] Hostname in nsdmnager is not compliant with matter #33474

Open yunhanw-google opened 4 months ago

yunhanw-google commented 4 months ago

In pixel 7, we see nsdManager is advertising host name with 40bytes, in android nsdManager, there is no API to set this hostname. The below is avahi-browse -r _matter._tcp = wlan0 IPv6 B6071280809E4E17-000000000001B669 _matter._tcp local hostname = [Android_25101c8afe6a479387b1d63318378d56.local] address = [192.168.0.102] port = [58586]

During ICD check-in validation, we see the mdns advertisment from android cannot be resolved in lit icd board because of the the above length issue so that lit cannot send the check-in message, In order to resolve this issue, we expand the kHostNameMaxLength to 40 in dnssd for workaround

But it would violate the matter spec description as below "For DNS‑SD a target host name is required, in addition to the instance name. The target host name SHALL be constructed using one of the available link-layer addresses, such as a 48-bit device MAC address (for Ethernet and Wi‑Fi) or a 64-bit MAC Extended Address (for Thread) expressed as a fixed-length twelve-character (or sixteen-character) hexadecimal string, encoded as ASCII (UTF-8) text using capital letters, e.g., B75AFB458ECD.. "

Potential solution:

  1. Modify the matter spec to adapt with android requirement.
  2. Extend a new API in android nsdmanager to set the hostname
yunhanw-google commented 4 months ago

@Damian-Nordic @andreilitvin @bzbarsky-apple @tennessee-google @mkardous-silabs

Following discussions with our Android team, because of privacy consideration, android has not yet encode mac address in hostname in android side , and it uses use android_UUID(32bytes)(total 40 bytes) in hostname. If android encoded the MAC address, this would allow apps that operate with higher-layer APIs, which don't necessarily have enough permissions, to deduce device identifiers.

In addition, the max should be 63 bytes instead of my current 40 bytes, as per the DNS-SD spec

We may update the matter spec in hostname section, "it must use a mechanism that ensures local uniqueness, which MAY be using the MAC address" It seems If future link layers are supported by Matter that do not use 48-bit MAC addresses or 64-bit MAC Extended Address identifiers, then a similar rule will be defined for those technologies in existing matter spec allow the future changes for different rules.

The below is matter spec clause for reference. "For DNS‑SD a target host name is required, in addition to the instance name. The target host name SHALL be constructed using one of the available link-layer addresses, such as a 48-bit device MAC address (for Ethernet and Wi‑Fi) or a 64-bit MAC Extended Address (for Thread) expressed as a fixed-length twelve-character (or sixteen-character) hexadecimal string, encoded as ASCII (UTF-8) text using capital letters, e.g., B75AFB458ECD.. In the event that a device performs MAC address randomization for privacy, then the target host name SHALL use the privacy-preserving randomized version and the hostname SHALL be updated in the record every time the underlying link-layer address rotates. Note that it is legal to reuse the same hostname on more than one interface, even if the underlying link-layer address does not match the hostname on that interface, since the goal of using a link-layer address is to ensure local uniqueness of the generated hostname. If future link layers are supported by Matter that do not use 48-bit MAC addresses or 64-bit MAC Extended Address identifiers, then a similar rule will be defined for those technologies."

bzbarsky-apple commented 4 months ago

@yunhanw-google Changing the spec requires spec issues, etc, none of which exist.

That said, changing the spec will not allow existing devices to talk to any device that uses the new longer names. This is a recipe for complete lack of interop.

In practice, what you need is to just use a random 64-bit hostname. That's what we ended up doing on Darwin. This does not have to be "the hostname of the device". It just needs to be a registered DNS record.

bzbarsky-apple commented 4 months ago

Oh, and this part:

the max should be 63 bytes instead of my current 40 bytes, as per the DNS-SD spec

I assume you mean the DNS spec (in the sense that DNS labels are capped at 63 octets)? DNS-SD does not define any length limits on hostnames per se.

RemiNVG commented 4 months ago

Currently the spec says "The target host name SHALL be constructed using one of the available link-layer addresses", so just using a random 64-bit hostname also does not follow the spec. If we agree this is acceptable behavior, that indicates that the spec should be changed to require that a mechanism that ensures local uniqueness is used, but not necessarily require that mechanism to be based on one of the link-layer addresses (in particular considering privacy implications) ?

It seems the bug here is slightly different though, where the implementation was assuming that the hostname is at most 16 bytes, which does not seem to be actually part of the spec: the spec says such as a 48-bit device MAC address or a 64-bit MAC Extended Address. Assuming a 16 bytes length could also cause interop problems when implementing the last part "If future link layers are supported by Matter that do not use 48-bit MAC addresses or 64-bit MAC Extended Address identifiers, then a similar rule will be defined for those technologies."

andy31415 commented 4 months ago

@RemiNVG one of the larger worries is backwards compatibility: 1.0 through 1.3 devices will refuse to talk with any device that sends "long names". As a result allowing the setting of a name including a "random name" is a vast improvement already as even though we would not use the exact MAC (maybe due to privacy) we would ensure backwards compatilbity: 1.0 controllers being able to see android devices.

bzbarsky-apple commented 4 months ago

Updating the spec to says "16-char random string" or "16-char random hex digit string" would be reasonably backwards-compatible and would in fact avoid the issues about link addresses.

RemiNVG commented 4 months ago

Agreed about saying "16-char random string" or "hex digits": this would make sure current implementations that don't use link addresses are in spec, and avoid interop issues like we are seeing now due to the length not being clearly mandated. It would have been nice to be less restrictive but I guess if there's many 1.0 controllers in the field that can't handle more than 16 characters already, making that a clear requirement would avoid future problems.

bzbarsky-apple commented 4 months ago

@RemiNVG It's not about controllers. Devices that try to request OTA from an OTA provider that uses a > 16 char hostname would fail, for example.

gmarcosb commented 1 month ago

Per @RemiNVG, we should be able to address this on Android T+ by simply having the Matter SDK set its own hostname in NsdServiceInfo when advertising

https://cs.android.com/android/platform/superproject/main/+/main:packages/modules/Connectivity/framework-t/src/android/net/nsd/NsdServiceInfo.java;l=233;drc=2fc115491a9c108707398aa56c1c3c26817f7178

yunhanw-google commented 4 weeks ago

@joonhaengHeo please take this work when you have time, roughly call SetHostName with 16-char random string when handling NsdServiceInfo when Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU when it is larger than android T