-
Notifications
You must be signed in to change notification settings - Fork 231
Add Apache Camel Advisories #1792
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: kunalsz <[email protected]>
Signed-off-by: kunalsz <[email protected]>
Signed-off-by: kunalsz <[email protected]>
Signed-off-by: kunalsz <[email protected]>
Signed-off-by: kunalsz <[email protected]>
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 @kunalsz, see some suggestions and feedback.
def __init__(self): | ||
super().__init__() |
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.
Any reason why we need this? I'm curious, is there something peculiar about the Apache Camel importer that requires it here, but not in the OpenSSL importer here https://github.com/aboutcode-org/vulnerablecode/blob/e2f03f59c276b5ca9554153f2b06da49ead0a889/vulnerabilities/pipelines/openssl_importer.py
return advisory_len | ||
|
||
|
||
def fetch_advisory_data(url): |
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.
It would be much cleaner and easier to parse advisory from text data. See example https://camel.apache.org/security/CVE-2023-34442.txt.asc
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.
Currently the advisories are fetched by parsing the html table. It is easier to parse the fixed rows and columns. In the text data we'll have to use regex to find the correct line corresponding to our need.
fixed_versions = [] | ||
for fixed_version in version_pattern.findall(fixed_version_out): | ||
fixed_versions.append(MavenVersion(fixed_version)) |
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.
Strange, you’re collecting the fixed versions but not including them in the final AdvisoryData
.
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 must have neglected it by mistake, I'll include that too
package=PackageURL( | ||
type="maven", | ||
namespace="org.apache.camel", | ||
name="camel", |
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.
Component is not always camel
. For example, in this advisory https://camel.apache.org/security/CVE-2023-34442.html component is camel-jira
.
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.
@keshav-space Yes,you are right about that. There are various components for camel
but they are not specified in an easier to parse fashion atleast on the advisory page/text file atleast. For example
- In https://camel.apache.org/security/CVE-2020-11973.html
Apache Camel Netty
is the component, but it is hard to parse instead there's a link available on the advisory page https://issues.apache.org/jira/browse/CAMEL-14477 which exclusively specifies the component ascamel-netty
.
Same goes for other advisories also, the component is more exclusively defined on https://issues.apache.org/jira/browse/CAMEL-{ID}. So, should I parse the component from there ?
return response["cveMetadata"]["datePublished"] | ||
|
||
|
||
def parse_apache_camel_versions(version_string): |
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.
This parse function doesn't work correctly. For example, given the affected package statement 2.15.0 up to 2.15.4, 2.16.0
function will produces vers:maven/2.15.0|2.15.4|2.16.0
which is not correct version range.
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'll update the parsing logic
@keshav-space Thanks for the review, I'll make the necessary changes and update you. |
@keshav-space I have update the version parsing logic and it works fine. But I have a doubt on how to incorporate For example:
Looking forward to your insights ! |
In reference to the issue #1515
Changes Made: