gradle icon indicating copy to clipboard operation
gradle copied to clipboard

./gradlew --write-verification-metadata sha256 should skip adding signed trusted artifacts

Open liutikas opened this issue 3 years ago • 7 comments

./gradlew --write-verification-metadata sha256 should skip adding signed trusted artifacts

Let's say you have gradle/verification-metadata.xml with trusted keys (example below)

<?xml version="1.0" encoding="UTF-8"?>
<verification-metadata xmlns="https://schema.gradle.org/dependency-verification" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="https://schema.gradle.org/dependency-verification https://schema.gradle.org/dependency-verification/dependency-verification-1.1.xsd">
   <configuration>
      <verify-metadata>true</verify-metadata>
      <verify-signatures>true</verify-signatures>
      <trusted-keys>
         <trusted-key id="c7cb325467893cc4" group="junit"/>
         <trusted-key id="efe8086f9e93774e" group="junit"/>
      </trusted-keys>
   </configuration>
</verification-metadata>

Then you run ./gradlew help -M sha256

Expected Behavior

entries are only created for artifacts that are not signed, or are signed but the key is not trusted

Current Behavior

Gradle generates entries for artifacts that are signed trusted keys (e.g. junit:junit in the example above).

Context

It is important to keep gradle/verification-metadata.xml minimal, to make reviews for it easy. Adding artifact entries for artifacts that do not actually need sha256 verification adds noise and is redundant.

Steps to Reproduce

It should be clear from instructions above.

Your Environment

Linux

Gradle 7.2

liutikas avatar Oct 08 '21 23:10 liutikas

Any more thoughts on this?

liutikas avatar May 19 '22 21:05 liutikas

When I change the "-M sha256" argument to "-M sha256,pgp" then I find that this no longer outputs a checksum for artifacts that are also trusted by signature. I think this means that with an argument of just "-M sha256" that Gradle doesn't currently consider pgp signatures when writing the verification metadata file.

It looks to me like the DependencyVerificationsXmlReader is updating the DependencyVerifierBuilder ( https://github.com/gradle/gradle/blob/d0c32a5dfc159832e213df016184f21ff8ebafe8/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/verification/writer/WriteDependencyVerificationFile.java#L259 ) but that WriteDependencyVerificationFile doesn't consider this when determining whether the artifact is already trusted by a key ( https://github.com/gradle/gradle/blob/d0c32a5dfc159832e213df016184f21ff8ebafe8/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/verification/writer/WriteDependencyVerificationFile.java#L307 )

Maybe we can update WriteDependencyVerificationFile to also make use of the information read by DependencyVerificationsXmlReader when considering whether an artifact is trusted

mathjeff avatar Sep 01 '22 19:09 mathjeff

@asodja WDYT?

liutikas avatar Sep 01 '22 21:09 liutikas

@asodja I think you mentioned at some point wanting some sort of flag for this. Do you know what the situation is where we would want to add checksums for artifacts that already have matching, trusted signatures?

Also interesting, -M sha256,pgp appears to mean "write signature trust entries for all artifacts that can use it, and write checksums for any artifacts that remain untrusted"

whereas -M sha256 appears to mean "write checksums for all artifacts that don't have them"

So, another possible option could be to determine when someone runs -M sha256 if there are already signatures in the verification metadata file and potentially print a message reminding them that they're running a command that adds checksums for all artifacts even if already trusted by signature

mathjeff avatar Sep 09 '22 19:09 mathjeff

Also slightly related, I managed to make a change at here that seems to address this issue as described but without the part about a flag from the previous comment yet

mathjeff avatar Sep 09 '22 19:09 mathjeff

Do you know what the situation is where we would want to add checksums for artifacts that already have matching, trusted signatures?

I remember #21272 where user would want to have both. Now I would have to think a bit more (or ask author of that issue), to see if there is any security benefit, since signature already checks the integrity of an artifact.

Also interesting, -M sha256,pgp appears to mean "write signature trust entries for all artifacts that can use it, and write checksums for any artifacts that remain untrusted" Whereas -M sha256 appears to mean "write checksums for all artifacts that don't have them"

Yeah, you noticed it right. That -M has two different behaviours unfortunately. When you add pgp it changes the original behaviour. With pgp first shaXYZ is used as a fallback for artifacts that are not signed . Maybe it would be better to have a bit different syntax for pgp with a fallback. Maybe: -M pgp|sha256,sha256 -> that would mean use pgp with sha256 as fallback and also write sha256. Or maybe having a separate flag, i.e.: --pgp-fallback. Not sure what would be the best solution for this.


My thoughts on ./gradlew --write-verification-metadata sha256 should skip adding signed trusted artifacts and having an option to write even signed artifacts to a verification-metadata.xml: I would guess that some users would want to have all artifacts in the verification-metadata.xml even if they are signed. I think that since with that option you could check what artifacts Gradle is actually validating. I could always be wrong though.

asodja avatar Sep 10 '22 23:09 asodja

I talked to Aurimas today, and he mentioned that his main opinion is that we'd like to be able to just regenerate the verification metadata xml without any edits and (I think) that when this issue was opened,./gradlew --write-verification-metadata sha256 was introducing fewer differences.

Later (today) I tried just running ./gradlew --write-verification-metadata sha256,pgp and I found that it introduced some differences. Today those differences appear to be because comments and reason fields (which aren't currently recognized by Gradle) are removed, see https://github.com/gradle/gradle/issues/20260

Today I also tried running ./gradlew --write-verification-metadata sha256 and found that it introduced the same set of differences. I think that means that some of the previous inconsistencies have been since resolved.

Perhaps this means for this issue we can just:

  1. Address https://github.com/gradle/gradle/issues/20260
  2. Make sure that users don't inadvertently leave out the ",pgp" when regenerating the verification metadata xml file, perhaps by leaving a short wrapper script in our git repository containing the appropriate regeneration command

which shouldn't make things worse for the case where another user wants signatures and checksums: https://github.com/gradle/gradle/issues/21272

Maybe to fully address that case we'd have to do something like what Anze was saying above about more syntaxes (https://github.com/gradle/gradle/issues/18569#issuecomment-1242817832)

mathjeff avatar Sep 19 '22 22:09 mathjeff

The way I've been thinking about this is that the SHA-256 is the ground truth. Maven repos are organized around the idea that artifacts are published then never changed. Using SHA-256 to verify fits perfectly with that idea, and is the most reliable way to verify since only one file can ever match that SHA-256 value. Signing keys can be compromised, so the PGP signature is worth less than the SHA-256. The ideal workflow as I see it would be to always store the SHA-256, then use the trusted keys to verify new artifacts.

If a team is actually manually inspecting each artifact they use, then the PGP signature adds no value, IMHO. But few teams actually do that. Trusting PGP signers for new artifacts, then using SHA-256 for verification provides a high level of security with much less work than manually inspecting all artifacts. And if that was well automated, then I could see a majority of developers using that workflow.

eighthave avatar Nov 29 '22 08:11 eighthave

While it is true that validating/tracking SHA256 for each artifact is the most explicit way to make sure that the dependencies are exactly what you expected them to be, it also provides a fairly heavy burden for it to be up to date, especially for large Gradle projects with thousands of dependencies. It causes merge conflicts, makes review fairly difficult.

Validating only PGP signature and not tracking SHA256 is a nice compromise that allows validating that the author of the artifacts have not changed from version to version without requiring explicit SHA256 updates on very version upgrade.

It is already how Gradle works, if a <trusted-key> is there and there is no <sha256> entry, it does what I am suggesting. The issue is that running gradlew to update the signatures also adds checksums, making it a manual step to undo the new entries added.

I would really love to see a flag that allows this updating behavior behavior.

liutikas avatar Mar 06 '24 19:03 liutikas