diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/plugin.xml b/bundles/org.eclipse.equinox.p2.artifact.repository/plugin.xml index 879eace3bb..4be690540b 100644 --- a/bundles/org.eclipse.equinox.p2.artifact.repository/plugin.xml +++ b/bundles/org.eclipse.equinox.p2.artifact.repository/plugin.xml @@ -41,8 +41,9 @@ + algorithm="MD5" + id="md5" + warnInsecure="true"> + + + + Since 1.4.300 +Set to true if this algorithm is now considered as insecure. A warning will be logged when this algorithm is used. An artifact that has no checksums or checksums only for insecure algorithms will emit a warning to user. + + + diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/checksum/ChecksumUtilities.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/checksum/ChecksumUtilities.java index f45b7f2033..ca091e5ea1 100644 --- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/checksum/ChecksumUtilities.java +++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/checksum/ChecksumUtilities.java @@ -19,12 +19,13 @@ import java.security.NoSuchProviderException; import java.util.*; import java.util.Map.Entry; +import java.util.stream.Collectors; import org.eclipse.core.runtime.*; import org.eclipse.equinox.internal.p2.artifact.repository.Messages; +import org.eclipse.equinox.internal.p2.core.helpers.LogHelper; import org.eclipse.equinox.internal.p2.repository.Activator; import org.eclipse.equinox.internal.p2.repository.helpers.ChecksumHelper; import org.eclipse.equinox.internal.p2.repository.helpers.ChecksumProducer; -import org.eclipse.equinox.internal.provisional.p2.artifact.repository.processing.ProcessingStep; import org.eclipse.equinox.p2.repository.artifact.IArtifactDescriptor; import org.eclipse.equinox.p2.repository.artifact.spi.ProcessingStepDescriptor; import org.eclipse.osgi.util.NLS; @@ -43,8 +44,9 @@ public class ChecksumUtilities { * @throws IllegalArgumentException if property neither {@link IArtifactDescriptor#ARTIFACT_CHECKSUM} nor {@link IArtifactDescriptor#DOWNLOAD_CHECKSUM} * @see ChecksumHelper#getChecksums(IArtifactDescriptor, String) */ - public static Collection getChecksumVerifiers(IArtifactDescriptor descriptor, String property, Set checksumsToSkip) throws IllegalArgumentException { - Collection steps = new ArrayList<>(); + public static Collection getChecksumVerifiers(IArtifactDescriptor descriptor, + String property, Set checksumsToSkip) throws IllegalArgumentException { + Collection steps = new ArrayList<>(); Map checksums = ChecksumHelper.getChecksums(descriptor, property); IConfigurationElement[] checksumVerifierConfigurations = getChecksumComparatorConfigurations(); @@ -58,17 +60,25 @@ public static Collection getChecksumVerifiers(IArtifactDescripto if (checksumEntry.getKey().equals(checksumId)) { String checksumAlgorithm = checksumVerifierConfiguration.getAttribute("algorithm"); //$NON-NLS-1$ String providerName = checksumVerifierConfiguration.getAttribute("providerName"); //$NON-NLS-1$ - ChecksumVerifier checksumVerifier = new ChecksumVerifier(checksumAlgorithm, providerName, checksumId); + boolean insecure = Boolean.parseBoolean(checksumVerifierConfiguration.getAttribute("warnInsecure")); //$NON-NLS-1$ + ChecksumVerifier checksumVerifier = new ChecksumVerifier(checksumAlgorithm, providerName, checksumId, insecure); checksumVerifier.initialize(null, new ProcessingStepDescriptor(null, checksumEntry.getValue(), true), descriptor); - if (checksumVerifier.getStatus().isOK()) + if (checksumVerifier.getStatus().isOK()) { steps.add(checksumVerifier); - else - // TODO log something? - continue; + } else { + LogHelper.log(checksumVerifier.getStatus()); + } } } } + if (!steps.isEmpty() && steps.stream().allMatch(ChecksumVerifier::isInsecureAlgorithm)) { + LogHelper.log(new Status(IStatus.WARNING, Activator.ID, + NLS.bind(Messages.onlyInsecureDigestAlgorithmUsed, + steps.stream().map(ChecksumVerifier::getAlgorithmId).collect(Collectors.joining(",")), //$NON-NLS-1$ + descriptor.getArtifactKey()))); + } + return steps; } diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/checksum/ChecksumVerifier.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/checksum/ChecksumVerifier.java index 4d913c46d0..a11041bb49 100644 --- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/checksum/ChecksumVerifier.java +++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/checksum/ChecksumVerifier.java @@ -33,12 +33,14 @@ final public class ChecksumVerifier extends MessageDigestProcessingStep { final private String algorithmName; final private String providerName; final private String algorithmId; + private final boolean insecureAlgorithm; // public to access from tests - public ChecksumVerifier(String digestAlgorithm, String providerName, String algorithmId) { + public ChecksumVerifier(String digestAlgorithm, String providerName, String algorithmId, boolean insecure) { this.algorithmName = digestAlgorithm; this.providerName = providerName; this.algorithmId = algorithmId; + this.insecureAlgorithm = insecure; basicInitialize(null); } @@ -91,4 +93,7 @@ public String getAlgorithmId() { return algorithmId; } + public boolean isInsecureAlgorithm() { + return insecureAlgorithm; + } } diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/md5/MD5Verifier.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/md5/MD5Verifier.java index 1128de4966..8d7899788d 100644 --- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/md5/MD5Verifier.java +++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/md5/MD5Verifier.java @@ -70,7 +70,9 @@ private void basicInitialize(IProcessingStepDescriptor descriptor) { @Override protected void onClose(String digestString) { // if the hashes don't line up set the status to error. - if (!digestString.equals(expectedMD5)) + if (!digestString.equals(expectedMD5)) { setStatus(new Status(IStatus.ERROR, Activator.ID, ProvisionException.ARTIFACT_MD5_NOT_MATCH, NLS.bind(Messages.Error_unexpected_hash, expectedMD5, digestString), null)); + } + setStatus(new Status(IStatus.WARNING, Activator.ID, Messages.MD5_deprecated)); } } diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/md5/Messages.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/md5/Messages.java index 3a873e912b..32636d0b82 100644 --- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/md5/Messages.java +++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/md5/Messages.java @@ -23,6 +23,8 @@ public class Messages extends NLS { public static String Error_MD5_unavailable; public static String Error_unexpected_hash; + public static String MD5_deprecated; + static { // initialize resource bundle NLS.initializeMessages(BUNDLE_NAME, Messages.class); diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/md5/messages.properties b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/md5/messages.properties index ff14156885..1e2c7e8437 100644 --- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/md5/messages.properties +++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/md5/messages.properties @@ -15,3 +15,4 @@ Error_invalid_hash=MD5 value not available or incorrect size, {0}. Error_MD5_unavailable=Could not create MD5 algorithm. Error_unexpected_hash=MD5 hash is not as expected. Expected: {0} and found {1}. +MD5_deprecated=MD5 is a deprecated and unsafe checksum algorithm. Consider abandoning it in favor of safer checksums. \ No newline at end of file diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/Messages.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/Messages.java index f4eba31a38..993ad714bf 100644 --- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/Messages.java +++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/Messages.java @@ -71,6 +71,8 @@ public class Messages extends NLS { public static String calculateChecksum_ok; public static String calculateChecksum_error; public static String calculateChecksum_providerError; + public static String onlyInsecureDigestAlgorithmUsed; + public static String noDigestAlgorithmToVerifyDownload; static { // initialize resource bundles diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/RawMirrorRequest.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/RawMirrorRequest.java index 3e103e9ccc..823bfc6479 100644 --- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/RawMirrorRequest.java +++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/RawMirrorRequest.java @@ -20,7 +20,10 @@ import java.util.Collections; import org.eclipse.core.runtime.*; import org.eclipse.equinox.internal.p2.artifact.processors.checksum.ChecksumUtilities; +import org.eclipse.equinox.internal.p2.artifact.processors.checksum.ChecksumVerifier; import org.eclipse.equinox.internal.p2.artifact.repository.simple.SimpleArtifactRepository; +import org.eclipse.equinox.internal.p2.core.helpers.LogHelper; +import org.eclipse.equinox.internal.p2.repository.Activator; import org.eclipse.equinox.internal.p2.repository.Transport; import org.eclipse.equinox.internal.provisional.p2.artifact.repository.processing.ProcessingStep; import org.eclipse.equinox.internal.provisional.p2.artifact.repository.processing.ProcessingStepHandler; @@ -57,7 +60,7 @@ public void perform(IArtifactRepository sourceRepository, IProgressMonitor monit } IStatus status = transfer(targetDescriptor, sourceDescriptor, monitor); - // if ok, cancelled or transfer has already been done with the canonical form return with status set + // if ok, cancelled or transfer has already been done with the canonical form return with status set if (status.getSeverity() == IStatus.CANCEL) { setResult(status); return; @@ -85,9 +88,13 @@ public IArtifactDescriptor getArtifactDescriptor() { // Perform the mirror operation without any processing steps @Override protected IStatus getArtifact(IArtifactDescriptor artifactDescriptor, OutputStream destination, IProgressMonitor monitor) { - if (SimpleArtifactRepository.CHECKSUMS_ENABLED) { - Collection steps = ChecksumUtilities.getChecksumVerifiers(artifactDescriptor, IArtifactDescriptor.DOWNLOAD_CHECKSUM, Collections.emptySet()); + Collection steps = ChecksumUtilities.getChecksumVerifiers(artifactDescriptor, + IArtifactDescriptor.DOWNLOAD_CHECKSUM, Collections.emptySet()); + if (steps.isEmpty()) { + LogHelper.log(new Status(IStatus.WARNING, Activator.ID, + NLS.bind(Messages.noDigestAlgorithmToVerifyDownload, artifactDescriptor.getArtifactKey()))); + } ProcessingStep[] stepArray = steps.toArray(new ProcessingStep[steps.size()]); // TODO should probably be using createAndLink here ProcessingStepHandler handler = new ProcessingStepHandler(); diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/messages.properties b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/messages.properties index fa54c3f8cb..088703d333 100644 --- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/messages.properties +++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/messages.properties @@ -62,4 +62,7 @@ calculateChecksum_providerError=Checksum provider id={0} algorithm={1} provider= exception_unableToCreateParentDir = Unable to create parent directory. folder_artifact_not_file_repo=Artifact {0} is a folder but the repository is an archive or remote location. retryRequest=Download of {0} failed on repository {1}. Retrying. -error_copying_local_file=An error occurred copying file {0}. \ No newline at end of file +error_copying_local_file=An error occurred copying file {0}. + +onlyInsecureDigestAlgorithmUsed = Only insecure digest algorithms ({0}) are being used to verify {1}. Consider using a safer digest algorithm. +noDigestAlgorithmToVerifyDownload = No digest algorithm is available to verify download of {0}. \ No newline at end of file diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java index e58c5c6daf..d627642d05 100644 --- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java +++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java @@ -29,10 +29,12 @@ import org.eclipse.core.runtime.*; import org.eclipse.core.runtime.jobs.Job; import org.eclipse.equinox.internal.p2.artifact.processors.checksum.ChecksumUtilities; +import org.eclipse.equinox.internal.p2.artifact.processors.checksum.ChecksumVerifier; import org.eclipse.equinox.internal.p2.artifact.processors.pgp.PGPSignatureVerifier; import org.eclipse.equinox.internal.p2.artifact.repository.*; import org.eclipse.equinox.internal.p2.artifact.repository.Messages; import org.eclipse.equinox.internal.p2.core.helpers.FileUtils; +import org.eclipse.equinox.internal.p2.core.helpers.LogHelper; import org.eclipse.equinox.internal.p2.metadata.ArtifactKey; import org.eclipse.equinox.internal.p2.metadata.expression.CompoundIterator; import org.eclipse.equinox.internal.p2.metadata.index.IndexProvider; @@ -502,7 +504,13 @@ private OutputStream addPreSteps(ProcessingStepHandler handler, IArtifactDescrip steps.add(new ZipVerifierStep()); Set skipChecksums = DOWNLOAD_MD5_CHECKSUM_ENABLED ? Collections.emptySet() : Collections.singleton(ChecksumHelper.MD5); - addChecksumVerifiers(descriptor, steps, skipChecksums, IArtifactDescriptor.DOWNLOAD_CHECKSUM); + ArrayList downloadChecksumSteps = new ArrayList<>(); + addChecksumVerifiers(descriptor, downloadChecksumSteps, skipChecksums, IArtifactDescriptor.DOWNLOAD_CHECKSUM); + if (downloadChecksumSteps.isEmpty()) { + LogHelper.log(new Status(IStatus.WARNING, Activator.ID, + NLS.bind(Messages.noDigestAlgorithmToVerifyDownload, descriptor.getArtifactKey()))); + } + steps.addAll(downloadChecksumSteps); // Add steps here if needed if (steps.isEmpty()) @@ -514,7 +522,8 @@ private OutputStream addPreSteps(ProcessingStepHandler handler, IArtifactDescrip private void addChecksumVerifiers(IArtifactDescriptor descriptor, ArrayList steps, Set skipChecksums, String property) { if (CHECKSUMS_ENABLED) { - Collection checksumVerifiers = ChecksumUtilities.getChecksumVerifiers(descriptor, property, skipChecksums); + Collection checksumVerifiers = ChecksumUtilities.getChecksumVerifiers(descriptor, + property, skipChecksums); steps.addAll(checksumVerifiers); } } diff --git a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/processors/ChecksumUtilitiesTest.java b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/processors/ChecksumUtilitiesTest.java index c0cc0dce22..3f5b739874 100644 --- a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/processors/ChecksumUtilitiesTest.java +++ b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/processors/ChecksumUtilitiesTest.java @@ -26,7 +26,6 @@ import org.eclipse.equinox.internal.p2.artifact.processors.checksum.ChecksumVerifier; import org.eclipse.equinox.internal.p2.metadata.ArtifactKey; import org.eclipse.equinox.internal.p2.metadata.OSGiVersion; -import org.eclipse.equinox.internal.provisional.p2.artifact.repository.processing.ProcessingStep; import org.eclipse.equinox.p2.repository.artifact.IArtifactDescriptor; import org.eclipse.equinox.p2.repository.artifact.spi.ArtifactDescriptor; import org.junit.Before; @@ -70,10 +69,11 @@ public void buildArtifactDescriptor() { @Test public void testChecksumProperty() { - Collection checksumVerifiers = ChecksumUtilities.getChecksumVerifiers(artifactDescriptor, propertyType, emptySet()); + Collection checksumVerifiers = ChecksumUtilities.getChecksumVerifiers(artifactDescriptor, + propertyType, emptySet()); assertEquals(format("Verifier for property=%s", property), 1, checksumVerifiers.size()); - ChecksumVerifier verifier = (ChecksumVerifier) checksumVerifiers.iterator().next(); + ChecksumVerifier verifier = checksumVerifiers.iterator().next(); assertEquals(digestAlgorithm, verifier.getAlgorithmName()); assertEquals(algorithmId, verifier.getAlgorithmId()); assertEquals(value, verifier.getExpectedChecksum()); @@ -82,7 +82,8 @@ public void testChecksumProperty() { @Test public void testChecksumsToSkip() { - Collection checksumVerifiers = ChecksumUtilities.getChecksumVerifiers(artifactDescriptor, propertyType, singleton(algorithmId)); + Collection checksumVerifiers = ChecksumUtilities.getChecksumVerifiers(artifactDescriptor, + propertyType, singleton(algorithmId)); assertEquals(emptyList(), checksumVerifiers); } diff --git a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/processors/ChecksumVerifierTest.java b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/processors/ChecksumVerifierTest.java index f16eb28e49..87a7e53fbb 100644 --- a/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/processors/ChecksumVerifierTest.java +++ b/bundles/org.eclipse.equinox.p2.tests/src/org/eclipse/equinox/p2/tests/artifact/processors/ChecksumVerifierTest.java @@ -70,7 +70,7 @@ public void testInitialize() throws IOException, IllegalArgumentException, Secur expect(processingStepDescriptor.getData()).andReturn(checksum); replay(processingStepDescriptor); - ChecksumVerifier verifier = new ChecksumVerifier(digestAlgorithm, providerName, algorithmId); + ChecksumVerifier verifier = new ChecksumVerifier(digestAlgorithm, providerName, algorithmId, false); verifier.initialize(null, processingStepDescriptor, null); @@ -93,7 +93,7 @@ public void testInitialize_DownloadChecksum() throws IOException, IllegalArgumen expect(artifactDescriptor.getProperties()).andReturn(properties); replay(artifactDescriptor); - ChecksumVerifier verifier = new ChecksumVerifier(digestAlgorithm, providerName, algorithmId); + ChecksumVerifier verifier = new ChecksumVerifier(digestAlgorithm, providerName, algorithmId, false); verifier.initialize(null, processingStepDescriptor, artifactDescriptor); @@ -116,7 +116,7 @@ public void testInitialize_ArtifactChecksum() throws IOException, IllegalArgumen expect(artifactDescriptor.getProperty(not(eq(artifactProperty)))).andReturn(null).times(1, 2); replay(artifactDescriptor); - ChecksumVerifier verifier = new ChecksumVerifier(digestAlgorithm, providerName, algorithmId); + ChecksumVerifier verifier = new ChecksumVerifier(digestAlgorithm, providerName, algorithmId, false); verifier.initialize(null, processingStepDescriptor, artifactDescriptor);