Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLIENT_HIDDDEN should never broadcast NeighborInfo #5464

Closed
wants to merge 1 commit into from

Conversation

fifieldt
Copy link
Contributor

@fifieldt fifieldt commented Nov 27, 2024

We set the broadcast interval to UINT_MAX in the NodeDB setup. However, in theory there would be a case where a HIDDEN node could reveal itself through this module if this setting is changed.

This change means it will never send, and will always wait UINT_MAX before trying again if called.

We set the broadcast interval to UINT_MAX in the NodeDB setup.
Howwever, in theory there would be a case where a HIDDEN node could
reveal itself through this module.

This change means it will never send, and will always wait UINT_MAX
before trying again if called.
@GUVWAF
Copy link
Member

GUVWAF commented Nov 27, 2024

Not sure this is needed, it’s an optional module already, there are other modules that would let a CLIENT_HIDDEN transmit.

Also, I don’t think it addresses the point made in #5286. It’s about another node broadcasting the node number of a CLIENT_HIDDEN when it has NeighborInfo enabled with LoRa transmit.
Anyway I don’t really see a problem with this, because when the CLIENT_HIDDEN transmits anything, it transmits its node number in the clear already, and it gets rebroadcasted by other nodes as well. From the docs: “Device that only broadcasts as needed for stealth or power savings.”, which doesn’t mean it cannot be detected in any case.

@fifieldt
Copy link
Contributor Author

Thanks for the review :) I was just looking at what DeviceTelemetryModule does and trying to make a parallel. I bow to those with more experience about whether this is something that needs to be addressed or not - this patch took very little effort :P

@GUVWAF
Copy link
Member

GUVWAF commented Nov 27, 2024

Yeah, DeviceTelemetryModule is enabled by default, NeighborInfoModule is not. I think if someone wants this on a CLIENT_HIDDEN then we should allow it, although it’s a weird choice.

@fifieldt
Copy link
Contributor Author

OK! Will wait another day in case someone else has an opinion and will abandon.

@thebentern
Copy link
Contributor

I would think this would be a very non-standard choice for a CLIENT_HIDDEN user, based on my conversation with that audience.

@fifieldt
Copy link
Contributor Author

Abandoning, not needed.

@fifieldt fifieldt closed this Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants