zipkin
zipkin copied to clipboard
openzipkin
Only expose the health endpoint from the Spring-Boot actuator
@coheigea could you please explain the context behind this?
Ping @llinder
It's an issue I raised on gitter already with the team:
The Spring Boot actuator endpoint exposes endpoints like /heapdump. It allows any user to obtain a heapdump by calling "actuator/heapdump". The "env" endpoint is also exposed which might expose credentials.
docker run -d -p 9411:9411 openzipkin/zipkin, and go to http://localhost:9411/actuator/env
I think it makes as a default, conservative setting to only expose the health endpoint. Should sites require more actuator endpoints for their infrastructure they can always override but it's their decision.
On Fri, Oct 15, 2021 at 2:02 PM Colm O hEigeartaigh < @.***> wrote:
It's an issue I raised on gitter already with the team:
The Spring Boot actuator endpoint exposes endpoints like /heapdump. It allows any user to obtain a heapdump by calling "actuator/heapdump". The "env" endpoint is also exposed which might expose credentials.
docker run -d -p 9411:9411 openzipkin/zipkin, and go to http://localhost:9411/actuator/env
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/openzipkin/zipkin/pull/3382#issuecomment-944241391, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABPKAAGYDLKFHDWUQT3XVTUHAJ5PANCNFSM5F7C62AA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
Can this be considered a breaking change? Is there any use case where people relay programmatically on this?
We have users that depend on the metrics and prometheus endpoints for sure. Even including those in the list in this PR would be a breaking change.
Zipkin is not intended to be deployed anywhere that is not a trusted environment, given the nature of the data collected and that we do not have an authorization system
I think @devinsba made a good point. Maybe the solution is to just add documentation describing this concern?
On Fri, Oct 15, 2021, 19:12 Brian Devins-Suresh @.***> wrote:
We have users that depend on the metrics and prometheus endpoints for sure. Even including those in the list in this PR would be a breaking change.
Zipkin is not intended to be deployed anywhere that is not a trusted environment, given the nature of the data collected and that we do not have an authorization system
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/openzipkin/zipkin/pull/3382#issuecomment-944462834, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXOYAQGTCOXMSD7V227Z2TUHBOJBANCNFSM5F7C62AA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
In my opinion, it is safer to be "secure by default", even if Zipkin is not supposed to be deployed in an untrusted environment. By default, Zipkin exposes the actuator heapdump endpoint - what is the use-case for this being reachable by default? I think it is better to only expose the minimal endpoint (/health) by default, which is what Spring Boot does by default, and if users want to expose additional endpoints they can do so by setting the MANAGEMENT_ENDPOINTS_WEB_EXPOSURE_INCLUDE env variable.
My general opinion on management endpoints is to only enable the bare minimum as well. I also agree with the argument that it shouldn't matter too much as the general guidance has always been that sites should provide their own security for Zipkin APIs and UI.
Since I don't use Spring Boot much these days, I wasn't aware of the environment variable where this can be set. Maybe the best compromise it to disable everything except the health check and document how to use that environment variable to re-enable things for debugging or other use cases? Then the challenge is communicating the change as it could break things for sites that might have been using other management endpoints. Alternatively we could leave the endpoints enabled and just document how to use that environment variable to control the settings?
By default, spring-boot only exposes the /health endpoint via http, so a decision to break current behaviour on this would not be all that unconventional. See https://docs.spring.io/spring-boot/docs/current/reference/htmlsingle/#actuator.endpoints.exposing .
In troubleshooting user-reported issues, I think some of the actuator endpoints have been invaluable. We usually start troubleshooting by asking users to check /info for the Zipkin version. Then, once we're all sure of the Zipkin version, we can ask them to check env and configprops to see what properties are actually applied vs what a user expects is applied. And if some configuration is not activating even though the properties are available, we can ask users to check conditions for auto-config conditions explaining why a configuration condition was or was not met. It can save a lot of time and back-and-forth to see at runtime what the configuration is and why, rather than guessing based on limited information provided.
That all said, we only officially support the endpoints mentioned at https://github.com/openzipkin/zipkin/tree/master/zipkin-server#endpoints, and as I mentioned in my prior review comment, the actuator exposure property does not affect any of those endpoints because Zipkin serves them outside Actuator.
We could have Actuator off by default and ask users to turn it on to troubleshoot. It's an additional hurdle to getting to resolution during troubleshooting, though. And as others have mentioned, ALL Zipkin endpoints are unsecured since it should be protected by external means. Any user with access to the network can use any of the APIs for any purpose. See https://github.com/openzipkin/zipkin/issues/782 about securing acess to the Zipkin Server. Heapdump is an extreme example since configured credentials (for example, storage credentials) could potentially be leaked to even an internal users that shouldn't directly have them. Not that you should probably use storage or storage credentials for Zipkin that is used for anything business-critical. Generally, if you're concerned about someone having access to Actuator endpoints, you should be equally concerned about them having access to the Zipkin API and doing nefarious things with those.
Ultimately, I'm fine disabling all of Actuator by default, but it is a breaking change in a sense - granted, it is functionality we never officially supported, but it's been there and used in troubleshooting for quite a long time. For support's sake, some documentation on how to enable those endpoints for troubleshooting will probably be important to point users at.
To recap, info/health/metrics/prometheus are officially supported endpoints and do not rely on Actuator. Whether we disable Actuator or not, they will continue to be exposed. I'm fine disabling Actuator by default, but it may negatively impact support. It's better to use management.endpoints.enabled-by-default=false rather than exposure, since it will avoid creating the endpoints at all in the application context.
