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.

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

Reports should depend on object types instead of plugins #2806

Open
dekkers opened this issue Apr 9, 2024 · 6 comments
Open

Reports should depend on object types instead of plugins #2806

dekkers opened this issue Apr 9, 2024 · 6 comments
Assignees

Comments

@dekkers
Copy link
Contributor

dekkers commented Apr 9, 2024

Currently reports can have a list of required and optional plugins. The optional plugins are presented as "suggested plugins" in the user interface. The problem with this is that reports don't depend on certain plugins, but on existence certain object types. Some object types are implemented by multiple plugins and might also be implemented by a plugins that are created by the user or third parties.

For example the open ports plugin depends on that IPPort objects being created when ports are open. It doesn't depend on any single specific plugin to do that and one plugin that does port scanning is enough. At the moment the open ports reports requires the nmap and has shodan, nmap-udp, nmap-ports, nmap-ip-range and masscan as optional plugins that are listed as suggested plugins. But that is not correct, because it just requires one of them and suggesting that all of them should be enabled also doesn't make sense.

What a report does is query certain object types. It would be more logical if a report would specify on which object types it depends and that we then figure out based on the consumes/produces which plugins fulfill these requirements.

@dekkers dekkers added this to KAT Apr 9, 2024
@github-project-automation github-project-automation bot moved this to Incoming features / Need assessment in KAT Apr 9, 2024
@dekkers dekkers moved this from Incoming features / Need assessment to To be discussed in KAT Apr 9, 2024
@dekkers dekkers changed the title Reports should depend on object types, not directly on plugins Reports should depend on object types instead of plugins Apr 9, 2024
@noamblitz
Copy link
Contributor

Interesting idea and I fully agree that having one plugin required and the others optional does not make sense. However, checking on the existence also has its flaws. For example, when an IPAddress does not have open ports, is that because there were no jobs, or is it because there are no open ports?

Then we could consider checking for completed jobs, but the same problem arises, is an nmap job required or a shodan job?

@dekkers
Copy link
Contributor Author

dekkers commented Apr 9, 2024

Interesting idea and I fully agree that having one plugin required and the others optional does not make sense. However, checking on the existence also has its flaws. For example, when an IPAddress does not have open ports, is that because there were no jobs, or is it because there are no open ports?

Then we could consider checking for completed jobs, but the same problem arises, is an nmap job required or a shodan job?

We should not check for the existence of objects or completed jobs, but that we check whether a combination of boefje and normalizer is enabled that produces the object type that the report depends on.

We should probably also check whether jobs for the enabled boefjes/normalizer have run, but I think that is a different issue/discussion.

@RomijnHumanoids
Copy link

Two things come to mind in terms of UX, I don't know if these fix the problem but it could make it more clear to the user.

  • If only one of a list of required plugins is enough to create input for the reports, we can design a new way to communicate something like: "choose at least one of the following plugins to meet the requirements for this report"
  • I'm not sure whether 'optional' and 'suggested' mean the same thing in this context, but in the example above it sounds more logical to use the term 'optional', as 'suggested' sounds like it would add more value to the report.

@zcrt
Copy link
Contributor

zcrt commented Apr 23, 2024

We should not check for the existence of objects or completed jobs, but that we check whether a combination of boefje and normalizer is enabled that produces the object type that the report depends on.

This introduces a (confidence) problem when for example Nmap and Shodan disagree about the open ports. Not every boefje producing object X is guaranteed to have the same objects produced by another boefje also producing object X

@dekkers dekkers self-assigned this Apr 30, 2024
@dekkers dekkers moved this from To be discussed to In Progress in KAT May 7, 2024
@dekkers
Copy link
Contributor Author

dekkers commented May 21, 2024

Overview of the current reports, input ooi types, required and optional plugins

Report input OOI types required plugins optional plugins
DNS Hostname dns-records, dns-sec dns-zone
Findings all OOI types
IPv6 Hostname, IPAddressV4, IPAddressV6 dns-records
Mail Hostname, IPAddressV4, IPAddressV6 dns-records
Multi Organization ReportData
Name server Hostname, IPAddressV4, IPAddressV6 nmap, dns-records, dns-sec
Open ports Hostname, IPAddressV4, IPAddressV6 nmap shodan, nmap-udp, nmap-ports, nmap-ip-range, masscan
RPKI Hostname, IPAddressV4, IPAddressV6 dns-records, rpki
Safe connections Hostname, IPAddressV4, IPAddressV6 dns-records, testssl-sh-ciphers, nmap
Systems Hostname, IPAddressV4, IPAddressV6 dns-records, nmap nmap-udp
TLS IPService testssl-sh-ciphers
Vulnerability Hostname, IPAddressV4, IPAddressV6 dns-records, nmap, webpage-analysis nmap-udp, nmap-ports, shodan
Web Hostname, IPAddressV4, IPAddressV6 nmap, dns-records, security_txt_downloader, testssl-sh-ciphers, ssl-version, ssl-certificates,webpage-analysis

@TwistMeister TwistMeister moved this from In Progress to To be discussed in KAT May 28, 2024
@madelondohmen madelondohmen moved this from To be discussed to In Progress in KAT May 28, 2024
@madelondohmen
Copy link
Contributor

Outcome of discussion meeting:
Tickets have to be created to implement this issue. @dekkers will do this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

No branches or pull requests

5 participants