Skip to content

Add openssh file test #274

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

Closed
terriko opened this issue Jan 8, 2020 · 5 comments
Closed

Add openssh file test #274

terriko opened this issue Jan 8, 2020 · 5 comments
Labels
good first issue Good for newcomers
Milestone

Comments

@terriko
Copy link
Contributor

terriko commented Jan 8, 2020

openssh has a cve mapping test, but not a file test.

Instructions here: Adding signature tests against real files

This is a beginner-suitable task. Since this is our first real-file test, it's possible you will discover an issue with the checker itself during testing. If you do, please mention it here!

@ghost
Copy link

ghost commented Jan 23, 2020

i'll work on this!

@ghost ghost mentioned this issue Jan 23, 2020
@terriko terriko added this to the 1.0 milestone Jan 25, 2020
@terriko
Copy link
Contributor Author

terriko commented Feb 29, 2020

Still looking for a good test here. Feel free to work on this even if someone else is already working on it -- there is no harm in havnig more than one file test for openssh and unless people happen to choose absolutely identical files, we can merge multiple solutions here.

@hur
Copy link
Contributor

hur commented Feb 29, 2020

I started working on this. I have a question regarding the version detection.

I ran the tests with some files and noticed that some files of version 7.9p1 are detected as 7.9 instead of 7.9p1. It seems to be based on whichever version is detected first in the binaries - the for loop determining the version breaks on the first match.

Is this the desired functionality? Should I write the tests to expect whatever is the first version occurrence in the binaries?

Here you can see 7.9 is detected before 7.9p1:

cve_bin_tool.Scanner - INFO - /tmp/cve-bin-tool-chj1kvwt/openssh-clients-7.9p1-6.fc29.x86_64.rpm.extracted/usr/bin/ssh-keyscan is openssh-client 7.9
cve_bin_tool.Scanner - INFO - Known CVEs in version 7.9
...
cve_bin_tool.Scanner - INFO - /tmp/cve-bin-tool-chj1kvwt/openssh-clients-7.9p1-6.fc29.x86_64.rpm.extracted/usr/bin/ssh is openssh-client 7.9p1
cve_bin_tool.Scanner - INFO - Known CVEs in version 7.9p1
cves = defaultdict(<class 'dict'>, {'openssh-client': {'7.9': {...}, '7.9p1': {...}}})

Also, sometimes the version detected will contain a space at the end. E.g. OpenSSH_7.4p1 Debian-10+deb9u7 ends up as 7.4p1 (with a space at the end), so a test expecting 7.4p1 would fail.

If these are unintended, I could take a shot at fixing them.

@terriko
Copy link
Contributor Author

terriko commented Mar 3, 2020

The first is unintended but sort of expected. It's a side effect of the way that python treats versions as bigger-than, and you can see how we worked around it in the openssl-specific code in cvedb.py. A fix would be good. For systemd (one of the others that has multiple version strings in a regular file) we actually just did a sort on the lines before doing matching, but maybe it's time to build a more generic solution.

However, once we get a fix, we're going to have another problem: we aren't actually comparing on revisions in getcves right now, and they're stored separately in NVD sometimes. So you may have to do some digging through cve.db and the json database to figure out how best to actually make use of the data once you've got it.

The space at the end is utterly unintended, though. We probably need a .strip() in get_version or something. That at least should be an easy fix, so you probably want to do that one separately.

@hur
Copy link
Contributor

hur commented Mar 4, 2020

Thank you, I'll take a look at how it's done currently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants