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

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

Merged
merged 30 commits into from
Apr 27, 2020

Conversation

andrewvc
Copy link
Contributor

@andrewvc andrewvc commented Apr 13, 2020

Work in support of elastic/uptime#161

This patch adds additional ECS TLS and x509 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.

{
	"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"
				}
			}
		}
	}
}

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

  • Verify that the fields here match the ECS spec

How to test this PR locally

Run against TLS/Non-TLS endpoints

This patch adds additional [ECS
fields](https://www.elastic.co/guide/en/ecs/current/ecs-tls.html).

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.

```json
{
  "version": "1.2",
  "version_protocol": "tls"
  "cipher": "ECDHE-RSA-AES-128-GCM-SHA256",
  "server": {
    "subject": "CN=r2.shared.global.fastly.net,O=Fastly\\, Inc.,L=San Francisco,ST=California,C=US",
    "hash": {
      "sha1": "b7b4b89ef0d0caf39d223736f0fdbb03c7b426f1",
      "sha256": "12b00d04db0db8caa302bfde043e88f95baceb91e86ac143e93830b4bbec726d"
    },
    "not_before": "2019-08-16T01:40:25.000Z",
    "not_after": "2019-08-16T01:40:25.000Z",
    "issuer": "CN=GlobalSign CloudSSL CA - SHA256 - G3,O=GlobalSign nv-sa,C=BE"
  },
  "certificate_not_valid_before": "2019-08-16T01:40:25.000Z",
  "certificate_not_valid_after": "2020-07-16T03:15:39.000Z",
  "established": true,
  "rtt": {
    "handshake": {
      "us": 42491
    }
  },
}
```

Work goes towards elastic/uptime#161
@andrewvc andrewvc added enhancement Heartbeat Team:obs-ds-hosted-services Label for the Observability Hosted Services team labels Apr 13, 2020
@andrewvc andrewvc self-assigned this Apr 13, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/uptime (:uptime)

@axrayn
Copy link

axrayn commented Apr 15, 2020

@andrewvc Looking forward to this addition, will save our current heartbeat/packetbeat kludge to report on TLS certificates validity.

Any chance that you could also include the server certificate key length? We have a need from our Cyber folks to be able to identify certificates with a KeyLength less than 2048 bits.

Packetbeat uses a function here that should be able to be replicated over to the tlsmeta/tls.go file for this.

Copy link
Contributor

@blakerouse blakerouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good. Just one comment, but not really a blocker.

}

fields.DeepUpdate(common.MapStr{"tls": tlsFields})
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be easier to test and use less memory to create a TLSMeta struct and place that inside of the event. Using map[string]interface{} everywhere is not very memory efficent. Example is #16538 where it was converted to a struct.

Its not really a blocker, just something that could help.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed this with @simitt . Looks like this is an area @urso is working on, but isn't possible today because beats processors expect all event fields to be maps. I agree that it would be nice.

Copy link
Contributor

@blakerouse blakerouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and the tests looks like it gives great coverage!

@blakerouse
Copy link
Contributor

Wildcard changes also look good.

@andrewvc andrewvc merged commit eb2dc26 into elastic:master Apr 27, 2020
@andrewvc andrewvc deleted the extra-tls-fields branch April 27, 2020 21:11
andrewvc added a commit to andrewvc/beats that referenced this pull request Apr 27, 2020
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)
andrewvc added a commit that referenced this pull request Apr 29, 2020
#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
@andrewvc andrewvc mentioned this pull request Apr 29, 2020
6 tasks
andrewvc added a commit to andrewvc/beats that referenced this pull request Apr 30, 2020
In elastic#17687 we added additional x509 fields. The mapping accidentally
double nested subject fields as subject.subject. This fixes that.

There are no tests here because we don't really have testing around
mappings, and it's sort of difficult to test without being repetitive,
so none are included here.
andrewvc added a commit that referenced this pull request May 4, 2020
In #17687 we added additional x509 fields. The mapping accidentally
double nested subject fields as subject.subject. This fixes that.

There are no tests here because we don't really have testing around
mappings, and it's sort of difficult to test without being repetitive,
so none are included here.
andrewvc added a commit to andrewvc/beats that referenced this pull request May 4, 2020
In elastic#17687 we added additional x509 fields. The mapping accidentally
double nested subject fields as subject.subject. This fixes that.

There are no tests here because we don't really have testing around
mappings, and it's sort of difficult to test without being repetitive,
so none are included here.

(cherry picked from commit a81bbda)
andrewvc added a commit that referenced this pull request May 4, 2020
In #17687 we added additional x509 fields. The mapping accidentally
double nested subject fields as subject.subject. This fixes that.

There are no tests here because we don't really have testing around
mappings, and it's sort of difficult to test without being repetitive,
so none are included here.

(cherry picked from commit a81bbda)
@andrewvc andrewvc assigned andrewvc and unassigned andrewvc May 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Heartbeat Team:obs-ds-hosted-services Label for the Observability Hosted Services team v7.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants