-
Couldn't load subscription status.
- Fork 2.1k
Grafana Detectors update #4411
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
base: main
Are you sure you want to change the base?
Grafana Detectors update #4411
Conversation
Updated grafana and grafanaserviceaccount and added a new detector for grafana api keys.
| } | ||
|
|
||
| if verify { | ||
| res.SetVerificationError(fmt.Errorf("no grafana instance detected to verify against"), match) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't seem to be verifying the matched credentials
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These kind of api keys are used for on-premise installations, so together with a custom endpoint that could be an IP address or a FQDN. Since it's hard to reliably identify it - there is no good way of verifying the matched credentials.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verification logic has to exist in a detector. In docs, it is mentioned that Grafana can be hosted on a VPS or as a subdomain. Gitlab detector is the best example for this
| // Grafana uses "eyJrIjoi" as a prefix for api keys, see for example. | ||
| // https://github.com/grafana/pyroscope-dotnet/blob/0c17634653af09befa7bc07b2e1c420b5dc8578c/tracer/src/Datadog.Trace/Iast/Analyzers/HardcodedSecretsAnalyzer.cs#L173 | ||
| func (s Scanner) Keywords() []string { | ||
| return []string{"grafanaapikey", "eyJrIjoi"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use "eyJrIjoi" exclusively since it's always part of the key. A keyword match for "grafanaapikey" won't necessarily contain eyJrIjoi in its input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doh. PEBKAC.... I'll update that :P
| } | ||
|
|
||
| if verify { | ||
| res.SetVerificationError(fmt.Errorf("no grafana instance detected to verify against"), match) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verification logic has to exist in a detector. In docs, it is mentioned that Grafana can be hosted on a VPS or as a subdomain. Gitlab detector is the best example for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for updating SATs too
| resp, err := client.Do(req) | ||
| if err == nil { | ||
| defer resp.Body.Close() | ||
| switch { | ||
| case resp.StatusCode >= 200 && resp.StatusCode < 300: | ||
| res.Verified = true | ||
| case resp.StatusCode == 401: | ||
| // determinately not verified | ||
| default: | ||
| res.SetVerificationError(fmt.Errorf("unexpected HTTP response status %d", resp.StatusCode), key) | ||
| } | ||
| } else { | ||
| res.SetVerificationError(err, key) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we're at it, can we add indeterminate verification support here?
Here's an example
Updated grafana and grafanaserviceaccount and added a new detector for grafana api keys.
Description:
Added one new detector for generic grafana api keys. Also updated the patterns and some of the logic for GLSA Service Account tokens and GLC tokens.
Checklist:
make test-community)?make lintthis requires golangci-lint)?