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

Verify hashes and PGP signatures for dependencies and plugins #10443

Closed
vlsi opened this issue Sep 4, 2019 · 10 comments
Closed

Verify hashes and PGP signatures for dependencies and plugins #10443

vlsi opened this issue Sep 4, 2019 · 10 comments
Assignees

Comments

@vlsi
Copy link
Contributor

@vlsi vlsi commented Sep 4, 2019

Expected Behavior

Gradle should allow to declare the expected hashes and/or PGP signatures for dependencies and plugins.
In other words, build script author should declare the set of trusted hashes/PGP signatures, and the builds should fail in case the resolved artifact fails the validation.

It is important that the build should fail before any class is loaded from the "invalid" jar (or whatever) file.

There should be a way to "fail at the build finish" to simplify CI checks (CI log would list all the violations not just the very first).

Current Behavior

Currently Gradle does not allow to enforce integrity of the downloaded/used artifacts, so build machines are susceptible to man-in-the-middle attacks.

Context

I want to improve the level of security across Gradle-based builds.
For instance, there are CVEs like "accidental use of HTTP instead of HTTPS for artifact repository" (see ben-manes/caffeine#301 )

If Gradle build allowed to declare the expected checksums and/or PGP signatures, then it would prevent man-in-the-middle attacks.

Lots of existing package managers use PGP for trust verification:

What other alternatives have you considered?

https://github.com/signalapp/gradle-witness is a Gradle plugin that verifies resolved artifacts. However it misses lots of important features like "ability to verify downloaded plugins".

I've implemented https://github.com/vlsi/vlsi-release-plugins/tree/master/plugins/checksum-dependency-plugin. It seems to work just fine. It captures resolution of plugins, and it captures resolutions of regular configurations.

However the sad thing is DependencyResolutionListener does not provide ConfigurationContainer object, so it is really hard to create a proper detached configuration for the resolution of .asc files.
As of now checksum-dependency-plugin relies on ResolvableDependencies.path. Then it analyzes the path and creates detached configuration accordingly.
Just in case: setttings, and different projects might have different set of repositories, so it is important to use a proper one for asc resolution.

@rgoldberg

This comment has been minimized.

Copy link
Contributor

@rgoldberg rgoldberg commented Sep 4, 2019

PGP signature verification would be great to have.

I think that, if Gradle otherwise supports checksum verification / locking, that code should be reused to provide the checksum portion of this feature request.

Given that #5633 seems to encompass checksum verification / locking, should this issue leave out checksums to focus solely on PGP, and just depend on #5633 as a precursor?

If the checksum portions of this issue won’t be satisfied by #5633, can you point out what is missing from #5633?

@vlsi

This comment has been minimized.

Copy link
Contributor Author

@vlsi vlsi commented Sep 4, 2019

should this issue leave out checksums to focus solely on PGP,

No. Please avoid mentioning #5633 here as that issue treats a "completely different" use case.

Please take into account that the key point of this issue is to prevent man-in-the-middle attacks.

There are artifacts that fail to publish PGP signatures (e.g. https://gitlab.ow2.org/asm/asm/issues/317878 and #10182 ). Build script author can't use PGP for those dependencies. The only available option becomes checksums.

In other words, man-in-the-middle mitigation requires both PGP and checksums.

@rgoldberg

This comment has been minimized.

Copy link
Contributor

@rgoldberg rgoldberg commented Sep 4, 2019

You’re saying that this issue requires allowing a build to somehow specify a checksum that a dependency must match. That is the exact definition of dependency content locking.

I imagine that this issue would require locking only for the specified dependencies instead of requiring it for all dependencies in a configuration, which would possibly be an additional requirement above #5633. But, in that case, I’d make a separate issue for enabling dependency content locking on a per-dependency basis, have that depend on #5633, and have this issue depend on that new issue.

Is there any new functionality (not use cases) from #5633 that won’t be necessary for this issue? If so, that would be a reason not to depend on #5633.

Are there any new checksum functionalities necessary for this issue that aren’t included in #5633? Besides the functionality I mentioned above…

@vlsi

This comment has been minimized.

Copy link
Contributor Author

@vlsi vlsi commented Sep 5, 2019

Here's a true story that could have been prevented by use of PGP for verification: https://twitter.com/jakewharton/status/1073102730443526144, https://blog.autsoft.hu/a-confusing-dependency/

@vlsi

This comment has been minimized.

Copy link
Contributor Author

@vlsi vlsi commented Sep 6, 2019

I've implemented keyserver failover, and it looks like it is important to resolve public keys in parallel.

Sample results.

Caffeine:

PGP key resolution time: 23975ms, resolution requests: 146, keys downloaded: 86
PGP verification time: 1240ms, files processed: 146, processed: 107MiB, skipped: 77MiB

Apache JMeter:

PGP key resolution time: 27578ms, resolution requests: 150, keys downloaded: 77
PGP verification time: 1757ms, files processed: 150, processed: 98MiB, skipped: 1309MiB
@vlsi

This comment has been minimized.

Copy link
Contributor Author

@vlsi vlsi commented Nov 6, 2019

This is relevant: raphw/byte-buddy#721 (comment)

@MartinDevi

This comment has been minimized.

Copy link

@MartinDevi MartinDevi commented Nov 13, 2019

I'd be very interested in this feature as well, and in general I think there's a lot of things that could be done in order to protect Gradle projects against malicious attacks through dependencies. Has any progress been made?

There's a few scenarios where this verification could be applied:

  • Configure automatic hash verification of downloaded artifacts, when available through Maven
  • Configure automatic PGP verification of downloaded artifacts, when available through Maven
  • Configure required fingerprints for PGP keys
  • Configure required constant hash for specific artifacts
  • Configure required PGP signature key local file for specific artifacts

These requirements could be configurable for artifacts belonging to a specific group, module, optionally also version. It could also be useful to define them for specific repositories, for example so that they're not required for Maven local.

I think there's a parallel between this issue and #1369 which is interesting to consider when designing the solution to this one. On a side note, IMO having the ability to enforce signatures is much more valuable and makes the ability to specify specific repositories for dependencies practically obsolete.

I'm curious to start a discussion about how this would take form in the API.

@vlsi

This comment has been minimized.

Copy link
Contributor Author

@vlsi vlsi commented Nov 13, 2019

Has any progress been made?

@MartinDevi, please check the issue description:

I've implemented https://github.com/vlsi/vlsi-release-plugins/tree/master/plugins/checksum-dependency-plugin

There's a ready to use plugin, and there's probably something being developed in Gradle Core.

@melix

This comment has been minimized.

Copy link
Member

@melix melix commented Nov 13, 2019

Indeed. I have spiked a built-in mechanism, but in the meantime, @vlsi 's plugin is the way to go.

@melix

This comment has been minimized.

Copy link
Member

@melix melix commented Dec 24, 2019

Feature has been merged into master, it's going to ship in Gradle 6.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.