Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/pgp/PGPSignatureVerifier.java34
-rw-r--r--bundles/org.eclipse.equinox.p2.engine/src/org/eclipse/equinox/internal/p2/engine/phases/CertificateChecker.java41
-rw-r--r--bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/processors/PGPSignatureVerifierTest.java8
-rw-r--r--bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/PGPVerifierTest.java5
4 files changed, 59 insertions, 29 deletions
diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/pgp/PGPSignatureVerifier.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/pgp/PGPSignatureVerifier.java
index d752d1b61..5a06ec3b7 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/pgp/PGPSignatureVerifier.java
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/pgp/PGPSignatureVerifier.java
@@ -21,7 +21,6 @@ import org.eclipse.equinox.internal.p2.artifact.repository.Activator;
import org.eclipse.equinox.internal.provisional.p2.artifact.repository.processing.ProcessingStep;
import org.eclipse.equinox.p2.core.IProvisioningAgent;
import org.eclipse.equinox.p2.repository.artifact.*;
-import org.eclipse.osgi.util.NLS;
/**
* This processing step verifies PGP signatures are correct (ie artifact was not
@@ -76,19 +75,21 @@ public final class PGPSignatureVerifier extends ProcessingStep {
IArtifactDescriptor context) {
super.initialize(agent, descriptor, context);
// 1. verify declared public keys have signature from a trusted key, if so, add to KeyStore
-// 2. verify artifact signature matches signture of given keys, and at least 1 of this key is trusted
+// 2. verify artifact signature matches signature of given keys, and at least 1 of this key is trusted
String signatureText = unnormalizedPGPProperty(context.getProperty(PGP_SIGNATURES_PROPERTY_NAME));
if (signatureText == null) {
setStatus(Status.OK_STATUS);
return;
}
+
+ Collection<PGPSignature> signatures;
try {
- signaturesToVerify = getSignatures(context);
+ signatures = getSignatures(context);
} catch (Exception ex) {
setStatus(new Status(IStatus.ERROR, Activator.ID, Messages.Error_CouldNotLoadSignature, ex));
return;
}
- if (signaturesToVerify.isEmpty()) {
+ if (signatures.isEmpty()) {
setStatus(Status.OK_STATUS);
return;
}
@@ -96,18 +97,21 @@ public final class PGPSignatureVerifier extends ProcessingStep {
IArtifactRepository repository = context.getRepository();
KNOWN_KEYS.addKeys(context.getProperty(PGP_SIGNER_KEYS_PROPERTY_NAME),
repository != null ? repository.getProperty(PGP_SIGNER_KEYS_PROPERTY_NAME) : null);
- for (PGPSignature signature : signaturesToVerify) {
+ for (PGPSignature signature : signatures) {
PGPPublicKey publicKey = KNOWN_KEYS.getKey(signature.getKeyID());
- if (publicKey == null) {
- setStatus(new Status(IStatus.ERROR, Activator.ID,
- NLS.bind(Messages.Error_publicKeyNotFound, Long.toHexString(signature.getKeyID()))));
- return;
- }
- try {
- signature.init(new BcPGPContentVerifierBuilderProvider(), publicKey);
- } catch (PGPException ex) {
- setStatus(new Status(IStatus.ERROR, Activator.ID, ex.getMessage(), ex));
- return;
+ if (publicKey != null) {
+ // Signatures without known a corresponding key will be treated like unsigned
+ // content.
+ try {
+ if (signaturesToVerify == null) {
+ signaturesToVerify = new ArrayList<>();
+ }
+ signature.init(new BcPGPContentVerifierBuilderProvider(), publicKey);
+ signaturesToVerify.add(signature);
+ } catch (PGPException ex) {
+ setStatus(new Status(IStatus.ERROR, Activator.ID, ex.getMessage(), ex));
+ return;
+ }
}
}
}
diff --git a/bundles/org.eclipse.equinox.p2.engine/src/org/eclipse/equinox/internal/p2/engine/phases/CertificateChecker.java b/bundles/org.eclipse.equinox.p2.engine/src/org/eclipse/equinox/internal/p2/engine/phases/CertificateChecker.java
index 92aee7199..83aef67a9 100644
--- a/bundles/org.eclipse.equinox.p2.engine/src/org/eclipse/equinox/internal/p2/engine/phases/CertificateChecker.java
+++ b/bundles/org.eclipse.equinox.p2.engine/src/org/eclipse/equinox/internal/p2/engine/phases/CertificateChecker.java
@@ -96,6 +96,7 @@ public class CertificateChecker {
private IStatus checkCertificates(SignedContentFactory verifierFactory) {
UIServices serviceUI = agent.getService(UIServices.class);
ArrayList<Certificate> untrustedCertificates = new ArrayList<>();
+ Map<IArtifactDescriptor, Collection<Long>> untrustedPGPKeyIDArtifacts = new HashMap<>();
Map<IArtifactDescriptor, Collection<PGPPublicKey>> untrustedPGPArtifacts = new HashMap<>();
Map<IArtifactDescriptor, File> unsigned = new HashMap<>();
ArrayList<Certificate[]> untrustedChain = new ArrayList<>();
@@ -132,14 +133,26 @@ public class CertificateChecker {
if (!signatures.isEmpty()) {
if (trustedKeysIds.isEmpty() && !trustedKeys.get().isEmpty()) {
trustedKeysIds.addAll(trustedKeys.get().all().stream()
- .map(PGPPublicKey::getKeyID).map(Long::valueOf).collect(Collectors.toSet()));
+ .map(PGPPublicKey::getKeyID).collect(Collectors.toSet()));
}
- if (signatures.stream().map(PGPSignature::getKeyID).noneMatch(trustedKeysIds::contains)) {
- untrustedPGPArtifacts.put(artifact.getKey(),
- signatures.stream().map(PGPSignature::getKeyID)
- .map(id -> findKey(id, artifact.getKey()))
- .filter(Objects::nonNull)
- .collect(Collectors.toList()));
+ Set<Long> untrustedKeyIds = signatures.stream().map(PGPSignature::getKeyID)
+ .collect(Collectors.toCollection(LinkedHashSet::new));
+ // If any key is already trusted, then we don't need to prompt for any of the
+ // other keys.
+ if (!untrustedKeyIds.removeAll(trustedKeysIds)) {
+ untrustedPGPKeyIDArtifacts.put(artifact.getKey(), untrustedKeyIds);
+ List<PGPPublicKey> untrustedKeys = untrustedKeyIds.stream()
+ .map(id -> findKey(id, artifact.getKey()))
+ .filter(Objects::nonNull)
+ .collect(Collectors.toList());
+ if (untrustedKeys.isEmpty()) {
+ // If no keys can be found for any of the signatures, treat the artifact like
+ // unsigned content because none of the signatures can actually be verified.
+ unsigned.put(artifact.getKey(), artifactFile);
+ } else {
+ // Otherwise, one of these keys needs to be trusted.
+ untrustedPGPArtifacts.put(artifact.getKey(), untrustedKeys);
+ }
}
} else {
unsigned.put(artifact.getKey(), artifactFile);
@@ -196,7 +209,15 @@ public class CertificateChecker {
}
String[] details = EngineActivator.UNSIGNED_ALLOW.equals(policy) || unsigned.isEmpty() ? null
- : unsigned.values().stream().map(Object::toString).toArray(String[]::new);
+ : unsigned.entrySet().stream().map(entry -> {
+ String detail = entry.getValue().toString();
+ Collection<Long> unknownIds = untrustedPGPKeyIDArtifacts.get(entry.getKey());
+ if (unknownIds != null) {
+ return detail + unknownIds.stream().map(Objects::toString)
+ .collect(Collectors.joining(", ", " [", "]")); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+ }
+ return detail;
+ }).toArray(String[]::new);
Certificate[][] unTrustedCertificateChains = untrustedCertificates.isEmpty() ? null
: untrustedChain.toArray(Certificate[][]::new);
// If there was no unsigned content, and nothing untrusted, no need to prompt.
@@ -326,8 +347,8 @@ public class CertificateChecker {
}
// load from bundles providing capability
for (IConfigurationElement extension : RegistryFactory.getRegistry()
- .getConfigurationElementsFor(EngineActivator.ID + ".pgp")) {
- if ("trustedKeys".equals(extension.getName())) {
+ .getConfigurationElementsFor(EngineActivator.ID + ".pgp")) { //$NON-NLS-1$
+ if ("trustedKeys".equals(extension.getName())) { //$NON-NLS-1$
String pathInBundle = extension.getAttribute("path"); //$NON-NLS-1$
if (pathInBundle != null) {
Stream.of(EngineActivator.getContext().getBundles())
diff --git a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/processors/PGPSignatureVerifierTest.java b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/processors/PGPSignatureVerifierTest.java
index 36b50a545..09d843f8c 100644
--- a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/processors/PGPSignatureVerifierTest.java
+++ b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/processors/PGPSignatureVerifierTest.java
@@ -71,13 +71,17 @@ public class PGPSignatureVerifierTest {
@Test
public void testNoPublicKeyFound() throws Exception {
+ // To address https://bugs.eclipse.org/bugs/show_bug.cgi?id=575541 a signature
+ // for which no key can be found is ignored.
+ // Such an artifact will be treated the same as an unsigned artifact.
+ // The missing key information will be in the details presented to the user.
IProcessingStepDescriptor processingStepDescriptor = new ProcessingStepDescriptor(null, null, false);
IArtifactDescriptor artifact = createArtifact("signed_by_signer_1", "public_signer2.pgp");
try (PGPSignatureVerifier verifier = new PGPSignatureVerifier()) {
verifier.initialize(null, processingStepDescriptor, artifact);
IStatus status = verifier.getStatus();
- assertEquals(IStatus.ERROR, status.getSeverity());
- assertTrue(status.getMessage().contains("Public key not found for"));
+ assertEquals(IStatus.OK, status.getSeverity());
+ // assertTrue(status.getMessage().contains("Public key not found for"));
}
}
diff --git a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/PGPVerifierTest.java b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/PGPVerifierTest.java
index c010af306..400ccdace 100644
--- a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/PGPVerifierTest.java
+++ b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/repository/PGPVerifierTest.java
@@ -62,9 +62,10 @@ public class PGPVerifierTest extends AbstractProvisioningTest {
@Test
public void testMissingPublicKey() throws Exception {
+ // See https://bugs.eclipse.org/bugs/show_bug.cgi?id=575541
+ // Missing PGP keys are ignored and the content is treated as unsigned.
IStatus mirrorStatus = performMirrorFrom("repoMissingPublicKey");
- assertNotOK(mirrorStatus);
- assertTrue(mirrorStatus.toString().contains("Public key not found"));
+ assertOK(mirrorStatus);
}
@Override

Back to the top