Implements x509 extended schema by dcode · Pull Request #762 · elastic/ecs

dcode · 2020-02-25T14:02:36Z

This intends to implement the common core fields of x509 certificates. This information is likely logged with TLS sessions, digital signatures found in executable binaries, S/MIME information in email bodies, or analysis of files on disk. When only a single certificate is logged in an event, I propose that it should be nested under file and include the hash object set as well. I also think it's reasonable for systems like packetbeat to log entries under client and server for the TLS metadata.

Related: #64

dcode · 2020-02-25T14:12:31Z

Here's a gist that shows data formats I considered that helped frame this approach.

dcode · 2020-03-20T13:49:50Z

Per elastic/kibana#60026, the SIEM currently uses the fields tls.server_certificate.issuer.common_name and tls.server.alternative_names from packetbeat to populate the TLS tab. The proposed schema already offers x509.alternatives_names, but we should update the subject and issuer fields to be objects split out into:

distinguished_name
common_name
organizational_unit
organization
province
country

cla-checker-service · 2020-03-25T13:39:28Z

💚 CLA has been signed

generated/beats/fields.ecs.yml

docs/field-details.asciidoc

webmat

Good work, thanks @dcode!

Noticed a bunch of little things to adjust, plus a few questions (feel free to answer those before making adjustments)

schemas/x509.yml

adriansr

Thanks, it's looking good. Just leaving you a couple of comments.

schemas/x509.yml

dcode · 2020-04-15T16:12:12Z

This PR is gross, and I'm sorry. I accidentally rebased against a stale branch earlier today and found myself cast into the pit of merge conflict dispair. Should be all striaghtened out now

andrewvc · 2020-04-15T21:40:14Z

code/go/ecs/x509.go

+	// Unique serial number issued by the certificate authority. For
+	// consistency, if this value is alphanumeric, it should be formatted
+	// without colons and uppercase characters.
+	SerialNumber string `ecs:"serial_number"`


Per https://tools.ietf.org/html/rfc5280#section-4.1.2.2 this should always be an integer. String is still appropriate since it may be up to 20 octets, but I think we should specify that we expect a base10 string representation.

andrewvc · 2020-04-15T22:38:14Z

Great stuff here, I'm working on support for this in Heartbeat in elastic/beats#17687 .

The only field not here that we'd use would be a sha1/sha256 hash of the DER encoded cert. This would be great for aggregation. While tls.hash and similar exist, it'd be nice to know this is the hash of the DER binary, and not, say a PEM file etc, which file.hash might do.

I don't think we need to block this issue on that FYI.

andrewvc · 2020-04-16T11:48:31Z

One other non-blocking change I'd recommend for a follow-up PR (which I'd be glad to contribute).

We could add chain_not_after, and chain_not_before which would contain the computed not_after/not_before based on the earliest/latest dates in the full chain, in case someone has a poorly setup CA issuing certs beyond its own lifetime.

peasead · 2020-04-16T12:35:56Z

Good point, Andrew. We had a federal customer who had a need for this kind of field recently.

On Thu, Apr 16, 2020 at 6:48 AM Andrew Cholakian ***@***.***> wrote: One other non-blocking change I'd recommend for a follow-up PR (which I'd be glad to contribute). We could add chain_not_after, and chain_not_before which would contain the computed not_after/not_before based on the earliest/latest dates in the full chain, in case someone has a poorly setup CA issuing certs beyond its own lifetime. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#762 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABYY5K5IOYNQ7XVL6E3RTCLRM3WCBANCNFSM4K3J2OXA> .

-- *---* *Andrew D. Pease* Senior Manager, Consulting m: 314.749.4249 andrew.pease@elastic.co

andrewvc · 2020-04-16T14:52:58Z

Having gone through the PR in detail, I think this is great where it is. I have some additive suggestions, but they don't need to be in this PR. I'm LGTM on merging this PR as-is, and following up with some additions.

webmat

LGTM after two minor rewordings, see comments below.

Please let me merge this, as we're still waiting for an official thumbs up from the SIEM UI point of view. I want to make sure this is reviewed from their POV so we don't have surprises there a second time.

@tsg you initially pinged me about the UI issues when tls got merged in ECS. Could you review this from the UI point of view, or point me to the person who will do the TLS view adjustments based on this new field set?

webmat · 2020-04-21T14:13:01Z

schemas/x509.yml

+  group: 2
+  short_description: These fields contain x509 certificate metadata.
+  description: >
+    This implements the common core fields for x509 certificates. This information is likely logged with TLS sessions,


Suggested change

This implements the common core fields for x509 certificates. This information is likely logged with TLS sessions,

These fields contain x509 certificate metadata. This information is likely logged with TLS sessions,

The current first sentence reads like a PR body ("we're implementing this"). Let's just reuse the short definition as the first sentence instead. The short and description are never displayed in the same place, so it's fine to reuse it as is.

webmat · 2020-04-21T14:40:56Z

schemas/x509.yml

+      type: keyword
+      normalize:
+        - array
+      description: List of state or province names (ST, S, or P)


Perhaps we should spell out more explicitly here that "any of the ST, S or P entries are to be added to this array.

Current description is perfect for a short description. Then the new description could read like.

Suggested change

description: List of state or province names (ST, S, or P)

short: List of state or province names (ST, S, or P)

description: List of state or province names.

All `ST`, `S` or `P` attributes should be added to the `x509.subject.state_or_province` array.

schemas/x509.yml

Work in support of elastic/uptime#161 This patch adds additional ECS [TLS](https://www.elastic.co/guide/en/ecs/current/ecs-tls.html) and [x509](elastic/ecs#762) fields. Note that we are blocked on the x509 fields which are not yet merged into ECS. Sample output of the `tls.*` fields with this patch is below. Note the somewhat strange nesting of data in `issuer` and `subject`. This is per the ECS spec, but a bit awkward. We may want to break this data out into the more specific ECS `x509` type in the future. For UI work we are likely fine to parse this on the client and display the CN section in most cases. I did break out the CN into its own field in `x509.subject/issuer.common_name`. However, if we do want to aggregate on issuer in the future it's good to have the full distinguished name to do that on. This PR also refactors some `libbeat` code around parsing TLS versions and adds test coverage there as well. ```json { "tls": { "certificate_not_valid_after": "2020-07-16T03:15:39Z", "certificate_not_valid_before": "2019-08-16T01:40:25Z", "server": { "hash": { "sha1": "b7b4b89ef0d0caf39d223736f0fdbb03c7b426f1", "sha256": "12b00d04db0db8caa302bfde043e88f95baceb91e86ac143e93830b4bbec726d" }, "x509": { "issuer": { "common_name": "GlobalSign CloudSSL CA - SHA256 - G3", "distinguished_name": "CN=GlobalSign CloudSSL CA - SHA256 - G3,O=GlobalSign nv-sa,C=BE" }, "not_after": "2020-07-16T03:15:39Z", "not_before": "2019-08-16T01:40:25Z", "public_key_algorithm": "RSA", "public_key_size": 2048, "serial_number": "26610543540289562361990401194", "signature_algorithm": "SHA256-RSA", "subject": { "common_name": "r2.shared.global.fastly.net", "distinguished_name": "CN=r2.shared.global.fastly.net,O=Fastly\\, Inc.,L=San Francisco,ST=California,C=US" } } } } } ``` ## How to test this PR locally Run against TLS/Non-TLS endpoints

Work in support of elastic/uptime#161 This patch adds additional ECS [TLS](https://www.elastic.co/guide/en/ecs/current/ecs-tls.html) and [x509](elastic/ecs#762) fields. Note that we are blocked on the x509 fields which are not yet merged into ECS. Sample output of the `tls.*` fields with this patch is below. Note the somewhat strange nesting of data in `issuer` and `subject`. This is per the ECS spec, but a bit awkward. We may want to break this data out into the more specific ECS `x509` type in the future. For UI work we are likely fine to parse this on the client and display the CN section in most cases. I did break out the CN into its own field in `x509.subject/issuer.common_name`. However, if we do want to aggregate on issuer in the future it's good to have the full distinguished name to do that on. This PR also refactors some `libbeat` code around parsing TLS versions and adds test coverage there as well. ```json { "tls": { "certificate_not_valid_after": "2020-07-16T03:15:39Z", "certificate_not_valid_before": "2019-08-16T01:40:25Z", "server": { "hash": { "sha1": "b7b4b89ef0d0caf39d223736f0fdbb03c7b426f1", "sha256": "12b00d04db0db8caa302bfde043e88f95baceb91e86ac143e93830b4bbec726d" }, "x509": { "issuer": { "common_name": "GlobalSign CloudSSL CA - SHA256 - G3", "distinguished_name": "CN=GlobalSign CloudSSL CA - SHA256 - G3,O=GlobalSign nv-sa,C=BE" }, "not_after": "2020-07-16T03:15:39Z", "not_before": "2019-08-16T01:40:25Z", "public_key_algorithm": "RSA", "public_key_size": 2048, "serial_number": "26610543540289562361990401194", "signature_algorithm": "SHA256-RSA", "subject": { "common_name": "r2.shared.global.fastly.net", "distinguished_name": "CN=r2.shared.global.fastly.net,O=Fastly\\, Inc.,L=San Francisco,ST=California,C=US" } } } } } ``` ## How to test this PR locally Run against TLS/Non-TLS endpoints (cherry picked from commit eb2dc26)

#18029) * [Heartbeat] Add Additional ECS tls.* fields (#17687) Work in support of elastic/uptime#161 This patch adds additional ECS [TLS](https://www.elastic.co/guide/en/ecs/current/ecs-tls.html) and [x509](elastic/ecs#762) fields. Note that we are blocked on the x509 fields which are not yet merged into ECS. Sample output of the `tls.*` fields with this patch is below. Note the somewhat strange nesting of data in `issuer` and `subject`. This is per the ECS spec, but a bit awkward. We may want to break this data out into the more specific ECS `x509` type in the future. For UI work we are likely fine to parse this on the client and display the CN section in most cases. I did break out the CN into its own field in `x509.subject/issuer.common_name`. However, if we do want to aggregate on issuer in the future it's good to have the full distinguished name to do that on. This PR also refactors some `libbeat` code around parsing TLS versions and adds test coverage there as well. ```json { "tls": { "certificate_not_valid_after": "2020-07-16T03:15:39Z", "certificate_not_valid_before": "2019-08-16T01:40:25Z", "server": { "hash": { "sha1": "b7b4b89ef0d0caf39d223736f0fdbb03c7b426f1", "sha256": "12b00d04db0db8caa302bfde043e88f95baceb91e86ac143e93830b4bbec726d" }, "x509": { "issuer": { "common_name": "GlobalSign CloudSSL CA - SHA256 - G3", "distinguished_name": "CN=GlobalSign CloudSSL CA - SHA256 - G3,O=GlobalSign nv-sa,C=BE" }, "not_after": "2020-07-16T03:15:39Z", "not_before": "2019-08-16T01:40:25Z", "public_key_algorithm": "RSA", "public_key_size": 2048, "serial_number": "26610543540289562361990401194", "signature_algorithm": "SHA256-RSA", "subject": { "common_name": "r2.shared.global.fastly.net", "distinguished_name": "CN=r2.shared.global.fastly.net,O=Fastly\\, Inc.,L=San Francisco,ST=California,C=US" } } } } } ``` Run against TLS/Non-TLS endpoints (cherry picked from commit eb2dc26) * Use non-wildcard field for text * Remove wildcard type

tsg

👍 this should be good for the SIEM app. issuer.common_name and subject.common_name are the ones that we were missing.

dcode force-pushed the dcode/x509 branch from d415db4 to a54d51f Compare March 20, 2020 13:42

dcode force-pushed the dcode/x509 branch from 3ff59df to 432c6cf Compare March 25, 2020 13:41

dcode requested review from dainperkins, peasead and webmat March 25, 2020 13:42

dcode force-pushed the dcode/x509 branch 2 times, most recently from 948c2d2 to e3e16d2 Compare March 25, 2020 13:46

peasead reviewed Mar 25, 2020

View reviewed changes

generated/beats/fields.ecs.yml Outdated Show resolved Hide resolved

dcode force-pushed the dcode/x509 branch from e3e16d2 to cb9a6a2 Compare March 25, 2020 17:30

webmat added the 1.6.0 label Mar 25, 2020

dainperkins reviewed Mar 25, 2020

View reviewed changes

docs/field-details.asciidoc Show resolved Hide resolved

webmat reviewed Apr 1, 2020

View reviewed changes

dcode force-pushed the dcode/x509 branch from 31d53b4 to 39f050c Compare April 2, 2020 17:14

andrewkroh requested a review from adriansr April 2, 2020 17:30

adriansr reviewed Apr 6, 2020

View reviewed changes

schemas/x509.yml Outdated Show resolved Hide resolved

schemas/x509.yml Show resolved Hide resolved

dcode force-pushed the dcode/x509 branch 2 times, most recently from c873c92 to 774e793 Compare April 15, 2020 15:43

dcode closed this Apr 15, 2020

dcode force-pushed the dcode/x509 branch from 774e793 to 9496824 Compare April 15, 2020 16:06

Adds x509 schema with all previously discussed changes.

da1cd46

dcode reopened this Apr 15, 2020

Adds generated documents

c2083c9

Forgot this little guy on the add

849b497

andrewvc reviewed Apr 15, 2020

View reviewed changes

andrewvc mentioned this pull request Apr 15, 2020

[Heartbeat] Add Additional ECS tls.* fields elastic/beats#17687

Merged

7 tasks

webmat approved these changes Apr 21, 2020

View reviewed changes

webmat requested a review from tsg April 21, 2020 14:50

webmat mentioned this pull request Apr 23, 2020

TLS Session Fields #452

Closed

andrewvc mentioned this pull request Apr 27, 2020

Cherry-pick #17687 to 7.x: [Heartbeat] Add Additional ECS tls.* fields elastic/beats#18029

Merged

7 tasks

tsg approved these changes Apr 30, 2020

View reviewed changes

Merge branch 'master' into dcode/x509

51d2372

webmat merged commit 8183908 into elastic:master Apr 30, 2020

dcode deleted the dcode/x509 branch April 30, 2020 18:55

andrewvc mentioned this pull request May 7, 2020

Enhanced TLS certificate metadata #64

Closed

leehinman mentioned this pull request Jun 29, 2020

[Meta] support new ECS 1.6 fields elastic/beats#19472

Closed

22 tasks

webmat mentioned this pull request Jul 22, 2020

ValueError exception using --ref #892

Closed

	This implements the common core fields for x509 certificates. This information is likely logged with TLS sessions,
	These fields contain x509 certificate metadata. This information is likely logged with TLS sessions,

-      description: List of state or province names (ST, S, or P)
+     short: List of state or province names (ST, S, or P)
+     description: List of state or province names.
+     All `ST`, `S` or `P` attributes should be added to the `x509.subject.state_or_province` array.

Conversation

dcode commented Feb 25, 2020

Uh oh!

dcode commented Feb 25, 2020

Uh oh!

dcode commented Mar 20, 2020

Uh oh!

cla-checker-service bot commented Mar 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

webmat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adriansr left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dcode commented Apr 15, 2020

Uh oh!

andrewvc Apr 15, 2020

Choose a reason for hiding this comment

Uh oh!

andrewvc commented Apr 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrewvc commented Apr 16, 2020

Uh oh!

peasead commented Apr 16, 2020 via email

Uh oh!

andrewvc commented Apr 16, 2020

Uh oh!

webmat left a comment

Choose a reason for hiding this comment

Uh oh!

webmat Apr 21, 2020

Choose a reason for hiding this comment

Uh oh!

webmat Apr 21, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tsg left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

cla-checker-service bot commented Mar 25, 2020 •

edited

Loading

adriansr left a comment •

edited

Loading

andrewvc commented Apr 15, 2020 •

edited

Loading