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

Support global vulnerability analysis policies #930

Open
nscuro opened this issue Nov 23, 2023 · 9 comments
Open

Support global vulnerability analysis policies #930

nscuro opened this issue Nov 23, 2023 · 9 comments
Labels
architecture domain/vuln-policy enhancement New feature or request p2 Non-critical bugs, and features that help organizations to identify and reduce risk size/XL Higher effort

Comments

@nscuro
Copy link
Member

nscuro commented Nov 23, 2023

Context

Since v4.x, Dependency-Track no longer supports global auditing of vulnerabilities.

This was done because analyses are not generally applicable to all projects depending on a given component. Context matters, and decisions like "not affected" can only ever be made on the project level.

It is thus also not possible to suppress false positives for all projects in a portfolio anymore. However, the community still has a need for it: DependencyTrack/dependency-track#1495

Automating analysis works, utilizing both Webhook notifications and Dependency-Track's REST API. An example implementation of such automation is available with dtapac.

What's problematic about this, is that at the point at which Webhooks are sent, it's already too late:

  • Users (and downstream systems) are already notified about a new vulnerability having popped up
  • CI systems already pulled findings via REST API, potentially failing a quality gate

Projects like OWASP Dependency-Check support suppression files. Typically based on regular expressions for component identifiers, and vulnerability IDs.

Dependency-Track supports more than just suppression though.

Organizations may wish to:

  • Suppress findings across their entire portfolio (false positives)
  • Apply a NOT_AFFECTED analysis to all versions (present and future) of a given project
  • Apply an EXPLOITABLE analysis if another component is present in the project
  • Apply an analysis to a specific version range of a component
  • Apply an analysis to a subset of projects with a given tag
  • Have an expiration date for such analyses

How does this differ from VEX / VDR?

While VEX and VDR can be used to communicate analysis of a given finding (which may lead to suppression), the analysis in both cases is static and can not be bound to additional constraints.

Why not use the existing policy feature?

Policy evaluation happens "too late" in the workflow. When the evaluation is executed, findings have already been created, and NEW_VULNERABILITY notification have already been sent. This is sub-optimal, as users are notified about findings that ultimately may be suppressed anyway. For CI use cases, build systems could fetch the list of findings before policy evaluation completed.

Existing policies are further scoped components. Their evaluation context includes:

  • 1 Component
  • 1 Project
  • 0-N Vulnerabilities

Thus, if a policy matches, it is not possible to tell for which of the 0-N vulnerabilities it matched.

Does this cover false negatives?

No. The proposed functionality exclusively applies to vulnerabilities that are identified. Dependency-Track's internal vulnerability database can be used to address false negatives.

Proposed solution

Introduce a new type of policy, entirely dedicated to applying analyses in an automated manner. For lack of better terms, this new policy type will be called "Vulnerability Policy" for now.

There will be a new tab in the UI for them. Existing policies will (likely) be renamed to "Component Policies".

image

Policy structure

Vulnerability policies consist of the following:

Field Description Required
name Name of the policy
description Short description of the policy
author Name of the policy author
validFrom When the policy should take effect (in RFC3339 format)
validUntil Until when the policy should take effect (in RFC3339 format)
conditions 1..N conditions as CEL expressions
analysis The analysis to apply when all expressions match
ratings 0..3 ratings to apply to the finding when all expressions match
Conditions

One or more conditions MUST be provided for each policy. Conditions MUST be valid CEL expressions, and MAY access everything that is already available in component policies.

When multiple conditions are provided, all of them have to match (conjunctive).

Analysis

A policy MUST define an analysis.

The analysis encompasses everything that can currently be set manually via UI or REST API:

Field Description Required
state State of the analysis (e.g. EXPLOITABLE, NOT_AFFECTED)
justification Justification for the state (e.g. CODE_NOT_REACHABLE)
vendor response What to communicate to consumers for this finding (from a vendor POV)
details Additional details about why this analysis was made
suppress Whether to suppress the finding

This is also in line with fields available in CycloneDX VDR and VEX.

Note

Analyses applied via policy will populate the audit trail, just like a manual application would.

Ratings

A policy may define up to three ratings that should be applied to matching findings, effectively overriding the ratings provided by vulnerability databases. This may be done to enrich CVSS vectors with temporal and environmental metrics, which can lead to severity reduction.

Vulnerabilities in Dependency-Track can currently have the following severity / risk ratings:

  • CVSSv2
  • CVSSv3 (includes CVSSv3.1)
  • OWASP Risk Rating

Thus, policies can define overrides for all of them. However, providing multiple ratings with identical method (e.g. two CVSSv2 ratings) is ambiguous and will not be supported.

Field Description Required
method Methodology of the rating (e.g. CVSSv2, OWASP)
severity Severity (LOW, MEDIUM, etc.)
vector Vector of the method
score Numeric score for the method

This is in line with fields available in CycloneDX VDR and VEX.

Example

Example policy (represented as YAML for brevity):

name: Example
description: Foo bar
author: Jane Doe
validUntil: 2024-01-01T00:00:00Z
conditions:
- vuln.id == "CVE-123" || vuln.aliases.exists(alias, alias.id == "CVE-123")
- |-
 component.name == "foo" 
   && project.name == "bar"
   && "internal" in project.tags
   && !component.is_dependency_of(v1.Component{group: "org.springframework.boot"})
analysis:
  state: NOT_AFFECTED
  justification: CODE_NOT_REACHABLE
  details: Because foo bar baz
  suppress: true
ratings:
- method: CVSSv3
  vector: CVSS:3.0/AV:N/AC:L/PR:L/UI:N/S:U/C:L/I:L/A:L
  severity: medium
  score: 6.3

Evaluation context

The context of condition evaluation will contain the following variables:

  • vuln: The reported vulnerability
  • component: The affected component
  • project: The project the component is part of
  • (Potentially) All other vulnerabilities reported for the component
    • This would allow suppression of duplicates based on aliases, e.g.
      vuln.aliases.exists(alias, alias,id == "CVE-123") && allVulns.exists(v, v.id == "CVE-123")

Policy integration

The following describes a potential workflow. Key components:

  • VulnScanResultProcessor: The Kafka consumer that processes ScanResults
  • AnalysisPolicyProvider: An abstraction for managing (creating, updating, retrieving) policies
    • Providers may have various implementations, ranging from hardcoded policies, policies stored in Git / S3, etc.
    • Providers may optionally perform additional filtering to decide which policy is applicable to which component/project
    • We can potentially allow for custom Plugins by offering an SPI
  • AnalysisPolicyEvaluator: A service that evaluates policies, given the context of:
    • A vulnerability
    • The affected component
    • The project the component belongs to

This workflow is executed for both ad-hoc vulnerability analysis upon BOM upload, as well as the scheduled continuous analysis of existing projects.

sequenceDiagram
    Kafka->>+VulnScanResultProcessor: Vuln Scan Result
    VulnScanResultProcessor->>VulnScanResultProcessor: Parse
    loop for each vulnerability
        VulnScanResultProcessor->>Database: Sync vuln details
        VulnScanResultProcessor->>AnalysisPolicyEvaluator: Evaluate policies
        Note over VulnScanResultProcessor, AnalysisPolicyEvaluator: Context: Component, Project, Vulnerability
        AnalysisPolicyEvaluator->>AnalysisPolicyProvider: Get applicable policies
        Note over AnalysisPolicyEvaluator, AnalysisPolicyProvider: Provider could be SPI to plug in various<br/>backends (Git, S3, Database, ...)
        AnalysisPolicyProvider->>AnalysisPolicyEvaluator: Applicable policies
        AnalysisPolicyEvaluator->>AnalysisPolicyEvaluator: Evaluate policies
        AnalysisPolicyEvaluator->>VulnScanResultProcessor: state=NOT_AFFECTED<br/>justification=CODE_NOT_REACHABLE<br/>detail=Because foo bar baz<br/>suppress=true
        Note over AnalysisPolicyEvaluator, VulnScanResultProcessor: What if multiple policies match?<br/>Short-circuit, prioritization?
        critical atomically
            VulnScanResultProcessor->>Database: Create finding
            VulnScanResultProcessor->>Database: Apply analysis
        end 
        opt finding not suppressed
            VulnScanResultProcessor->>Kafka: Send NEW_VULNERABILITY<br/>notification 
        end
    end
Loading

Note

"Finding" refers to the association of a component with a vulnerability.

Change control

As analysis policies not only notify, but modify, there is a need to optionally have a change control mechanism in place. There are multiple options to achieve this.

Generally, we do not believe that it's worth having approval workflows baked into Dependency-Track. It just adds a lot of complexity without much benefit, as other systems are available that already enforce such workflows perfectly.

Note

This will be entirely optional. Users who prefer to use the UI for policy management will be able to continue to do so.

Policy management in Git, synchronization via REST API
  • Users do not have the permission to update policies manually
  • CI server has a Dependency-Track API key with policy management permission
  • Policies are managed in Git (e.g. as YAML files)
  • Repository is setup to enforce PRs, PR review, Linting
sequenceDiagram
    actor User A
    actor User B
    User A->>Git Server: Push policies
    User A->>Git Server: Create PR
    User B->>Git Server: Review / Approve
    Git Server->>Git Server: Perform merge to<br/>main branch
    Git Server->>CI Server: Webhook
    CI Server->>Git Server: Clone repository
    CI Server->>Dependency-Track: Reconcile policies<br/>(CUD)
Loading
  • ✅ Possible today, without modifications
  • ✅ Complete audit history for policy changes
  • ✅ Updates are immediately applied to Dependency-Track
  • ❌ Updates are not necessarily atomic
  • ❌ Bound to availability of Dependency-Track
Policy management in Git, synchronization directly
  • Users do not have the permission to update policies manually
  • CI server not involved
  • Policies are managed in Git (e.g. as YAML files)
  • Repository is setup to enforce PRs, PR review, Linting
sequenceDiagram
    actor User A
    actor User B
    User A->>Git Server: Push policies
    User A->>Git Server: Create PR
    User B->>Git Server: Review / Approve
    Git Server->>Git Server: Perform merge to<br/>main branch
    loop regularly
        Dependency-Track->>Git Server: Clone / pull
        Dependency-Track->>Dependency-Track: Reconcile policies<br/>(CUD)
    end
Loading
  • ✅ Complete audit history for policy changes
  • ✅ Updates can be atomic
  • ✅ Not bound to availability of DT
  • ❌ Updates can be delayed due to polling
  • ❌ Dependency-Track needs access to Git
    • Potentially requires additional keys
    • Problematic in environments with strict network segmentation
    • Some Git servers disallow cloning via HTTPS, DT would need SSH keys
Policy management in Git, synchronization via bundle

Similar to OPA's policy bundles, we can follow a similar approach to allow for atomic application of updates. Managing access to blob storage / HTTP file servers is a lot simpler than for Git.

  • Users do not have the permission to update policies manually
  • CI server packages policies into a bundle and uploads that to blob storage (e.g. S3) or file server (e.g. NGINX)
  • Policies are managed in Git (e.g. as YAML files)
  • Repository is setup to enforce PRs, PR review, Linting
sequenceDiagram
    actor User A
    actor User B
    User A->>Git Server: Push policies
    User A->>Git Server: Create PR
    User B->>Git Server: Review / Approve
    Git Server->>Git Server: Perform merge to<br/>main branch
    Git Server->>CI Server: Webhook
    CI Server->>Git Server: Clone repository
    CI Server->>CI Server: Create policy bundle<br/>(.tgz / .zip)
    CI Server->>Blob storage: Upload bundle
    loop regularly
        Dependency-Track->>Blob storage: Download bundle
        Dependency-Track->>Dependency-Track: Reconcile policies<br/>(CUD)
    end
Loading
  • ✅ Complete audit history for policy changes
  • ✅ Updates can be atomic
  • ✅ Not bound to availability of DT
  • ❌ Updates can be delayed due to polling

Note

In addition, it should be possible to upload bundles directly via REST API.

@nscuro nscuro added enhancement New feature or request p2 Non-critical bugs, and features that help organizations to identify and reduce risk architecture size/L High effort spike/research Requires more research before implementation labels Nov 23, 2023
@nscuro

This comment was marked as resolved.

@nscuro
Copy link
Member Author

nscuro commented Nov 27, 2023

High-level tasks for an initial MVP:

  • Define a JSON schema for vulnerability policies in YAML format #939
    • The schema will be used both in CI, and by DT during policy reconciliation
    • Should be as strict as possible, using formats (e.g. date-time) and regex validation wherever possible
    • Consider enforcing a mandatory apiVersion field, to make schema evolution easier
    • Consider enforcing a mandatory kind / type field that denotes the type of policy (VulnerabilityPolicy, ComponentPolicy, etc.)
    • Write unit tests that assert correct behavior of YAML deserialization and schema validation
  • Define a new database table schema for vulnerability policies #942
    • Consider storing analysis and ratings in JSONB columns (avoids having to add new tables and is still index- and queryable if needed)
    • Reminder: schema is managed via Liquibase now
  • Define and implement vulnerability policy reconciliation #938
    • Fetching of policy bundles from blob storage / file server
      • Schedule and endpoint must be configurable (via UI?)
    • Extraction of the bundle
    • Validation of all included policy files
    • Compilation / validation of condition CEL expressions
      • Nice-to-have: Check if expensive functions are being called first (e.g. dependency graph traversal), which could be too expensive. Can use visitor to validate this on the abstract syntax tree of the CEL script.
    • Synchronization with database (single DB transaction, this needs to be atomic)
    • Appropriate logging of errors, such that they can be monitored for and reacted to
  • Implement VulnerabilityPolicyEvaluator #937
    • Either as new class, or integrate into existing CelPolicyEngine
    • Can develop against a VulnerabilityPolicyProvider interface at first, which can easily be mocked
    • Same as CelPolicyEngine, it will potentially need to load additional data from DB based on condition expression requirements
    • Think about where caching may be used to avoid redundant expensive evaluation
  • Integrate VulnerabilityPolicyEvaluator in VulnerabilityScanResultProcessor #940
    • Add Micrometer Timer to track time taken for evaluation
    • Consider logging warnings when evaluation takes too long
      • Add a timeout to prevent evaluation from taking too long? We don't want policies to DOS the system. Is this practical?
    • Profile the integration, it's crucial that it performs well!
    • Should be disabled unless a feature flag (environment variable) is set
  • Extend Analysis model to additionally hold rating overrides #941
    • Overrides MUST reflect in metrics calculation!
    • When overrides are present, original ratings of the Vulnerability MUST be ignored

@valentijnscholten
Copy link
Contributor

I suggest next to entering raw CEL expressions (which can be complicated and makes the user think), to also have a "simple" way to suppress vulnerabilities for:

  • all existing versions
  • all existing and future versions
  • possible other common use cases (suprress for all projects maybe?)

May be this can be a button that puts a template into the expression field or something like that.

@nscuro
Copy link
Member Author

nscuro commented Nov 27, 2023

@valentijnscholten Good suggestion. The way we did it for the existing policy functionality, is that users can continue to use the pre-defined conditions (e.g. Coordinates), but in the background we re-write them to CEL and execute them that way.

Offering a set of condition templates as you say also makes sense to me.

@tomaszn
Copy link

tomaszn commented Nov 28, 2023

@nscuro This is fabulous.

One thing seems missing to me. Can the scope of the rule updates be limited per user? For example, I would like to limit users having access only to projects A and B to be only permitted to create policies that cover only these projects. This check would apply to changes both via UI, and also via API, so separate bot accounts could be used per team, to prevent one team from suppressing findings in other team's projects. This would be useful in the scenario of policy management in Git with synchronization via REST API, where each team or project could maintain their policies and update them e.g. in CI just before uploading the SBOM.

@skhokhlov
Copy link

I'm so excited about this feature. One thing I want to suggest is to have a policies archive push endpoint in api and ui. In this case policies can be properly controlled and DT won't need any integration with file storage systems. Also it allows to rollback policies to a previous version without any troubles. In case of polling an external endpoint for policies archive, the problem may be that artefact storage won't allow to override file with a new version. It means that for each new version the link will be different.

@nscuro
Copy link
Member Author

nscuro commented Dec 6, 2023

Thanks @skhokhlov, your input is much appreciated!

One thing I want to suggest is to have a policies archive push endpoint in api and ui. In this case policies can be properly controlled and DT won't need any integration with file storage systems.

Good point. I think DT should support both push and pull models. Ideally, it shouldn't matter to DT how bundles are supplied.

Also it allows to rollback policies to a previous version without any troubles.

Yeah I guess having to wait a few minutes for polling to kick in is not something you want when a faulty policy was deployed.

In case of polling an external endpoint for policies archive, the problem may be that artefact storage won't allow to override file with a new version. It means that for each new version the link will be different.

Hmmm true. Hadn't thought about this limitation.

@nscuro nscuro added domain/vuln-policy size/XL Higher effort and removed size/L High effort spike/research Requires more research before implementation labels Dec 11, 2023
@nscuro nscuro pinned this issue Dec 11, 2023
@rkg-mm
Copy link

rkg-mm commented Dec 19, 2023

This would address DependencyTrack/dependency-track#2183, and I totally need this yesterday 😆

One idea: In the existing vulnerability analysis view, add a button like "Apply to all project versions", clicking it opens the UI to create a corresponding policy, fields pre-filled and customizable if the user wants to. This would make the workflow much easier (teams usually work in the vulnerability view, do an analysis starting from there and add the result in this view. Forcing them to then switch to another view entering details that already were there in the view before would be a usability nightmare for this use case)

Also I have questions about the permissions this requires. Creating policies today requires specific permissions the teams usually don't have in my org (and won't have in future). However, creating policies affecting a specific project (can it include children?) could be done if the vulnerability analysis permission is there for the user and this project.

@nscuro
Copy link
Member Author

nscuro commented Dec 20, 2023

@rkg-mm One idea: In the existing vulnerability analysis view, add a button like "Apply to all project versions", clicking it opens the UI to create a corresponding policy, fields pre-filled and customizable if the user wants to.

I like this idea!

However, creating policies affecting a specific project (can it include children?) could be done if the vulnerability analysis permission is there for the user and this project.

I think this is an ACL check we should be able to do. At least off the top of my head I don't see anything that would prevent us from doing that. But, this and the idea you mentioned earlier necessitate some form of policy "ownership", which likely needs to be scoped to teams. So related to what @tomaszn mentioned.

When users create policies from the audit view, the policies are owned by the team(s) that user is a member of.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
architecture domain/vuln-policy enhancement New feature or request p2 Non-critical bugs, and features that help organizations to identify and reduce risk size/XL Higher effort
Projects
None yet
Development

No branches or pull requests

5 participants