Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorEd Merks2022-01-13 10:33:44 +0000
committerEd Merks2022-01-13 14:15:20 +0000
commitc193e5197535846f3f546f0e9ce210660c190484 (patch)
treeb532bca39279af99cde2e0ef6f0c66e7ff27ccbb
parentbcc7488f0d8c02b4a6e2f8dd4910fb6f629f8a82 (diff)
downloadrt.equinox.p2-c193e5197535846f3f546f0e9ce210660c190484.tar.gz
rt.equinox.p2-c193e5197535846f3f546f0e9ce210660c190484.tar.xz
rt.equinox.p2-c193e5197535846f3f546f0e9ce210660c190484.zip
Bug 575541 - [pgp] P2 should not fail completely if a public key is notY20220114-0600Y20220113-0900I20220114-1800I20220113-1800
found Ignore signatures for which no corresponding key can be found. If all signatures are ignored, the artifact will be effectively treated the same as an unsigned artifact. In the unsigned artifact details presented to the user, include the key ID(s) of the ignored signature(s). Update the test to expect the artifact request status to be OK. Change-Id: If433a81144d2eafed51e12d38396e9ffa5b09787 Signed-off-by: Ed Merks <ed.merks@gmail.com> Reviewed-on: https://git.eclipse.org/r/c/equinox/rt.equinox.p2/+/189588 Tested-by: Equinox Bot <equinox-bot@eclipse.org>
-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