[Cloudflare Logpush] map cloudflare_logpush.http_request.client.ssl.cipher to tls.cipher

[moxarth-rathod]

Proposed commit message

cloudflare_logpush: map cloudflare_logpush.http_request.client.ssl.cipher to tls.cipher Map cloudflare_logpush.http_request.client.ssl.cipher to the tls.cipher in order to pick up its search behavior without risking potential breaking behavior. 

Checklist

• I have reviewed tips for building integrations and this pull request is aligned with them.
• I have verified that all data streams collect metrics or logs.
• I have added an entry to my package's changelog.yml file.
• I have verified that Kibana version constraints are current according to guidelines.
• I have verified that any added dashboard complies with Kibana's Dashboard good practices

How to test this PR locally

Clone integrations repo.
Install elastic package locally.
Start elastic stack using elastic-package.
Move to integrations/packages/cloudflare_logpush directory.
Run the following command to run tests.

elastic-package test

Related issues

• Closes [Cloudflare Logpush]: The field cloudflare_logpush.http_request.client.ssl.cipher has the wrong mapping #13596

[elasticmachine]

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

[elastic-vault-github-plugin-prod]

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

[efd6]

I can see the justification for this, but I do not agree that this is a fix; at best it is an enhancement since it is enabling something that could not be done in the past, but it is not a conflict with the ECS since the types under custom field hierachies are not constrained by the ECS (arguably), and the field here is not mentioned in the ECS (ssl v tls, yes, tenuous, but valid).

I think this needs thought, and the commit message rationale needs to be clarified.

/cc @ShourieG @leandrojmp

[efd6]

I do not believe that this is a fix since I am not convinced that this is a bug. It is unfortunate, but it is not a strict conflict.

[leandrojmp]

I can see the justification for this, but I do not agree that this is a fix; at best it is an enhancement since it is enabling something that could not be done in the past, but it is not a conflict with the ECS since the types under custom field hierachies are not constrained by the ECS (arguably), and the field here is not mentioned in the ECS (ssl v tls, yes, tenuous, but valid).

I think this needs thought, and the commit message rationale needs to be clarified.

/cc @ShourieG @leandrojmp

I think this can be considered an enhancement, I mentioned on the Issue that I've opened that the current mapping is wrong because the value for the field cloudflare_logpush.http_request.client.ssl.cipher is a keyword and the equivalent ECS field would be tls.cipher, but this is indeed not a conflict.

Some values for this field are:

• AEAD-AES256-GCM-SHA384
• ECDHE-ECDSA-AES128-GCM-SHA256
• NONE

Which is a string that indicates the cipher used during the connection, the same description for ecs tls.cipher field.

A future improvement could be just renaming this into tls.cipher to make it easier to correlate with other data sources, this is something that we are doing using the custom ingest pipeline.

[efd6]

@leandrojmp

Which is a string that indicates the cipher used during the connection, the same description for ecs tls.cipher field.

Oh, I absolutely agree that it is intended to be semantically the same, but ECS legalism…

A future improvement could be just renaming this into tls.cipher to make it easier to correlate with other data sources, this is something that we are doing using the custom ingest pipeline.

TBH, this would be my preferred solution — copying the value over to tls.cipher — but I was concerned that the semantics of client might be lost in the move to that field where client is not indicated in the field name. As a user, do you think that would an issue?

[leandrojmp]

TBH, this would be my preferred solution — copying the value over to tls.cipher — but I was concerned that the semantics of client might be lost in the move to that field where client is not indicated in the field name. As a user, do you think that would an issue?

I don't think this would be an issue on this dataset, also there are other tls.* fields being created by the ingest pipeline from some client fields already.

 - grok: field: cloudflare_logpush.http_request.client.ssl.protocol patterns: - "^%{DATA:tls.version_protocol}v%{DATA:tls.version}$" ignore_missing: true ignore_failure: true - lowercase: field: tls.version_protocol ignore_missing: true 

So creating tls.cipher from cloudflare_logpush.http_request.client.ssl.cipher would be aligned with the other fields.

[efd6]

@leandrojmp, thanks. Let's wait for @ShourieG, but @moxarth-rathod I think we may want to do that rather than what is here now.

[moxarth-rathod]

@leandrojmp, thanks. Let's wait for @ShourieG, but @moxarth-rathod I think we may want to do that rather than what is here now.

Got it @efd6. I'll update this as per the suggestions.

[ShourieG]

@moxarth-rathod LGTM from my end, but wait for Dan's approval before merging.

[efd6]

Please also update the proposed commit message to match the behaviour here.

cloudflare_logpush: map cloudflare_logpush.http_request.client.ssl.cipher to tls.cipher Map cloudflare_logpush.http_request.client.ssl.cipher to the tls.cipher in order to pick up its search behavior without risking potential breaking behavior. 

[elastic-sonarqube]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
92.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: dab25a5

History

• 💚 Build #25899 succeeded b56b608
• 💚 Build #25895 succeeded 03f4ef9
• 💚 Build #25874 succeeded 8c2bf92

cc @moxarth-rathod

[elastic-vault-github-plugin-prod]

Package cloudflare_logpush - 1.38.0 containing this change is available at https://epr.elastic.co/package/cloudflare_logpush/1.38.0/