search

tinkerbell / hegel

Instance Metadata Service
https://tinkerbell.org
Apache License 2.0
97 stars 32 forks source link

gRPC subscribe call ignores request's struct ID field #90

Closed chrisdoherty4 closed 2 years ago

chrisdoherty4 commented 2 years ago

When refactoring the gRPC handlers I noticed the SubscribeRequest has an ID field that isn't used anywhere in the Subscribe() handler. The Subscribe() handler extracts a peer IP, if present, using Google's peer pkg and retrieves a hardware ID based on that IP.

Either the payload has superfluous data that can be removed or the Subscribe() handler isn't doing the right thing. If the intended behavior is to operate on an IP, the IP should be made part of the payload given its required data fundamental to the logic.

Given there's a lack of code comments its unclear what the behavior of the handler should be.

mmlb commented 2 years ago

Hmm yeah the SubscribeRequest.ID is not needed and can be dropped. We use the IP info given by the kernel or trusted proxies to look up the client's info. Doing otherwise would allow spoofing and access to potentially sensitive information.

chrisdoherty4 commented 2 years ago

To avoid breakages in .7 this will be deprecated (hopefully with proto file labeling) and scheduled for removal in .8.

chrisdoherty4 commented 2 years ago

We're introducing a Hegel metadata API and dropping support for Equinix specific APIs which will see the removal of gRPC endpoints. We'll close this issue and await the Hegel API switch.