-
Notifications
You must be signed in to change notification settings - Fork 171
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/compute dependency hashes #143
Conversation
builders/builderutil/hashes.go
Outdated
package builderutil | ||
|
||
import ( | ||
"crypto/md5" |
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.
G501: Blacklisted import crypto/md5: weak cryptographic primitive
builders/builderutil/hashes.go
Outdated
} | ||
hashes.SHA1 = string(sha1Hash.Sum(nil)) | ||
|
||
md5Hash := md5.New() |
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.
G401: Use of weak cryptographic primitive
builders/builderutil/hashes.go
Outdated
} | ||
hashes.SHA1 = hex.EncodeToString(sha1Hash.Sum(nil)) | ||
|
||
md5Hash := md5.New() |
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.
G401: Use of weak cryptographic primitive
builders/builderutil/hashes.go
Outdated
package builderutil | ||
|
||
import ( | ||
"crypto/md5" // #nosec |
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.
G501: Blacklisted import crypto/md5: weak cryptographic primitive
builders/builderutil/hashes.go
Outdated
} | ||
hashes.SHA1 = hex.EncodeToString(sha1Hash.Sum(nil)) | ||
|
||
md5Hash := md5.New() // #nosec |
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.
G401: Use of weak cryptographic primitive
I'm not convinced that dependency hashing is the right way forward for detecting vendored dependencies. I'm especially not convinced that using standard MD5/etc. hashing is the correct way to implement dependency hashing. I suspect this implementation might be JAR-specific. Here are two other examples where hashing might help us:
I don't think it's clear that this is a good design for hashing dependencies in general, but I'm happy to LGTM for JARs specifically if that is the intention and you move this into the Ant builder package. |
We need a narrative to address a package that we truly don't have access to, but is considered 3rd-party; like an old vendorized JAR from who knows where. This doesn't need to be relied upon by all languages, but there are some environments where this is just common. FOSSA should have a large variety of ways to resolve a dependency; a sha is a good, generic way to represent "here's a bit of code". The CLI should just support this as part of the spec, but not necessarily implement it across everything if there are better methods available. This seems to be particularly useful for Java and C#, see VersionEye's approach here: https://blog.versioneye.com/tag/sha/ https://github.com/versioneye/veye-checker An alternative method of doing this is just to create a "hash" locator spec that resolves to a known artifact; but my hesitation is that hashes are not very semantic. It's hard to understand what it represents which would render an unresolved hash locator useless to a user. |
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.
LGTM for a v0, but we'll have to revisit this.
package module | ||
|
||
// Hashes contains hexadecimal checksums of code libraries brought in by running a Build | ||
type Hashes struct { |
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.
Why are we computing all of these? Isn't there just one hash that's relevant per dependency?
* Differentiate between locators with and without revisions * Retrieve and store project scan filters
WIP, proposal to support using dependency hashes as an alternative resolution method for builds that heavily depend on vendorized dependencies.