Bug 576705 - Disable pgp.trustedPublicKeys from IUsI20211103-1800

Only stick to trusted keys defined in profile as it's not safe to import
trusted keys from installed IUs because metadata is an attack vector
(Bug 577029)

Change-Id: I3d4c97b69d7b85a8f5aceed0c50383975f7f6f6a
Reviewed-on: https://git.eclipse.org/r/c/equinox/rt.equinox.p2/+/187264
Tested-by: Equinox Bot <equinox-bot@eclipse.org>
Reviewed-by: Mickael Istria <mistria@redhat.com>

2 files changed, 44 insertions, 43 deletions

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 e1a3d5f83..12bb4cee8 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
@@ -21,7 +21,8 @@ import java.util.*;
 import java.util.Map.Entry;
 import java.util.stream.Collectors;
 import org.bouncycastle.openpgp.*;
-import org.eclipse.core.runtime.*;
+import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.Status;
 import org.eclipse.equinox.internal.p2.artifact.processors.pgp.PGPSignatureVerifier;
 import org.eclipse.equinox.internal.p2.engine.*;
 import org.eclipse.equinox.p2.core.IProvisioningAgent;
@@ -29,7 +30,6 @@ import org.eclipse.equinox.p2.core.UIServices;
 import org.eclipse.equinox.p2.core.UIServices.TrustInfo;
 import org.eclipse.equinox.p2.engine.IProfile;
 import org.eclipse.equinox.p2.engine.IProfileRegistry;
-import org.eclipse.equinox.p2.query.QueryUtil;
 import org.eclipse.equinox.p2.repository.artifact.IArtifactDescriptor;
 import org.eclipse.osgi.service.security.TrustEngine;
 import org.eclipse.osgi.signedcontent.*;
@@ -290,8 +290,12 @@ public class CertificateChecker {
 		IProfile profile = agent.getService(IProfileRegistry.class).getProfile(IProfileRegistry.SELF);
 		Set<PGPPublicKey> store = new HashSet<>(
 				PGPSignatureVerifier.readPublicKeys(profile.getProperty(TRUSTED_KEY_STORE_PROPERTY)));
-		profile.query(QueryUtil.ALL_UNITS, new NullProgressMonitor()).forEach(
-				iu -> store.addAll(PGPSignatureVerifier.readPublicKeys(iu.getProperty(TRUSTED_KEY_STORE_PROPERTY))));
+		//// SECURITY ISSUE: next lines become an attack vector as we have no guarantee
+		//// the metadata of those IUs is safe/were signed.
+		//// https://bugs.eclipse.org/bugs/show_bug.cgi?id=576705#c4
+		// profile.query(QueryUtil.ALL_UNITS, new NullProgressMonitor()).forEach(
+		// iu ->
+		// store.addAll(PGPSignatureVerifier.readPublicKeys(iu.getProperty(TRUSTED_KEY_STORE_PROPERTY))));
 		store.forEach(PGPSignatureVerifier.keystore::addKey);
 		return store;
 	}
diff --git a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/engine/CertificateCheckerTest.java b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/engine/CertificateCheckerTest.java
index c9e4bc8a5..5f4049a5a 100644
--- a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/engine/CertificateCheckerTest.java
+++ b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/engine/CertificateCheckerTest.java
@@ -28,13 +28,7 @@ import org.eclipse.equinox.internal.p2.metadata.ArtifactKey;
 import org.eclipse.equinox.p2.core.IAgentLocation;
 import org.eclipse.equinox.p2.core.ProvisionException;
 import org.eclipse.equinox.p2.core.UIServices;
-import org.eclipse.equinox.p2.engine.IEngine;
-import org.eclipse.equinox.p2.engine.IProfile;
 import org.eclipse.equinox.p2.engine.IProfileRegistry;
-import org.eclipse.equinox.p2.engine.IProvisioningPlan;
-import org.eclipse.equinox.p2.engine.ProvisioningContext;
-import org.eclipse.equinox.p2.metadata.MetadataFactory;
-import org.eclipse.equinox.p2.metadata.MetadataFactory.InstallableUnitDescription;
 import org.eclipse.equinox.p2.metadata.Version;
 import org.eclipse.equinox.p2.repository.artifact.spi.ArtifactDescriptor;
 import org.eclipse.equinox.p2.tests.AbstractProvisioningTest;
@@ -287,37 +281,40 @@ public class CertificateCheckerTest extends AbstractProvisioningTest {
 		}
 	}
 
-	public void testPGPSignedArtifactTrustedKeyInInstalledIU() throws ProvisionException, IOException {
-		try {
-			// create a test profile
-			testAgent.registerService("FORCED_SELF", IProfileRegistry.SELF);
-			testAgent
-					.registerService(IAgentLocation.SERVICE_NAME,
-							new AgentLocation(Files.createTempDirectory(
-									CertificateCheckerTest.class.getName() + "testPGPSignedArtifactTrustedKey-profile")
-									.toUri()));
-			IProfile testProfile = testAgent.getService(IProfileRegistry.class).addProfile(IProfileRegistry.SELF);
-			// install an IU that declares trusted keys
-			InstallableUnitDescription desc = new InstallableUnitDescription();
-			desc.setProperty(CertificateChecker.TRUSTED_KEY_STORE_PROPERTY, PGP_PUBLIC_KEY);
-			desc.setId("unitWithTrustedKeys");
-			desc.setVersion(Version.create("1.0.0"));
-			IEngine engine = testAgent.getService(IEngine.class);
-			IProvisioningPlan plan = engine.createPlan(testProfile, new ProvisioningContext(testAgent));
-			plan.addInstallableUnit(MetadataFactory.createInstallableUnit(desc));
-			assertTrue(engine.perform(plan, getMonitor()).isOK());
-
-			unsigned = TestData.getFile("pgp/repoPGPOK/plugins", "blah_1.0.0.123456.jar");
-			ArtifactDescriptor artifactDescriptor = new ArtifactDescriptor(
-					new ArtifactKey("what", "ever", Version.create("1")));
-			artifactDescriptor.addProperties(Map.of(PGPSignatureVerifier.PGP_SIGNATURES_PROPERTY_NAME, PGP_SIGNATURE));
-			checker.add(Map.of(artifactDescriptor, unsigned));
-			System.getProperties().setProperty(EngineActivator.PROP_UNSIGNED_POLICY, EngineActivator.UNSIGNED_PROMPT);
-			IStatus result = checker.start();
-			assertTrue(result.isOK());
-			assertFalse(serviceUI.wasPrompted);
-		} finally {
-			System.getProperties().remove(EngineActivator.PROP_UNSIGNED_POLICY);
-		}
-	}
+	//// SECURITY ISSUE: next lines become an attack vector as we have no guarantee
+	//// the metadata of those IUs is safe/were signed.
+	//// https://bugs.eclipse.org/bugs/show_bug.cgi?id=576705#c4
+//	public void testPGPSignedArtifactTrustedKeyInInstalledIU() throws ProvisionException, IOException {
+//		try {
+//			// create a test profile
+//			testAgent.registerService("FORCED_SELF", IProfileRegistry.SELF);
+//			testAgent
+//					.registerService(IAgentLocation.SERVICE_NAME,
+//							new AgentLocation(Files.createTempDirectory(
+//									CertificateCheckerTest.class.getName() + "testPGPSignedArtifactTrustedKey-profile")
+//									.toUri()));
+//			IProfile testProfile = testAgent.getService(IProfileRegistry.class).addProfile(IProfileRegistry.SELF);
+//			// install an IU that declares trusted keys
+//			InstallableUnitDescription desc = new InstallableUnitDescription();
+//			desc.setProperty(CertificateChecker.TRUSTED_KEY_STORE_PROPERTY, PGP_PUBLIC_KEY);
+//			desc.setId("unitWithTrustedKeys");
+//			desc.setVersion(Version.create("1.0.0"));
+//			IEngine engine = testAgent.getService(IEngine.class);
+//			IProvisioningPlan plan = engine.createPlan(testProfile, new ProvisioningContext(testAgent));
+//			plan.addInstallableUnit(MetadataFactory.createInstallableUnit(desc));
+//			assertTrue(engine.perform(plan, getMonitor()).isOK());
+//
+//			unsigned = TestData.getFile("pgp/repoPGPOK/plugins", "blah_1.0.0.123456.jar");
+//			ArtifactDescriptor artifactDescriptor = new ArtifactDescriptor(
+//					new ArtifactKey("what", "ever", Version.create("1")));
+//			artifactDescriptor.addProperties(Map.of(PGPSignatureVerifier.PGP_SIGNATURES_PROPERTY_NAME, PGP_SIGNATURE));
+//			checker.add(Map.of(artifactDescriptor, unsigned));
+//			System.getProperties().setProperty(EngineActivator.PROP_UNSIGNED_POLICY, EngineActivator.UNSIGNED_PROMPT);
+//			IStatus result = checker.start();
+//			assertTrue(result.isOK());
+//			assertFalse(serviceUI.wasPrompted);
+//		} finally {
+//			System.getProperties().remove(EngineActivator.PROP_UNSIGNED_POLICY);
+//		}
+//	}
 }