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

Extend HTTPClient usage for downloads to ArtifactorySearch #7254

Closed
aikebah opened this issue Dec 15, 2024 · 21 comments · Fixed by #7293
Closed

Extend HTTPClient usage for downloads to ArtifactorySearch #7254

aikebah opened this issue Dec 15, 2024 · 21 comments · Fixed by #7293

Comments

@aikebah
Copy link
Collaborator

aikebah commented Dec 15, 2024

          Confirmed... only NexusSearch and ArtifactorySearch are pending. As both are typically solutions within the enterprise datacenter not requiring proxy access I think it would be fine to finish up this part and leave the remainder phase-out of URLConnectionFactory for a later feature-release in the 11.x series.

Originally posted by @aikebah in #6949 (comment)

Follow-up issue to #6949 to use HTTPClient for ArtifactorySearch in order to honor the standard proxy configuration also for the (expected rare) cases in which an Artifactory repository is only reachable via a proxy

@aikebah aikebah changed the title Confirmed... only NexusSearch and ArtifactorySearch are pending. As both are typically solutions within the enterprise datacenter not requiring proxy access I think it would be fine to finish up this part and leave the remainder phase-out of URLConnectionFactory for a later feature-release in the 11.x series. Extend HTTPClient usage for downloads to ArtifactorySearch Dec 15, 2024
@aikebah aikebah self-assigned this Dec 17, 2024
@aikebah
Copy link
Collaborator Author

aikebah commented Dec 18, 2024

@irrandon, @jthurne Are you still using Artifactory with authentication?
If so, would you be able to test a snapshot version of DependencyCheck?

I'm looking for someone who can verify the correct working of ODC snapshot against an artifactory server requiring authentication by the time I've finished coding the required changes for this enhancement in order to not break its connectivity in a future ODC release.

@aikebah
Copy link
Collaborator Author

aikebah commented Jan 1, 2025

@marcelstoer Spotted in other tickets that you have some items stored in Artifactory. Are you using Artifactory Pro version and would you be able to help validate correct working of authentication towards it (preferably both basic and bearer) by the ArtifactoryAnalyzer using a snapshot-build of a feature-branch in this project?

@marcelstoer
Copy link
Contributor

@aikebah I'd be happy to help. I am authenticating through <username> and <password> in settings.xml. I know we have a commercial Artifactory license but I have no idea what version we use.

@aikebah
Copy link
Collaborator Author

aikebah commented Jan 6, 2025

@marcelstoer any version would do, as long as it's the Artifactory Pro. Open source setup responds on the search with HTTP 400:

This REST API is available only in Artifactory Pro (see: jfrog.com/artifactory/features). If you are already running Artifactory Pro please make sure your server is activated with a valid license key.

I hope to work on polishing up the ArtifactorySearch transformation to httpclient5 coming Wednesday and will ping back here when a testable branch has been pushed to this repo

@aikebah
Copy link
Collaborator Author

aikebah commented Jan 8, 2025

@marcelstoer Implementation is complete. I would appreciate a test using a snapshot built from the feat/artifactory branch.

@marcelstoer
Copy link
Contributor

All good, the check passed w/o problem. Would I need to enable the Artifactory analyzer for what you need me to test?

@aikebah
Copy link
Collaborator Author

aikebah commented Jan 9, 2025

@marcelstoer Yes, artifactoryAnalyzer would have to be enabled. And would be best if at least one of your project's dependencies would be findable by sha1 on your Artifactory instance's search API - that/those dependencies should then have artifactory evidences included in the evidences section of the dependency-check report.

@marcelstoer
Copy link
Contributor

I had no idea what the Artifactory analyzer is or does and the best documentation I could find appears to be #60. Hence, I didn't really know what I was doing here 😄

Anyway, I hope the output I collected is of any use for you.

Command

mvn --debug org.owasp:dependency-check-maven:11.1.2-SNAPSHOT:aggregate \
-DartifactoryAnalyzerEnabled -DartifactoryAnalyzerUrl=https://my-bin/artifactory -DartifactoryAnalyzerServerId=my-bin \
-DnvdDatafeedUrl="https://dependency-check.github.io/DependencyCheck_Builder/nvd_cache/nvdcve-{0}.json.gz" \
-DfailBuildOnCVSS=7 -DcveStartYear=2020 -DskipProvidedScope=true -DnodeAuditSkipDevDependencies=true -DnodePackageSkipDevDependencies=true > odc.log

Debug snippet

...
[DEBUG] Initializing Artifactory Analyzer
[DEBUG] Initializing Artifactory analyzer
[DEBUG] Artifactory analyzer enabled: true
[DEBUG] Artifactory Search URL https://my-bin/artifactory
[DEBUG] Using default non-legacy proxy configuration
[DEBUG] Starting Artifactory Analyzer
[DEBUG] Parallel processing with up to 16 threads: Artifactory Analyzer.
[DEBUG] Begin Analysis of '/Users/marcelstoer/.m2/repository/some-jar.jar' (Artifactory Analyzer)
[DEBUG] Begin Analysis of '/Users/marcelstoer/.m2/repository/some-other-jar.jar' (Artifactory Analyzer)
...

ODC report excerpt

Screenshot 2025-01-09 at 10 33 18

@aikebah
Copy link
Collaborator Author

aikebah commented Jan 9, 2025

@marcelstoer at least no errors encountered, otherwise you'd shown those I guess.
For that aws sdk library, in the evidences section, are there also rows with artifactory in the source-column?

And if not: is
https://my-bin/artifactory/api/search/checksum?sha1=1e02ef5a66694ed83bab70b5bdc3f73fe6ec0998 indeed also not returning hits?

@marcelstoer
Copy link
Contributor

Yes, no errors.

For that aws sdk library, in the evidences section, are there also rows with artifactory in the source-column?

No, maybe because the dependencies are all present in the local repo?

is https://my-bin/artifactory/api/search/checksum?sha1=1e02ef5a66694ed83bab70b5bdc3f73fe6ec0998 indeed also not returning hits?

It's returning the AWS SDK

{
  "results" : [ {
    "uri" : "https://my-bin/artifactory/api/storage/maven-central-cache/com/amazonaws/aws-java-sdk-core/1.11.277/aws-java-sdk-core-1.11.277.jar"
  } ]
}

@aikebah
Copy link
Collaborator Author

aikebah commented Jan 9, 2025

Ah... I'd missed the detail of...

        for (Evidence e : dependency.getEvidence(EvidenceType.VENDOR)) {
            if ("pom".equals(e.getSource())) {
                return;
            }
        }

in the artifactoryAnalyzer so indeed... it would not use the Artifactory search as the jar will no doubt already have the pom associated with it already and therefor pom-based evidences are already present.

I guess best way to try and get it to really hit Artifactory with a request would be to put the jar in some folder and use the CLI from within that folder to scan the current folder (with artifactory analyzer enabled and centralAnalyzer disabled)

path/to/odc/bin/dependency-check.sh \
-s . \
--disableCentral \
--enableArtifactory \
--artifactoryUrl https://my-bin/artifactory \
--artifactoryUsername $ARTIFACTORY_USER \
--artifactoryApiToken $ARTIFCATORY_TOKEN \
-d ~/.m2/repository/org/owasp/dependency-check-data/11.0 \
--noupdate \
-l odc.log

should allow you to run it and simply re-use the already cached cveDB in your maven local repo, assuming that your register your login credentials for artifactory in the referenced env variables
(after build of the snapshot you would typically be able to run dependency-check.sh directly from the project target folder at <path/to/DependencyCheck>/cli/target/release/bin/dependency-check.sh)

@aikebah
Copy link
Collaborator Author

aikebah commented Jan 19, 2025

@marcelstoer Would you be able to run a trial with the CLI against your artifactory? When 'seen working' I would mark the PR ready and have it merged for the next release. I'd rather not merge and then find out that it 'breaks when used' if we can avoid that by testing the snapshot.

@marcelstoer
Copy link
Contributor

I guess best way to try and get it to really hit Artifactory with a request would be to put the jar in some folder and use the CLI from within that folder to scan the current folder

This fails with the following stacktrace

[ERROR] Could not connect to Artifactory search. Analysis failed.
java.io.IOException: Download failed, unable to retrieve and parse 'https://my-bin/artifactory/api/search/checksum?sha1=bca243d0af0db4758fbae45c5f4995cb5dabb612'; Cannot invoke "org.owasp.dependencycheck.data.artifactory.ChecksumsImpl.getMd5()" because "checksums" is null
        at org.owasp.dependencycheck.utils.Downloader.fetchAndHandle(Downloader.java:728)
        at org.owasp.dependencycheck.utils.Downloader.fetchAndHandle(Downloader.java:685)
        at org.owasp.dependencycheck.data.artifactory.ArtifactorySearch.search(ArtifactorySearch.java:111)
        at org.owasp.dependencycheck.analyzer.ArtifactoryAnalyzer.analyzeDependency(ArtifactoryAnalyzer.java:199)
        at org.owasp.dependencycheck.analyzer.AbstractAnalyzer.analyze(AbstractAnalyzer.java:131)
        at org.owasp.dependencycheck.AnalysisTask.call(AnalysisTask.java:88)
        at org.owasp.dependencycheck.AnalysisTask.call(AnalysisTask.java:37)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.lang.NullPointerException: Cannot invoke "org.owasp.dependencycheck.data.artifactory.ChecksumsImpl.getMd5()" because "checksums" is null
        at org.owasp.dependencycheck.data.artifactory.ArtifactorySearchResponseHandler.checkHashes(ArtifactorySearchResponseHandler.java:107)
        at org.owasp.dependencycheck.data.artifactory.ArtifactorySearchResponseHandler.validateUsability(ArtifactorySearchResponseHandler.java:187)
        at org.owasp.dependencycheck.data.artifactory.ArtifactorySearchResponseHandler.handleResponse(ArtifactorySearchResponseHandler.java:151)
        at org.owasp.dependencycheck.data.artifactory.ArtifactorySearchResponseHandler.handleResponse(ArtifactorySearchResponseHandler.java:42)
        at org.apache.hc.client5.http.impl.classic.CloseableHttpClient.execute(CloseableHttpClient.java:247)
        at org.apache.hc.client5.http.impl.classic.CloseableHttpClient.execute(CloseableHttpClient.java:188)
        at org.owasp.dependencycheck.utils.Downloader.fetchAndHandle(Downloader.java:715)
        ... 10 common frames omitted

The https://my-bin/artifactory/api/search/checksum?sha1=bca243d0af0db4758fbae45c5f4995cb5dabb612 resource does not contain any checksums indeed.

{
  "results": [
    {
      "uri": "https://bin.ti8m.ch:443/artifactory/api/storage/maven-central-cache/org/freemarker/freemarker/2.3.33/freemarker-2.3.33.jar"
    },
    {
      "uri": "https://bin.ti8m.ch:443/artifactory/api/storage/gradle-plugins-extended-cache/org/freemarker/freemarker/2.3.33/freemarker-2.3.33.jar"
    }
  ]
}

@aikebah
Copy link
Collaborator Author

aikebah commented Jan 22, 2025

@marcelstoer A simple HTTP-GET of the URL (e.g. from the browser) indeed returns only the URL, that is to be expected. The request that dependency-check sends includes (or at least is expected to include based on the code I wrote as a replacement for the old code) an additional HTTP Header X-Result-Detail with value info.

With that header present Artifactory is expected to respond with a much more detailed response (as also indicated at the Headers (Optionally) at Artifactory REST API documentation for search

@marcelstoer
Copy link
Contributor

I assumed you wanted me to try that and so I did 😄

curl -H "Authorization: Bearer <my-token>" -H "X-Result-Detail: info" https://my-bin/artifactory/api/search/checksum?sha1=bca243d0af0db4758fbae45c5f4995cb5dabb612

The result, though, is the same: no checksums returned, only URIs.

Their documentation says

to add all extra information of the found artifact

Maybe this is to be interpreted as "all information is optional"?

@aikebah
Copy link
Collaborator Author

aikebah commented Jan 22, 2025

Based on stored responses in the testcases from when the artifactory analyzer was first contributed I would expect at least some of the additional details to be 'always available'.

Based on the current Artifactory OSS sources for any found artifact I would expect at the very least its 'BaseStorageInfo' to be added into the response:

        storageInfoRest.repo = itemInfo.getRepoKey();
        storageInfoRest.path = "/" + itemInfo.getRelPath();
        storageInfoRest.created = RestUtils.toIsoDateString(itemInfo.getCreated());
        storageInfoRest.createdBy = itemInfo.getCreatedBy();
        storageInfoRest.lastModified = RestUtils.toIsoDateString(itemInfo.getLastModified());
        storageInfoRest.modifiedBy = itemInfo.getModifiedBy();
        storageInfoRest.lastUpdated = RestUtils.toIsoDateString(itemInfo.getLastUpdated());

@aikebah
Copy link
Collaborator Author

aikebah commented Jan 22, 2025

@marcelstoer Regarding how it can be that you don't get them: is the connection from curl to your artifactory direct, or might there be some WebApplicationFirewall technology in-between that strips out any 'not explicitly allowed' HTTP headers from each request that passes through?

@marcelstoer
Copy link
Contributor

Hah, spot on, genius! It's indeed the WAF dropping the X-Result-Detail header. I verified this on a host that is "closer" (network-wise) to Artifactory.

I guess I need to redo the entire ODC CLI test on said host. It'll take a while but I'll report back once done.

@aikebah
Copy link
Collaborator Author

aikebah commented Jan 22, 2025

Well, at least some other useful outcome of this testing: the code should be prepared to receive responses without the detail and handle those gracefully (try to detect we get the 'basic' response and inform the user in some way that there is likely network infrastructure dropping the X-Result-Detail HTTP-header in between their system and the Artifactory server that is rendering the ArtifactoryAnalyzer useless)

aikebah added a commit that referenced this issue Jan 22, 2025
…of detail

Resolves the NPE reported in #7254 (comment)

Logs a warning for each entry in the response that does not have the checksums block that is to be expected for any
request that has the `X-Result-Detail: info` HTTP header.
@aikebah
Copy link
Collaborator Author

aikebah commented Jan 22, 2025

@marcelstoer Have you mixed up the search-URL and the search response, or is your artifactory actually hosting a jar for freemarker 2.3.33 that has a sha1 collision with the Maven Central hosted org.aspectj:aspectj-weaver:1.9.22.1)?
Maven Central has a different sha1 hash for Freemarker 2.3.33 jar-file

@marcelstoer
Copy link
Contributor

I guess I need to redo the entire ODC CLI test on said host. It'll take a while but I'll report back once done.

Ok, all good now I suppose.

[admin@t8tstjnode10 ~]$ bin/dependency-check.sh \
-s ./my.war \
--disableCentral \
--enableArtifactory \
--artifactoryUrl https://my-bin/artifactory \
--artifactoryUsername user \
--artifactoryApiToken token \
-l odc.log
[INFO] Checking for updates
...
[INFO] End database defrag (6967 ms)
[INFO] Check for updates complete (1269499 ms)
[INFO] 

...
[INFO] Analysis Started
[INFO] Finished Archive Analyzer (3 seconds)
[INFO] Finished File Name Analyzer (0 seconds)
[INFO] Finished Jar Analyzer (0 seconds)
[INFO] Finished Artifactory Analyzer (0 seconds)
[INFO] Finished Dependency Merging Analyzer (0 seconds)
[INFO] Finished Hint Analyzer (0 seconds)
[INFO] Finished Version Filter Analyzer (0 seconds)
[INFO] Created CPE Index (1 seconds)
[INFO] Finished CPE Analyzer (3 seconds)
[INFO] Finished False Positive Analyzer (0 seconds)
[INFO] Finished NVD CVE Analyzer (0 seconds)
[INFO] Finished RetireJS Analyzer (0 seconds)
[INFO] Finished Sonatype OSS Index Analyzer (2 seconds)
[INFO] Finished Vulnerability Suppression Analyzer (0 seconds)
[INFO] Finished Known Exploited Vulnerability Analyzer (0 seconds)
[INFO] Finished Dependency Bundling Analyzer (0 seconds)
[INFO] Finished Unused Suppression Rule Analyzer (0 seconds)
[INFO] Analysis Complete (10 seconds)
[INFO] Writing HTML report to: /home/admin/./dependency-check-report.html

The report is fine i.e. as expected and the odc.log contains the expected Artifactory statements:

...
DEBUG - Initializing Artifactory Analyzer
2025-01-24 17:28:56,646 org.owasp.dependencycheck.analyzer.ArtifactoryAnalyzer:135
DEBUG - Initializing Artifactory analyzer
2025-01-24 17:28:56,646 org.owasp.dependencycheck.analyzer.ArtifactoryAnalyzer:137
DEBUG - Artifactory analyzer enabled: true
2025-01-24 17:28:56,647 org.owasp.dependencycheck.data.artifactory.ArtifactorySearch:80
DEBUG - Artifactory Search URL https://my-bin/artifactory
2025-01-24 17:28:56,647 org.owasp.dependencycheck.data.artifactory.ArtifactorySearch:87
DEBUG - Using default non-legacy proxy configuration
2025-01-24 17:28:56,720 org.owasp.dependencycheck.Engine:764
DEBUG - Starting Artifactory Analyzer
2025-01-24 17:28:56,721 org.owasp.dependencycheck.Engine:812
DEBUG - Parallel processing with up to 16 threads: Artifactory Analyzer.
2025-01-24 17:28:56,721 org.owasp.dependencycheck.AnalysisTask:86
DEBUG - Begin Analysis of '/tmp/dctemp32106972-2169-46b6-b69a-f12266d7ed90/check16211674009853962537tmp/1/WEB-INF/lib/error_prone_annotations-2.28.0.jar' (Artifactory Analyzer)
2025-01-24 17:28:56,722 org.owasp.dependencycheck.AnalysisTask:86
DEBUG - Begin Analysis of '/tmp/dctemp32106972-2169-46b6-b69a-f12266d7ed90/check16211674009853962537tmp/1/WEB-INF/lib/spectator-api-1.7.3.jar' (Artifactory Analyzer)
...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants