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.

Sign up for GitHub

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

Jump to bottom

Sanitize event.host in Metricbeat #21022

Merged
merged 3 commits into from
Sep 9, 2020
Merged

Sanitize event.host in Metricbeat #21022

merged 3 commits into from
Sep 9, 2020

Conversation

mtojek
Copy link
Contributor

@mtojek mtojek commented Sep 8, 2020 •
edited
Loading

What does this PR do?

This PR sanitizes the event.host not to leak any credentials. It fixes also a bug related to a defined but not used host parser.

Why is it important?

It's a security threat.

Checklist

  • My code follows the style guidelines of this project
  • [x 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

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

@mtojek mtojek requested a review from a team September 8, 2020 16:45
@mtojek mtojek self-assigned this Sep 8, 2020
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Sep 8, 2020
@mtojek mtojek changed the title Sanitize event.host Sanitize event.host in Metricbeat Sep 8, 2020
@mtojek mtojek added Team:Integrations Label for the Integrations team needs_backport PR is waiting to be backported to other branches. and removed needs_team Indicates that the issue/PR needs a Team:* label labels Sep 8, 2020
@elasticmachine
Copy link
Collaborator

elasticmachine commented Sep 8, 2020 •
edited by jenkins-beats-ci bot
Loading

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #21022 updated]

  • Start Time: 2020-09-08T20:53:54.417+0000

  • Duration: 60 min 24 sec

Test stats 🧪

Test Results
Failed 0
Passed 4155
Skipped 880
Total 5035

@mtojek
Copy link
Contributor Author

mtojek commented Sep 8, 2020

CI errors unrelated:

  1. ERROR: failed to create cluster: failed to generate kubeadm config content: failed to get kubernetes version from node: failed to get file: command "docker exec --privileged metricbeat-8-0-0-f39f985656-snapshot-control-plane cat /kind/version" failed with error: exit status 1
  2. github.com/aws/[email protected]: Get "https://storage.googleapis.com/proxy-golang-org-prod/c6bb391792cc261f-github.aaakk.us.kg:aws:aws-sdk-go-v1.30.15.zip?Expires=1599686198&GoogleAccessId=gcs-urlsigner-prod%40golang-modproxy.iam.gserviceaccount.com&Signature=IWVc2O4FODmmtHA%2F79iVG9AWUuPB02oYCGxRPm3YlJeaHkfzHt%2BsWDi39Jw2rDArC%2FhGFUVhqyKF4pSr6APL8ZZK0BnC57uTLmGNStj2uOyrf%2B1H0c5a8PNzeiFWitjYF7K1qalJ6RhhXXGLcKPCr8RM14v500bS66KyIp7I2NJgXvJ51HX%2B6mcczmz%2FcWObVc25sKPMAUWvxwMQ4cul813veFuxxBKNWAh3LDF2RhLYH%2F44DoRbi7CUAFj8nho0rHS5297T2z%2FPoiHJMzsayqOpmZtg9dLqz7ouFhemKdBvR%2BXTqVdsGmxodhgnWIysTthYkLwe0Szh68MbYwLF1g%3D%3D": unexpected EOF

@mtojek mtojek marked this pull request as ready for review September 8, 2020 21:49
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

@mtojek mtojek requested a review from andrewkroh September 8, 2020 21:49
andrewkroh
andrewkroh approved these changes Sep 8, 2020
View reviewed changes
Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

LGTM

@mtojek mtojek merged commit 3ecf7e6 into elastic:master Sep 9, 2020
@mtojek mtojek mentioned this pull request Sep 9, 2020
Merged
5 tasks
@mtojek mtojek added v7.10.0 and removed needs_backport PR is waiting to be backported to other branches. labels Sep 9, 2020
mtojek added a commit to mtojek/beats that referenced this pull request Sep 9, 2020
@mtojek
Sanitize event.host in Metricbeat (elastic#21022)
189b73d 
* Sanitize event.host

* Update CHANGELOG

* Fix: enable host parser

(cherry picked from commit 3ecf7e6)
mtojek added a commit that referenced this pull request Sep 9, 2020
@mtojek
Sanitize event.host in Metricbeat (#21022) (#21030)
577f67d 
* Sanitize event.host

* Update CHANGELOG

* Fix: enable host parser

(cherry picked from commit 3ecf7e6)
v1v added a commit to v1v/beats that referenced this pull request Sep 14, 2020
@v1v
Merge remote-tracking branch 'upstream/master' into feature/windows-10
cc07c42 
* upstream/master: (362 commits)
  Add vendoring to Google Cloud Functions again (elastic#21070)
  [Elastic Agent] Add fleet.host.id for sending to endpoint. (elastic#21042)
  Do not need Google credentials before using it (elastic#21072)
  [Filebeat][New Module] Zoom webhook module (elastic#20414)
  Add support for GMT timezone offset in decode_cef (elastic#20993)
  Filebeat: Fix random error on harvester close (elastic#21048)
  Add ingress controller dashboards (elastic#21052)
  Fix loggers in composable module. (elastic#21047)
  [Ingest Manager] Increase kibana client timeout to 5 minutes (elastic#21037)
  Add changelog. (elastic#21041)
  [Elastic Agent] Add support for EQL based conditions (elastic#20994)
  Disable Kafka metricsets based on Jolokia (elastic#20989)
  Update apm agent (elastic#21031)
  Add container ECS fields in kubernetes metadata (elastic#20984)
  Sanitize event.host in Metricbeat (elastic#21022)
  Update api-keys.asciidoc - API key prerequisites (elastic#21026)
  [Filebeat][suricata] Map x509 for suricata/eve fileset (elastic#20973)
  [Filebeat][santa] Map x509 fields in santa module (elastic#20976)
  [Filebeat][fortinet] Map x509 ecs fields for fortinet fw fileset (elastic#20983)
  Bump zeek kerberos/ssl/x509 ecs version (elastic#21003)
  ...
v1v added a commit to v1v/beats that referenced this pull request Sep 14, 2020
@v1v
Merge remote-tracking branch 'upstream/master' into feature/windows-7-64
fcc41b4 
* upstream/master: (364 commits)
  Add vendoring to Google Cloud Functions again (elastic#21070)
  [Elastic Agent] Add fleet.host.id for sending to endpoint. (elastic#21042)
  Do not need Google credentials before using it (elastic#21072)
  [Filebeat][New Module] Zoom webhook module (elastic#20414)
  Add support for GMT timezone offset in decode_cef (elastic#20993)
  Filebeat: Fix random error on harvester close (elastic#21048)
  Add ingress controller dashboards (elastic#21052)
  Fix loggers in composable module. (elastic#21047)
  [Ingest Manager] Increase kibana client timeout to 5 minutes (elastic#21037)
  Add changelog. (elastic#21041)
  [Elastic Agent] Add support for EQL based conditions (elastic#20994)
  Disable Kafka metricsets based on Jolokia (elastic#20989)
  Update apm agent (elastic#21031)
  Add container ECS fields in kubernetes metadata (elastic#20984)
  Sanitize event.host in Metricbeat (elastic#21022)
  Update api-keys.asciidoc - API key prerequisites (elastic#21026)
  [Filebeat][suricata] Map x509 for suricata/eve fileset (elastic#20973)
  [Filebeat][santa] Map x509 fields in santa module (elastic#20976)
  [Filebeat][fortinet] Map x509 ecs fields for fortinet fw fileset (elastic#20983)
  Bump zeek kerberos/ssl/x509 ecs version (elastic#21003)
  ...
This was referenced Jul 15, 2021
Closed
Merged
jsoriano added a commit that referenced this pull request Jul 15, 2021
@jsoriano
Use common host parser in vsphere module (#26904)
68e9909 
Use the common host parser builder to parse hosts defined in vsphere module configuration.

Since #21022, sanitized URIs included in modules host data are used as `service.address`.
vsphere did a custom parsing that didn't fill the sanitized URI and then `service.address` was not filled.
mergify bot pushed a commit that referenced this pull request Jul 15, 2021
@jsoriano
Use common host parser in vsphere module (#26904)
4e99ee8 
Use the common host parser builder to parse hosts defined in vsphere module configuration.

Since #21022, sanitized URIs included in modules host data are used as `service.address`.
vsphere did a custom parsing that didn't fill the sanitized URI and then `service.address` was not filled.

(cherry picked from commit 68e9909)
mergify bot pushed a commit that referenced this pull request Jul 15, 2021
@jsoriano
Use common host parser in vsphere module (#26904)
7223f72 
Use the common host parser builder to parse hosts defined in vsphere module configuration.

Since #21022, sanitized URIs included in modules host data are used as `service.address`.
vsphere did a custom parsing that didn't fill the sanitized URI and then `service.address` was not filled.

(cherry picked from commit 68e9909)
jsoriano added a commit that referenced this pull request Jul 15, 2021
@mergify @jsoriano
Use common host parser in vsphere module (#26904) (#26909)
c4b10fe 
Use the common host parser builder to parse hosts defined in vsphere module configuration.

Since #21022, sanitized URIs included in modules host data are used as `service.address`.
vsphere did a custom parsing that didn't fill the sanitized URI and then `service.address` was not filled.

(cherry picked from commit 68e9909)

Co-authored-by: Jaime Soriano Pastor <[email protected]>
jsoriano added a commit that referenced this pull request Jul 15, 2021
@mergify @jsoriano
[7.14](backport #26904) Use common host parser in vsphere module (#26910
5a03e06 
)

Use the common host parser builder to parse hosts defined in vsphere module configuration.

Since #21022, sanitized URIs included in modules host data are used as `service.address`.
vsphere did a custom parsing that didn't fill the sanitized URI and then `service.address` was not filled.

(cherry picked from commit 68e9909)

Co-authored-by: Jaime Soriano Pastor <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewkroh andrewkroh andrewkroh approved these changes

Assignees

@mtojek mtojek

Labels
enhancement Team:Integrations Label for the Integrations team v7.10.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Automatically mask passwords
3 participants
@mtojek @elasticmachine @andrewkroh