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

API Key and App Key Logged With Failed Requests #274

Closed
CoryOwens opened this issue May 17, 2018 · 13 comments · May be fixed by #518
Closed

API Key and App Key Logged With Failed Requests #274

CoryOwens opened this issue May 17, 2018 · 13 comments · May be fixed by #518
Labels
kind/bug Bug related issue resource/api severity/major Major severity issue stale Stale - Bot reminder

Comments

@CoryOwens
Copy link

Currently, if anything goes wrong with a post to Datadog, the API Key and App Key are logged by multiple modules (e.g. datadog.api, urllib3.connectionpool, urllib3.util.retry, etc.) because they are included in the request URL (e.g. /api/v1/series?api_key=[OBFUSCATED_API_KEY]&application_key=[OBFUSCATED_APP_KEY]).

These keys should be sent as request headers, instead.

[   DEBUG] [datadog.api] [pid:3275 ] [    MainThread] Use `requests` based HTTP client.
[   DEBUG] [urllib3.util.retry] [pid:3275 ] [    MainThread] Converted retries value: 3 -> Retry(total=3, connect=None, read=None, redirect=None, status=None)
[   DEBUG] [urllib3.util.retry] [pid:3275 ] [    MainThread] Incremented Retry for (url='/api/v1/series?api_key=[OBFUSCATED_API_KEY]&application_key=[OBFUSCATED_APP_KEY]'): Retry(total=2, connect=None, read=None, redirect=None, status=None)
[ WARNING] [urllib3.connectionpool] [pid:3275 ] [    MainThread] Retrying (Retry(total=2, connect=None, read=None, redirect=None, status=None)) after connection broken by 'NewConnectionError('<urllib3.connection.VerifiedHTTPSConnection object at 0x7fe8a2e3d5c0>: Failed to establish a new connection: [Errno -2] Name or service not known',)': /api/v1/series?api_key=[OBFUSCATED_API_KEY]&application_key=[OBFUSCATED_APP_KEY]
[   DEBUG] [urllib3.util.retry] [pid:3275 ] [    MainThread] Incremented Retry for (url='/api/v1/series?api_key=[OBFUSCATED_API_KEY]&application_key=[OBFUSCATED_APP_KEY]'): Retry(total=1, connect=None, read=None, redirect=None, status=None)
[ WARNING] [urllib3.connectionpool] [pid:3275 ] [    MainThread] Retrying (Retry(total=1, connect=None, read=None, redirect=None, status=None)) after connection broken by 'NewConnectionError('<urllib3.connection.VerifiedHTTPSConnection object at 0x7fe8a2e3d630>: Failed to establish a new connection: [Errno -2] Name or service not known',)': /api/v1/series?api_key=[OBFUSCATED_API_KEY]&application_key=[OBFUSCATED_APP_KEY]
[   DEBUG] [urllib3.util.retry] [pid:3275 ] [    MainThread] Incremented Retry for (url='/api/v1/series?api_key=[OBFUSCATED_API_KEY]&application_key=[OBFUSCATED_APP_KEY]'): Retry(total=0, connect=None, read=None, redirect=None, status=None)
[ WARNING] [urllib3.connectionpool] [pid:3275 ] [    MainThread] Retrying (Retry(total=0, connect=None, read=None, redirect=None, status=None)) after connection broken by 'NewConnectionError('<urllib3.connection.VerifiedHTTPSConnection object at 0x7fe8a2e3d6d8>: Failed to establish a new connection: [Errno -2] Name or service not known',)': /api/v1/series?api_key=[OBFUSCATED_API_KEY]&application_key=[OBFUSCATED_APP_KEY]
[   ERROR] [datadog.api] [pid:3275 ] [    MainThread] Could not request POST https://app.datadoghq.com/api/v1/series: HTTPSConnectionPool(host='app.datadoghq.com', port=443): Max retries exceeded with url: /api/v1/series?api_key=[OBFUSCATED_API_KEY]&application_key=[OBFUSCATED_APP_KEY] (Caused by NewConnectionError('<urllib3.connection.VerifiedHTTPSConnection object at 0x7fe8a2e3d7b8>: Failed to establish a new connection: [Errno -2] Name or service not known',)). Please check the network connection or try again later. If the problem persists, please contact [email protected]
@chenrui333
Copy link

chenrui333 commented Jun 11, 2018

+1, I have the same issue to submit metrics from AWS lambda

@ssvaddiparthy
Copy link

ssvaddiparthy commented Jul 16, 2018

@CoryOwens and @chenrui333 since this is a log from a dependency, I suggest you mask logs by have a masking function at your project level. The code for the filter will be something like this.

import logging

class LoggingFilter(logging.Filter):
    """Filters the Datadog API and Datadog APP keys from the logs."""

    def filter(self, log_record):
        """
        Filters the patterns from the Log record

        :type log_record: logging.LogRecord
        :param log_record: A single logging record that needs filtering (or not)

        :rtype: bool
        :return: True or False
        """

        if isinstance(log_record.args, dict):
            for message in log_record.args:
                log_record.args[message] = self.__mask_data(message)
        else:
            log_record.args = tuple(self.__mask_data(record_arg) for record_arg in log_record.args)

        return True

    def __get_param_value(self, query_string, param_name):
        """
        Given a string if it has the api_key/application_key, get its value

        :type query_string: str
        :param query_string: Query String containing api or app key

        :type param_name: str
        :param param_name: api_key= or application_key=

        :return: value of api or app key
        """

        value = query_string.split(param_name)[-1]
        if ' ' in value:
            value = value.split(' ')[0]
        else:
            value = value.split('&')[0]

        return value

    def __mask_data(self, log_msg):
        """
        Replace the sensitive data in a dictionary

        :type str
        :param log_msg: Log Message

        :rtype: dict
        :return: received dictionary after replacement of sensitive data
        """

        if type(log_msg) != str:
            return log_msg

        if 'api_key' in log_msg:
            api_key_value = self.__get_param_value(log_msg, 'api_key=')
            log_msg = log_msg.replace(api_key_value, '*'*len(api_key_value))

        if 'application_key' in log_msg:
            app_key_value = self.__get_param_value(log_msg, 'application_key=')
            log_msg = log_msg.replace(app_key_value, '*' * len(app_key_value))

        return log_msg

When you setup logging, you can then add a filter to the logger.

@CoryOwens
Copy link
Author

@vaddipar - My issue isn't with the library; the library is doing exactly what it is supposed to do. My issue is with how DataDog is using that library. Private keys should be sent as request headers, not as query parameters. Log filtering is a workaround with a noticeable performance cost, not an ideal solution.

@dil048 dil048 closed this as completed Apr 11, 2019
@dil048 dil048 reopened this Apr 11, 2019
@katezaps
Copy link

katezaps commented May 8, 2019

hey 👋 still seeing this now - is there any fix coming forward? we shouldn't be logging API keys at all

@ssvaddiparthy
Copy link

@katezaps I can fix the client end but how about the API actually reading the api and app key from the headers instead of params?

@gzussa
Copy link
Contributor

gzussa commented May 13, 2019

Thanks everyone for raising this. This is much appreciated :). Indeed, we want to fix this! We raised the problem to the proper team. They actually already knew about it and they will do their best to fix it ASAP. Unfortunately, i cannot provide any ETA at the moment.

@dabcoder
Copy link
Contributor

cc @ssc3 as I believe your #446 PR addresses this issue?

@ssc3
Copy link
Contributor

ssc3 commented Oct 10, 2019

cc @ssc3 as I believe your #446 PR addresses this issue?

Yup. #446 fixes it. Thanks!

@zippolyte
Copy link
Contributor

Actually it doesn't work for all the endpoints at the moment, see

def _set_api_and_app_keys_in_params(cls, api_version, path):
"""
Some endpoints need api and app keys to be set in params only
For these endpoints, api and app keys in headers are ignored
:return: True if this endpoint needs api and app keys params set
"""
constructed_path = construct_path(api_version, path)
set_of_paths = {
"v1/series",
"v1/check_run",
"v1/events",
"v1/screen",
}

So this issue is still not fully resolved

@github-actions
Copy link

github-actions bot commented Jan 7, 2020

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 Stale - Bot reminder label Jan 7, 2020
@jirikuncar jirikuncar added kind/bug Bug related issue resource/api severity/major Major severity issue and removed stale Stale - Bot reminder labels Jan 23, 2020
jirikuncar added a commit to jirikuncar/datadogpy that referenced this issue Jan 23, 2020
jirikuncar added a commit to jirikuncar/datadogpy that referenced this issue Jan 23, 2020
jirikuncar added a commit to jirikuncar/datadogpy that referenced this issue Jan 23, 2020
jirikuncar added a commit to jirikuncar/datadogpy that referenced this issue Jan 23, 2020
jirikuncar added a commit to jirikuncar/datadogpy that referenced this issue Jan 23, 2020
@github-actions
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 Stale - Bot reminder label Feb 23, 2020
@spiliopoulos
Copy link

This is affecting us as well. Can we revisit if after 3 years its now possible to only send api/app keys through headers and not request parameters?

@therve
Copy link
Contributor

therve commented Jan 24, 2023

Fixed in #754

@therve therve closed this as completed Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Bug related issue resource/api severity/major Major severity issue stale Stale - Bot reminder
Projects
None yet
Development

Successfully merging a pull request may close this issue.