[INS-241] Datadogapikey detector #4627
Conversation
| case "datadog": | ||
| inputs = []textinputs.InputConfig{{ | ||
| Label: "API Key", | ||
| Key: "apiKey", | ||
| Required: true, | ||
| RedactInput: true, | ||
| }, { | ||
| Label: "Application Key", | ||
| Key: "appKey", | ||
| Required: true, | ||
| RedactInput: true, | ||
| }, { |
There was a problem hiding this comment.
From my understanding of the PR description, the API key and application key are distinct credentials and can be verified independently, which is why we now have separate detectors for each. However, since we use a single analyzer for Datadog, it appears that the analyzer treats the application key as optional while requiring the API key. Is this the intended behavior? Also, do both keys provide access to the same set of resources?
There was a problem hiding this comment.
Yup, you are right both app key and apikey are distinct credentials and can we verified independently, I will/have introduced a validation check in Datadog's analyzer to make sure both appkey and apikey are passed to the analyzer.
Also all the api key have the same permissions(Send metrics, logs and event) so we wont't need a analyzer for it.
| if endpoint != "" { | ||
| baseURL = endpoint + "/api" | ||
| } else { | ||
| baseURL, err = DetectDomain(client, apiKey, appKey) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("[x] %v", err) | ||
| } |
There was a problem hiding this comment.
I think this is right, keeping the domain detection helps OSS.
In Enterprise, the Analyzer would only run after successfully detecting so I think we're good.
| client = common.SaneHttpClient() | ||
|
|
||
| // Make sure that your group is surrounded in boundary characters such as below to reduce false positives. | ||
| apiKeyPat = regexp.MustCompile(detectors.PrefixRegex([]string{"datadog", "dd"}) + `\b([a-zA-Z-0-9]{32})\b`) |
There was a problem hiding this comment.
Checking for a 32-character string that contains numbers or letters seems a little broad. Maybe an entropy test on the found matches would filter out the "non secret" strings?
| resApiMatch := strings.TrimSpace(apiMatch[1]) | ||
| s1 := detectors.Result{ | ||
| DetectorType: detectorspb.DetectorType_DatadogApikey, | ||
| Raw: []byte(resApiMatch), |
There was a problem hiding this comment.
I think associated endpoint the detected key is being tested on should be part of RawV2 in the results. It will help distinguish verified key-endpoint pairs with unverified ones.
There was a problem hiding this comment.
Could add the URL in the result's extra data as well.
There was a problem hiding this comment.
URL added in ExtraData.
5 participants
Description:
Previously, the
datadogtokendetector was responsible for verifying both API keys and Application keys for Datadog. This caused an issue where an invalid Application key would incorrectly mark a valid API key as unverified, leading to false positives.To resolve this, we have introduced a new detector:
datadogapikey. This detector is focused solely on validating Datadog API keys, ensuring that API key verification is isolated and accurate, independent of the App key status.Checklist:
make test-community)?make lintthis requires golangci-lint)?