-
Notifications
You must be signed in to change notification settings - Fork 142
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
feat(openEuler): Add openEuler CVE database #397
base: main
Are you sure you want to change the base?
Conversation
The testing result follows:
|
+1 to add support for openEuler OS! |
Could someone run tests for this PR? Thanks! |
@knqyf263 Could you please review this PR currently? Thank you! |
@DmitriyLewen Thank you very much!
|
@DmitriyLewen I have changed the code as we discussed above. The result looks good
|
@DmitriyLewen Do you have some other suggestions for this PR? |
Hello @wjunLu
If this is okay - please update aquasecurity/trivy#6475 (you can use |
Thank you very much! I'm checking this. |
Thank you again! I have no problem for this! I will update aquasecurity/trivy#6475 soon. |
|
@DmitriyLewen So sorry! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DmitriyLewen I have updated my branch from upstream, please re-run the tests. Thank you! |
I'll take a look today. |
pkg/vulnsrc/openeuler/openeuler.go
Outdated
return "" | ||
} | ||
version := parts[4] | ||
// e.g. 23.09, 22.03-LTS, 22.03-LTS-SP3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we treat 22.03-LTS, 22.03-LTS-SP1, and other versions differently? For example, alpine 3.19.0 and 3.19.1 are considered the same version in Trivy from the security advisory perspective, as their fixed versions are the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tend to preserve those versions, since they have different lifecycles (https://www.openeuler.org/en/other/lifecycle/) and different fixed versions.
For example:
The fixed version of kernel
in openEuler-22.03-LTS-SP3 is 5.10.0-217.0.0.120
, but that in 22.03-LTS-SP4 is kernel-5.10.0-217.0.0.116
see https://repo.openeuler.org/security/data/cvrf/2024/cvrf-openEuler-SA-2024-1792.xml and https://repo.openeuler.org/security/data/cvrf/2024/cvrf-openEuler-SA-2024-1795.xml
pkg/vulnsrc/openeuler/openeuler.go
Outdated
func getAffectedPackages(productTree ProductTree) []Package { | ||
var pkgs = make(map[string]Package) // pkgID => Package | ||
for _, branch := range productTree.Branches { | ||
// `src` pkgs are not installed in openEuler. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can get a source package name from RPMBDB.
$ docker run --rm -it openeuler/openeuler:20.03-lts
[root@5462ae7535d3 /]# rpm -qa --qf "%{NAME} %{SOURCERPM}\n" | grep glibc
glibc-common glibc-2.28-36.oe1.src.rpm
glibc glibc-2.28-36.oe1.src.rpm
So, I'm wondering if a source package name and a list of binary package names are equivalent in terms of vulnerability detection. In other words, wouldn't it be enough to save only the source package name and the fixed version in the DB? It has a great benefit from the perspective of DB size.
Let's take an example. Currently, we save all binary names here, like glibc-devel
, nscd
, etc.
https://github.com/aquasecurity/vuln-list/blob/ecca778da16828205caa70ba6941582f70041104/openeuler/2021/openEuler-SA-2021-1013.json#L79-L210
I'm wondering if storing only glibc
is enough.
https://github.com/aquasecurity/vuln-list/blob/ecca778da16828205caa70ba6941582f70041104/openeuler/2021/openEuler-SA-2021-1013.json#L231-L242
We do that for other distributions. If only the binary packages really affected by the vulnerability are listed, they should be preserved, but the current advisory appears to list all binary packages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to find out this, the result is as the example shows above that the really affected package is only glibc
. I will change the code, for aarch64
and x86_64
packages, only the binary like glibc
will be preserved, others like glibc-common
or glibc-debuginfo
... will not be preserved. Is that OK?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only the binary like glibc will be preserved, others like glibc-common or glibc-debuginfo ... will not be preserved. Is that OK?
I meant we may want to store only source packages. The source glibc and the binary glibc are technically different. In this example, by chance, there is a binary package name identical to the source package name, but there should also be examples where it does not exist.
How about storing the source package name and the affected architectures, since I think there are cases where x86_64 is affected but aarch64 is not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, such as Affected package: glibc
with Arches: [x86_64, aarch64, noaarch]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly. That's my idea. But I'm not confident as I'm not familiar with openEuler. If it turns out we need binary package names in the future, we will be able to fix it without breaking. If you also don't find any issues, we can go with source names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, @DmitriyLewen, could you please check the changes currently? Thank you very much!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @DmitriyLewen @knqyf263, I'm still looking forward to your suggestions, please review the new changes as soon as you have time.
Thank you very much!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wjunLu Please be patient. Maintainers have numerous other tasks and might be taking the summer vacation. It is good to remind maintainers, but please do that only once a week. There is not much point in notifying daily.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, sorry for that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pkg/vulnsrc/openeuler/openeuler.go
Outdated
// e.g. cpe:/a:openEuler:openEuler:22.03-LTS-SP3 | ||
parts := strings.Split(cpe, ":") | ||
if len(parts) != 5 || parts[2] != "openEuler" { | ||
return "" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wjunLu I found an incorrect CPE format - https://github.com/aquasecurity/vuln-list/blob/ecca778da16828205caa70ba6941582f70041104/openeuler/2022/openEuler-SA-2022-1773.json#L97
I checked other advisories. This format only appears in this file.
Do we need to parse this case or is it a typo by openEular
and they should fix it on their end?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I confirmed that some files exist such typo, and those errors occurred in the earlier advisories.
I will try to ask the community to fix this, but I think those advisories that have been released may not be easy to modify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think those advisories that have been released may not be easy to modify.
I hope that it is possible to correct one advisory.
If no - we must provide for such cases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If no - we must provide for such cases
How to do this? list such cases in README or some other place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, i meant we need to update our logic to add the ability to get the OS version for these cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DmitriyLewen Thanks!
Please see
trivy-db/pkg/vulnsrc/openeuler/openeuler.go
Line 197 in 4fa6f19
if len(parts) != 5 || parts[2] != "openEuler" { |
I added codes to use the wrong format getting the OS version, is that OK?
pkg/vulnsrc/openeuler/openeuler.go
Outdated
// e.g., cpe:/a:openEuler:openEuler:22.03-LTS-SP3 | ||
parts := strings.Split(cpe, ":") | ||
if len(parts) != 5 || parts[2] != "openEuler" { | ||
// e.g., cpe:/a:openEuler:openEuler-22.03-LTS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DmitriyLewen I did some changes here, to get OS version from the wrong format, please check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DmitriyLewen Please re-run the tests and tell me what to do next to push this PR merged. Thank you very much!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Your commit is better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, @DmitriyLewen @knqyf263 ! |
Hi @knqyf263 ! Please take a look when you have time. |
Looks like it‘s ready to support openEuler CVE database. @knqyf263 Would you mind taking a final look? |
aquasecurity/trivy#6475 has been closed due to inactivity while it actually waits for this PR... |
@DmitriyLewen I just merged the upstream's latest code into my forked branch, please re-run the test when you have time. Thank you! |
Description
What's openEuler?
openEuler is an open source, free Linux distribution platform. The platform provides an open community for global developers to build an open, diversified, and architecture-inclusive software ecosystem. openEuler is also an innovative platform that
encourages everyone to propose new ideas, explore new approaches, and practice new solutions.
Learn more, please visit https://www.openeuler.org/en/
Trivy does not support openEuler
We can see that the operating systems currently supported by trivy for security detection does not include openEuler(see https://aquasecurity.github.io/trivy/v0.50/docs/coverage/os/).
To support openEuler
Now, openEuler has 2,345,659 users, 18,072 contributors and 1,501 organization members(see https://datastat.openeuler.org/en/overview). It is necessary to support such a very mature open source operating system.
Discussion
Our discussion is here aquasecurity/trivy#6400
Relatived PRs
aquasecurity/vuln-list-update#284