Merge pull request #357 from adorsys/feature/221_PathTraversal_RootBu…

…cket Feature/221 path traversal root bucket
adorsys · Oct 17, 2024 · 28f411a · 28f411a
2 parents 5333827 + 29e0de0
commit 28f411a
Show file tree

Hide file tree

Showing 12 changed files with 199 additions and 104 deletions.
diff --git a/...business/src/test/java/de/adorsys/datasafe/business/impl/e2e/MultiDFSFunctionalityIT.java b/...business/src/test/java/de/adorsys/datasafe/business/impl/e2e/MultiDFSFunctionalityIT.java
@@ -1,8 +1,5 @@
 package de.adorsys.datasafe.business.impl.e2e;
 
-import static de.adorsys.datasafe.types.api.shared.DockerUtil.getDockerUri;
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.jupiter.api.Assertions.assertThrows;
 import com.amazonaws.services.s3.AmazonS3;
 import com.amazonaws.services.s3.model.S3ObjectSummary;
 import dagger.Lazy;
@@ -39,6 +36,7 @@
 import de.adorsys.datasafe.types.api.types.ReadStorePassword;
 import de.adorsys.datasafe.types.api.utils.ExecutorServiceUtil;
 import de.adorsys.datasafe.types.api.utils.ReadKeyPasswordTestFactory;
+
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.security.UnrecoverableKeyException;
@@ -50,6 +48,7 @@
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
+
 import lombok.SneakyThrows;
 import lombok.experimental.Delegate;
 import lombok.extern.slf4j.Slf4j;
@@ -62,6 +61,10 @@
 import org.testcontainers.containers.wait.strategy.Wait;
 import org.testcontainers.shaded.com.google.common.collect.ImmutableMap;
 
+import static de.adorsys.datasafe.types.api.shared.DockerUtil.getDockerUri;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
 /**
  * This test distributes users' storage access keystore, document encryption keystore,
  * users' private files into buckets that reside on different buckets. Bootstrap knows only how to
@@ -135,6 +138,7 @@ void initDatasafe() {
                         accessKey(CREDENTIALS),
                         secretKey(CREDENTIALS)
                 ),
+                REGION,
                 CREDENTIALS,
                 EXECUTOR
         );
@@ -156,6 +160,7 @@ void initDatasafe() {
                                                                 acc.getAccessKey(),
                                                                 acc.getSecretKey()
                                                         ),
+                                                        acc.getRegion(),
                                                         acc.getBucketName(),
                                                         EXECUTOR
                                                 )

diff --git a/datasafe-cli/src/main/java/de/adorsys/datasafe/cli/config/DatasafeFactory.java b/datasafe-cli/src/main/java/de/adorsys/datasafe/cli/config/DatasafeFactory.java
@@ -86,6 +86,7 @@ private static StorageService amazonS3() {
                                 acc.getAccessKey(),
                                 acc.getSecretKey()
                         ),
+                        acc.getRegion(),
                         // Bucket name is encoded in first path segment
                         acc.getBucketName(),
                         ExecutorServiceUtil.submitterExecutesOnStarvationExecutingService()
@@ -134,6 +135,7 @@ private static S3StorageService getStorageService(String accessKey, String secre
 
         return new S3StorageService(
                 amazons3,
+                region,
                 bucket,
                 ExecutorServiceUtil
                         .submitterExecutesOnStarvationExecutingService(

diff --git a/.../test/java/de/adorsys/datasafe/examples/business/s3/MultiDfsWithCredentialsExampleIT.java b/.../test/java/de/adorsys/datasafe/examples/business/s3/MultiDfsWithCredentialsExampleIT.java
@@ -1,9 +1,5 @@
 package de.adorsys.datasafe.examples.business.s3;
 
-import static de.adorsys.datasafe.examples.business.s3.MinioContainerId.DIRECTORY_BUCKET;
-import static de.adorsys.datasafe.examples.business.s3.MinioContainerId.FILES_BUCKET_ONE;
-import static de.adorsys.datasafe.examples.business.s3.MinioContainerId.FILES_BUCKET_TWO;
-import static org.assertj.core.api.Assertions.assertThat;
 import com.amazonaws.services.s3.AmazonS3;
 import dagger.Lazy;
 import de.adorsys.datasafe.business.impl.service.DaggerDefaultDatasafeServices;
@@ -30,6 +26,7 @@
 import de.adorsys.datasafe.types.api.resource.StorageIdentifier;
 import de.adorsys.datasafe.types.api.shared.AwsClientRetry;
 import de.adorsys.datasafe.types.api.utils.ExecutorServiceUtil;
+
 import java.io.OutputStream;
 import java.net.URI;
 import java.nio.charset.StandardCharsets;
@@ -39,6 +36,7 @@
 import java.util.Map;
 import java.util.concurrent.ExecutorService;
 import java.util.regex.Pattern;
+
 import lombok.SneakyThrows;
 import lombok.experimental.Delegate;
 import lombok.extern.slf4j.Slf4j;
@@ -49,6 +47,11 @@
 import org.testcontainers.containers.wait.strategy.Wait;
 import org.testcontainers.shaded.com.google.common.collect.ImmutableMap;
 
+import static de.adorsys.datasafe.examples.business.s3.MinioContainerId.DIRECTORY_BUCKET;
+import static de.adorsys.datasafe.examples.business.s3.MinioContainerId.FILES_BUCKET_ONE;
+import static de.adorsys.datasafe.examples.business.s3.MinioContainerId.FILES_BUCKET_TWO;
+import static org.assertj.core.api.Assertions.assertThat;
+
 /**
  * This example shows how client can register storage system and securely store its access details.
  * Here, we will use 2 Datasafe class instances - one for securely storing user access credentials
@@ -105,6 +108,7 @@ void testMultiUserStorageUserSetup() {
         // static client that will be used to access `directory` bucket:
         StorageService directoryStorage = new S3StorageService(
                 directoryClient,
+                REGION,
                 DIRECTORY_BUCKET.getBucketName(),
                 EXECUTOR
         );
@@ -133,6 +137,7 @@ void testMultiUserStorageUserSetup() {
                                                                         acc.getAccessKey(),
                                                                         acc.getSecretKey()
                                                                 ),
+                                                                acc.getRegion(),
                                                                 // Bucket name is encoded in first path segment
                                                                 acc.getBucketName(),
                                                                 EXECUTOR

diff --git a/...asafe/examples/business/s3/BaseUserOperationsWithDefaultDatasafeOnVersionedStorageIT.java b/...asafe/examples/business/s3/BaseUserOperationsWithDefaultDatasafeOnVersionedStorageIT.java
@@ -121,6 +121,7 @@ void init() {
                 .config(new DefaultDFSConfig(cephMappedUrl, "secret"::toCharArray))
                 .storage(new S3StorageService(
                         cephS3,
+                        "",
                         VERSIONED_BUCKET_NAME,
                         ExecutorServiceUtil.submitterExecutesOnStarvationExecutingService()))
                 .build();

diff --git a/datasafe-rest-impl/src/main/java/de/adorsys/datasafe/rest/impl/config/DatasafeConfig.java b/datasafe-rest-impl/src/main/java/de/adorsys/datasafe/rest/impl/config/DatasafeConfig.java
@@ -34,12 +34,14 @@
 import de.adorsys.datasafe.types.api.context.overrides.OverridesRegistry;
 import de.adorsys.datasafe.types.api.types.ReadStorePassword;
 import de.adorsys.datasafe.types.api.utils.ExecutorServiceUtil;
+
 import java.net.URI;
 import java.nio.file.Paths;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
 import java.util.regex.Pattern;
+
 import lombok.experimental.Delegate;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
@@ -131,6 +133,7 @@ StorageService clientCredentials(AmazonS3 s3, S3Factory factory, DatasafePropert
         ExecutorService executorService = ExecutorServiceUtil.submitterExecutesOnStarvationExecutingService();
         S3StorageService basicStorage = new S3StorageService(
                 s3,
+                properties.getAmazonRegion(),
                 properties.getBucketName(),
                 executorService
         );
@@ -184,6 +187,7 @@ StorageService singleStorageServiceFilesystem(DatasafeProperties properties) {
     StorageService singleStorageServiceS3(AmazonS3 s3, DatasafeProperties properties) {
         return new S3StorageService(
                 s3,
+                properties.getAmazonRegion(),
                 properties.getBucketName(),
                 ExecutorServiceUtil.submitterExecutesOnStarvationExecutingService()
         );
@@ -202,7 +206,7 @@ StorageService multiStorageService(DatasafeProperties properties) {
         )
         );
 
-        S3StorageService s3StorageService = new S3StorageService(s3(properties), properties.getBucketName(),
+        S3StorageService s3StorageService = new S3StorageService(s3(properties), properties.getAmazonRegion(), properties.getBucketName(),
                 ExecutorServiceUtil.submitterExecutesOnStarvationExecutingService()
         );
 

diff --git a/...impl/src/main/java/de/adorsys/datasafe/simple/adapter/impl/SimpleDatasafeServiceImpl.java b/...impl/src/main/java/de/adorsys/datasafe/simple/adapter/impl/SimpleDatasafeServiceImpl.java
@@ -45,12 +45,14 @@
 import de.adorsys.datasafe.types.api.types.ReadKeyPassword;
 import de.adorsys.datasafe.types.api.types.ReadStorePassword;
 import de.adorsys.datasafe.types.api.utils.ExecutorServiceUtil;
+
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.URI;
 import java.nio.file.FileSystems;
 import java.util.List;
 import java.util.stream.Collectors;
+
 import lombok.AllArgsConstructor;
 import lombok.Getter;
 import lombok.SneakyThrows;
@@ -298,6 +300,7 @@ private static SystemRootAndStorageService useAmazonS3(AmazonS3DFSCredentials df
         }
         StorageService storageService = new S3StorageService(
                 amazons3,
+                amazonS3DFSCredentials.getRegion(),
                 amazonS3DFSCredentials.getContainer(),
                 ExecutorServiceUtil
                         .submitterExecutesOnStarvationExecutingService(

diff --git a/...e-adapter-spring/src/test/java/de/adorsys/datasafe/simple/adapter/spring/InjectionIT.java b/...e-adapter-spring/src/test/java/de/adorsys/datasafe/simple/adapter/spring/InjectionIT.java
@@ -103,7 +103,7 @@ private AbsoluteLocation<PrivateResource> getPrivateResourceAbsoluteLocation(DFS
         }
         if (dfsCredentials instanceof AmazonS3DFSCredentials) {
             AmazonS3DFSCredentials a = (AmazonS3DFSCredentials) dfsCredentials;
-            return new AbsoluteLocation<>(BasePrivateResource.forPrivate(new URI(a.getUrl() + "/" + a.getRootBucket())));
+            return new AbsoluteLocation<>(BasePrivateResource.forPrivate(new URI(a.getUrl() + "/" + a.getRegion() + "/" + a.getRootBucket())));
         }
         throw new TestException("NYI");
     }

diff --git a/...e-storage-impl-s3/src/main/java/de/adorsys/datasafe/storage/impl/s3/S3StorageService.java b/...e-storage-impl-s3/src/main/java/de/adorsys/datasafe/storage/impl/s3/S3StorageService.java
@@ -45,12 +45,13 @@ public class S3StorageService implements StorageService {
 
     /**
      * @param s3 Connection to S3
-     * @param bucketName Bucket to use
+     * @param region Region to use
+     * @param bucket Bucket to use
      * @param executorService Multipart sending threadpool (file chunks are sent in parallel)
      */
-    public S3StorageService(AmazonS3 s3, String bucketName, ExecutorService executorService) {
+    public S3StorageService(AmazonS3 s3, String region, String bucket, ExecutorService executorService) {
         this.s3 = s3;
-        this.router = new StaticBucketRouter(bucketName);
+        this.router = new StaticBucketRouter(region, bucket);
         this.executorService = executorService;
     }
 

diff --git a/...storage-impl-s3/src/main/java/de/adorsys/datasafe/storage/impl/s3/StaticBucketRouter.java b/...storage-impl-s3/src/main/java/de/adorsys/datasafe/storage/impl/s3/StaticBucketRouter.java
@@ -8,6 +8,7 @@
 @RequiredArgsConstructor
 public class StaticBucketRouter implements BucketRouter {
 
+    private final String region;
     private final String bucketName;
 
     @Override
@@ -20,10 +21,16 @@ public String resourceKey(AbsoluteLocation resource) {
         UnaryOperator<String> trimStartingSlash = str -> str.replaceFirst("^/", "");
 
         String resourcePath = trimStartingSlash.apply(resource.location().getRawPath());
-        if (bucketName == null || "".equals(bucketName) || !resourcePath.contains(bucketName)) {
-            return resourcePath;
-        }
 
-        return trimStartingSlash.apply(resourcePath.substring(resourcePath.indexOf(bucketName) + bucketName.length()));
+        if (bucketName != null && !bucketName.isEmpty()) {
+            if (resourcePath.startsWith(bucketName)) {
+                return trimStartingSlash.apply(resourcePath.substring(resourcePath.indexOf(bucketName) + bucketName.length()));
+            }
+            String bucketNameWithRegion = region + "/" + bucketName;
+            if (resourcePath.startsWith(bucketNameWithRegion)) {
+                return trimStartingSlash.apply(resourcePath.substring(resourcePath.indexOf(bucketNameWithRegion) + bucketNameWithRegion.length()));
+            }
+        }
+        return resourcePath;
     }
 }
diff --git a/...e-impl-s3/src/test/java/de/adorsys/datasafe/storage/impl/s3/S3SystemStorageServiceIT.java b/...e-impl-s3/src/test/java/de/adorsys/datasafe/storage/impl/s3/S3SystemStorageServiceIT.java
@@ -16,6 +16,7 @@
 import de.adorsys.datasafe.types.api.utils.ExecutorServiceUtil;
 import lombok.SneakyThrows;
 import lombok.extern.slf4j.Slf4j;
+import org.assertj.core.api.AbstractStringAssert;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
@@ -27,6 +28,7 @@
 import java.io.OutputStream;
 import java.util.List;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static de.adorsys.datasafe.types.api.shared.DockerUtil.getDockerUri;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -82,6 +84,7 @@ static void beforeAll() {
     void init() {
         this.storageService = new S3StorageService(
                 s3,
+                "eu-central-1",
                 bucketName,
                 ExecutorServiceUtil.submitterExecutesOnStarvationExecutingService()
         );
@@ -91,12 +94,31 @@ void init() {
     void list() {
         createFileWithMessage();
 
-        assertThat(storageService.list(root))
-                .hasSize(1)
-                .extracting(AbsoluteLocation::location)
-                .asString().contains(FILE);
+        // Log root and fileWithMsg URIs for debugging
+
+        Stream<AbsoluteLocation<ResolvedResource>> list = storageService.list(root);
+        List<AbsoluteLocation<ResolvedResource>> resultList = list.collect(Collectors.toList());
+
+        // Check if the size of the list is correct
+        assertThat(resultList).hasSize(1);
+
+        // Log the returned URI
+        String uriString = resultList.get(0).location().toASCIIString();
+        //        log.info("Returned URI in CI/CD: " + uriString);
+
+        //        // Add environment-related logging
+        //        log.info("Running in region: " + System.getenv("AWS_REGION"));
+        //        log.info("S3 Bucket Name: " + bucketName);
+        //        log.info("AWS_ACCESS_KEY_ID: " + System.getenv("AWS_ACCESS_KEY_ID"));
+        //        log.info("AWS_SECRET_ACCESS_KEY: " + System.getenv("AWS_SECRET_ACCESS_KEY"));
+        //        log.info("AWS_REGION: " + System.getenv("AWS_REGION"));
+        //        log.info("Minio container started at port: " + minio.getMappedPort(9000));
+        //        log.info("Minio container is running: " + minio.isRunning());
+
+        assertThat(uriString).contains(FILE);
     }
 
+
     @Test
     void testListOutOfStandardListFilesLimit() {
         int numberOfFilesOverLimit = 1010;

diff --git a/...age-impl-s3/src/test/java/de/adorsys/datasafe/storage/impl/s3/StaticBucketRouterTest.java b/...age-impl-s3/src/test/java/de/adorsys/datasafe/storage/impl/s3/StaticBucketRouterTest.java
@@ -0,0 +1,45 @@
+package de.adorsys.datasafe.storage.impl.s3;
+
+import de.adorsys.datasafe.types.api.resource.AbsoluteLocation;
+import de.adorsys.datasafe.types.api.resource.BasePrivateResource;
+import de.adorsys.datasafe.types.api.resource.Uri;
+import lombok.extern.slf4j.Slf4j;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+@Slf4j
+public class StaticBucketRouterTest {
+
+    private StaticBucketRouter router;
+
+    @BeforeEach
+    void setup() {
+        String region = "region";
+        String bucketName = "bucket";
+        router = new StaticBucketRouter(region, bucketName);
+    }
+
+    @Test
+    void resourceKeyTest() {
+        var root = new AbsoluteLocation<>(BasePrivateResource.forPrivate(new Uri("http://s3-us-west-2.amazonaws.com/bucket/users/myuserid/private/files/bucket/users/otheruser/private/files/somefile.aes")));
+        log.info(String.valueOf(root));
+        String resourcePath = router.resourceKey(root);
+        assertThat(resourcePath).hasToString("users/myuserid/private/files/bucket/users/otheruser/private/files/somefile.aes");
+    }
+
+    @Test
+    void noBucketInPath() {
+        var root = new AbsoluteLocation<>(BasePrivateResource.forPrivate(new Uri("http://bucket.s3-us-west-2.amazonaws.com/users/myuserid/private/files/bucket/users/otheruser/private/files/somefile.aes")));
+        String resourcePath = router.resourceKey(root);
+        assertThat(resourcePath).hasToString("users/myuserid/private/files/bucket/users/otheruser/private/files/somefile.aes");
+    }
+
+    @Test
+    void regionAndBucketInPath() {
+        var root = new AbsoluteLocation<>(BasePrivateResource.forPrivate(new Uri("s3://host/region/bucket/users/myuserid/private/files/bucket/users/otheruser/private/files/somefile.aes")));
+        String resourcePath = router.resourceKey(root);
+        assertThat(resourcePath).hasToString("users/myuserid/private/files/bucket/users/otheruser/private/files/somefile.aes");
+    }
+}