Skip to content

aws.securityhub_findings*: Fix null reference when extracting host.ip #14264

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
Merged
kcreddy merged 4 commits into from
Jun 20, 2025
Merged

aws.securityhub_findings*: Fix null reference when extracting host.ip #14264

kcreddy merged 4 commits into from
Jun 20, 2025

Conversation

@kcreddy
Copy link
Contributor

@kcreddy kcreddy commented Jun 19, 2025

Proposed commit message

There is no null check on resources[].Details.AwsEc2Instance.IpV4Addresses 
or resources[].Details.AwsEc2Instance.IpV6Addresses when extracting host.ip 
leading to pipeline_error.
This PR adds the required null check to prevent pipeline_error.

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

Added new pipeline test samples to simulate the behaviour.
Before:

--- Test results for package: aws - START ---
FAILURE DETAILS:
aws/securityhub_findings_full_posture test-securityhub-findings-full-posture.log:
[0] unexpected pipeline error: [Processor 'conditional' with tag 'script_extract_fields_from_single_resource' failed with message 'cannot access method/field [iterator] from a null def reference']


╭─────────┬───────────────────────────────────┬───────────┬───────────────────────────────────────────────────────────────────────┬─────────────────────────────────────────────────────────────────────────────┬──────────────╮
│ PACKAGE │ DATA STREAM                       │ TEST TYPE │ TEST NAME                                                             │ RESULT                                                                      │ TIME ELAPSED │
├─────────┼───────────────────────────────────┼───────────┼───────────────────────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────────┼──────────────┤
│ aws     │ securityhub_findings_full_posture │ pipeline  │ (ingest pipeline warnings test-securityhub-findings-full-posture.log) │ PASS                                                                        │ 440.180209ms │
│ aws     │ securityhub_findings_full_posture │ pipeline  │ test-securityhub-findings-full-posture.log                            │ FAIL: test case failed: one or more problems with fields found in documents │ 296.372708ms │
╰─────────┴───────────────────────────────────┴───────────┴───────────────────────────────────────────────────────────────────────┴─────────────────────────────────────────────────────────────────────────────┴──────────────╯
--- Test results for package: aws - END   ---
Done

After:

--- Test results for package: aws - START ---
╭─────────┬───────────────────────────────────┬───────────┬───────────────────────────────────────────────────────────────────────┬────────┬──────────────╮
│ PACKAGE │ DATA STREAM                       │ TEST TYPE │ TEST NAME                                                             │ RESULT │ TIME ELAPSED │
├─────────┼───────────────────────────────────┼───────────┼───────────────────────────────────────────────────────────────────────┼────────┼──────────────┤
│ aws     │ securityhub_findings              │ pipeline  │ (ingest pipeline warnings test-securityhub-findings.log)              │ PASS   │ 431.320292ms │
│ aws     │ securityhub_findings              │ pipeline  │ test-securityhub-findings.log                                         │ PASS   │  288.70975ms │
│ aws     │ securityhub_findings_full_posture │ pipeline  │ (ingest pipeline warnings test-securityhub-findings-full-posture.log) │ PASS   │ 453.879208ms │
│ aws     │ securityhub_findings_full_posture │ pipeline  │ test-securityhub-findings-full-posture.log                            │ PASS   │ 275.653208ms │
╰─────────┴───────────────────────────────────┴───────────┴───────────────────────────────────────────────────────────────────────┴────────┴──────────────╯
--- Test results for package: aws - END   ---
Done
@kcreddy kcreddy self-assigned this Jun 19, 2025
@kcreddy kcreddy added Integration:aws AWS bugfix Pull request that fixes a bug issue Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations] labels Jun 19, 2025
@kcreddy kcreddy marked this pull request as ready for review June 19, 2025 12:42
@kcreddy kcreddy requested review from a team as code owners June 19, 2025 12:42
@elasticmachine
Copy link

elasticmachine commented Jun 19, 2025

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)
@elastic-vault-github-plugin-prod
Copy link

elastic-vault-github-plugin-prod bot commented Jun 19, 2025

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport
andrewkroh
andrewkroh reviewed Jun 19, 2025
View reviewed changes
...a_stream/securityhub_findings/_dev/test/pipeline/test-securityhub-findings.log-expected.json Outdated
"IamInstanceProfileArn": "arn:aws:iam::1111111111:instance-profile/eks-00c3ac5a-8d2a-1a50-9bd3-6c1939f83db6",
"ImageId": "ami-0e0ff40957f238bdd",
"IpV4Addresses": [
"67.43.156.245",
Copy link
Member

@andrewkroh andrewkroh Jun 19, 2025

Choose a reason for hiding this comment

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

Can we change those IPs to be within the range reserved for documentation by RFC 5737.

https://github.com/elastic/elastic-package/blob/6f9523b118c70377de3c792abb228cf5757dca78/docs/howto/ingest_geoip.md
Copy link
Contributor Author

@kcreddy kcreddy Jun 19, 2025

Choose a reason for hiding this comment

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

Done in 397c0ac
packages/aws/data_stream/securityhub_findings/elasticsearch/ingest_pipeline/default.yml Outdated
if (res.Type == 'AwsEcsContainer' && res.Details.AwsEcsContainer?.Name != null) {
ctx.host.name = res.Details.AwsEcsContainer.Name;
}
if (res.Type == 'AwsEc2Instance' && (res.Details.AwsEc2Instance?.IpV4Addresses != null || res.Details.AwsEc2Instance?.IpV6Addresses != null)) {
Copy link
Member

@andrewkroh andrewkroh Jun 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It looks like this && (res.Details.AwsEc2Instance?.IpV4Addresses != null || res.Details.AwsEc2Instance?.IpV6Addresses != null) is now redundant and could be removed.

Same comment for L1832 and for the full_posture pipeline.
Copy link
Contributor Author

@kcreddy kcreddy Jun 19, 2025

Choose a reason for hiding this comment

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

Updated as per suggestion in 397c0ac
@kcreddy kcreddy requested a review from andrewkroh June 19, 2025 16:25
@elastic-sonarqube
Copy link

elastic-sonarqube bot commented Jun 19, 2025

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

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

See analysis details on SonarQube
@elasticmachine
Copy link

elasticmachine commented Jun 19, 2025

💚 Build Succeeded

History

cc @kcreddy
andrewkroh
andrewkroh approved these changes Jun 20, 2025
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
@kcreddy kcreddy merged commit b4bd0e2 into elastic:main Jun 20, 2025
7 checks passed
@elastic-vault-github-plugin-prod
Copy link

elastic-vault-github-plugin-prod bot commented Jun 20, 2025

Package aws - 3.8.2 containing this change is available at https://epr.elastic.co/package/aws/3.8.2/
shmsr pushed a commit to shmsr/integrations that referenced this pull request Jun 30, 2025
@kcreddy @shmsr
aws.securityhub_findings*: Fix null reference when extracting host.ip (…
494f8a0 
…elastic#14264)

There is no null check on resources[].Details.AwsEc2Instance.IpV4Addresses 
or resources[].Details.AwsEc2Instance.IpV6Addresses when extracting host.ip 
leading to pipeline_error.
This PR adds the required null check to prevent pipeline_error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Pull request that fixes a bug issue Integration:aws AWS Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations]

3 participants

@kcreddy @elasticmachine @andrewkroh