Basic Auth annotation Support

[r4j4h]

Problem:

When trying to protect some endpoints with Basic Auth I learned that the Base64-carrying Authorization Header is passed even if proxy_hide_header is used. Apparently proxy_set_header Authorization "" is necessary to qualm it.

See "Issues with reverse proxy authentication" from this page for a source reference. Also note that practically all other related references that show up from searches only mention proxy_set_header, further implying this is the case.

Currently:

proxy-hide-headers and proxy-pass-headers are directly provided by this Ingress controller, and while proxy-set-header is not directly provided it is documented to be indirectly provided in tandem with other options and states, namely around keepalive.

Request:

Since this is probably one of a few edge cases where this is necessary, fully exposing proxy_set_header is not necessarily needed, a terminate-authorization-header: "true" annotation or configmap entry could suffice.

While it would be nice to expose the set header functionality I feel it might be a bigger problem than it appears on surface with key-value pairs and potentially nested quotes.

Alternatively, perhaps upstream nginx could hide this header if configured to, instead of apparently ignoring it or treating it as a non-header? As it was hard to find any examples of others trying to do this with proxy_hide_header and the official docs simply imply it should work.

[pleshakov]

@r4j4h
proxy_hide_header removes a header from a response of an upstream server. The directive cannot be used to clear a header that is passed in a request to an upstream server. As you mentioned, to clear a header it is required to use proxy_set_header directive.

I don't think it makes sense to have an annotation that clears only the Authorization header. I suggest the following alternatives:

1. Use the location snippets annotation to clear the header. An example is available here.

2. Have an annotation that can set the value of any header(s). You're right, adding this annotation is a bigger problem.

3. Since the use-case is configuring basic authentication, have annotations for configuring basic authentication, similar to JWT. Again, it is a bigger problem. Implementation of JWT is in commit 239fd88 It makes sense to disable passing the Authorization header with those annotations.

[r4j4h]

@pleshakov

Thank you for the detailed response! The intended direction for the hide/set directives was enlightening and makes sense.

I am a little sad it did not occur to me to use location snippets for this purpose. 😊
In light of that this annotation is definitely not needed, so I am totally fine with closing this Issue and the accompanying MR and switching my implementation to use location snippets for this purpose.

Regarding alternative number 3 though, if you feel annotations for configuring basic auth would be useful as independent annotations, I'm not against working towards it.

I think between http-snippets and location-snippets everything needed is there already, but after looking over the JWT links you provided I noticed it references Secrets by resource name instead of having the user mount them manually in the Ingress' Spec. If I understand the code I am reading properly, I think the JWT code is pulling a secret by name and writing its contents locally, avoiding the manual volume mount. That could be useful for people.

A further use for this sort of annotation would be if it worked as a global setting in the Controller's ConfigMap to automatically apply to all Location blocks or something, but I am not sure that is worth the complexity. One-off sweeping behavior from one flag might confuse some users while having them include the location snippet on each Ingress like they currently would need to do is less likely to catch someone by surprise. (I believe they could still opt out specific Ingresses by using location-snippet with "basic-auth off", or perhaps we would add an Ingress annotation for doing that if we went this global route).

As for the annotations, here's a sketch of what they might look like after seeing the JWT ones:

• nginx.com/basic-auth-realm: "Restricted"
• nginx.com/basic-auth-clear-authorization-header: "True" # defaults to "False"

If we kept with file mounting, as in the linked example, and how I am doing it now, we might have something like:

• nginx.com/basic-auth-user-file: /mounted/basic/auth/file/on/ingress/controller

and have user be responsible for mounting it in to Ingress Controller.

If we try and re-use the JWT logic and fetch the secret and write it locally, it might allow something like:

• nginx.com/basic-auth-secret: secret-with-basic-auth

I'm not sure if in that case we'd want another annotation for describing the key in that Secret resource, or if we would hardcode an expectation like I think the JWT code is doing though.

[pleshakov]

@r4j4h

Thanks for your response.

Regarding alternative number 3 though, if you feel annotations for configuring basic auth would be useful as independent annotations, I'm not against working towards it.

That would be helpful!

If I understand the code I am reading properly, I think the JWT code is pulling a secret by name and writing its contents locally, avoiding the manual volume mount. That could be useful for people.

Yes, the main benefits is avoiding the manual volume mount.

A further use for this sort of annotation would be if it worked as a global setting in the Controller's ConfigMap to automatically apply to all Location blocks or something, but I am not sure that is worth the complexity.

I think it is better to start with annotations and add configMap keys only if they are needed.

nginx.org/basic-auth-realm: "Restricted", nginx.org/basic-auth-clear-authorization-header: "True" # defaults to "False", nginx.org/basic-auth-secret: secret-with-basic-auth

The annotations look good. I only changed nginx.com to nginx.org, as nginx.com/* annotations refer to features available in NGINX Plus, the paid version of NGINX.

I think the main benefit of this feature is dynamic secret fetching. Otherwise, the same can be accomplished using the snippets annotation.

[r4j4h]

@pleshakov

Thanks for your response.

It is my pleasure, and thank YOU for providing and maintaining this Ingress controller! :)

That would be helpful!

Sweet deal, I won't have time immediately but I will start working on this then.

I think in the meantime we should retitle this Issue, how does "Basic Auth annotation Support" sound?

I think it is better to start with annotations and add configMap keys only if they are needed.

I agree, lets wait until someone actually asks for it.

After setting my implementation to using location-snippets, the idea of being able to avoid Ingress resource's coupling with the Ingress' volume mount path feels all the more attractive..

I will be sure to use nginx.org, good call out - I didn't even notice the switch! :)

[r4j4h]

Just writing in to let you know I have not forgotten this, just been extremely busy!

[pleshakov]

@r4j4h thx. Happy new year!

Hi @pleshakov @r4j4h

any updates on this? We would like to secure some third party applications running in our cluster with basic auth. For example prometheus, so that we can federate it securely over https + auth to a central point.

👍

[r4j4h]

Hi @rjrhaverkamp no I have not progressed on this effort yet.

In the meantime I found mounting in a basic-auth file to the nginx controller and using annotations that reference that mounted file works very well, aside from the Ingresses having to know the file path to the DB:

    nginx.org/location-snippets: |
        auth_basic "Restricted";
        auth_basic_user_file "/mounted/path/in/nginx/dev-basic-auth";

Do note that if it's not particular routes, you can also put a similar thing in the nginx config configmap to affect all locations:

    http-snippets: |
     auth_basic "Restricted";
     auth_basic_user_file "/mounted/path/in/nginx/dev-basic-auth";

For a while I actually used ingress classes to do this, running one nginx with basic auth on everything, the other without, and using the ingress.class to distinguish which for a given ingress.

Hopefully one of those two solutions will meet your needs in the meantime!

[splichy]

+1 for this

[hixichen]

+1 for this