[ENT] fix issue with index on artifact #1835

pxp928 · 2024-04-11T17:15:54Z

Description of the PR

No errors when ingesting cdx_vuln.json and spdx_vuln.json

The trivy CDX SBOM has algo as "SHA-1" while SPDX has "SHA1" (with the same digest). This caused the issue.

{
          "alg": "SHA-1",
          "content": "75045e7ec628424fc4a0fd1006dd997b82215433"
}

 go run ./cmd/guacone collect files ../guac-data/cdx_vuln.json
{"level":"info","ts":1712854913.4478111,"caller":"logging/logger.go:70","msg":"Logging at info level"}
{"level":"info","ts":1712854913.447982,"caller":"cli/init.go:69","msg":"Using config file: /Users/parth/Documents/pxp928/artifact-ff/guac.yaml"}
{"level":"info","ts":1712854913.4492428,"caller":"process/process.go:254","msg":"Decoding document with encoding:  "}
{"level":"info","ts":1712854913.4587939,"caller":"parser/parser.go:81","msg":"parsing document tree with root type: CycloneDX"}
{"level":"info","ts":1712854913.461613,"caller":"ingestor/ingestor.go:59","msg":"unable to create entries in collectsub server, but continuing: unable to add collect entries: rpc error: code = Unavailable desc = connection error: desc = \"transport: Error while dialing: dial tcp [::1]:2782: connect: connection refused\""}
{"level":"info","ts":1712854913.4621491,"caller":"helpers/bulk.go:39","msg":"assembling Package: 105"}
{"level":"info","ts":1712854913.4748049,"caller":"helpers/bulk.go:55","msg":"assembling Source: 0"}
{"level":"info","ts":1712854913.476955,"caller":"helpers/bulk.go:65","msg":"assembling Artifact: 1"}
{"level":"info","ts":1712854913.480494,"caller":"helpers/bulk.go:80","msg":"assembling Materials (Artifact): 0"}
{"level":"info","ts":1712854913.481659,"caller":"helpers/bulk.go:89","msg":"assembling Builder: 0"}
{"level":"info","ts":1712854913.4826949,"caller":"helpers/bulk.go:98","msg":"assembling Vulnerability: 0"}
{"level":"info","ts":1712854913.483598,"caller":"helpers/bulk.go:107","msg":"assembling Licenses: 0"}
{"level":"info","ts":1712854913.48462,"caller":"helpers/bulk.go:114","msg":"assembling CertifyScorecard: 0"}
{"level":"info","ts":1712854913.484643,"caller":"helpers/bulk.go:119","msg":"assembling IsDependency: 273"}
{"level":"info","ts":1712854913.5215921,"caller":"helpers/bulk.go:127","msg":"assembling IsOccurrence: 4"}
{"level":"info","ts":1712854913.5254002,"caller":"helpers/bulk.go:135","msg":"assembling HasSLSA: 0"}
{"level":"info","ts":1712854913.5254211,"caller":"helpers/bulk.go:140","msg":"assembling CertifyVuln: 0"}
{"level":"info","ts":1712854913.525426,"caller":"helpers/bulk.go:145","msg":"assembling VulnMetadata: 0"}
{"level":"info","ts":1712854913.52543,"caller":"helpers/bulk.go:150","msg":"assembling VulnEqual: 0"}
{"level":"info","ts":1712854913.525434,"caller":"helpers/bulk.go:156","msg":"assembling HasSourceAt: 0"}
{"level":"info","ts":1712854913.5254369,"caller":"helpers/bulk.go:161","msg":"assembling CertifyBad: 0"}
{"level":"info","ts":1712854913.525441,"caller":"helpers/bulk.go:167","msg":"assembling CertifyGood: 0"}
{"level":"info","ts":1712854913.5254662,"caller":"helpers/bulk.go:173","msg":"assembling PointOfContact: 0"}
{"level":"info","ts":1712854913.5254688,"caller":"helpers/bulk.go:178","msg":"assembling HasMetadata: 0"}
{"level":"info","ts":1712854913.525473,"caller":"helpers/bulk.go:183","msg":"assembling HasSBOM: 1"}
{"level":"info","ts":1712854913.541128,"caller":"helpers/bulk.go:193","msg":"assembling VEX : 0"}
{"level":"info","ts":1712854913.541159,"caller":"helpers/bulk.go:198","msg":"assembling HashEqual : 0"}
{"level":"info","ts":1712854913.541164,"caller":"helpers/bulk.go:203","msg":"assembling PkgEqual : 0"}
{"level":"info","ts":1712854913.5411682,"caller":"helpers/bulk.go:208","msg":"assembling CertifyLegal : 0"}
{"level":"info","ts":1712854913.541179,"caller":"ingestor/ingestor.go:68","msg":"[91.931625ms] completed doc {Collector:FileCollector Source:file:///../guac-data/cdx_vuln.json}"}
{"level":"info","ts":1712854913.541188,"caller":"cmd/files.go:138","msg":"collector ended gracefully"}
{"level":"info","ts":1712854913.541217,"caller":"cmd/files.go:152","msg":"completed ingesting 1 documents of 1"}

spdx_vuln.json

go run ./cmd/guacone collect files ../guac-data/docs/spdx/spdx_vuln.json
{"level":"info","ts":1712854902.853546,"caller":"logging/logger.go:70","msg":"Logging at info level"}
{"level":"info","ts":1712854902.853662,"caller":"cli/init.go:69","msg":"Using config file: /Users/parth/Documents/pxp928/artifact-ff/guac.yaml"}
{"level":"info","ts":1712854902.855564,"caller":"process/process.go:254","msg":"Decoding document with encoding:  "}
{"level":"info","ts":1712854903.090552,"caller":"parser/parser.go:81","msg":"parsing document tree with root type: SPDX"}
{"level":"info","ts":1712854903.208218,"caller":"ingestor/ingestor.go:59","msg":"unable to create entries in collectsub server, but continuing: unable to add collect entries: rpc error: code = Unavailable desc = connection error: desc = \"transport: Error while dialing: dial tcp [::1]:2782: connect: connection refused\""}
{"level":"info","ts":1712854903.2215161,"caller":"helpers/bulk.go:39","msg":"assembling Package: 3981"}
{"level":"info","ts":1712854903.468142,"caller":"helpers/bulk.go:55","msg":"assembling Source: 0"}
{"level":"info","ts":1712854903.472548,"caller":"helpers/bulk.go:65","msg":"assembling Artifact: 3384"}
{"level":"info","ts":1712854903.529508,"caller":"helpers/bulk.go:80","msg":"assembling Materials (Artifact): 0"}
{"level":"info","ts":1712854903.531965,"caller":"helpers/bulk.go:89","msg":"assembling Builder: 0"}
{"level":"info","ts":1712854903.533203,"caller":"helpers/bulk.go:98","msg":"assembling Vulnerability: 0"}
{"level":"info","ts":1712854903.5346181,"caller":"helpers/bulk.go:107","msg":"assembling Licenses: 29"}
{"level":"info","ts":1712854903.5388029,"caller":"helpers/bulk.go:114","msg":"assembling CertifyScorecard: 0"}
{"level":"info","ts":1712854903.53886,"caller":"helpers/bulk.go:119","msg":"assembling IsDependency: 7161"}
{"level":"info","ts":1712854903.9924579,"caller":"helpers/bulk.go:127","msg":"assembling IsOccurrence: 3878"}
{"level":"info","ts":1712854904.180903,"caller":"helpers/bulk.go:135","msg":"assembling HasSLSA: 0"}
{"level":"info","ts":1712854904.180953,"caller":"helpers/bulk.go:140","msg":"assembling CertifyVuln: 0"}
{"level":"info","ts":1712854904.180964,"caller":"helpers/bulk.go:145","msg":"assembling VulnMetadata: 0"}
{"level":"info","ts":1712854904.180968,"caller":"helpers/bulk.go:150","msg":"assembling VulnEqual: 0"}
{"level":"info","ts":1712854904.180972,"caller":"helpers/bulk.go:156","msg":"assembling HasSourceAt: 0"}
{"level":"info","ts":1712854904.1809762,"caller":"helpers/bulk.go:161","msg":"assembling CertifyBad: 0"}
{"level":"info","ts":1712854904.18098,"caller":"helpers/bulk.go:167","msg":"assembling CertifyGood: 0"}
{"level":"info","ts":1712854904.181005,"caller":"helpers/bulk.go:173","msg":"assembling PointOfContact: 0"}
{"level":"info","ts":1712854904.18101,"caller":"helpers/bulk.go:178","msg":"assembling HasMetadata: 462"}
{"level":"info","ts":1712854904.207297,"caller":"helpers/bulk.go:183","msg":"assembling HasSBOM: 1"}
{"level":"info","ts":1712854904.426201,"caller":"helpers/bulk.go:193","msg":"assembling VEX : 0"}
{"level":"info","ts":1712854904.4262269,"caller":"helpers/bulk.go:198","msg":"assembling HashEqual : 0"}
{"level":"info","ts":1712854904.426232,"caller":"helpers/bulk.go:203","msg":"assembling PkgEqual : 0"}
{"level":"info","ts":1712854904.426236,"caller":"helpers/bulk.go:208","msg":"assembling CertifyLegal : 125"}
{"level":"info","ts":1712854904.450565,"caller":"ingestor/ingestor.go:68","msg":"[1.594984334s] completed doc {Collector:FileCollector Source:file:///../guac-data/docs/spdx/spdx_vuln.json}"}
{"level":"info","ts":1712854904.45059,"caller":"cmd/files.go:138","msg":"collector ended gracefully"}
{"level":"info","ts":1712854904.450625,"caller":"cmd/files.go:152","msg":"completed ingesting 1 documents of 1"}

PR Checklist

All commits have a Developer Certificate of Origin (DCO) -- they are generated using -s flag to git commit.
All new changes are covered by tests
If GraphQL schema is changed, make generate has been run
If OpenAPI spec is changed, make generate has been run
If collectsub protobuf has been changed, make proto has been run
All CI checks are passing (tests and formatting)
All dependent PRs have already been merged

Signed-off-by: pxp928 <parth.psu@gmail.com>

pkg/assembler/backends/ent/backend/artifact.go

Signed-off-by: pxp928 <parth.psu@gmail.com>

mihaimaruseac

So, it makes sense to key on both algorithm and digest. We would prevent cases when an artifact with algorithm A has hash H and another artifact with algorithm B has the same hash H, same length, same contents.

But I don't see how this matches with the issue. Here it looks like the same jar was given 4 different paths, if I understand the PR message correctly. Just checking, do we also canonicalize algorithm to lower case?

pxp928 · 2024-04-11T17:27:39Z

So, it makes sense to key on both algorithm and digest. We would prevent cases when an artifact with algorithm A has hash H and another artifact with algorithm B has the same hash H, same length, same contents.

But I don't see how this matches with the issue. Here it looks like the same jar was given 4 different paths, if I understand the PR message correctly. Just checking, do we also canonicalize algorithm to lower case?

The question was if that is valid. The same jar with different paths having the same SHA1?

Just checking, do we also canonicalize algorithm to lower case?

Yes we do. This is also tested in the unit test.

mihaimaruseac · 2024-04-11T17:40:27Z

The same jar with different paths having the same SHA1?

This should still be just one artifact, we don't care about the path in the noun. Is the path from the SBOM? We'd then have 4 different HasSBOM nodes for the same artifact?

Signed-off-by: pxp928 <parth.psu@gmail.com>

pxp928 · 2024-04-11T18:05:13Z

The same jar with different paths having the same SHA1?

This should still be just one artifact, we don't care about the path in the noun. Is the path from the SBOM? We'd then have 4 different HasSBOM nodes for the same artifact?

yeah disregard that. It is not part of the issue. Mistake on my end.

pkg/assembler/backends/arangodb/backend.go

mrizzi

@pxp928 thanks 👍

* fix issue with index on artifact Signed-off-by: pxp928 <parth.psu@gmail.com> * add test to capture this usecase Signed-off-by: pxp928 <parth.psu@gmail.com> * remove digest only index in arrango Signed-off-by: pxp928 <parth.psu@gmail.com> --------- Signed-off-by: pxp928 <parth.psu@gmail.com> Signed-off-by: Soham Arora <arorasoham9@gmail.com>

fix issue with index on artifact

ba70a7e

Signed-off-by: pxp928 <parth.psu@gmail.com>

pxp928 requested a review from mihaimaruseac as a code owner April 11, 2024 17:15

pull-request-size bot added the size/S label Apr 11, 2024

pxp928 commented Apr 11, 2024

View reviewed changes

pkg/assembler/backends/ent/backend/artifact.go Show resolved Hide resolved

pull-request-size bot added size/M and removed size/S labels Apr 11, 2024

add test to capture this usecase

a070f94

Signed-off-by: pxp928 <parth.psu@gmail.com>

pxp928 force-pushed the issue-1816-ent branch from 933384c to a070f94 Compare April 11, 2024 17:19

pull-request-size bot added size/S and removed size/M labels Apr 11, 2024

mihaimaruseac approved these changes Apr 11, 2024

View reviewed changes

remove digest only index in arrango

a6deee6

Signed-off-by: pxp928 <parth.psu@gmail.com>

pull-request-size bot added size/M and removed size/S labels Apr 11, 2024

pxp928 requested review from jeffmendoza and mrizzi April 11, 2024 18:13

mihaimaruseac approved these changes Apr 11, 2024

View reviewed changes

pkg/assembler/backends/arangodb/backend.go Show resolved Hide resolved

mrizzi approved these changes Apr 11, 2024

View reviewed changes

lumjjb approved these changes Apr 11, 2024

View reviewed changes

kodiakhq bot merged commit 2ec6bc9 into guacsec:main Apr 11, 2024
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ENT] fix issue with index on artifact #1835

[ENT] fix issue with index on artifact #1835

pxp928 commented Apr 11, 2024 •

edited

mihaimaruseac left a comment

pxp928 commented Apr 11, 2024

mihaimaruseac commented Apr 11, 2024

pxp928 commented Apr 11, 2024

mrizzi left a comment

[ENT] fix issue with index on artifact #1835

[ENT] fix issue with index on artifact #1835

Conversation

pxp928 commented Apr 11, 2024 • edited

Description of the PR

PR Checklist

mihaimaruseac left a comment

Choose a reason for hiding this comment

pxp928 commented Apr 11, 2024

mihaimaruseac commented Apr 11, 2024

pxp928 commented Apr 11, 2024

mrizzi left a comment

Choose a reason for hiding this comment

pxp928 commented Apr 11, 2024 •

edited