Feature/enforce hash validation for uploads (#1158)

* moved artifact duplication check after hash check

Signed-off-by: Robert Sing <robert.sing@bosch-si.com>

* added unit tests

Signed-off-by: Robert Sing <robert.sing@bosch-si.com>

* fixed copyright header

Signed-off-by: Robert Sing <robert.sing@bosch-si.com>

* fixed review finding

Signed-off-by: Robert Sing <robert.sing@bosch-si.com>

hawkbit-core/src/main/java/org/eclipse/hawkbit/artifact/repository/AbstractArtifactRepository.java

@@ -55,9 +55,9 @@ public abstract class AbstractArtifactRepository implements ArtifactRepository {
         }
 
         String tempFile = null;
-        try (final DigestInputStream inputstream = wrapInDigestInputStream(content, mdSHA1, mdMD5, mdSHA256)) {
+        try (final DigestInputStream inputStream = wrapInDigestInputStream(content, mdSHA1, mdMD5, mdSHA256)) {
 
-            tempFile = storeTempFile(inputstream);
+            tempFile = storeTempFile(inputStream);
 
             final String sha1Hash16 = BaseEncoding.base16().lowerCase().encode(mdSHA1.digest());
             final String md5Hash16 = BaseEncoding.base16().lowerCase().encode(mdMD5.digest());
@@ -65,6 +65,11 @@ public abstract class AbstractArtifactRepository implements ArtifactRepository {
 
             checkHashes(sha1Hash16, md5Hash16, sha256Hash16, providedHashes);
 
+            // Check if file with same sha1 hash exists and if so return it
+            if (existsByTenantAndSha1(tenant, sha1Hash16)) {
+                return addMissingHashes(getArtifactBySha1(tenant, sha1Hash16), sha1Hash16, md5Hash16, sha256Hash16);
+            }
+
             return store(sanitizeTenant(tenant), new DbArtifactHash(sha1Hash16, md5Hash16, sha256Hash16), contentType,
                     tempFile);
         } catch (final IOException e) {
@@ -76,6 +81,21 @@ public abstract class AbstractArtifactRepository implements ArtifactRepository {
         }
     }
 
+    private AbstractDbArtifact addMissingHashes(final AbstractDbArtifact existing, final String calculatedSha1,
+            final String calculatedMd5, final String calculatedSha256) {
+
+        final String sha1 = checkEmpty(existing.getHashes().getSha1(), calculatedSha1);
+        final String md5 = checkEmpty(existing.getHashes().getMd5(), calculatedMd5);
+        final String sha256 = checkEmpty(existing.getHashes().getSha256(), calculatedSha256);
+
+        existing.setHashes(new DbArtifactHash(sha1, md5, sha256));
+        return existing;
+    }
+
+    private String checkEmpty(final String value, final String fallback) {
+        return StringUtils.isEmpty(value) ? fallback : value;
+    }
+
     protected void deleteTempFile(final String tempFile) {
         final File file = new File(tempFile);
 
@@ -118,12 +138,14 @@ public abstract class AbstractArtifactRepository implements ArtifactRepository {
                     + " does not match the calculated md5 hash " + md5Hash16, HashNotMatchException.MD5);
         }
         if (areHashesNotMatching(providedHashes.getSha256(), sha256Hash16)) {
-            throw new HashNotMatchException("The given sha256 hash " + providedHashes.getSha256()
+            throw new HashNotMatchException(
-                    + " does not match the calculated sha256 hash " + sha256Hash16, HashNotMatchException.SHA256);
+                    "The given sha256 hash " + providedHashes.getSha256()
+                            + " does not match the calculated sha256 hash " + sha256Hash16,
+                    HashNotMatchException.SHA256);
         }
     }
 
-    private static boolean areHashesNotMatching(String providedHashValue, String hashValue) {
+    private static boolean areHashesNotMatching(final String providedHashValue, final String hashValue) {
         return providedHashValue != null && !hashValue.equals(providedHashValue);
     }
 

hawkbit-core/src/main/java/org/eclipse/hawkbit/artifact/repository/model/AbstractDbArtifact.java

@@ -19,10 +19,11 @@ import org.springframework.util.Assert;
 public abstract class AbstractDbArtifact {
 
     private final String artifactId;
-    private final DbArtifactHash hashes;
     private final long size;
     private final String contentType;
 
+    private DbArtifactHash hashes;
+
     protected AbstractDbArtifact(final String artifactId, final DbArtifactHash hashes, final long size,
             final String contentType) {
         Assert.notNull(artifactId, "Artifact ID cannot be null");
@@ -47,6 +48,13 @@ public abstract class AbstractDbArtifact {
         return hashes;
     }
 
+    /**
+     * Set hashes of the artifact
+     */
+    public void setHashes(final DbArtifactHash hashes) {
+        this.hashes = hashes;
+    }
+
     /**
      * @return site of the artifact in bytes
      */
@@ -62,8 +70,8 @@ public abstract class AbstractDbArtifact {
     }
 
     /**
-     * Creates an {@link InputStream} on this artifact. Caller has to take care
+     * Creates an {@link InputStream} on this artifact. Caller has to take care of
-     * of closing the stream. Repeatable calls open a new {@link InputStream}.
+     * closing the stream. Repeatable calls open a new {@link InputStream}.
      * 
      * @return {@link InputStream} to read from artifact.
      */

hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaArtifactManagement.java

@@ -43,7 +43,6 @@ import org.springframework.data.domain.Pageable;
 import org.springframework.retry.annotation.Backoff;
 import org.springframework.retry.annotation.Retryable;
 import org.springframework.transaction.annotation.Transactional;
-import org.springframework.util.StringUtils;
 import org.springframework.validation.annotation.Validated;
 
 /**
@@ -105,20 +104,8 @@ public class JpaArtifactManagement implements ArtifactManagement {
 
         assertArtifactQuota(moduleId, 1);
 
-        return getOrCreateArtifact(artifactUpload)
+        final AbstractDbArtifact artifact = storeArtifact(artifactUpload);
-                .map(artifact -> storeArtifactMetadata(softwareModule, filename, artifact, existing)).orElse(null);
+        return storeArtifactMetadata(softwareModule, filename, artifact, existing);
-    }
-
-    private Optional<AbstractDbArtifact> getOrCreateArtifact(final ArtifactUpload artifactUpload) {
-        final String providedSha1Sum = artifactUpload.getProvidedSha1Sum();
-
-        if (!StringUtils.isEmpty(providedSha1Sum)
-                && artifactRepository.existsByTenantAndSha1(tenantAware.getCurrentTenant(), providedSha1Sum)) {
-            return Optional
-                    .ofNullable(artifactRepository.getArtifactBySha1(tenantAware.getCurrentTenant(), providedSha1Sum));
-        }
-
-        return Optional.of(storeArtifact(artifactUpload));
     }
 
     private AbstractDbArtifact storeArtifact(final ArtifactUpload artifactUpload) {

hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/ArtifactManagementTest.java

@@ -16,6 +16,8 @@ import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.URISyntaxException;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
 import java.util.List;
 import java.util.Optional;
 import java.util.concurrent.Callable;
@@ -25,6 +27,7 @@ import javax.validation.ConstraintViolationException;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.RandomStringUtils;
 import org.eclipse.hawkbit.artifact.repository.model.AbstractDbArtifact;
+import org.eclipse.hawkbit.artifact.repository.model.DbArtifactHash;
 import org.eclipse.hawkbit.im.authentication.SpPermission;
 import org.eclipse.hawkbit.repository.ArtifactManagement;
 import org.eclipse.hawkbit.repository.event.remote.SoftwareModuleDeletedEvent;
@@ -32,6 +35,9 @@ import org.eclipse.hawkbit.repository.event.remote.entity.SoftwareModuleCreatedE
 import org.eclipse.hawkbit.repository.exception.AssignmentQuotaExceededException;
 import org.eclipse.hawkbit.repository.exception.FileSizeQuotaExceededException;
 import org.eclipse.hawkbit.repository.exception.InsufficientPermissionException;
+import org.eclipse.hawkbit.repository.exception.InvalidMD5HashException;
+import org.eclipse.hawkbit.repository.exception.InvalidSHA1HashException;
+import org.eclipse.hawkbit.repository.exception.InvalidSHA256HashException;
 import org.eclipse.hawkbit.repository.exception.StorageQuotaExceededException;
 import org.eclipse.hawkbit.repository.jpa.model.JpaArtifact;
 import org.eclipse.hawkbit.repository.jpa.model.JpaSoftwareModule;
@@ -46,6 +52,7 @@ import org.eclipse.hawkbit.repository.test.util.WithUser;
 import org.junit.jupiter.api.Test;
 
 import com.google.common.collect.Lists;
+import com.google.common.io.BaseEncoding;
 
 import io.qameta.allure.Description;
 import io.qameta.allure.Feature;
@@ -484,6 +491,103 @@ public class ArtifactManagementTest extends AbstractJpaIntegrationTest {
         }
     }
 
+    @Test
+    @Description("Verifies that creation of an artifact with none matching hashes fails.")
+    public void createArtifactWithNoneMatchingHashes() throws IOException, NoSuchAlgorithmException {
+        final SoftwareModule sm = testdataFactory.createSoftwareModuleOs();
+
+        final byte[] testData = RandomStringUtils.randomAlphanumeric(100).getBytes();
+        final DbArtifactHash artifactHashes = calcHashes(testData);
+
+        try (final InputStream inputStream = new ByteArrayInputStream(testData)) {
+            final ArtifactUpload artifactUploadWithInvalidSha1 = new ArtifactUpload(inputStream, sm.getId(),
+                    "test-file", artifactHashes.getMd5(), "sha1", artifactHashes.getSha256(), false, null,
+                    testData.length);
+            assertThatExceptionOfType(InvalidSHA1HashException.class)
+                    .isThrownBy(() -> artifactManagement.create(artifactUploadWithInvalidSha1));
+        }
+
+        try (final InputStream inputStream = new ByteArrayInputStream(testData)) {
+            final ArtifactUpload artifactUploadWithInvalidMd5 = new ArtifactUpload(inputStream, sm.getId(), "test-file",
+                    "md5", artifactHashes.getSha1(), artifactHashes.getSha256(), false, null, testData.length);
+            assertThatExceptionOfType(InvalidMD5HashException.class)
+                    .isThrownBy(() -> artifactManagement.create(artifactUploadWithInvalidMd5));
+        }
+
+        try (final InputStream inputStream = new ByteArrayInputStream(testData)) {
+            final ArtifactUpload artifactUploadWithInvalidSha256 = new ArtifactUpload(inputStream, sm.getId(),
+                    "test-file", artifactHashes.getMd5(), artifactHashes.getSha1(), "sha256", false, null,
+                    testData.length);
+            assertThatExceptionOfType(InvalidSHA256HashException.class)
+                    .isThrownBy(() -> artifactManagement.create(artifactUploadWithInvalidSha256));
+        }
+    }
+
+    @Test
+    @Description("Verifies that creation of an artifact with matching hashes works.")
+    public void createArtifactWithMatchingHashes() throws IOException, NoSuchAlgorithmException {
+        final SoftwareModule sm = testdataFactory.createSoftwareModuleOs();
+
+        final byte[] testData = RandomStringUtils.randomAlphanumeric(100).getBytes();
+        final DbArtifactHash artifactHashes = calcHashes(testData);
+
+        try (final InputStream inputStream = new ByteArrayInputStream(testData)) {
+            final ArtifactUpload artifactUpload = new ArtifactUpload(inputStream, sm.getId(), "test-file",
+                    artifactHashes.getMd5(), artifactHashes.getSha1(), artifactHashes.getSha256(), false, null,
+                    testData.length);
+            final Artifact createdArtifact = artifactManagement.create(artifactUpload);
+            assertThat(createdArtifact.getSha1Hash()).isEqualTo(artifactHashes.getSha1());
+            assertThat(createdArtifact.getMd5Hash()).isEqualTo(artifactHashes.getMd5());
+            assertThat(createdArtifact.getSha256Hash()).isEqualTo(artifactHashes.getSha256());
+        }
+
+        final Optional<AbstractDbArtifact> dbArtifact = artifactManagement.loadArtifactBinary(artifactHashes.getSha1());
+        assertThat(dbArtifact).isPresent();
+    }
+
+    @Test
+    @Description("Verifies that creation of an existing artifact returns a full hash list.")
+    public void createExistingArtifactReturnsFullHashList() throws IOException, NoSuchAlgorithmException {
+        final SoftwareModule smOs = testdataFactory.createSoftwareModuleOs();
+        final SoftwareModule smApp = testdataFactory.createSoftwareModuleApp();
+
+        final byte[] testData = RandomStringUtils.randomAlphanumeric(100).getBytes();
+        final DbArtifactHash artifactHashes = calcHashes(testData);
+
+        try (final InputStream inputStream = new ByteArrayInputStream(testData)) {
+            final ArtifactUpload artifactUpload = new ArtifactUpload(inputStream, smOs.getId(), "test-file",
+                    artifactHashes.getMd5(), artifactHashes.getSha1(), artifactHashes.getSha256(), false, null,
+                    testData.length);
+            final Artifact artifact = artifactManagement.create(artifactUpload);
+            assertThat(artifact).isNotNull();
+        }
+        final Optional<AbstractDbArtifact> dbArtifact = artifactManagement.loadArtifactBinary(artifactHashes.getSha1());
+        assertThat(dbArtifact).isPresent();
+
+        try (final InputStream inputStream = new ByteArrayInputStream(testData)) {
+            final ArtifactUpload existingArtifactUpload = new ArtifactUpload(inputStream, smApp.getId(), "test-file",
+                    false, testData.length);
+            final Artifact createdArtifact = artifactManagement.create(existingArtifactUpload);
+            assertThat(createdArtifact.getSha1Hash()).isEqualTo(artifactHashes.getSha1());
+            assertThat(createdArtifact.getMd5Hash()).isEqualTo(artifactHashes.getMd5());
+            assertThat(createdArtifact.getSha256Hash()).isEqualTo(artifactHashes.getSha256());
+        }
+    }
+
+    private DbArtifactHash calcHashes(final byte[] input) throws NoSuchAlgorithmException {
+        final String sha1Hash = toBase16Hash("SHA1", input);
+        final String md5Hash = toBase16Hash("MD5", input);
+        final String sha256Hash = toBase16Hash("SHA-256", input);
+
+        return new DbArtifactHash(sha1Hash, md5Hash, sha256Hash);
+    }
+
+    private String toBase16Hash(final String algorithm, final byte[] input) throws NoSuchAlgorithmException {
+        final MessageDigest messageDigest = MessageDigest.getInstance(algorithm);
+        messageDigest.update(input);
+        return BaseEncoding.base16().lowerCase().encode(messageDigest.digest());
+    }
+
     private Artifact createArtifactForSoftwareModule(final String filename, final long moduleId, final int artifactSize)
             throws IOException {
         final byte[] randomBytes = randomBytes(artifactSize);