-
Notifications
You must be signed in to change notification settings - Fork 483
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
Add osquery ingestion for host certificates feature #26426
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat-23235-host-certificates #26426 +/- ##
===============================================================
Coverage ? 63.82%
===============================================================
Files ? 1657
Lines ? 159035
Branches ? 4093
===============================================================
Hits ? 101505
Misses ? 49583
Partials ? 7947
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
LGTM!
@@ -139,8 +140,39 @@ type MDMAppleErrorChainItem struct { | |||
// | |||
// See https://osquery.io/schema/5.15.0/#certificates | |||
func ExtractDetailsFromOsqueryDistinguishedName(str string) (*HostCertificateNameDetails, error) { | |||
// TODO | |||
return nil, errors.New("not implemented") | |||
str = strings.TrimSpace(str) |
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.
It feels like there should be a built-in Go function to parse this format, but I couldn't find one (I'm not sure how that format is called). I found this: https://pkg.go.dev/crypto/x509/pkix#Name, but it takes a RDNSequence
that is unmarshalled from asn1
binary data, not something like the string osquery gives us. In any case, your implementation looks fine!
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.
Yeah, it took me some time trying to track down the provenance of the format used by osquery since it doesn't seem to be an official specification. Based on some online sleuthing, my best guess it that osquery is following "openSSL-style" formatting, which refers to the formatting requirements of the -subj
flag for openssl req
.
351f402
into
feat-23235-host-certificates
For #23235 and #25459
Checklist for submitter
SELECT *
is avoided, SQL injection is prevented (using placeholders for values in statements)NOTE: Other submitter checklist items (osquery-perf, changes file, etc.) will be covered in subsequent PRs