eclipse-hawkbit / hawkbit

Eclipse hawkBit™
https://projects.eclipse.org/projects/iot.hawkbit
Eclipse Public License 2.0
466 stars 189 forks source link

Bug or feature: Support multiple Issuer-Hashes in Client TLS reverse proxy scenarios #1471

Open mdaur opened 1 year ago

mdaur commented 1 year ago

Hello,

this question is about the forwarding of TLS information like certificate CN, Issuer Hash etc. in reverse proxy scenarios when it comes to the authentication of targets using the DDI protocol (X-Ssl-Issuer-Hash-%d).

I was not aware of a separator ";" and especially the additional separator "," for values used as Issuer Hashes (hawkbit.server.ddi.security.sslIssuerHashHeader) see source code: https://github.com/eclipse/hawkbit/blob/ac946e76ef0cf0436d8cb929d215a53acaec927b/hawkbit-security-integration/src/main/java/org/eclipse/hawkbit/security/ControllerPreAuthenticatedSecurityHeaderFilter.java#L159

So is this really a feature or is it a bug having "," as an additional possible separator (in a function called splitMultiHashBySemicolon)?

Background: As far as I know it is not possible to have the Issuer Cert Hash extracted oob using k8s ingress nginx as a reverse proxy, but you can have the issuer DN and I would love to see "SSL Issuer Hash" as "Issuer Identifier" in the future. And going with DN's having a "," separator is really a bad starting point :-)

Do we get a consent to remove "," and go only with ";" separator for multiple Issuer Hashes for the time being or do we even agree to rename the setting e.g. "Issuer Identifier"?

/Martin

avgustinmm commented 1 year ago

I suppose this feature has been intended just for a hash and that comma is added as commonly used separator. So, I suppose it's a feature :-) At https://eclipse.dev/hawkbit/concepts/authentication/ there was recently contributed an example that uses nginx as a reverse proxy. It somehow uses pre-shared id assuming that all trusted for nginx will be trusted for hawkbit. There I see:

proxy_set_header X-Ssl-Client-Cn $ssl_client_s_dn_cn;
proxy_set_header X-Ssl-Issuer-Hash-1 Hawkbit;

I suppose this could be used as approach if usecase is that all trusted for nginx are trusted for hawkbit tenant (in single tenant case only/!) I suppose the client CN (mapped in the example) could be used as hash if do not contain commas. Maybe also some hash could be calculated based on DN that doesn't contain commas. This will keep backward compatibility. There is also option to introduce escaping for ; and , so to be possible to add hashes containing these symbols. I also don't like the "hash" terminology and would like to go in "Issuer Identifier" terminology. However, we this will made things incompatible - if we also decide to rename the header. Of course we could also, for some time support the legacy headers (in parallel).