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

Setting host in Configuration causes credentials to be sent in the clear #1794

Open
roy-work opened this issue Dec 8, 2023 · 4 comments
Open

Comments

@roy-work
Copy link

roy-work commented Dec 8, 2023

Describe the bug
Setting host in the Configuration ctor causes credentials to be sent in the clear.

To Reproduce

Take this example:

import os

from datadog_api_client import ApiClient, Configuration
from datadog_api_client.v1.api.monitors_api import MonitorsApi

configuration = Configuration(
    host=os.environ['DATADOG_HOST'],
)
configuration.api_key["apiKeyAuth"] = os.environ['DATADOG_API_KEY']
configuration.api_key["appKeyAuth"] = os.environ['DATADOG_APP_KEY']
configuration.debug = True
with ApiClient(configuration) as api_client:
    monitors_api = MonitorsApi(api_client)
    monitors = monitors_api.list_monitors()

print('{} monitors'.format(len(monitors)))

You can tell from the debug output that we're not doing TLS:

send: b'GET /api/v1/monitor HTTP/1.1\r\nHost: <subdomain>.datadoghq.com\r\nAccept: application/json\r\n<snip>\r\n\r\n'
reply: 'HTTP/1.1 308 Permanent Redirect\r\n'
header: Date: Fri, 08 Dec 2023 23:22:03 GMT
header: Content-Length: 0
header: Connection: keep-alive
header: location: https://<subdomain>.datadoghq.com/api/v1/monitor
header: x-content-type-options: nosniff
header: strict-transport-security: max-age=31536000; includeSubDomains; preload
send: b'GET /api/v1/monitor HTTP/1.1\r\nHost: <subdomain>.datadoghq.com\r\nAccept: application/json\r\n<snip>\r\n\r\n'
reply: 'HTTP/1.1 200 OK\r\n'
header: Date: Fri, 08 Dec 2023 23:22:03 GMT
header: Content-Type: application/json
header: Transfer-Encoding: chunked
header: Connection: keep-alive
header: etag: W/"721e351f3b20293a0d2bc1301301df83"
header: x-frame-options: SAMEORIGIN
header: content-security-policy: frame-ancestors 'self'; report-uri https://logs.browser-intake-datadoghq.com/api/v2/logs?dd-api-key=pube4f163c23bbf91c16b8f57f56af9fc58&dd-evp-origin=content-security-policy&ddsource=csp-report&ddtags=site%3Adatadoghq.com
header: vary: Accept-Encoding
header: content-encoding: gzip
header: x-ratelimit-limit: 1000
header: x-ratelimit-period: 10
header: x-ratelimit-remaining: 999
header: x-ratelimit-reset: 7
header: x-ratelimit-name: get_all_monitors
header: x-content-type-options: nosniff
header: strict-transport-security: max-age=31536000; includeSubDomains; preload
240 monitors

That double request is pretty clearly an HTTP→HTTPS redirect. Wireshark confirms as much. Oddly, whatever the default for host is doesn't do this, but I didn't realize until much later that setting host isn't required, see the Additional Context section. Initially, I was under the impression that configuration of what DD tenant I was was required.

Expected behavior
TLS is enabled by default … and ideally only switch off very explicitly. Certainly overriding things like hostname don't then also remove TLS, in addition…

Environment and Versions (please complete the following information):
A clear and precise description of your setup:

  • version for this project in use: 2.19.0
  • services, libraries, languages and tools list and versions: the minimal example above

Additional context
I ended up setting host almost by accident. Initially, I was getting 403 Forbidden from the API, and wasn't sure why; I was using the "Getting Started" example, and setting an API key. It turns out DD requires both an API key and an "app" key (the README does note this, but I missed that in my first pass), which is somewhat unusual. So, AFAICT, I wasn't authenticated, but since the wrong HTTP status was getting returned, that never clicked, and I tried other things (such as configuring my host) prior to returning to the what-if of "perhaps it is, indeed, authn?"

I'm not clear on why one can construct a DD client without (full) credentials without error in the first place.

@therve
Copy link
Contributor

therve commented Dec 9, 2023

Hi,

I think the confusion is that host is supposed to be a base path. Maybe we can enforce that, as the name is fairly confusing, but you don't really have any reason to use this and not the regular server name for API usage?

@roy-work
Copy link
Author

I think the confusion is that host is supposed to be a base path.

What is a "base path"? (Perhaps an example?)

@therve
Copy link
Contributor

therve commented Dec 11, 2023

Something like "https://api.datadoghq.com/".

Copy link

Thanks for your contribution!

This issue has been automatically marked as stale because it has not had activity in the last 30 days. Note that the issue will not be automatically closed, but this notification will remind us to investigate why there's been inactivity. Thank you for participating in the Datadog open source community.

If you would like this issue to remain open:

  1. Verify that you can still reproduce the issue in the latest version of this project.

  2. Comment that the issue is still reproducible and include updated details requested in the issue template.

@github-actions github-actions bot added the stale label Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants