Skip to content

Commit

Permalink
Merge pull request #23946 Trusted GPG-key should only accept 160-bit …
Browse files Browse the repository at this point in the history
…fingerprints

Backport from #23667 to 6.9.4

Fixes #23945

Co-authored-by: Balint Hegyi <bhegyi@gradle.com>
  • Loading branch information
bot-gradle and hegyibalint committed Feb 20, 2023
2 parents aba2e18 + ff1d3bb commit bf3cc0f
Show file tree
Hide file tree
Showing 14 changed files with 408 additions and 129 deletions.
Expand Up @@ -27,7 +27,7 @@ import spock.lang.Unroll
import static org.gradle.security.fixtures.SigningFixtures.getValidPublicKeyLongIdHexString
import static org.gradle.security.fixtures.SigningFixtures.signAsciiArmored
import static org.gradle.security.fixtures.SigningFixtures.validPublicKeyHexString
import static org.gradle.security.internal.SecuritySupport.toLongIdHexString
import static org.gradle.security.internal.SecuritySupport.toHexString

class DependencyVerificationSignatureCheckIntegTest extends AbstractSignatureVerificationIntegrationTest {

Expand Down Expand Up @@ -60,36 +60,6 @@ class DependencyVerificationSignatureCheckIntegTest extends AbstractSignatureVer
outputContains("Dependency verification is an incubating feature.")
}

def "doesn't need checksums if signature is verified and trust using long id"() {
createMetadataFile {
keyServer(keyServerFixture.uri)
verifySignatures()
addTrustedKey("org:foo:1.0", validPublicKeyLongIdHexString)
addTrustedKey("org:foo:1.0", validPublicKeyLongIdHexString, "pom", "pom")
}

given:
javaLibrary()
uncheckedModule("org", "foo", "1.0") {
withSignature {
signAsciiArmored(it)
}
}
buildFile << """
dependencies {
implementation "org:foo:1.0"
}
"""

when:
serveValidKey()
succeeds ":compileJava"

then:
outputContains("Dependency verification is an incubating feature.")
}

@Unroll
def "if signature is verified and checksum is declared in configuration, verify checksum (terse output=#terse)"() {
createMetadataFile {
keyServer(keyServerFixture.uri)
Expand Down Expand Up @@ -490,8 +460,8 @@ If the artifacts are trustworthy, you will need to update the gradle/verificatio
createMetadataFile {
keyServer(keyServerFixture.uri)
verifySignatures()
addTrustedKeyByFileName("org:foo:1.0", "foo-1.0-classy.jar", trustedKey)
addTrustedKey("org:foo:1.0", trustedKey, "pom", "pom")
addTrustedKeyByFileName("org:foo:1.0", "foo-1.0-classy.jar", validPublicKeyHexString)
addTrustedKey("org:foo:1.0", validPublicKeyHexString, "pom", "pom")
}
given:
Expand Down Expand Up @@ -519,11 +489,6 @@ If the artifacts are trustworthy, you will need to update the gradle/verificatio
If the artifacts are trustworthy, you will need to update the gradle/verification-metadata.xml file by following the instructions at ${docsUrl}"""
where:
trustedKey << [
validPublicKeyHexString,
validPublicKeyLongIdHexString
]
}
@Unroll
Expand Down Expand Up @@ -575,7 +540,7 @@ This can indicate that a dependency has been compromised. Please carefully verif
def keyring = newKeyRing()
def secondServer = new KeyServer(temporaryFolder.createDir("keyserver-${UUID.randomUUID()}"))
secondServer.registerPublicKey(keyring.publicKey)
def pkId = toLongIdHexString(keyring.publicKey.keyID)
def pkId = toHexString(keyring.publicKey.fingerprint)
secondServer.start()
createMetadataFile {
keyServer(keyServerFixture.uri)
Expand Down Expand Up @@ -1145,7 +1110,7 @@ This can indicate that a dependency has been compromised. Please carefully verif
def "passes verification if an artifact is signed with multiple keys and one of them is ignored"() {
def keyring = newKeyRing()
keyServerFixture.registerPublicKey(keyring.publicKey)
def pkId = toLongIdHexString(keyring.publicKey.keyID)
def pkId = toHexString(keyring.publicKey.fingerprint)
createMetadataFile {
keyServer(keyServerFixture.uri)
verifySignatures()
Expand Down Expand Up @@ -1311,7 +1276,7 @@ If the artifacts are trustworthy, you will need to update the gradle/verificatio
def "can read public keys from keyring"() {
// key will not be published on the server fixture but available locally
def keyring = newKeyRing()
def pkId = toLongIdHexString(keyring.publicKey.keyID)
def pkId = toHexString(keyring.publicKey.fingerprint)

createMetadataFile {
keyServer(keyServerFixture.uri)
Expand Down
Expand Up @@ -16,7 +16,6 @@
package org.gradle.api.internal.artifacts.ivyservice.ivyresolve;

import org.gradle.StartParameter;
import org.gradle.api.GradleException;
import org.gradle.api.artifacts.component.ModuleComponentIdentifier;
import org.gradle.api.artifacts.result.ResolvedArtifactResult;
import org.gradle.api.artifacts.verification.DependencyVerificationMode;
Expand All @@ -28,6 +27,7 @@
import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.verification.writer.WriteDependencyVerificationFile;
import org.gradle.api.internal.artifacts.ivyservice.resolutionstrategy.ExternalResourceCachePolicy;
import org.gradle.api.internal.artifacts.repositories.resolver.MetadataFetchingCost;
import org.gradle.api.internal.artifacts.verification.exceptions.DependencyVerificationException;
import org.gradle.api.internal.artifacts.verification.signatures.SignatureVerificationServiceFactory;
import org.gradle.api.internal.component.ArtifactType;
import org.gradle.api.internal.properties.GradleProperties;
Expand Down Expand Up @@ -237,7 +237,7 @@ private FailureVerificationOverride(Exception error) {

@Override
public ModuleComponentRepository overrideDependencyVerification(ModuleComponentRepository original, String resolveContextName, ResolutionStrategyInternal resolutionStrategy) {
throw new GradleException("Dependency verification cannot be performed", error);
throw new DependencyVerificationException("Dependency verification cannot be performed", error);
}
}

Expand Down
Expand Up @@ -31,6 +31,7 @@
import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.ModuleComponentRepository;
import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.verification.report.DependencyVerificationReportWriter;
import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.verification.report.VerificationReport;
import org.gradle.api.internal.artifacts.verification.exceptions.DependencyVerificationException;
import org.gradle.api.internal.artifacts.verification.serializer.DependencyVerificationsXmlReader;
import org.gradle.api.internal.artifacts.verification.signatures.SignatureVerificationService;
import org.gradle.api.internal.artifacts.verification.signatures.SignatureVerificationServiceFactory;
Expand Down Expand Up @@ -94,8 +95,8 @@ public ChecksumAndSignatureVerificationOverride(BuildOperationExecutor buildOper
this.reportWriter = new DependencyVerificationReportWriter(gradleUserHome.toPath(), documentationRegistry, verificationsFile, verifier.getSuggestedWriteFlags(), reportsDirectory, gradlePropertiesFactory);
} catch (FileNotFoundException e) {
throw UncheckedException.throwAsUncheckedException(e);
} catch (InvalidUserDataException e) {
throw new InvalidUserDataException("Unable to read dependency verification metadata from " + verificationsFile, e.getCause());
} catch (DependencyVerificationException e) {
throw new DependencyVerificationException("Unable to read dependency verification metadata from " + verificationsFile, e.getCause());
}
this.signatureVerificationService = signatureVerificationServiceFactory.create(keyRingsFile, keyServers());
}
Expand Down
@@ -0,0 +1,72 @@
/*
* Copyright 2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.gradle.api.internal.artifacts.verification.exceptions;

import org.gradle.api.GradleException;
import org.gradle.api.artifacts.component.ModuleComponentIdentifier;
import org.gradle.internal.logging.text.TreeFormatter;

import java.util.function.Consumer;

public class ComponentVerificationException extends GradleException {

private final ModuleComponentIdentifier component;
private final Consumer<TreeFormatter> causeErrorFormatter;

/**
* Creates a new exception when a component cannot be verified - because of some reason.
*
* @param component the component which failed the verification
*/
public ComponentVerificationException(String message, ModuleComponentIdentifier component) {
super(message);
this.component = component;
this.causeErrorFormatter = null;
}

/**
* Creates a new exception when a component cannot be verified - because of some reason.
*
* @param component the component which failed the verification
* @param causeErrorFormatter a consumer, which will be called with a {@link TreeFormatter}, and can put extra details what happened
*/
public ComponentVerificationException(ModuleComponentIdentifier component, Consumer<TreeFormatter> causeErrorFormatter) {
this.component = component;
this.causeErrorFormatter = causeErrorFormatter;
}

@Override
public String getMessage() {
final TreeFormatter treeFormatter = new TreeFormatter();
// Add our header first
treeFormatter.node(
String.format(
"An error happened meanwhile verifying '%s:%s:%s':",
component.getGroup(), component.getModule(), component.getVersion()
)
);

if (this.causeErrorFormatter != null) {
treeFormatter.startChildren();
// Let the underlying exception explain the situation
causeErrorFormatter.accept(treeFormatter);
treeFormatter.endChildren();

}
return treeFormatter.toString();
}
}
@@ -0,0 +1,32 @@
/*
* Copyright 2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.gradle.api.internal.artifacts.verification.exceptions;

import org.gradle.api.GradleException;
import org.gradle.internal.exceptions.Contextual;

@Contextual
public class DependencyVerificationException extends GradleException {

public DependencyVerificationException(String message) {
super(message);
}

public DependencyVerificationException(String message, Throwable cause) {
super(message, cause);
}
}
@@ -0,0 +1,70 @@
/*
* Copyright 2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.gradle.api.internal.artifacts.verification.exceptions;

import org.gradle.api.GradleException;
import org.gradle.api.internal.DocumentationRegistry;
import org.gradle.internal.logging.text.TreeFormatter;

import java.util.List;

/**
* Exception class used when a GPG IDs were not correct.
*
* <p>
* An example is using short/long IDs instead of fingerprints when trusting keys
*/
public class InvalidGpgKeyIdsException extends GradleException {
private final List<String> wrongKeys;

/**
* Creates a new exception with a list of incorrect keys.
*
* @param wrongKeys the list of incorrect IDs, which will be nicely formatted as part of the exception messages so the user can find them
*/
public InvalidGpgKeyIdsException(List<String> wrongKeys) {
this.wrongKeys = wrongKeys;
}

/**
* Formats a nice error message by using a {@link TreeFormatter}.
*
* <p>
* Idea for this method is that you can pass a higher-level {@link TreeFormatter} into here, and get a coherent, nice error message printed out - so the user will see a nice chain of causes.
*/
public void formatMessage(TreeFormatter formatter) {
final DocumentationRegistry documentationRegistry = new DocumentationRegistry();
final String documentLink = documentationRegistry.getDocumentationFor("dependency_verification", "sec:understanding-signature-verification");

formatter.node(
String.format("The following trusted GPG IDs are not in a minimum 160-bit fingerprint format (see: %s):", documentLink)
);
formatter.startChildren();
wrongKeys
.stream()
.map(key -> String.format("'%s'", key))
.forEach(formatter::node);
formatter.endChildren();
}

@Override
public String getMessage() {
final TreeFormatter treeFormatter = new TreeFormatter();
formatMessage(treeFormatter);
return treeFormatter.toString();
}
}
Expand Up @@ -21,6 +21,7 @@
import org.gradle.api.artifacts.ModuleIdentifier;
import org.gradle.api.artifacts.component.ModuleComponentIdentifier;
import org.gradle.api.internal.artifacts.DefaultModuleIdentifier;
import org.gradle.api.internal.artifacts.verification.exceptions.DependencyVerificationException;
import org.gradle.api.internal.artifacts.verification.model.ChecksumKind;
import org.gradle.api.internal.artifacts.verification.model.IgnoredKey;
import org.gradle.api.internal.artifacts.verification.verifier.DependencyVerifier;
Expand Down Expand Up @@ -81,7 +82,7 @@ public static void readFromXml(InputStream in, DependencyVerifierBuilder builder
xmlReader.setContentHandler(handler);
xmlReader.parse(new InputSource(in));
} catch (Exception e) {
throw new InvalidUserDataException("Unable to read dependency verification metadata", e);
throw new DependencyVerificationException("Unable to read dependency verification metadata", e);
} finally {
try {
in.close();
Expand Down
Expand Up @@ -17,11 +17,14 @@

import com.google.common.collect.ImmutableList;
import org.gradle.api.artifacts.component.ModuleComponentIdentifier;
import org.gradle.api.internal.artifacts.verification.exceptions.InvalidGpgKeyIdsException;
import org.gradle.api.internal.artifacts.verification.model.IgnoredKey;
import org.gradle.internal.component.external.model.ModuleComponentArtifactIdentifier;

import javax.annotation.Nullable;
import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Set;
Expand Down Expand Up @@ -168,7 +171,19 @@ public static class TrustedKey extends TrustCoordinates {

TrustedKey(String keyId, @Nullable String group, @Nullable String name, @Nullable String version, @Nullable String fileName, boolean regex) {
super(group, name, version, fileName, regex);
this.keyId = keyId;

// The key is 160 bits long, encoded in base32 (case-insensitive characters).
//
// Base32 gives us 4 bits per character, so the whole fingerprint will be:
// (160 bits) / (4 bits / character) = 40 characters
//
// By getting ASCII bytes (aka. strictly 1 byte per character, no variable-length magic)
// we can safely check if the fingerprint is of the correct length.
if (keyId.getBytes(StandardCharsets.US_ASCII).length < 40) {
throw new InvalidGpgKeyIdsException(Collections.singletonList(keyId));
} else {
this.keyId = keyId;
}
}

public String getKeyId() {
Expand Down

0 comments on commit bf3cc0f

Please sign in to comment.