Skip to content

Conversation

@wesleya
Copy link
Contributor

@wesleya wesleya commented Dec 17, 2024

Initial integration for DomainTools Newly Observed Domains feed.

Please explain:

  • WHAT: Initial datastream, ingest pipeline, fields for DomainTools Newly Observed Domains feed.
  • WHY: First DomainTools feed integration

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

Author's Checklist

  • Newly Observed Domains feed

How to test this PR locally

Install and configure DomainTools integration. Search for feed results: data_stream.dataset: "domaintools.nod_feed"

@cla-checker-service
Copy link

cla-checker-service bot commented Dec 17, 2024

💚 CLA has been signed

@andrewkroh andrewkroh added needs CLA User must sign the Elastic Contributor License before review. New Integration Issue or pull request for creating a new integration package. labels Dec 17, 2024
@wesleya
Copy link
Contributor Author

wesleya commented Dec 17, 2024

Hi @andrewkroh I went ahead and signed the CLA, thank you!

@andrewkroh andrewkroh removed the needs CLA User must sign the Elastic Contributor License before review. label Dec 17, 2024
@wesleya
Copy link
Contributor Author

wesleya commented Dec 19, 2024

Hi @andrewkroh apologies if I'm missing something obvious here, first time going through this process. Was there anything else you needed from me to get this PR approved and merged? Thank you for the guidance!

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.

I took quick (non-thorough) look and left a few comments. I'm asking to find the right internal team to help review this.

The integration kind of sounds like it should be treated as a threat intel integration with naming to match (e.g. ti_domaintools). Is that the general use case for this data?

This will need system and pipeline testing.

Copy link
Member

Choose a reason for hiding this comment

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

What does this timestamp represent? It is the time when the domain was registered? Or first observed through passive DNS means?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

documentation has been updated. You are correct, first observed through passive DNS.

Copy link
Member

Choose a reason for hiding this comment

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

Are these going to be eTLDs only or any domain observed anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

documentation has been updated. no eTLDS. Apex-level domains (e.g. example.com but not www.example.com) that we observe for the first time, and have not observed previously with our global DNS sensor network.

Copy link
Member

Choose a reason for hiding this comment

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

This field table and the sample event should be generated from a template that is placed in the _build/ dir (see other integrations as an example).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apologies, I wasn't able to figure this one out. I’m not sure what how to generate the sample event and field table here? I don’t see any template in integrations/build directory. Am I in the wrong place?

I used the ti_recordedfuture README as an example. And just replaced the fields we export by hand. Please let me know what I'm missing here.

@wesleya
Copy link
Contributor Author

wesleya commented Dec 20, 2024

Hi @andrewkroh,

Thank you for the review! Your comments make perfect sense. We've just started our company holiday, so we'll be able to address the feedback and submit an update early next year.

Happy holidays to you as well!

Copy link
Contributor

@kcreddy kcreddy left a comment

Choose a reason for hiding this comment

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

All of our existing threat intel integrations have a prefix ti_.
I would suggest you rename the integration to ti_domaintools.

For threat intel indicators, we also want to support an expiration.
You can take a look at other ti_* integrations how it is done. Example.

Essentially you need to setup a latest transform and expire the indicators based on a time period or based on a data point.

@kcreddy
Copy link
Contributor

kcreddy commented Dec 31, 2024

/test

@kcreddy kcreddy added the Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations] label Dec 31, 2024
@elasticmachine
Copy link

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

@wesleya
Copy link
Contributor Author

wesleya commented Jan 21, 2025

@kcreddy I've renamed the integration to ti_domaintools. I've also taken a first pass at adding a latest transform to support expiration. It seems to be working as expected. I've also addressed the feedback from @andrewkroh.

Can you please take another look and let me know if we're missing anything. Appreciate your patience and support as first-timers working with Elastic, thank you!

@wesleya
Copy link
Contributor Author

wesleya commented Jan 28, 2025

@kcreddy @andrewkroh just checking in on this. Anything else needed on our side right now?

As always, appreciate the patience as elastic first-timers, thank you!

@kcreddy
Copy link
Contributor

kcreddy commented Jan 30, 2025

/test

Copy link
Contributor

@kcreddy kcreddy left a comment

Choose a reason for hiding this comment

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

Hey @wesleya, sorry for the delay.

The CI is currently failing with error:

Error: there is no owner for "packages/ti_domaintools" in ".github/CODEOWNERS"

Could you please add an entry here for this integration?

Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest matching this max_age value with default interval.
So, the latest domain indicators are populated into latest indices.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussing with our product lead, we wanted to know if it's possible to have default interval of 10m for pulling in the feed, but expire the indicators after 7d? Reasoning is to keep the feed close to real time, but leave folks time to see them? Or let us know if we're misunderstanding max_age here. Thank you!

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it. So, with current configuration:
interval: 10m
max_age: 7d

This would mean when these domain indicators are deleted inside your platform (or expired since no longer valid), and when the API is called next time, this indicator will not be fetched from there on. But since max_age is set to 7d, this indicator will still be present in your feed in Elastic and only deleted after 7d, does this seem okay?

@wesleya
Copy link
Contributor Author

wesleya commented Jan 31, 2025

@kcreddy I believe we've addressed your feedback. Please let me know if there are more changes to make, or if my requests around max_age, and date above don't make sense. Thank you!

@kcreddy
Copy link
Contributor

kcreddy commented Feb 3, 2025

/test

@kcreddy
Copy link
Contributor

kcreddy commented Feb 3, 2025

@wesleya, thanks for resolving comments. I will take a look.

Could you please add an entry into .github/CODEOWNERS

The manifest.yml owner.github value should match with the one added in CODEOWNERS file. Could you please update manifest.yml?

owner:
  github: elastic/security-service-integrations

@wesleya
Copy link
Contributor Author

wesleya commented Feb 5, 2025

@kcreddy thank you for the guidance, I believe I've updated the branch and things are looking as expected, testing looks good as well.

You might see 2 or 3 additional force push attempts previous to my final attempt. You can ignore those, it just took me a few tries to work things out correctly. Let me know if I'm missing anything, thank you!

@kcreddy
Copy link
Contributor

kcreddy commented Feb 5, 2025

/test

@kcreddy kcreddy removed request for a team, VihasMakwana and belimawr February 5, 2025 19:19
@kcreddy kcreddy added Partner and removed Integration:1password 1Password (Partner supported) Integration:abnormal_security Abnormal AI labels Feb 5, 2025
@elasticmachine
Copy link

elasticmachine commented Feb 5, 2025

💔 Build Failed

Failed CI Steps

History

  • 💔 Build #21441 failed 8dfaf8258b2ea6a889d1d9c385f8786cdfa18897
  • 💔 Build #21258 failed 7a00771f33182f5a2cd41ecce77f998b708af991
  • 💔 Build #19933 failed cbb8b5dcbab457b0b4d0d2536cfb36af734f94df

@kcreddy kcreddy closed this Feb 5, 2025
@kcreddy kcreddy reopened this Feb 5, 2025
@kcreddy
Copy link
Contributor

kcreddy commented Feb 5, 2025

There's a CI error unrelated to this package. #12630 should fix the CI error. Once that is merged, CI needs to be run again.
Once the CI passes, I can approve and merge this PR.

@wesleya
Copy link
Contributor Author

wesleya commented Feb 5, 2025

@kcreddy I saw another force-push come in causing more conflicts on this PR. Should I repeat the resolution steps again? Or should I wait on #12630 or other changes to come in first?

@kcreddy
Copy link
Contributor

kcreddy commented Feb 6, 2025

Should I repeat the resolution steps again? Or should I wait on #12630 or other changes to come in first?

I will let you know as I get some updates. Right now merging is blocked. Apologies for the inconvenience.

@wesleya
Copy link
Contributor Author

wesleya commented Feb 6, 2025

I just received this email from [email protected]:

We are working with GitHub support to clean up https://github.com/DomainTools/integrations/tree/domaintools. The GitHub engineering team has identified that you have an active branch against the fork of elastic/integrations. This prevents GitHub from taking action on the files and their references.

We kindly request that you delete the branch, delete and re-fork the repo, and please confirm when complete.

I will re-create this PR on a brand new branch on the re-forked repo. Thank you

@wesleya wesleya closed this Feb 6, 2025
@wesleya wesleya deleted the domaintools branch February 6, 2025 23:06
@wesleya wesleya mentioned this pull request Feb 7, 2025
6 tasks
@elastic-vault-github-plugin-prod

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

New Integration Issue or pull request for creating a new integration package. Partner Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations]

Projects

None yet

Development

Successfully merging this pull request may close these issues.