Traefik x509 SPI Provider

[Romain Tribotté]

Hi Team, 

I'm one of the [Traefik proxy](https://github.com/traefik/traefik) maintainers and I recently opened a proposal issue to introduce an x509 certificate SPI Provider for Traefik (https://github.com/keycloak/keycloak/issues/17171).

As @ahus1 suggested on this issue, I am joining this mailing list to ask if a Keycloak maintainer could take a look at the proposal and evaluate it.

Thanks in advance!

[Stian Thorgersen]

I don't have that deep knowledge about the SPI and a bit limited knowledge on how various proxies pass the cert, but I would suggest we add a configurable provider instead of keep adding more and more alternatives.

I would imagine it's usually a PEM, that the header name varies, and sometimes it's url-encoded, and sometimes it's not?

I'd argue that the apache/haproxy/nginx providers should basically just be default config options for a default provider rather than us having many different implementations?

--
You received this message because you are subscribed to the Google Groups "Keycloak Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to keycloak-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/keycloak-dev/c18d6d09-4f95-4b4a-a140-0d18482329fbn%40googlegroups.com.

[Romain Tribotté]

Thanks for the reply!

This makes total sense to me.
There is already an abstract provider that constitutes a sort of generic boilerplate.
Each vendor-specific implementation seems to extend and adapt it.

The main pain point with Traefik and the existing providers is that the client cert header format is different, and the Nginx one worked with the past versions of Traefik (<v2.9.4) but that was unintended and seemed to work only with single certs, not chains.

It seems that the current model is already based on a decent effort for genericity, and it does not seem that there's much more that could be abstracted and set in common for all implementations.

Adding a Traefik-only implementation on top of it would still follow this initial intent of genericity without overcomplicating things IMHO.

WDYT?

[Steve Lewis]

Is there a way to allow it to handle both single cert and cert chains in the parent class? It seems like one case should logically reduce to the other.

[Welton Rodrigo Torres Nascimento]

Taking a look at the code, I see that the Apache and HAProxy variations can be fused (they only diffed on how the PEM cert is presented) because the apache code converts to single line format, which the PemUtilsProvider already does. (Maybe it made sense in a previous version of the code).

The nginx version vary in how the cert is encoded (url encoded instead of plain PEM format) and how the intermediate certs are obtained (via truststore instead of headers. Nginx doesn’t support sending the chain in the header).

So, based on what I see, I suppose we can indeed have a single configurable provider with an option to choose the format of the major proxy providers. Compare this with the current approach of one provider for each proxy, with the added complexity of the configuration variable names. Three variables, three providers: 9 combinations. And one of them (nginx) ignore two.

That being said, I also don’t see too much benefit from removing the variables and breaking compatibility for so little code removal.

> To view this discussion on the web visit https://groups.google.com/d/msgid/keycloak-dev/17d6e2dc-fa3c-475b-8ee0-865bfe23629cn%40googlegroups.com.

[Steve Lewis]

Maybe create a configurable one that references the common code in A and B so they can eventually go away?

You received this message because you are subscribed to a topic in the Google Groups "Keycloak Dev" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/keycloak-dev/NCl6ZrvBaY0/unsubscribe.
To unsubscribe from this group and all its topics, send an email to keycloak-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/keycloak-dev/D5B175C9-9E2B-4C56-A120-48F639526A23%40familianascimento.org.

[Romain Tribotté]

Hello,

Sorry for the late reply.
I understand that the project would be more inclined to introduce a common implementation that can satisfy all specificities by exposing configuration, or at least be reluctant to see yet another vendor-specific added to the Keycloak repository.

I am looking for guidance because I'm not familiar with the project. I am looking for a solution that would be likely gladly welcomed and accepted, to ease the use of Traefik with Keycloak.
In that regard, it is still unclear what would be accepted or not.

Having said that, I can still share my opinion. If I didn’t overlook this, I don't think unifying implementations is a good idea. I didn't dig deep, but it already seems to me that for instance the Nginx implementation, being the more complex one, brings unique features, uncommon to any other vendor, and I am not sure it is worth working at merging it with other implementations. I think that having a common implementation would add unnecessary complexity with low benefits, other than removing code that seems to need, as of today, low maintenance.
So I still suggest that the Traefik implementation should be added to the Keycloak repository to keep things simple.


WDYT? Should I still consider opening a PR for a new x509 Traefik provider?

Thanks!

[Welton Rodrigo Torres Nascimento]

Based on what I saw in the code and given the initial issue https://github.com/keycloak/keycloak/issues/17171:

Since v2.9.2, the PassTLSClientCert middleware behavior changed and the client certificate PEM bytes are not URL encoded anymore.

I suppose a better approach to avoid both breaking backwards compatibility and introducing yet another x509 provider, would be to expose configuration for the existing Nginx provider to make it traefik compatible again.

If that is only the PEM encoding and the issue states, better yet.

The problem with merging the existing providers is that a compatibility break is mandatory, as IMHO there is no option in quarkus to “redirect” a conf. So we cannot have the new x509 provider recognize, say, settings from the apache x509 provider, which would keep compatibility.

So, based on the amount of code being deleted, given that the existing nginx provider can be made configurable to support traefik, my opinion (I’m note a maintainer nor developer) is that using the nginx x509 provider as a generic would be better.

To view this discussion on the web visit https://groups.google.com/d/msgid/keycloak-dev/1677b7e7-fa84-4bd7-b6fa-eba5fbdc8a99n%40googlegroups.com.