Proposal: TLS related fields

[ph]

While working on the TCP inputs for beats I am able to extract information about the TLS envelope,
this might be useful information for post-mortems or audit. Also, this was a feature requested on
the beats input on the Logstash side.

We are also extracting some information in packetbeat from the work of @adriansr.

Concerning the TCP or the beats input, I think we can extract the following field and provide some
useful information.

• TLS version
• Remote Certificates (mutual auth)
• ServerName
• Cipher used.

Where OR should this information be in the ECS schema? Maybe we could add a transport top level
key and have something like this.

Field	Description	Type	Example
tls.version	string representation of the tls version	keyword	TLSv1.1
tls.certificates	array of certificates used by the client.	text	raw certificate
tls.servername	server name requested by client	keyword	localhost1
tls.ciphersuite	Name of the cipher used	keyword	ECDHE-ECDSA-AES-128-CBC-SHA

[ph]

@ruflin Good job, the tooling for this repository works great. 🎉

[praseodym]

What would be the format of this field? If one were to use base64-encoded certificates, a keyword field would make more sense than text.

[ph]

You are absolutely right for the keyword, I’ll update the PR.

[ph]

@ruflin to move the certificate type from a text field to a keyword I will need to make a small change on the template generator, we have the ignore_above value hardcoded, I think we should allow users to override that from the schema definition. I need to do some checks on the openssl documentation but it can be over 1024.

I don't expect any problems since the ES default is much higher.

Do not index any string longer than this value. Defaults to 2147483647 so that all values would be accepted. 

[ruflin]

@ph We should add support for ignore_above config in field.go: https://github.com/elastic/beats/blob/master/libbeat/common/field.go#L36

[ph]

@ruflin The required support for ignore_above was merged into master, I've rebased and regenerated the files in this PR. I've gone with a 4096 upper bound limit on the base64 certificate; I think it should cover the maximum size, look at the commit message for more details.

[praseodym]

Is there a reason to specify a value for ignore_above? Current documentation doesn't list any advantages.

It is very common for certificates to be >4096 bytes, e.g. check out this certificate for elastic.co at 4885 bytes or this certificate for microsoft.com at 5697 bytes. There is no set limit on a certificate's size; e.g. the number of subject alternative names is unbounded.

[ph]

Right, thanks for enlightening me, I am OK to remove it relies on ES keyword default which will be in this case 2147483647, I will need to make changes on the template generator on libbeat, since there is no way to opt out of it.

@ruflin what you think if we make it an explicit choice, so users have to use -1 in the fields.yaml?

[ph]

@ruflin Also, defining this kind of size limits should be in another document we generated like the CSV, I don't think this information should only be kept in the template. This could be part of a new discussion or issue.

[ruflin]

We started to set for some fields index: false and doc_values: false (see https://github.com/elastic/ecs/blob/master/schemas/event.yml#L73) I wonder if this also applies to certificate as it will not be a field that anyone queries on I would assume?

@ph +1 on having the option in the generator to disable it. For the discussion on where these things should show up, let's move it to an other Github issue as proposed.

[ph]

@ruflin I think it would make sense in that context to set doc_values to false, we won't sort or aggregate on that field. So that would be a space saver.

[ruflin]

I would also set index:false. Like this the keyword length becomes irrelevant.

[ph]

Setting index: false will make it not queryable, in the context of certificates, querying it can be useful when doing forensic, no? What is your reasoning for not indexing it?

[ph]

Created #23 for exposing keyword limit.

[ruflin]

My assumption was that there is not going to be a query on the certificate field but as you mentioned above, there are potential queries. So index should be set to true.

[ph]

@ruflin updated the PR with the latest review.

[ruflin]

s/contains/contain/

[ruflin]

Can you add a . at the end?

[karenzone]

Consider tagging as footnote: >to push this example to after the table

[ruflin]

@ph I think you need to run make again.

[ph]

@ruflin @karenzone added footnote and run make again, I didn't know about footnote.

[ruflin]

@ph footnote is something we just introduced a few days ago.