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.java36
-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, 30 insertions, 53 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 0f2569b87..9c8caf9f9 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
@@ -22,6 +22,7 @@ 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
@@ -125,21 +126,19 @@ 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 signature of given keys, and at least 1 of this key is trusted
+// 2. verify artifact signature matches signture 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 {
- signatures = getSignatures(context);
+ signaturesToVerify = getSignatures(context);
} catch (Exception ex) {
setStatus(new Status(IStatus.ERROR, Activator.ID, Messages.Error_CouldNotLoadSignature, ex));
return;
}
- if (signatures.isEmpty()) {
+ if (signaturesToVerify.isEmpty()) {
setStatus(Status.OK_STATUS);
return;
}
@@ -147,21 +146,18 @@ 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 : signatures) {
+ for (PGPSignature signature : signaturesToVerify) {
PGPPublicKey publicKey = KNOWN_KEYS.getKey(signature.getKeyID());
- 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;
- }
+ 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;
}
}
}
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 03656bfbc..34aa50fc9 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,7 +96,6 @@ 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<>();
@@ -133,26 +132,14 @@ public class CertificateChecker {
if (!signatures.isEmpty()) {
if (trustedKeysIds.isEmpty() && !trustedKeys.get().isEmpty()) {
trustedKeysIds.addAll(trustedKeys.get().all().stream()
- .map(PGPPublicKey::getKeyID).collect(Collectors.toSet()));
+ .map(PGPPublicKey::getKeyID).map(Long::valueOf).collect(Collectors.toSet()));
}
- 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);
- }
+ 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()));
}
} else {
unsigned.put(artifact.getKey(), artifactFile);
@@ -211,9 +198,8 @@ public class CertificateChecker {
String[] details = EngineActivator.UNSIGNED_ALLOW.equals(policy) || unsigned.isEmpty() ? null
: 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(Long::toHexString)
+ if (untrustedPGPKeys != null && !untrustedPGPKeys.isEmpty()) {
+ return detail + untrustedPGPKeys.stream().map(PGPPublicKey::getKeyID).map(Long::toHexString)
.collect(Collectors.joining(", ", " [", "]")); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
}
return detail;
@@ -347,8 +333,8 @@ public class CertificateChecker {
}
// load from bundles providing capability
for (IConfigurationElement extension : RegistryFactory.getRegistry()
- .getConfigurationElementsFor(EngineActivator.ID + ".pgp")) { //$NON-NLS-1$
- if ("trustedKeys".equals(extension.getName())) { //$NON-NLS-1$
+ .getConfigurationElementsFor(EngineActivator.ID + ".pgp")) {
+ if ("trustedKeys".equals(extension.getName())) {
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 09d843f8c..36b50a545 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,17 +71,13 @@ 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.OK, status.getSeverity());
- // assertTrue(status.getMessage().contains("Public key not found for"));
+ assertEquals(IStatus.ERROR, 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 400ccdace..c010af306 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,10 +62,9 @@ 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");
- assertOK(mirrorStatus);
+ assertNotOK(mirrorStatus);
+ assertTrue(mirrorStatus.toString().contains("Public key not found"));
}
@Override

Back to the top