| Message ID | 20200222085715.23769-13-heiko.thiery@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | pkg-stats json output improvements | expand |
Heiko, all, On 2/22/20 9:57 AM, Heiko Thiery wrote: > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com> > --- > support/scripts/pkg-stats | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats > index ed22f6b650..4efff8624f 100755 > --- a/support/scripts/pkg-stats > +++ b/support/scripts/pkg-stats > @@ -617,8 +617,14 @@ def check_package_cves(nvd_path, packages): > > for cve in CVE.read_nvd_dir(nvd_path): > for pkg_name in cve.pkg_names: > - if pkg_name in packages and cve.affects(packages[pkg_name]): > - packages[pkg_name].cves.append(cve.identifier) > + if pkg_name in packages: > + if cve.affects(packages[pkg_name]): > + packages[pkg_name].cves.append(cve.identifier) > + if len(packages[pkg_name].cves): > + packages[pkg_name].status['cve'] = ('error', 'affected by cve') > + else: > + packages[pkg_name].status['cve'] = ('ok', 'no cve found') > + > > > def calculate_stats(packages): > In the current state, that would give: * If a CVE applies to a br package -> error * If a CVE does not applies to a br package -> ok * If no CVE points to a br package -> na (no status check done) I would argue that the latest case is not correct. The status should be ok, because we ran through the whole list of CVEs from the NVD feed, and we did not find any of them that applies to this package. I would rather write it like this: ######################## diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats index ed22f6b650..91477d583e 100755 --- a/support/scripts/pkg-stats +++ b/support/scripts/pkg-stats @@ -620,6 +620,12 @@ def check_package_cves(nvd_path, packages): if pkg_name in packages and cve.affects(packages[pkg_name]): packages[pkg_name].cves.append(cve.identifier) + for pkg_name, pkg in packages.items(): + if len(pkg.cves) > 0: + pkg.status['cve'] = ('error', 'affected by CVE(s)') + else: + pkg.status['cve'] = ('ok', 'no CVE found') + def calculate_stats(packages): stats = defaultdict(int) ######################## Best regards, Titouan
Hi Titouan and all, Am So., 23. Feb. 2020 um 15:24 Uhr schrieb Titouan Christophe <titouan.christophe@railnova.eu>: > > Heiko, all, > > On 2/22/20 9:57 AM, Heiko Thiery wrote: > > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com> > > --- > > support/scripts/pkg-stats | 10 ++++++++-- > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats > > index ed22f6b650..4efff8624f 100755 > > --- a/support/scripts/pkg-stats > > +++ b/support/scripts/pkg-stats > > @@ -617,8 +617,14 @@ def check_package_cves(nvd_path, packages): > > > > for cve in CVE.read_nvd_dir(nvd_path): > > for pkg_name in cve.pkg_names: > > - if pkg_name in packages and cve.affects(packages[pkg_name]): > > - packages[pkg_name].cves.append(cve.identifier) > > + if pkg_name in packages: > > + if cve.affects(packages[pkg_name]): > > + packages[pkg_name].cves.append(cve.identifier) > > + if len(packages[pkg_name].cves): > > + packages[pkg_name].status['cve'] = ('error', 'affected by cve') > > + else: > > + packages[pkg_name].status['cve'] = ('ok', 'no cve found') > > + > > > > > > def calculate_stats(packages): > > > > In the current state, that would give: > > * If a CVE applies to a br package -> error > * If a CVE does not applies to a br package -> ok > * If no CVE points to a br package -> na (no status check done) > > I would argue that the latest case is not correct. The status should be > ok, because we ran through the whole list of CVEs from the NVD feed, and > we did not find any of them that applies to this package. I see .. I think you are right. This will lead to a wrong status. I will remove the initializer for the status. > > I would rather write it like this: > > ######################## > diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats > index ed22f6b650..91477d583e 100755 > --- a/support/scripts/pkg-stats > +++ b/support/scripts/pkg-stats > @@ -620,6 +620,12 @@ def check_package_cves(nvd_path, packages): > if pkg_name in packages and cve.affects(packages[pkg_name]): > packages[pkg_name].cves.append(cve.identifier) > > + for pkg_name, pkg in packages.items(): > + if len(pkg.cves) > 0: > + pkg.status['cve'] = ('error', 'affected by CVE(s)') > + else: > + pkg.status['cve'] = ('ok', 'no CVE found') > + Isn't it right that we loop then (depending on the amount of nvd pathes) several thousend times? e.g. packages ~2600, nvds ~ 20 => 20*2600=52000 On the other hand we loop over the list of packages all over the place ;-/ > > def calculate_stats(packages): > stats = defaultdict(int) > ######################## > > > Best regards, > > Titouan
On 2/24/20 8:06 AM, Heiko Thiery wrote: > Hi Titouan and all, > > Am So., 23. Feb. 2020 um 15:24 Uhr schrieb Titouan Christophe > <titouan.christophe@railnova.eu>: >> >> Heiko, all, >> >> On 2/22/20 9:57 AM, Heiko Thiery wrote: >>> Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com> [--SNIP--] > >> >> I would rather write it like this: >> >> ######################## >> diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats >> index ed22f6b650..91477d583e 100755 >> --- a/support/scripts/pkg-stats >> +++ b/support/scripts/pkg-stats >> @@ -620,6 +620,12 @@ def check_package_cves(nvd_path, packages): >> if pkg_name in packages and cve.affects(packages[pkg_name]): >> packages[pkg_name].cves.append(cve.identifier) >> >> + for pkg_name, pkg in packages.items(): >> + if len(pkg.cves) > 0: >> + pkg.status['cve'] = ('error', 'affected by CVE(s)') >> + else: >> + pkg.status['cve'] = ('ok', 'no CVE found') >> + > > Isn't it right that we loop then (depending on the amount of nvd > pathes) several thousend times? > > e.g. packages ~2600, nvds ~ 20 => 20*2600=52000 Except that each NVD file contains a few thousands CVEs :). > > On the other hand we loop over the list of packages all over the place ;-/ Looping over all CVEs in a single NVD file yields 5 to 10 more iterations than looping over all packages (for instance year 2018 alone has 16039 CVE items) > >> >> def calculate_stats(packages): >> stats = defaultdict(int) >> ######################## >> >> >> Best regards, >> >> Titouan
Hi Titouan, Am Mo., 24. Feb. 2020 um 10:35 Uhr schrieb Titouan Christophe <titouan.christophe@railnova.eu>: > > > On 2/24/20 8:06 AM, Heiko Thiery wrote: > > Hi Titouan and all, > > > > Am So., 23. Feb. 2020 um 15:24 Uhr schrieb Titouan Christophe > > <titouan.christophe@railnova.eu>: > >> > >> Heiko, all, > >> > >> On 2/22/20 9:57 AM, Heiko Thiery wrote: > >>> Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com> > [--SNIP--] > > Isn't it right that we loop then (depending on the amount of nvd > > pathes) several thousend times? > > > > e.g. packages ~2600, nvds ~ 20 => 20*2600=52000 > > Except that each NVD file contains a few thousands CVEs :). > > > > > On the other hand we loop over the list of packages all over the place ;-/ > > Looping over all CVEs in a single NVD file yields 5 to 10 more > iterations than looping over all packages (for instance year 2018 alone > has 16039 CVE items) > you're right ... compared to this it doesn't matter. > > > >> > >> def calculate_stats(packages): > >> stats = defaultdict(int) > >> ######################## > >> > >> > >> Best regards, > >> > >> Titouan Thank you, Heiko
diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats index ed22f6b650..4efff8624f 100755 --- a/support/scripts/pkg-stats +++ b/support/scripts/pkg-stats @@ -617,8 +617,14 @@ def check_package_cves(nvd_path, packages): for cve in CVE.read_nvd_dir(nvd_path): for pkg_name in cve.pkg_names: - if pkg_name in packages and cve.affects(packages[pkg_name]): - packages[pkg_name].cves.append(cve.identifier) + if pkg_name in packages: + if cve.affects(packages[pkg_name]): + packages[pkg_name].cves.append(cve.identifier) + if len(packages[pkg_name].cves): + packages[pkg_name].status['cve'] = ('error', 'affected by cve') + else: + packages[pkg_name].status['cve'] = ('ok', 'no cve found') + def calculate_stats(packages):
Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com> --- support/scripts/pkg-stats | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)