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

Make search for vulnerabilities faster #955

Merged
merged 10 commits into from
Oct 20, 2022
2 changes: 2 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ dev =
ipython==8.0.1
# used for testing
commoncode
# debug
django-debug-toolbar

[options.entry_points]
console_scripts =
Expand Down
5 changes: 4 additions & 1 deletion vulnerabilities/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,10 @@ def bulk_search(self, request):

@action(detail=False, methods=["get"])
def all(self, request):
vulnerable_packages = Package.objects.vulnerable().only(*PackageURL._fields)
"""
Return all the vulnerable Package URLs.
"""
vulnerable_packages = Package.objects.vulnerable().only(*PackageURL._fields).distinct()
vulnerable_purls = [str(package.purl) for package in vulnerable_packages]
return Response(vulnerable_purls)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 4.0.7 on 2022-10-19 16:18

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('vulnerabilities', '0027_alter_vulnerabilityreference_url'),
]

operations = [
migrations.AlterField(
model_name='packagerelatedvulnerability',
name='fix',
field=models.BooleanField(db_index=True, default=False, help_text='Does this relation fix the specified vulnerability ?'),
),
]
23 changes: 19 additions & 4 deletions vulnerabilities/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
from django.core.validators import MaxValueValidator
from django.core.validators import MinValueValidator
from django.db import models
from django.db.models import Count
from django.db.models import Q
from django.db.models.functions import Length
from django.db.models.functions import Trim
from django.dispatch import receiver
Expand Down Expand Up @@ -86,8 +88,7 @@ def __str__(self):

@property
def severities(self):
for reference in self.references.all():
yield from VulnerabilitySeverity.objects.filter(reference=reference.id)
return VulnerabilitySeverity.objects.filter(reference__in=self.references.all())

@property
def vulnerable_to(self):
Expand Down Expand Up @@ -202,7 +203,19 @@ def vulnerable(self):
"""
Return all vulnerable packages.
"""
return Package.objects.filter(packagerelatedvulnerability__fix=False).distinct()
return self.filter(packagerelatedvulnerability__fix=False)

def with_vulnerability_counts(self):
return self.annotate(
vulnerability_count=Count(
"vulnerabilities",
filter=Q(packagerelatedvulnerability__fix=False),
),
patched_vulnerability_count=Count(
"vulnerabilities",
filter=Q(packagerelatedvulnerability__fix=True),
),
)


class Package(PackageURLMixin):
Expand Down Expand Up @@ -310,7 +323,9 @@ class PackageRelatedVulnerability(models.Model):
)

fix = models.BooleanField(
default=False, help_text="Does this relation fix the specified vulnerability ?"
default=False,
db_index=True,
help_text="Does this relation fix the specified vulnerability ?",
)

class Meta:
Expand Down
50 changes: 25 additions & 25 deletions vulnerabilities/templates/vulnerability_details.html
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,21 @@
<li data-tab="fixed-by">
<a>
<span>
Fixed by packages ({{ vulnerability.resolved_to|length }})
Fixed by packages ({{ resolved_to|length }})
</span>
</a>
</li>
<li data-tab="affected-packages">
<a>
<span>
Affected packages ({{ vulnerability.vulnerable_to|length }})
Affected packages ({{ vulnerable_to|length }})
</span>
</a>
</li>
<li data-tab="references">
<a>
<span>
References ({{ vulnerability.references.all|length }})
References ({{ references|length }})
</span>
</a>
</li>
Expand All @@ -69,7 +69,7 @@
<tr>
<td class="two-col-left">Aliases</td>
<td class="two-col-right">
{% for alias in vulnerability.aliases.all %}
{% for alias in aliases %}
{% if alias.url %}
<a href={{ alias.url }} target="_blank">{{ alias }}<i class="fa fa-external-link fa_link_custom"></i></a>
{% else %}
Expand Down Expand Up @@ -121,11 +121,11 @@


<div class="has-text-weight-bold tab-nested-div ml-1 mb-1 mt-6">
Fixed by packages ({{ vulnerability.resolved_to.all|length }})
Fixed by packages ({{ resolved_to|length }})
</div>
<div class="tab-nested-div">
<table class="table is-bordered is-striped is-narrow is-hoverable is-fullwidth gray-header-border">
{% for package in vulnerability.resolved_to.all|slice:":3" %}
{% for package in resolved_to|slice:":3" %}
<tr>
<td>
<a href="{{ package.get_absolute_url }}" target="_self">{{ package.purl }}</a>
Expand All @@ -139,7 +139,7 @@
</td>
</tr>
{% endfor %}
{% if vulnerability.resolved_to.all|length > 3 %}
{% if resolved_to|length > 3 %}
<tr>
<td>
... see <i>Fixed by packages</i> tab for more
Expand All @@ -150,11 +150,11 @@
</div>

<div class="has-text-weight-bold tab-nested-div ml-1 mb-1 mt-6">
Affected packages ({{ vulnerability.vulnerable_to.all|length }})
Affected packages ({{ vulnerable_to|length }})
</div>
<div class="tab-nested-div">
<table class="table is-bordered is-striped is-narrow is-hoverable is-fullwidth gray-header-border">
{% for package in vulnerability.vulnerable_to.all|slice:":3" %}
{% for package in vulnerable_to|slice:":3" %}
<tr>
<td>
<a href="{{ package.get_absolute_url }}" target="_self">{{ package.purl }}</a>
Expand All @@ -168,7 +168,7 @@
</td>
</tr>
{% endfor %}
{% if vulnerability.vulnerable_to.all|length > 3 %}
{% if vulnerable_to|length > 3 %}
<tr>
<td>
... see <i>Affected packages</i> tab for more
Expand All @@ -187,7 +187,7 @@
<th> URL </th>
</tr>
</thead>
{% for ref in vulnerability.references.all %}
{% for ref in references %}
<tr>
{% if ref.reference_id %}
<td class="wrap-strings">{{ ref.reference_id }}</td>
Expand All @@ -210,23 +210,23 @@
<table class="table is-bordered is-striped is-narrow is-hoverable is-fullwidth">
<thead>
<tr>
<th><span class="has-tooltip-multiline has-tooltip-black has-tooltip-arrow has-tooltip-text-left" data-tooltip="The package url or purl is a URL string used to identify and locate a software package.">Package URL</span></th>
<th style="width: 225px;"><span class="has-tooltip-multiline has-tooltip-black has-tooltip-arrow has-tooltip-text-left" data-tooltip="This is the number of vulnerabilities that affect the package.">Affected by vulnerabilities</span></th>
<th style="width: 225px;"><span class="has-tooltip-multiline has-tooltip-black has-tooltip-arrow has-tooltip-text-left" data-tooltip="This is the number of vulnerabilities fixed by the package.">Fixing vulnerabilities</span></th>
<th><span
class="has-tooltip-multiline has-tooltip-black has-tooltip-arrow has-tooltip-text-left"
data-tooltip="The package url or purl is a URL string used to identify and locate a software package.">
Package URL</span>
</th>
</tr>
</thead>
<tbody>
{% for package in vulnerability.vulnerable_to.all %}
{% for package in vulnerable_to %}
<tr>
<td>
<a href="{{ package.get_absolute_url }}?search={{ package.purl }}" target="_self">{{ package.purl }}</a>
</td>
<td>{{ package.vulnerable_to|length }}</td>
<td>{{ package.resolved_to|length }}</td>
</tr>
{% empty %}
<tr>
<td colspan="3">
<td>
This vulnerability is not known to affect any packages.
</td>
</tr>
Expand All @@ -239,23 +239,23 @@
<table class="table is-bordered is-striped is-narrow is-hoverable is-fullwidth">
<thead>
<tr>
<th><span class="has-tooltip-multiline has-tooltip-black has-tooltip-arrow has-tooltip-text-left" data-tooltip="The package url or purl is a URL string used to identify and locate a software package.">Package URL</span></th>
<th style="width: 225px;"><span class="has-tooltip-multiline has-tooltip-black has-tooltip-arrow has-tooltip-text-left" data-tooltip="This is the number of vulnerabilities that affect the package.">Affected by vulnerabilities</span></th>
<th style="width: 225px;"><span class="has-tooltip-multiline has-tooltip-black has-tooltip-arrow has-tooltip-text-left" data-tooltip="This is the number of vulnerabilities fixed by the package.">Fixing vulnerabilities</span></th>
<th><span
class="has-tooltip-multiline has-tooltip-black has-tooltip-arrow has-tooltip-text-left"
data-tooltip="The package url or purl is a URL string used to identify and locate a software package.">
Package URL</span>
</th>
</tr>
</thead>
<tbody>
{% for package in vulnerability.resolved_to.all %}
{% for package in resolved_to %}
<tr>
<td>
<a href="{{ package.get_absolute_url }}?search={{ package.purl }}" target="_self">{{ package.purl }}</a>
</td>
<td>{{ package.vulnerable_to|length }}</td>
<td>{{ package.resolved_to|length }}</td>
</tr>
{% empty %}
<tr>
<td colspan="3">
<td>
This vulnerability is not known to be fixed by any packages.
</td>
</tr>
Expand Down
5 changes: 3 additions & 2 deletions vulnerabilities/tests/test_fix_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ def setUp(self):

def test_get_vulnerable_packages(self):
vuln_packages = Package.objects.vulnerable()
assert vuln_packages.count() == 10
vuln_purls = [pkg.purl for pkg in vuln_packages.only(*PackageURL._fields)]
assert vuln_packages.count() == 20
assert vuln_packages.distinct().count() == 10
vuln_purls = [pkg.purl for pkg in vuln_packages.distinct().only(*PackageURL._fields)]
assert vuln_purls == [
"pkg:generic/nginx/test@0",
"pkg:generic/nginx/test@1",
Expand Down
60 changes: 39 additions & 21 deletions vulnerabilities/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,27 +126,34 @@ def get_context_data(self, **kwargs):
def get_queryset(self, query=None):
query = query or self.request.GET.get("search") or ""
qs = self.model.objects
query = query.strip()
if not query:
return qs.none()
qs = (
qs.filter(
Q(vulnerability_id__icontains=query)
| Q(aliases__alias__icontains=query)
| Q(references__id__icontains=query)
| Q(summary__icontains=query)
)
.order_by("vulnerability_id")
.annotate(
vulnerable_package_count=Count(
"packages", filter=Q(packagerelatedvulnerability__fix=False), distinct=True
),
patched_package_count=Count(
"packages", filter=Q(packagerelatedvulnerability__fix=True), distinct=True
),
)
.prefetch_related()

# middle ground, exact on vulnerability_id
qssearch = qs.filter(vulnerability_id=query)
if not qssearch.exists():
# middle ground, exact on alias
qssearch = qs.filter(aliases__alias=query)
if not qssearch.exists():
# middle ground, slow enough
qssearch = qs.filter(
Q(vulnerability_id__icontains=query) | Q(aliases__alias__icontains=query)
)
if not qssearch.exists():
# last resort super slow
qssearch = qs.filter(
Q(references__id__icontains=query) | Q(summary__icontains=query)
)

return qssearch.order_by("vulnerability_id").annotate(
vulnerable_package_count=Count(
"packages", filter=Q(packagerelatedvulnerability__fix=False), distinct=True
),
patched_package_count=Count(
"packages", filter=Q(packagerelatedvulnerability__fix=True), distinct=True
),
)
return qs


class PackageDetails(DetailView):
Expand Down Expand Up @@ -190,11 +197,22 @@ class VulnerabilityDetails(DetailView):
slug_url_kwarg = "vulnerability_id"
slug_field = "vulnerability_id"

def get_queryset(self):
return super().get_queryset().prefetch_related("references", "aliases")

def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
context["vulnerability"] = self.object
context["vulnerability_search_form"] = VulnerabilitySearchForm(self.request.GET)
context["severities"] = list(self.object.severities)
context.update(
{
"vulnerability": self.object,
"vulnerability_search_form": VulnerabilitySearchForm(self.request.GET),
"severities": list(self.object.severities),
"references": self.object.references.all(),
"aliases": self.object.aliases.all(),
"resolved_to": self.object.resolved_to.all(),
"vulnerable_to": self.object.vulnerable_to.all(),
}
)
return context


Expand Down
31 changes: 31 additions & 0 deletions vulnerablecode/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@
# SECURITY WARNING: don't run with debug turned on in production
DEBUG = env.bool("VULNERABLECODE_DEBUG", default=False)

# SECURITY WARNING: don't run with debug turned on in production
DEBUG_TOOLBAR = env.bool("VULNERABLECODE_DEBUG_TOOLBAR", default=False)

# SECURITY WARNING: don't run with debug turned on in production
DEBUG_UI = env.bool("VULNERABLECODE_DEBUG_UI", default=False)

Expand All @@ -57,6 +60,7 @@
"widget_tweaks",
)


MIDDLEWARE = (
"django.middleware.security.SecurityMiddleware",
"django.contrib.sessions.middleware.SessionMiddleware",
Expand Down Expand Up @@ -187,3 +191,30 @@

if not VULNERABLECODEIO_REQUIRE_AUTHENTICATION:
REST_FRAMEWORK["DEFAULT_PERMISSION_CLASSES"] = ("rest_framework.permissions.AllowAny",)


if DEBUG_TOOLBAR:
INSTALLED_APPS += ("debug_toolbar",)

MIDDLEWARE += ("debug_toolbar.middleware.DebugToolbarMiddleware",)

DEBUG_TOOLBAR_PANELS = (
"debug_toolbar.panels.history.HistoryPanel",
"debug_toolbar.panels.versions.VersionsPanel",
"debug_toolbar.panels.timer.TimerPanel",
"debug_toolbar.panels.settings.SettingsPanel",
"debug_toolbar.panels.headers.HeadersPanel",
"debug_toolbar.panels.request.RequestPanel",
"debug_toolbar.panels.sql.SQLPanel",
"debug_toolbar.panels.staticfiles.StaticFilesPanel",
"debug_toolbar.panels.templates.TemplatesPanel",
"debug_toolbar.panels.cache.CachePanel",
"debug_toolbar.panels.signals.SignalsPanel",
"debug_toolbar.panels.logging.LoggingPanel",
"debug_toolbar.panels.redirects.RedirectsPanel",
"debug_toolbar.panels.profiling.ProfilingPanel",
)

INTERNAL_IPS = [
"127.0.0.1",
]
6 changes: 6 additions & 0 deletions vulnerablecode/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from vulnerabilities.views import VulnerabilityDetails
from vulnerabilities.views import VulnerabilitySearch
from vulnerabilities.views import schema_view
from vulnerablecode.settings import DEBUG_TOOLBAR


# See the comment at https://stackoverflow.com/a/46163870.
Expand Down Expand Up @@ -54,3 +55,8 @@ def __init__(self, *args, **kwargs):
# disabled for now
# path("admin/", admin.site.urls),
]

if DEBUG_TOOLBAR:
urlpatterns += [
path("__debug__/", include("debug_toolbar.urls")),
]