besu icon indicating copy to clipboard operation
besu copied to clipboard

Enable dependency checksum verification

Open diega opened this issue 3 years ago • 5 comments
trafficstars

PR description

This PR enables dependency verification using a mechanism introduced in Gradle 6.2.

The gradle/verification-metadata.xml file was autogenerated relying on the TOFU scheme.

  • HEAD~1: adds auto generated sha-256 checksums
  • HEAD: adds missing sha-256 checksums manually

The proposal lets pgp signatures out of scope just to check if we feel confortable with this verification step.

Accepting this PR means that every time a dependency is upgraded, this file needs to be updated too. I think this is a positive thing, since it will make the dev more aware of what the upgrade really implies dependecy wise.

Everytime you update a dependency, ./gradlew --write-verification-metadata pgp,sha256 + tasks_to_execute must be called in order to update the verification-metadata.xml file, but this command will only add entries, so the developer should be careful to remove the replaced entry to prevent perpetual grow of the file (more info in the official documentation).

Documentation

  • [x] I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

diega avatar Aug 10 '22 18:08 diega

Generally speaking, I really like this. Some ideas (not for this PR)

  • Is there a way to require all dependencies to be verified? The way I read this is it's only verifying what's in the build
  • Can we write a task to update values rather than append new values.
  • It would be nice if CODEOWNERS allowed multi-apprve, this would be a primary target.

I'll click approve later in the week, I want to give the consensys maintainers a few business days to digest this.

shemnon avatar Aug 30 '22 01:08 shemnon

Hi @shemnon, I tried commenting out an entry from the gradle/verification-metadata.xml file and it failed with a failed verification message, so I think once this file is present every dependency needs to be verified.

diega avatar Aug 30 '22 18:08 diega

Generally speaking, "I'm for it" and the following questions should not be considered blocking adoption.

  • What is the security actually being added?
    • Checksums are only partially sufficient in catching a supply chain issue, and even then they only compare against the published checksum. If an attacker pushes a bad artifact, can't they also push the corresponding checksum? Sometimes, not always- this quickly becomes a per-artifact problem, which the upgrader needs to address each time a dependency is upgraded. Might need comments in the verification file indicating the checksum source. Protecting against lousy attackers still has value!
    • GPG sig checking seems like a superior superset of security, but I appreciate adopting the "easier" one first. Do we have any idea how much of our supply chain even offers signed artifacts? Do we need to maintain a web of trust for the keys used to sign?
  • What are the maintenance frictions?
    • Developer must update file with new signingkeys/checksums and remove old ones on dependency upgrade. This is a reasonable amount of friction.
    • How does this work with an aggressively multi-module gradle project like Besu? I had a hard time understanding the documentation of the verification scope here (https://docs.gradle.org/current/userguide/dependency_verification.html#sec:verification-scope)
    • Did we figure out how much build overhead we're taking on?
    • I am unclear if transitive dependencies are also verified, but I am assuming they must or this loses almost all utility.

jflo avatar Aug 31 '22 14:08 jflo

@jflo I'm sorry for the late reply. Allow me to go through some of your comments

What is the security actually being added?

Regarding the supply chain protection of checksums, it's true that an attacker can compromise the original server, but this mechanism can protect other layers used for building, like proxies or tainted local caches. The verification file format supports, in the sha256 element, an origin field which originally is filled with Generated by Gradle but for the future that could be manually filled with the value from the officially provided artifacts (and that would probably be the recommended practice).

What are the maintenance frictions?

Regarding the overhead, for checksums verification this is negligible. For signatures, Gradle by default downloads all the required keys which certainly adds time to the build. One option could be to distribute a local keyring, with the potential security implications that this would carry. What is good is that verification can be disabled but it is not so granular i.e. you cannot disable just signatures and let the checksums run. By the time we add signatures, we can analyze if it makes sense to disable all verification by default and only make this strict on CI. About the scope, multi modules are well supported and transitive dependencies are also verified.

diega avatar Sep 19 '22 18:09 diega

Thanks for the responses Diego, if you can rebase this onto main maybe we can get it into the next RC.

jflo avatar Oct 05 '22 19:10 jflo

Any reason not to merge this one @jflo @diega ?

macfarla avatar Nov 10 '22 06:11 macfarla

@macfarla not for me. Let me rebase it just to see if there has been some new dependency

diega avatar Nov 10 '22 21:11 diega