search

openzipkin / zipkin

Zipkin is a distributed tracing system
https://zipkin.io/
Apache License 2.0
17.02k stars 3.09k forks source link

Basic Auth setup for the new (armeria) http collector endpoint #2467

Closed gquintana closed 5 years ago

gquintana commented 5 years ago

Feature:

Add authentication and autorisation control on HTTP REST API and UIs. This is a continuation of #782, specific to adding basic auth example integration instructions.

Rational

I admit security is a complicated topic. The MVP for me would be file based basic auth.

Example Scenario

Prior Art

codefromthecrypt commented 5 years ago

there is an existing issue about this. hard coding basic auth credentials is probably not something we would want to standardize on for obvious reasons. however, it should be possible.

maybe the below no longer works due to drift from the armeria update will have a look

https://github.com/openzipkin/zipkin/issues/782#issuecomment-433306013

gquintana commented 5 years ago

there is an existing issue about this. hard coding basic auth credentials is probably not something we would want to standardize on for obvious reasons.

Reasons are not so obvious to me. Can you explain?

I tried the recipe described in the comment:

  1. Add spring-security to class path 
    $ jar -tvf zipkin-server-2.12.7-SNAPSHOT-exec.jar |grep security
408 Fri Feb 15 10:42:02 CET 2019 BOOT-INF/lib/spring-boot-starter-security-2.1.3.RELEASE.jar
778579 Wed Feb 13 17:37:46 CET 2019 BOOT-INF/lib/spring-security-config-5.1.4.RELEASE.jar
433224 Wed Feb 13 17:37:14 CET 2019 BOOT-INF/lib/spring-security-core-5.1.4.RELEASE.jar
537635 Wed Feb 13 17:37:30 CET 2019 BOOT-INF/lib/spring-security-web-5.1.4.RELEASE.jar
  2. Add properties to Spring boot config: 
    spring:
security:
user:
  name: zipkin
  password: zipkin

The security filter chain appears to be created:

2019-03-25T17:14:04,244 INFO  org.springframework.security.web.DefaultSecurityFilterChain - Creating filter chain: any request, [org.springframework.security.web.context.request.async.WebAsyncManagerIntegrationFilter@b70da4c, org.springframework.security.web.context.SecurityContextPersistenceFilter@5286c33a, org.springframework.security.web.header.HeaderWriterFilter@47a5b70d, org.springframework.security.web.csrf.CsrfFilter@14dda234, org.springframework.security.web.authentication.logout.LogoutFilter@bf1ec20, org.springframework.security.web.savedrequest.RequestCacheAwareFilter@5c530d1e, org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter@c5ee75e, org.springframework security.web.authentication.AnonymousAuthenticationFilter@4a11eb84, org.springframework.security.web.session.SessionManagementFilter@1a45193b, org.springframework.security.web.access.ExceptionTranslationFilter@54d18072]

But doesn't seem to be plugged in in the HTTP request handler because filters are not invoked.

codefromthecrypt commented 5 years ago

On Tue, Mar 26, 2019 at 12:55 AM Gerald Quintana notifications@github.com wrote:

there is an existing issue about this. hard coding basic auth credentials is probably not something we would want to standardize on for obvious reasons.

Reasons are not so obvious to me. Can you explain?

https://www.owasp.org/index.php/Use_of_hard-coded_password and a bunch of other posts. The background issue (the one I linked, not what you created) also mentions that people have different ideas when it comes to auth. For example api keys, mutual TLS, oauth, etc. That and many are not using http for span data either (ex disable it), and the options for messaging security are a bit random. I don't want to wander this issue, all I meant to say is that there are some problems including the vulnerability aspects and depending on impl whether the server reboots to see a new password and how exactly such a password change would roll out to your instrumented apps. I remember this a decade ago was a bit of an issue. Making a default for basic auth would imply for us at least thinking that through which I think some would reasonably say "please don't do basic auth". This is why I'm more interested in seeing how to allow you to opt in. I would prefer debates about whether basic is smart or not to remain on the other issue. This one, I'm trying to help see if you can enable it on your own.

I tried the recipe described in the comment:

Add spring-security to class path

$ jar -tvf zipkin-server-2.12.7-SNAPSHOT-exec.jar |grep security 408 Fri Feb 15 10:42:02 CET 2019 BOOT-INF/lib/spring-boot-starter-security-2.1.3.RELEASE.jar 778579 Wed Feb 13 17:37:46 CET 2019 BOOT-INF/lib/spring-security-config-5.1.4.RELEASE.jar 433224 Wed Feb 13 17:37:14 CET 2019 BOOT-INF/lib/spring-security-core-5.1.4.RELEASE.jar 537635 Wed Feb 13 17:37:30 CET 2019 BOOT-INF/lib/spring-security-web-5.1.4.RELEASE.jar

Add properties to Spring boot config:

spring: security: user: name: zipkin password: zipkin

The security filter chain appears to be created:

But doesn't seem to be plugged in in the HTTP request handler because filters are not invoked.

I appreciate you trying that, that's what I was going to try also. We need to see if it is viable to hook armeria to spring security, or if it is easier in your case to wire in an armeria handler cc @openzipkin/armeria

codefromthecrypt commented 5 years ago

@gquintana would the features available here get you by? if so I could try to get an example together https://github.com/line/armeria/blob/master/core/src/main/java/com/linecorp/armeria/server/auth/HttpAuthServiceBuilder.java

codefromthecrypt commented 5 years ago

some findings I think.. so I looked at some change to let spring-security work with reactive (by @rwinch). It seems that spring-security is largely servlet based. Currently our web controllers are armeria which directly implement over netty I think... this is both read and write endpoints.

To solve things at the correct layer of abstraction, if spring-security implementation is important to you, I think it should be an issue request here https://github.com/line/armeria

Regardless, in general we suggest using a proxy or gateway and/or turning off endpoints not needed. We made this example for exactly the reason of security templating. Another example, Tyro are a bank and they simply disable query where it should not be used and visa versa for the collector endpoint. I'm repeating stuff already discussed in #782 for context only. We can track this issue independently for what is viable for basic auth as a "at your own risk" add-on to the server.

codefromthecrypt commented 5 years ago

Note: I found a couple classes that might be relevant for armeria+spring security.

As far as I understand, this is wrapping the servlet-based code so that it can be re-used in spring webflux

https://github.com/spring-projects/spring-security/blob/b93528138e2b2f7ad34d10a040e6b7ac50fc587a/web/src/main/java/org/springframework/security/web/server/WebFilterChainProxy.java https://github.com/spring-projects/spring-security/blob/b93528138e2b2f7ad34d10a040e6b7ac50fc587a/config/src/main/java/org/springframework/security/config/annotation/web/reactive/WebFluxSecurityConfiguration.java#L60

codefromthecrypt commented 5 years ago

what's going out as 2.12.7 can configure SSL. just added a test case to prove it https://github.com/openzipkin/zipkin/pull/2480

basic auth can definitely be done, and authenticating POST via spring-security managed credentials is likely not a big deal. I will make an example once release goes through

However, full-bore spring-security authorization is beyond the scope of what we'd do in this repo. I'd recommend anyone interested to raise an issue for armeria if they desire that level after seeing the basic example.

I'll leave this issue open until there's a code example of how to self-support basic auth on the POST endpoint.

gquintana commented 5 years ago

I confirm SSL can be configured. However cipher suites and TLS version can not be tuned: https://github.com/line/armeria/issues/1711

Connecting Spring Security to Armeria and Zipkin looks very complicated, because Spring Security is based on Servlet API (or WebFlux API) contrary to Armeria.

codefromthecrypt commented 5 years ago

we may not be able to support spring security.. something we never supported anyway. it won't happen in this repo regardless.

we can put an example of basic auth configuration in a unit test that does not use spring security.

On Mon, Apr 15, 2019, 11:19 PM Gerald Quintana notifications@github.com wrote:

I confirm SSL can be configured. However cipher suites and TLS version can not be tuned: line/armeria#1711 https://github.com/line/armeria/issues/1711

Connecting Spring Security to Armeria and Zipkin looks very complicated, because Spring Security is based on Servlet API (or WebFlux API) contrary to Armeria.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/apache/incubator-zipkin/issues/2467#issuecomment-483271266, or mute the thread https://github.com/notifications/unsubscribe-auth/AAD613MUC19qzU4nae9goDyZgpJeGi7rks5vhIqOgaJpZM4cGe9A .

gquintana commented 5 years ago

Thanks for pointing out the HttpAuthServiceBuilder class.

The SAML SSO is another interesting lead.

Using a reverse proxy to do security is not good solution in our case because despite firewalls Zipkin would still be exposed in some way.

How can I add security related Armeria decorators to Zipkin HTTP services? Can you see an easy and cross-cutting extension point?

codefromthecrypt commented 5 years ago

will make an example project now. feel free to ping me https://gitter.im/openzipkin/zipkin

On Tue, Apr 16, 2019 at 4:39 PM Gerald Quintana notifications@github.com wrote:

Thanks for pointing out the HttpAuthServiceBuilder https://github.com/line/armeria/blob/master/core/src/main/java/com/linecorp/armeria/server/auth/HttpAuthServiceBuilder.java class.

The SAML SSO https://line.github.io/armeria/advanced-saml.html is another interesting lead.

Using a reverse proxy to do security is not good solution in our case because despite firewalls Zipkin would still be exposed in some way.

How can I add security related Armeria decorators to Zipkin HTTP services? Can you see an easy and cross-cutting extension point?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/apache/incubator-zipkin/issues/2467#issuecomment-483547521, or mute the thread https://github.com/notifications/unsubscribe-auth/AAD61zegi4LL9zNN7w5HhafN0Woxt30Sks5vhX4ogaJpZM4cGe9A .

codefromthecrypt commented 5 years ago

I created an example project to show how someone can do this. usual disclaimers apply.. if someone makes a custom integration, that will eventually need maintenance when we upgrade to spring boot 3 or some new version of a library etc. https://github.com/adriancole/zipkin-security