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

/metrics endpoint always exposed regardless -expose-metrics=f #760

Closed
CompuRoot opened this issue Jun 30, 2022 · 7 comments
Closed

/metrics endpoint always exposed regardless -expose-metrics=f #760

CompuRoot opened this issue Jun 30, 2022 · 7 comments
Labels

Comments

@CompuRoot
Copy link

CompuRoot commented Jun 30, 2022

/metrics endpoint always exposed regardless -expose-metrics=f

Even adding custom metrics path via -metrics-path still expose /metrics URL

Expected behavior
flag -expose-metrics=false should disable metrics endpoint completely

Setup details

  • Operating System: Linux
  • Used tusd version:
    • Version = v1.9.0
    • GitCommit = 3a3916a
    • BuildDate = Tue Apr 5 18:41:49 UTC 2022
  • Used tusd data storage: disk store
  • Used tusd configuration:
    • -host x.x.x.x
    • -port 12345
    • -upload-dir ./storage
    • -base-path /black-hole
    • -expose-metrics=f
    • -metrics-path /N1NYcEFzdzcvbElNN3hsZ2JzM1hC
  • Used tus client library: direct access via browser to /metrics

Suggestion

if option -expose-metrics=f is in use then option -show-greeting=f must be activated too to prevent disabled endpoint /metrics to reply with greetings where also disclosed hidden metrics path in message

@CompuRoot CompuRoot added the bug label Jun 30, 2022
@Acconut
Copy link
Member

Acconut commented Jul 1, 2022

I cannot reproduce this. If I run tusd -expose-metrics=false, then http://localhost:1080/metrics returns the greeting and not the metrics endpoint as expected. The presence of the -metrics-path does not change this behavior. If I run tusd -expose-metrics=false -show-greeting=false then a 404 is returned. Everything as expected for me.

@CompuRoot
Copy link
Author

CompuRoot commented Jul 1, 2022

Set -metrics-path to some secret path, then try to visit /metrics it will reveal secret metrics path.

That's why my suggestion to activate automatically -show-greeting=f in case of -expose-metrics=f is set.

Thanks for looking into it !

@Acconut
Copy link
Member

Acconut commented Jul 2, 2022

Set -metrics-path to some secret path, then try to visit /metrics it will reveal secret metrics path.

I cannot reproduce this. I run tusd -expose-metrics=false -metrics-path=/foo, but http://localhost:1080/metrics only returns the greeting. No metrics are shown. What do you mean exactly with "it will reveal secret metrics path"?

@CompuRoot
Copy link
Author

To reproduce, script to start tusd

#!/bin/sh

/srv/bin/tusd                  \
  -upload-dir=/srv/tusd/data   \
  -host 1.2.3.4 -port 1234     \
  -base-path /blobs/           \
  -expose-metrics=f                              \
  -metrics-path /superSecretPath                 \
  -tls-certificate=/etc/ssl/certs/1.2.3.4.crt    \
  -tls-key=/etc/ssl/private/1.2.3.4.key

Where 1.2.3.4 real public IP address.

Even -expose-metrics=f is set to false, /merics is still exposed and returns:

Welcome to tusd
===============

Congratulations on setting up tusd! Thanks for joining our cause, you have taken
the first step towards making the future of resumable uploading a reality! We
hope you are as excited about this as we are!

While you did an awesome job on getting tusd running, this is just the welcome
message, so let's talk about the places that really matter:

- /blobs/ - send your tus uploads to this endpoint                    <<<<<<<<<<<<<<<<<<<<<<
- /superSecretPath - gather statistics to keep tusd running smoothly  <<<<<<<<<<<<<<<<<<<<<<
- https://github.com/tus/tusd/issues - report your bugs here

So quit lollygagging, send over your files and experience the future!

Version = v1.9.0
GitCommit = 3a3916af460ee41c6bbdd4d059a9dcf51a323d9a
BuildDate = Tue Apr  5 18:41:49 UTC 2022

As you can see, both upload endpoint /blobs/ and "secret" metrics endpoint is exposed to anyone who knows that /metrics won't be disabled even option -expose-metrics=f explicitly requested

@Acconut
Copy link
Member

Acconut commented Jul 4, 2022

Ok, now I see what you mean with exposing the metrics endpoint. Thanks for clarifying. You are referring to it being displayed in the greeting. That wasn't clear to me until now.

However, I am still struggling to see what the underlying issue is here. If -expose-metrics=false, then there will be no metrics endpoint mounted. I.e. no metrics are available under the /metrics endpoint (or whatever you set -metrics-endpoint to). This URL just leads to nowhere.

So what is your concern here? That we should not display a URL in the greeting which is not available?

@CompuRoot
Copy link
Author

That wasn't clear to me until now.

Im sorry that I wasn't clear enough with description!

So what is your concern here?

I think if options set to not expose metrics should mean exactly this - do not expose, but endpoint /metrics is still exposed with greetings (which isn't related to metrics at all) and disclose "hidden/custom" -metrics-endpoint path that was used previously or will be used in a future again.

That we should not display a URL in the greeting which is not available?

The issue is that /metrics exposed always, regardless what have been set with -metrics-endpoint, which makes option -metrics-endpoint simply useless.

I think the best would be to stop show greetings on /metrics at all since it isn't related to metrics. Greetings is already present on / path, so I see no point to duplicate it on non related path. Also, if -metrics-endpoint is in use (the only purpose I see for it is to prevent public access to it), then it shouldn't be shown in greetings at / otherwise it void the whole purpose of custom metrics endpoint.

Acconut added a commit that referenced this issue Aug 4, 2022
@Acconut Acconut closed this as completed in 83015bf Aug 4, 2022
@Acconut
Copy link
Member

Acconut commented Aug 4, 2022

I just added a patch, which removes the metrics information from the greeting, if -expose-metrics=false. Hope this resolves your question.

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

No branches or pull requests

2 participants