Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Empty auth-scheme support #349

Open
sepich opened this issue Dec 20, 2021 · 9 comments
Open

Empty auth-scheme support #349

sepich opened this issue Dec 20, 2021 · 9 comments

Comments

@sepich
Copy link

sepich commented Dec 20, 2021

What did you do?
I want to scrape target secured via Authorization header, curl example:

curl -H "Authorization: $token" ...

Per docs i should be able to not use Bearer
https://prometheus.io/docs/prometheus/latest/configuration/configuration/#scrape_config

authorization:
  # Sets the authentication type of the request.
  [ type: <string> | default: Bearer ]

So i've set:

authorization:
  type: ''

But in tcpdump i still see that Bearer prefix is added before token.
Validation code:

c.Authorization.Type = strings.TrimSpace(c.Authorization.Type)

What did you expect to see?
I expect to being able to choose any auth-scheme, including nil one.

What did you see instead? Under which circumstances?
I see that Bearer auth-scheme is forced, i.e.:

curl -H "Authorization: Bearer $token" ...

instead of:

curl -H "Authorization: $token" ...

Environment

  • Prometheus version:
prometheus, version 2.31.1 (branch: HEAD, revision: 411021ada9ab41095923b8d2df9365b632fd40c3)
  build user:       root@9419c9c2d4e0
  build date:       20211105-20:35:02
  go version:       go1.17.3
  platform:         linux/amd64
  • Prometheus configuration file:
scrape_configs:
  - job_name: "auth"
    authorization:
      type: ''
      credentials: SECRET_TOKEN
    static_configs:
    - targets: 
      - a.b.c:443

This could be fixed by verifying that type is explicitly set to empty string before callingTrimSpace. I can send PR if such a solution is fine.

@LeviHarrison LeviHarrison transferred this issue from prometheus/prometheus Dec 20, 2021
@LeviHarrison
Copy link
Member

I agree that we should support an empty auth scheme. Ideally, we can go with the solution you proposed, although I'm not sure if bearer token configurations expect no string to be "Bearer", because if so we would have to come up with something else in order to avoid breaking backward compatibility.

cc @roidelapluie

@roidelapluie
Copy link
Member

I do think the opposite. The type is mandatory. Accepting something else would be confusing. Here is the page I used when designing this: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Authorization

@sepich
Copy link
Author

sepich commented Dec 21, 2021

Unfortunately reality is more complicated than expectations. As we know already, there are many systems avoiding log4shell by not updating from v1 ;) Also there are many vendor systems that expect raw token in Authorization header, and nobody here to update them.
Standard allows to use only auth-scheme in the header: https://datatracker.ietf.org/doc/html/rfc7235#appendix-C

Authorization = credentials
credentials = auth-scheme [ 1*SP ( token68 / [ ( "," / auth-param )
*( OWS "," [ OWS auth-param ] ) ] ) ]

And it is possible to workaround this via prometheus config:

    authorization:
      type: SECRET_TOKEN
      credentials: ' '

But in this case token in plain text is displayed in /config web-ui. So is better to fix it via "hiding" authorization.type in web-ui?

@roidelapluie
Copy link
Member

the intersection between vendors not following rfc's and supporting prometheus is too small at the moment to consider exceptions like this upstream.

@nuved
Copy link

nuved commented Jan 5, 2023

please let's set an empty value for the type
right now I have to set a proxy up and add a header there and ask Prometheus for scrapping that endpoint.

@roidelapluie
Copy link
Member

roidelapluie commented Jan 5, 2023 via email

@nuved
Copy link

nuved commented Jan 5, 2023

@roidelapluie BTW, we have to set up a proxy and add the header of Authorization there because Prometheus can't add a simple header like this one .
Authorization ${TOKEN} and add

@timlynch2k
Copy link

@roidelapluie please reconsider. authorization stanza could be full general for Authorization request header value, with basic_auth the convenience alternative.

Relax this validation:

if strings.ToLower(c.Authorization.Type) == "basic" {

Already authorization.credentials_file allows a path that could be a mounted Secret. If authorization.type validation is relaxed, this solves a case where the remote requires Basic auth with a secret username and no password.

Almost all bytes on Authorization request header are user controlled. Please allow full control.

@shurkus
Copy link

shurkus commented Mar 22, 2024

Another case when authorization.type is unnecessary and does not allow data to be collected normally https://grafana.com/docs/oncall/latest/oncall-api-reference/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants