Bug 575541 - [pgp] P2 should not fail completely if a public key is not
found
Treat missing keys as if no signatures are present and ensure that the
transfered artifact's metadata has no signature nor key metadata exactly
like an unsigned artifact.
Change-Id: Id079b4da0ef9acc7d7d8c538a7d57d4aa181c65a
Signed-off-by: Ed Merks <ed.merks@gmail.com>
Reviewed-on: https://git.eclipse.org/r/c/equinox/rt.equinox.p2/+/191120
Reviewed-by: Alexander Kurtakov <akurtako@redhat.com>
diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/pgp/Messages.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/pgp/Messages.java
index f02a440..4f522f0 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/pgp/Messages.java
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/pgp/Messages.java
@@ -23,7 +23,7 @@
public static String Error_CouldNotLoadSignature;
- public static String Error_publicKeyNotFound;
+ public static String Warning_publicKeyNotFound;
static {
// initialize resource bundle
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 d6267b6..b994b77 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
@@ -137,25 +137,25 @@
long keyID = signature.getKeyID();
Collection<PGPPublicKey> keys = keyService.getKeys(keyID);
if (keys.isEmpty()) {
- setStatus(new Status(IStatus.ERROR, Activator.ID,
- NLS.bind(Messages.Error_publicKeyNotFound, PGPPublicKeyService.toHex(keyID))));
- return;
- }
-
- try {
- PGPContentVerifierBuilder verifierBuilder = new BcPGPContentVerifierBuilderProvider()
- .get(signature.getKeyAlgorithm(), signature.getHashAlgorithm());
- List<PGPContentVerifier> verifiers = new ArrayList<>();
- signaturesToVerify.put(signature, verifiers);
- for (PGPPublicKey key : keys) {
- PGPContentVerifier verifier = verifierBuilder.build(key);
- verifierKeys.put(verifier, key);
- verifiers.add(verifier);
- signatureVerifiers.add(verifier.getOutputStream());
+ LogHelper.log(new Status(IStatus.WARNING, Activator.ID,
+ NLS.bind(Messages.Warning_publicKeyNotFound, PGPPublicKeyService.toHex(keyID),
+ context.getArtifactKey().getId())));
+ } else {
+ try {
+ PGPContentVerifierBuilder verifierBuilder = new BcPGPContentVerifierBuilderProvider()
+ .get(signature.getKeyAlgorithm(), signature.getHashAlgorithm());
+ List<PGPContentVerifier> verifiers = new ArrayList<>();
+ signaturesToVerify.put(signature, verifiers);
+ for (PGPPublicKey key : keys) {
+ PGPContentVerifier verifier = verifierBuilder.build(key);
+ verifierKeys.put(verifier, key);
+ verifiers.add(verifier);
+ signatureVerifiers.add(verifier.getOutputStream());
+ }
+ } catch (PGPException ex) {
+ setStatus(new Status(IStatus.ERROR, Activator.ID, ex.getMessage(), ex));
+ return;
}
- } catch (PGPException ex) {
- setStatus(new Status(IStatus.ERROR, Activator.ID, ex.getMessage(), ex));
- return;
}
}
}
diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/pgp/messages.properties b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/pgp/messages.properties
index 1d12500..09a1e72 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/pgp/messages.properties
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/pgp/messages.properties
@@ -13,4 +13,4 @@
Error_SignatureAfterKeyExpiration=The signature is created after the expiration time of the signature's key {0}
Error_SignatureAndFileDontMatch=The signature is invalid for current content
Error_CouldNotLoadSignature=The signatures could not be loaded
-Error_publicKeyNotFound=A public key with ID {0} needed to verify the signatures could not be found
+Warning_publicKeyNotFound=A public key with ID {0} needed to verify the signatures of {1} could not be found
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 2dd294f..dd33567 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
@@ -13,6 +13,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
@@ -20,7 +21,9 @@
import java.nio.file.Files;
import java.util.Set;
import org.eclipse.core.runtime.FileLocator;
+import org.eclipse.core.runtime.IAdaptable;
import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.equinox.internal.p2.artifact.processors.pgp.PGPSignatureVerifier;
import org.eclipse.equinox.internal.p2.metadata.ArtifactKey;
import org.eclipse.equinox.internal.provisional.p2.repository.DefaultPGPPublicKeyService;
@@ -73,6 +76,23 @@
return res;
}
+ private static class ArtifactOutputStream extends ByteArrayOutputStream implements IAdaptable {
+ IArtifactDescriptor descriptor = new ArtifactDescriptor(
+ new ArtifactKey("whatever", "whatever", Version.parseVersion("1.0.0")));
+
+ public IArtifactDescriptor getDescriptor() {
+ return descriptor;
+ }
+
+ @Override
+ public <T> T getAdapter(Class<T> adapter) {
+ if (adapter.isInstance(descriptor)) {
+ return adapter.cast(descriptor);
+ }
+ return null;
+ }
+ }
+
private String read(String resource) throws IOException, URISyntaxException {
return Files.readString(new File(FileLocator.toFileURL(getClass().getResource(resource)).toURI()).toPath());
}
@@ -84,6 +104,8 @@
@SuppressWarnings("resource")
PGPSignatureVerifier verifier = new PGPSignatureVerifier();
verifier.initialize(agentProvider.getAgent(), processingStepDescriptor, artifact);
+ ArtifactOutputStream artifactOutputStream = new ArtifactOutputStream();
+ verifier.link(artifactOutputStream, new NullProgressMonitor());
Assert.assertTrue(verifier.getStatus().toString(), verifier.getStatus().isOK());
try (InputStream bytes = getClass().getResourceAsStream("testArtifact")) {
bytes.transferTo(verifier);
@@ -91,6 +113,12 @@
Assert.assertTrue(verifier.getStatus().isOK());
verifier.close();
Assert.assertTrue(verifier.getStatus().isOK());
+
+ IArtifactDescriptor descriptor = artifactOutputStream.getDescriptor();
+ Assert.assertNotNull("Signatures should be present",
+ descriptor.getProperty(PGPSignatureVerifier.PGP_SIGNATURES_PROPERTY_NAME));
+ Assert.assertNotNull("Keys should be present",
+ descriptor.getProperty(PGPSignatureVerifier.PGP_SIGNER_KEYS_PROPERTY_NAME));
}
@Test
@@ -99,9 +127,21 @@
IArtifactDescriptor artifact = createArtifact("signed_by_signer_1", "public_signer2.pgp");
try (PGPSignatureVerifier verifier = new PGPSignatureVerifier()) {
verifier.initialize(agentProvider.getAgent(), processingStepDescriptor, artifact);
- IStatus status = verifier.getStatus();
- assertEquals(IStatus.ERROR, status.getSeverity());
- assertTrue(status.getMessage().matches("A public key.*could not be found.*"));
+ ArtifactOutputStream artifactOutputStream = new ArtifactOutputStream();
+ verifier.link(artifactOutputStream, new NullProgressMonitor());
+ Assert.assertTrue(verifier.getStatus().toString(), verifier.getStatus().isOK());
+ try (InputStream bytes = getClass().getResourceAsStream("testArtifact")) {
+ bytes.transferTo(verifier);
+ }
+ Assert.assertTrue(verifier.getStatus().isOK());
+ verifier.close();
+ Assert.assertTrue(verifier.getStatus().isOK());
+
+ IArtifactDescriptor descriptor = artifactOutputStream.getDescriptor();
+ Assert.assertNull("No signatures should be present",
+ descriptor.getProperty(PGPSignatureVerifier.PGP_SIGNATURES_PROPERTY_NAME));
+ Assert.assertNull("No keys should be present",
+ descriptor.getProperty(PGPSignatureVerifier.PGP_SIGNER_KEYS_PROPERTY_NAME));
}
}
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 fe9d5fc..0d4525b 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
@@ -19,14 +19,17 @@
import java.util.stream.Stream;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.NullProgressMonitor;
+import org.eclipse.equinox.internal.p2.artifact.processors.pgp.PGPSignatureVerifier;
import org.eclipse.equinox.internal.p2.artifact.repository.MirrorRequest;
import org.eclipse.equinox.internal.p2.metadata.ArtifactKey;
import org.eclipse.equinox.internal.provisional.p2.repository.DefaultPGPPublicKeyService;
import org.eclipse.equinox.p2.core.IAgentLocation;
import org.eclipse.equinox.p2.metadata.Version;
+import org.eclipse.equinox.p2.repository.artifact.IArtifactDescriptor;
import org.eclipse.equinox.p2.repository.artifact.IArtifactRepository;
import org.eclipse.equinox.p2.repository.spi.PGPPublicKeyService;
import org.eclipse.equinox.p2.tests.AbstractProvisioningTest;
+import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
@@ -49,17 +52,35 @@
@Test
public void testAllGood() throws Exception {
- IStatus mirrorStatus = performMirrorFrom("repoPGPOK");
+ MirrorRequest mirrorRequest = performMirrorFrom("repoPGPOK");
+ IStatus mirrorStatus = mirrorRequest.getResult();
assertOK(mirrorStatus);
+
+ IArtifactDescriptor[] artifactDescriptors = targetRepo.getArtifactDescriptors(mirrorRequest.getArtifactKey());
+ Assert.assertEquals(1, artifactDescriptors.length);
+ IArtifactDescriptor descriptor = artifactDescriptors[0];
+ Assert.assertNotNull("Signatures should be present",
+ descriptor.getProperty(PGPSignatureVerifier.PGP_SIGNATURES_PROPERTY_NAME));
+ Assert.assertNotNull("Keys should be present",
+ descriptor.getProperty(PGPSignatureVerifier.PGP_SIGNER_KEYS_PROPERTY_NAME));
}
@Test
public void testAllGoodWithEncodedProperties() throws Exception {
- IStatus mirrorStatus = performMirrorFrom("repoPGPOK_encoded");
+ MirrorRequest mirrorRequest = performMirrorFrom("repoPGPOK_encoded");
+ IStatus mirrorStatus = mirrorRequest.getResult();
assertOK(mirrorStatus);
+
+ IArtifactDescriptor[] artifactDescriptors = targetRepo.getArtifactDescriptors(mirrorRequest.getArtifactKey());
+ Assert.assertEquals(1, artifactDescriptors.length);
+ IArtifactDescriptor descriptor = artifactDescriptors[0];
+ Assert.assertNotNull("Signatures should be present",
+ descriptor.getProperty(PGPSignatureVerifier.PGP_SIGNATURES_PROPERTY_NAME));
+ Assert.assertNotNull("Keys should be present",
+ descriptor.getProperty(PGPSignatureVerifier.PGP_SIGNER_KEYS_PROPERTY_NAME));
}
- private IStatus performMirrorFrom(String repoName) throws Exception {
+ private MirrorRequest performMirrorFrom(String repoName) throws Exception {
// Clear the remembered keys/cache of the agent.
IAgentLocation agentLocation = getAgent().getService(IAgentLocation.class);
Path repositoryCache = Paths
@@ -84,14 +105,22 @@
ArtifactKey key = new ArtifactKey("osgi.bundle", "blah", Version.create("1.0.0.123456"));
MirrorRequest mirrorRequest = new MirrorRequest(key, targetRepo, NO_PROPERTIES, NO_PROPERTIES, getTransport());
mirrorRequest.perform(sourceRepo, getMonitor());
- return mirrorRequest.getResult();
+ return mirrorRequest;
}
@Test
public void testMissingPublicKey() throws Exception {
- IStatus mirrorStatus = performMirrorFrom("repoMissingPublicKey");
- assertNotOK(mirrorStatus);
- assertTrue(mirrorStatus.toString().matches(".*public key.*not be found.*"));
+ MirrorRequest mirrorRequest = performMirrorFrom("repoMissingPublicKey");
+ IStatus mirrorStatus = mirrorRequest.getResult();
+ assertOK(mirrorStatus);
+
+ IArtifactDescriptor[] artifactDescriptors = targetRepo.getArtifactDescriptors(mirrorRequest.getArtifactKey());
+ Assert.assertEquals(1, artifactDescriptors.length);
+ IArtifactDescriptor descriptor = artifactDescriptors[0];
+ Assert.assertNull("Signatures should not be present",
+ descriptor.getProperty(PGPSignatureVerifier.PGP_SIGNATURES_PROPERTY_NAME));
+ Assert.assertNull("Keys should not be present",
+ descriptor.getProperty(PGPSignatureVerifier.PGP_SIGNER_KEYS_PROPERTY_NAME));
}
@Override
