Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/221 path traversal root bucket #357

Merged
merged 11 commits into from
Oct 17, 2024
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -135,6 +138,7 @@ void initDatasafe() {
accessKey(CREDENTIALS),
secretKey(CREDENTIALS)
),
REGION,
CREDENTIALS,
EXECUTOR
);
Expand All @@ -156,6 +160,7 @@ void initDatasafe() {
acc.getAccessKey(),
acc.getSecretKey()
),
acc.getRegion(),
acc.getBucketName(),
EXECUTOR
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -134,6 +135,7 @@ private static S3StorageService getStorageService(String accessKey, String secre

return new S3StorageService(
amazons3,
region,
bucket,
ExecutorServiceUtil
.submitterExecutesOnStarvationExecutingService(
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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
);
Expand Down Expand Up @@ -133,6 +137,7 @@ void testMultiUserStorageUserSetup() {
acc.getAccessKey(),
acc.getSecretKey()
),
acc.getRegion(),
// Bucket name is encoded in first path segment
acc.getBucketName(),
EXECUTOR
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ void init() {
.config(new DefaultDFSConfig(cephMappedUrl, "secret"::toCharArray))
.storage(new S3StorageService(
cephS3,
"",
VERSIONED_BUCKET_NAME,
ExecutorServiceUtil.submitterExecutesOnStarvationExecutingService()))
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
);
Expand Down Expand Up @@ -184,6 +187,7 @@ StorageService singleStorageServiceFilesystem(DatasafeProperties properties) {
StorageService singleStorageServiceS3(AmazonS3 s3, DatasafeProperties properties) {
return new S3StorageService(
s3,
properties.getAmazonRegion(),
properties.getBucketName(),
ExecutorServiceUtil.submitterExecutesOnStarvationExecutingService()
);
Expand All @@ -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()
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -298,6 +300,7 @@ private static SystemRootAndStorageService useAmazonS3(AmazonS3DFSCredentials df
}
StorageService storageService = new S3StorageService(
amazons3,
amazonS3DFSCredentials.getRegion(),
amazonS3DFSCredentials.getContainer(),
ExecutorServiceUtil
.submitterExecutesOnStarvationExecutingService(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
@RequiredArgsConstructor
public class StaticBucketRouter implements BucketRouter {

private final String region;
private final String bucketName;

@Override
Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -82,6 +84,7 @@ static void beforeAll() {
void init() {
this.storageService = new S3StorageService(
s3,
"eu-central-1",
bucketName,
ExecutorServiceUtil.submitterExecutesOnStarvationExecutingService()
);
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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");
}
}
Loading