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

fix: Ensure paths are securely prefixed with root bucket #348

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,4 @@ public abstract class DefaultInboxActionsModule {
*/
@Binds
abstract InboxService inboxService(InboxServiceImplRuntimeDelegatable impl);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ interface Builder {
@BindsInstance
Builder config(DFSConfig config);

@BindsInstance
Builder rootBucket( String rootBucket);
/**
* Binds (configures) all storage operations - not necessary to call {@code storageList} after.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ interface Builder {
@BindsInstance
Builder storage(StorageService storageService);

@BindsInstance
Builder rootBucket( String rootBucket);
/**
* Provides class overriding functionality, so that you can disable i.e. path encryption
* @param overridesRegistry Map with class-overrides (note: you can override classes that are
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ protected void writeDataToPrivate(UserIDAuth auth, String path, String data) {
@SneakyThrows
protected void writeDataToInbox(UserIDAuth owner, UserIDAuth auth, String path, String data) {
try (OutputStream stream = writeToInbox.write(
WriteInboxRequest.forDefaultPublic(owner, Collections.singleton(auth.getUserID()), path)
WriteInboxRequest.forDefaultPublic(owner, Collections.singleton(auth.getUserID()), path)
)) {

stream.write(data.getBytes(UTF_8));
Expand All @@ -134,7 +134,7 @@ protected AbsoluteLocation<ResolvedResource> getFirstFileInPrivate(UserIDAuth ow

protected List<AbsoluteLocation<ResolvedResource>> getAllFilesInPrivate(UserIDAuth owner) {
try (Stream<AbsoluteLocation<ResolvedResource>> ls = listPrivate.list(
ListRequest.forDefaultPrivate(owner, "./")
ListRequest.forDefaultPrivate(owner, "./")
)) {
List<AbsoluteLocation<ResolvedResource>> files = ls.collect(Collectors.toList());
log.info("{} has {} in PRIVATE", owner.getUserID(), files);
Expand Down Expand Up @@ -178,7 +178,7 @@ protected AbsoluteLocation<ResolvedResource> getFirstFileInInbox(UserIDAuth inbo

protected List<AbsoluteLocation<ResolvedResource>> getAllFilesInInbox(UserIDAuth inboxOwner) {
try (Stream<AbsoluteLocation<ResolvedResource>> ls = listInbox.list(
ListRequest.forDefaultPrivate(inboxOwner, "./")
ListRequest.forDefaultPrivate(inboxOwner, "./")
)) {
List<AbsoluteLocation<ResolvedResource>> files = ls.collect(Collectors.toList());
log.info("{} has {} in INBOX", inboxOwner, files);
Expand All @@ -194,7 +194,7 @@ protected void registerJohnAndJane() {
@SneakyThrows
protected void sendToInbox(UserIDAuth from, UserID to, String filename, String data) {
try (OutputStream stream = writeToInbox.write(
WriteInboxRequest.forDefaultPublic(from, Collections.singleton(to), "./" + filename)
WriteInboxRequest.forDefaultPublic(from, Collections.singleton(to), "./" + filename)
)) {
stream.write(data.getBytes());
}
Expand Down Expand Up @@ -229,11 +229,11 @@ protected UserIDAuth createJohnTestUser(int i) {
protected void assertPrivateSpaceList(UserIDAuth user, String root, String... expected) {
List<String> paths;
try (Stream<AbsoluteLocation<ResolvedResource>> ls =
listPrivate.list(ListRequest.forDefaultPrivate(user, root))
listPrivate.list(ListRequest.forDefaultPrivate(user, root))
) {
paths = ls
.map(it -> it.getResource().asPrivate().decryptedPath().asString())
.collect(Collectors.toList());
.map(it -> it.getResource().asPrivate().decryptedPath().asString())
.collect(Collectors.toList());
}

assertThat(paths).containsExactlyInAnyOrder(expected);
Expand All @@ -242,11 +242,11 @@ protected void assertPrivateSpaceList(UserIDAuth user, String root, String... ex
protected void assertInboxSpaceList(UserIDAuth user, String root, String... expected) {
List<String> paths;
try (Stream<AbsoluteLocation<ResolvedResource>> ls =
listInbox.list(ListRequest.forDefaultPrivate(user, root))
listInbox.list(ListRequest.forDefaultPrivate(user, root))
) {
paths = ls
.map(it -> it.getResource().asPrivate().decryptedPath().asString())
.collect(Collectors.toList());
.map(it -> it.getResource().asPrivate().decryptedPath().asString())
.collect(Collectors.toList());
}

assertThat(paths).containsExactlyInAnyOrder(expected);
Expand All @@ -255,8 +255,8 @@ protected void assertInboxSpaceList(UserIDAuth user, String root, String... expe
@SneakyThrows
protected void assertRootDirIsEmpty(WithStorageProvider.StorageDescriptor descriptor) {
try (Stream<AbsoluteLocation<ResolvedResource>> ls = descriptor.getStorageService().get()
.list(
new AbsoluteLocation<>(BasePrivateResource.forPrivate(descriptor.getLocation())))
.list(
new AbsoluteLocation<>(BasePrivateResource.forPrivate(descriptor.getLocation())))
) {
assertThat(ls).isEmpty();
}
Expand All @@ -267,18 +267,18 @@ protected void assertRootDirIsEmpty(WithStorageProvider.StorageDescriptor descri
// however we can't remove anything above
try (Stream<Path> files = Files.walk(Paths.get(descriptor.getLocation().asURI()))) {
assertThat(files)
.allMatch(it -> it.toFile().isDirectory())
.extracting(Path::toUri)
.extracting(it -> descriptor.getLocation().asURI().relativize(it))
.extracting(URI::toString)
.containsExactlyInAnyOrder(
"",
"users/",
"profiles/",
"profiles/public/",
"profiles/private/"
);
.allMatch(it -> it.toFile().isDirectory())
.extracting(Path::toUri)
.extracting(it -> descriptor.getLocation().asURI().relativize(it))
.extracting(URI::toString)
.containsExactlyInAnyOrder(
"",
"users/",
"profiles/",
"profiles/public/",
"profiles/private/"
);
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -449,4 +449,4 @@ private static ObjectMapper createMapper() {
mapper.setSerializationInclusion(JsonInclude.Include.NON_NULL);
return mapper;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -130,4 +130,4 @@ private void init(StorageDescriptor descriptor) {
.defaultDatasafeServices(descriptor.getStorageService().get(), descriptor.getLocation());
initialize(DatasafeServicesProvider.dfsConfig(descriptor.getLocation()), datasafeServices);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -215,4 +215,4 @@ private void init(WithStorageProvider.StorageDescriptor descriptor) {
.defaultDatasafeServices(descriptor.getStorageService().get(), descriptor.getLocation());
initialize(DatasafeServicesProvider.dfsConfig(descriptor.getLocation()), datasafeServices);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -179,4 +179,4 @@ private byte randomByte(byte notEqualTo) {

return resultArray[0];
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -316,4 +316,4 @@ private void shareMessage(UserIDAuth fromUser, UserID forUser, String messageNam
os.write(message.getBytes(StandardCharsets.UTF_8));
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@
import de.adorsys.datasafe.inbox.api.actions.WriteToInbox;
import de.adorsys.datasafe.types.api.actions.WriteInboxRequest;
import de.adorsys.datasafe.types.api.context.annotations.RuntimeDelegate;
import de.adorsys.datasafe.types.api.resource.BasePublicResource;
import de.adorsys.datasafe.types.api.resource.PublicResource;

import javax.inject.Inject;
import java.io.OutputStream;
import java.net.URI;
import java.util.Set;
import java.util.stream.Collectors;

Expand All @@ -30,14 +32,16 @@ public class WriteToInboxImpl implements WriteToInbox {
private final PrivateKeyService privateKeyService;
private final ResourceResolver resolver;
private final EncryptedDocumentWriteService writer;
private final String rootBucket;

@Inject
public WriteToInboxImpl(PublicKeyService publicKeyService, PrivateKeyService privateKeyService, ResourceResolver resolver,
EncryptedDocumentWriteService writer) {
EncryptedDocumentWriteService writer, String rootBucket) {
this.publicKeyService = publicKeyService;
this.privateKeyService = privateKeyService;
this.resolver = resolver;
this.writer = writer;
this.rootBucket = rootBucket;
}

/**
Expand All @@ -47,10 +51,16 @@ public WriteToInboxImpl(PublicKeyService publicKeyService, PrivateKeyService pri
*/
@Override
public OutputStream write(WriteInboxRequest<UserIDAuth, Set<UserID>, PublicResource> request) {
URI location = request.getLocation().location().asURI();
String bucket = rootBucket != null ? rootBucket : "";
if (!location.getPath().startsWith(bucket)) {
location = URI.create(bucket + "/" + location.getPath());
}
final URI finalLocation = location;
return writer.write(
request.getRecipients().stream().collect(Collectors.toMap(
publicKeyService::publicKey,
it -> resolver.resolveRelativeToPublicInbox(it, request.getLocation())
it -> resolver.resolveRelativeToPublicInbox(it, new BasePublicResource(finalLocation))
)),
privateKeyService.getKeyPair(request.getOwner())
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,17 @@
import org.mockito.Mock;

import java.io.ByteArrayOutputStream;
import java.io.OutputStream;
import java.net.URI;
import java.security.KeyPair;
import java.security.KeyPairGenerator;
import java.security.NoSuchAlgorithmException;
import java.security.PublicKey;
import java.util.Collections;
import java.util.Set;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.when;

class WriteToInboxImplTest extends BaseMockitoTest {
Expand Down Expand Up @@ -61,8 +65,15 @@ class WriteToInboxImplTest extends BaseMockitoTest {
private WriteToInboxImpl inbox;

@BeforeEach
void init() {
void init() throws NoSuchAlgorithmException {
this.publicKeyWithId = new PublicKeyIDWithPublicKey(new KeyID(""), publicKey);
KeyPairGenerator keyPairGenerator = KeyPairGenerator.getInstance("RSA");
keyPairGenerator.initialize(2048);
this.senderKeyPair = keyPairGenerator.generateKeyPair();
String rootBucket = "bucket";
if (inbox == null) {
inbox = new WriteToInboxImpl(publicKeyService, privateKeyService, resolver, writeService, rootBucket);
}
}

@Test
Expand All @@ -73,11 +84,15 @@ void write() {
.forDefaultPublic(ownerAuth, Collections.singleton(auth), ABSOLUTE_PATH);
when(publicKeyService.publicKey(auth)).thenReturn(publicKeyWithId);
when(privateKeyService.getKeyPair(ownerAuth)).thenReturn(senderKeyPair);
when(resolver.resolveRelativeToPublicInbox(auth, request.getLocation())).thenReturn(resource);
when(resolver.resolveRelativeToPublicInbox(any(), any())).thenReturn(resource);
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
when(writeService.write(Collections.singletonMap(publicKeyWithId, resource), senderKeyPair)).thenReturn(outputStream);

inbox.write(request).write(BYTES.getBytes());
if (inbox != null && request != null) {
try (OutputStream stream = inbox.write(request)) {
stream.write(BYTES.getBytes());
}
}

assertThat(outputStream.toByteArray()).contains(BYTES.getBytes());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ 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)) {
if (bucketName == null || "".equals(bucketName) || !resourcePath.startsWith(bucketName)) {
return resourcePath;
}

return trimStartingSlash.apply(resourcePath.substring(resourcePath.indexOf(bucketName) + bucketName.length()));
return trimStartingSlash.apply(resourcePath.substring(bucketName.length()));
}
}
Loading