You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by "vttranlina (via GitHub)" <gi...@apache.org> on 2024/11/05 09:38:14 UTC
[PR] WIP - JAMES-4085 [S3 SSEC] S3 multitenancy: SSEC basic implementation [james-project]
vttranlina opened a new pull request, #2488:
URL: https://github.com/apache/james-project/pull/2488
resolve https://github.com/linagora/james-project/issues/5278
jira https://issues.apache.org/jira/browse/JAMES-4085
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-4085 [S3 SSEC] SSEC basic implementation [james-project]
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #2488:
URL: https://github.com/apache/james-project/pull/2488#discussion_r1847579052
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreDAO.java:
##########
@@ -166,64 +177,79 @@ private static class FluxResponse {
}
private Mono<FluxResponse> getObject(BucketName bucketName, BlobId blobId) {
- return Mono.fromFuture(() ->
- client.getObject(
- builder -> builder.bucket(bucketName.asString()).key(blobId.asString()),
- new AsyncResponseTransformer<GetObjectResponse, FluxResponse>() {
-
- FluxResponse response;
-
- @Override
- public CompletableFuture<FluxResponse> prepare() {
- response = new FluxResponse();
- return response.supportingCompletableFuture;
- }
-
- @Override
- public void onResponse(GetObjectResponse response) {
- this.response.sdkResponse = response;
- }
-
- @Override
- public void exceptionOccurred(Throwable error) {
- this.response.supportingCompletableFuture.completeExceptionally(error);
- }
-
- @Override
- public void onStream(SdkPublisher<ByteBuffer> publisher) {
- response.flux = Flux.from(publisher);
- response.supportingCompletableFuture.complete(response);
- }
- }))
- .switchIfEmpty(Mono.error(() -> new ObjectStoreIOException("Request was unexpectedly canceled, no GetObjectResponse")));
+ return buildGetObjectRequestBuilder(bucketName, blobId)
+ .flatMap(getObjectRequestBuilder -> Mono.fromFuture(() ->
+ client.getObject(getObjectRequestBuilder.build(),
+ new AsyncResponseTransformer<GetObjectResponse, FluxResponse>() {
+
+ FluxResponse response;
+
+ @Override
+ public CompletableFuture<FluxResponse> prepare() {
+ response = new FluxResponse();
+ return response.supportingCompletableFuture;
+ }
+
+ @Override
+ public void onResponse(GetObjectResponse response) {
+ this.response.sdkResponse = response;
+ }
+
+ @Override
+ public void exceptionOccurred(Throwable error) {
+ this.response.supportingCompletableFuture.completeExceptionally(error);
+ }
+
+ @Override
+ public void onStream(SdkPublisher<ByteBuffer> publisher) {
+ response.flux = Flux.from(publisher);
+ response.supportingCompletableFuture.complete(response);
+ }
+ }))
+ .switchIfEmpty(Mono.error(() -> new ObjectStoreIOException("Request was unexpectedly canceled, no GetObjectResponse"))));
}
@Override
public Mono<byte[]> readBytes(BucketName bucketName, BlobId blobId) {
BucketName resolvedBucketName = bucketNameResolver.resolve(bucketName);
- return Mono.fromFuture(() ->
- client.getObject(
- builder -> builder.bucket(resolvedBucketName.asString()).key(blobId.asString()),
- new MinimalCopyBytesResponseTransformer(configuration, blobId)))
- .onErrorMap(NoSuchBucketException.class, e -> new ObjectNotFoundException("Bucket not found " + resolvedBucketName.asString(), e))
- .onErrorMap(NoSuchKeyException.class, e -> new ObjectNotFoundException("Blob not found " + blobId.asString() + " in bucket " + resolvedBucketName.asString(), e))
- .publishOn(Schedulers.parallel())
- .map(BytesWrapper::asByteArrayUnsafe)
- .onErrorMap(e -> e.getCause() instanceof OutOfMemoryError, Throwable::getCause);
+ return buildGetObjectRequestBuilder(resolvedBucketName, blobId)
+ .flatMap(putObjectRequest -> Mono.fromFuture(() ->
+ client.getObject(putObjectRequest.build(), new MinimalCopyBytesResponseTransformer(configuration, blobId)))
+ .onErrorMap(NoSuchBucketException.class, e -> new ObjectNotFoundException("Bucket not found " + resolvedBucketName.asString(), e))
+ .onErrorMap(NoSuchKeyException.class, e -> new ObjectNotFoundException("Blob not found " + blobId.asString() + " in bucket " + resolvedBucketName.asString(), e))
+ .publishOn(Schedulers.parallel())
+ .map(BytesWrapper::asByteArrayUnsafe)
+ .onErrorMap(e -> e.getCause() instanceof OutOfMemoryError, Throwable::getCause));
+ }
+
+ private Mono<GetObjectRequest.Builder> buildGetObjectRequestBuilder(BucketName bucketName, BlobId blobId) {
+ GetObjectRequest.Builder baseBuilder = GetObjectRequest.builder()
+ .bucket(bucketName.asString())
+ .key(blobId.asString());
+
+ if (s3RequestOption.ssec().enable()) {
+ return Mono.from(s3RequestOption.ssec().sseCustomerKeyFactory().get()
+ .generate(bucketName, blobId))
+ .map(sseCustomerKey -> baseBuilder
+ .sseCustomerAlgorithm(sseCustomerKey.ssecAlgorithm())
Review Comment:
> Are we certain the ssecAlgorithm used to create the object is the same as the one currently configured?
The configuration is specified in the `blobstore.properties `file, which is consistent.
During a quick test, I attempted to put data using AES256 and retrieve it with AES128, resulting in the error: `Requests specifying Server Side Encryption with Customer provided keys must provide a valid encryption algorithm.`
Additionally, MinIO look like only supports AES256. Using any other algorithm will result in the error (Test with DES, AES128): `The SSE-C algorithm is not supported.`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-4085 [S3 SSEC] SSEC basic implementation [james-project]
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #2488:
URL: https://github.com/apache/james-project/pull/2488#discussion_r1835707444
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/sse/S3SSECConfiguration.java:
##########
@@ -0,0 +1,45 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws.sse;
+
+import java.util.List;
+
+import com.google.common.base.Preconditions;
+
+public interface S3SSECConfiguration {
+
+ List<String> SUPPORTED_ALGORITHMS = List.of("AES256");
+
+ String algorithm();
+
+ record Basic(String algorithm,
+ String masterPassword,
+ String salt) implements S3SSECConfiguration {
Review Comment:
It was did in another ticket
https://github.com/apache/james-project/pull/2495
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-4085 [S3 SSEC] SSEC basic implementation [james-project]
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #2488:
URL: https://github.com/apache/james-project/pull/2488#discussion_r1839343316
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/sse/S3SSECConfiguration.java:
##########
@@ -23,20 +23,23 @@
import com.google.common.base.Preconditions;
-public record S3SSECustomerConfiguration(boolean enabled,
- String algorithm,
- String masterPassword,
- String salt) {
+public interface S3SSECConfiguration {
- public static final List<String> SUPPORTED_ALGORITHMS = List.of("AES256");
+ List<String> SUPPORTED_ALGORITHMS = List.of("AES256");
- public S3SSECustomerConfiguration(boolean enabled, String algorithm, String masterPassword, String salt) {
- this.enabled = enabled;
- if (enabled) {
- Preconditions.checkArgument(SUPPORTED_ALGORITHMS.contains(algorithm), "Unsupported algorithm: %s. The supported algorithms are: %s", algorithm, SUPPORTED_ALGORITHMS);
+ String algorithm();
+
+ record Basic(String algorithm,
+ String masterPassword,
+ String salt) implements S3SSECConfiguration {
+ public Basic {
+ Preconditions.checkArgument(SUPPORTED_ALGORITHMS.contains(algorithm), "Unsupported algorithm: " + algorithm + ". The supported algorithms are: " + SUPPORTED_ALGORITHMS);
+ }
+ }
+
+ record KeyDerivationEnabled(String algorithm) implements S3SSECConfiguration {
Review Comment:
Indeed, this remark is completely correct 100%.
I'll remove it in the next fixup commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-4085 [S3 SSEC] SSEC basic implementation [james-project]
Posted by "HoussemNasri (via GitHub)" <gi...@apache.org>.
HoussemNasri commented on code in PR #2488:
URL: https://github.com/apache/james-project/pull/2488#discussion_r1847911827
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreDAO.java:
##########
@@ -166,64 +177,79 @@ private static class FluxResponse {
}
private Mono<FluxResponse> getObject(BucketName bucketName, BlobId blobId) {
- return Mono.fromFuture(() ->
- client.getObject(
- builder -> builder.bucket(bucketName.asString()).key(blobId.asString()),
- new AsyncResponseTransformer<GetObjectResponse, FluxResponse>() {
-
- FluxResponse response;
-
- @Override
- public CompletableFuture<FluxResponse> prepare() {
- response = new FluxResponse();
- return response.supportingCompletableFuture;
- }
-
- @Override
- public void onResponse(GetObjectResponse response) {
- this.response.sdkResponse = response;
- }
-
- @Override
- public void exceptionOccurred(Throwable error) {
- this.response.supportingCompletableFuture.completeExceptionally(error);
- }
-
- @Override
- public void onStream(SdkPublisher<ByteBuffer> publisher) {
- response.flux = Flux.from(publisher);
- response.supportingCompletableFuture.complete(response);
- }
- }))
- .switchIfEmpty(Mono.error(() -> new ObjectStoreIOException("Request was unexpectedly canceled, no GetObjectResponse")));
+ return buildGetObjectRequestBuilder(bucketName, blobId)
+ .flatMap(getObjectRequestBuilder -> Mono.fromFuture(() ->
+ client.getObject(getObjectRequestBuilder.build(),
+ new AsyncResponseTransformer<GetObjectResponse, FluxResponse>() {
+
+ FluxResponse response;
+
+ @Override
+ public CompletableFuture<FluxResponse> prepare() {
+ response = new FluxResponse();
+ return response.supportingCompletableFuture;
+ }
+
+ @Override
+ public void onResponse(GetObjectResponse response) {
+ this.response.sdkResponse = response;
+ }
+
+ @Override
+ public void exceptionOccurred(Throwable error) {
+ this.response.supportingCompletableFuture.completeExceptionally(error);
+ }
+
+ @Override
+ public void onStream(SdkPublisher<ByteBuffer> publisher) {
+ response.flux = Flux.from(publisher);
+ response.supportingCompletableFuture.complete(response);
+ }
+ }))
+ .switchIfEmpty(Mono.error(() -> new ObjectStoreIOException("Request was unexpectedly canceled, no GetObjectResponse"))));
}
@Override
public Mono<byte[]> readBytes(BucketName bucketName, BlobId blobId) {
BucketName resolvedBucketName = bucketNameResolver.resolve(bucketName);
- return Mono.fromFuture(() ->
- client.getObject(
- builder -> builder.bucket(resolvedBucketName.asString()).key(blobId.asString()),
- new MinimalCopyBytesResponseTransformer(configuration, blobId)))
- .onErrorMap(NoSuchBucketException.class, e -> new ObjectNotFoundException("Bucket not found " + resolvedBucketName.asString(), e))
- .onErrorMap(NoSuchKeyException.class, e -> new ObjectNotFoundException("Blob not found " + blobId.asString() + " in bucket " + resolvedBucketName.asString(), e))
- .publishOn(Schedulers.parallel())
- .map(BytesWrapper::asByteArrayUnsafe)
- .onErrorMap(e -> e.getCause() instanceof OutOfMemoryError, Throwable::getCause);
+ return buildGetObjectRequestBuilder(resolvedBucketName, blobId)
+ .flatMap(putObjectRequest -> Mono.fromFuture(() ->
+ client.getObject(putObjectRequest.build(), new MinimalCopyBytesResponseTransformer(configuration, blobId)))
+ .onErrorMap(NoSuchBucketException.class, e -> new ObjectNotFoundException("Bucket not found " + resolvedBucketName.asString(), e))
+ .onErrorMap(NoSuchKeyException.class, e -> new ObjectNotFoundException("Blob not found " + blobId.asString() + " in bucket " + resolvedBucketName.asString(), e))
+ .publishOn(Schedulers.parallel())
+ .map(BytesWrapper::asByteArrayUnsafe)
+ .onErrorMap(e -> e.getCause() instanceof OutOfMemoryError, Throwable::getCause));
+ }
+
+ private Mono<GetObjectRequest.Builder> buildGetObjectRequestBuilder(BucketName bucketName, BlobId blobId) {
+ GetObjectRequest.Builder baseBuilder = GetObjectRequest.builder()
+ .bucket(bucketName.asString())
+ .key(blobId.asString());
+
+ if (s3RequestOption.ssec().enable()) {
+ return Mono.from(s3RequestOption.ssec().sseCustomerKeyFactory().get()
+ .generate(bucketName, blobId))
+ .map(sseCustomerKey -> baseBuilder
+ .sseCustomerAlgorithm(sseCustomerKey.ssecAlgorithm())
Review Comment:
@quantranhong1999 @vttranlina It's clearer now, it should be good, thanks you for clarifying.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-4085 [S3 SSEC] SSEC basic implementation [james-project]
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael merged PR #2488:
URL: https://github.com/apache/james-project/pull/2488
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-4085 [S3 SSEC] SSEC basic implementation [james-project]
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on PR #2488:
URL: https://github.com/apache/james-project/pull/2488#issuecomment-2484626741
Rebase master & squash
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-4085 [S3 SSEC] SSEC basic implementation [james-project]
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #2488:
URL: https://github.com/apache/james-project/pull/2488#discussion_r1835710466
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/sse/S3WithSSECBlobStoreDAO.java:
##########
@@ -0,0 +1,68 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws.sse;
+
+import org.apache.james.blob.api.BlobId;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.blob.objectstorage.aws.S3BlobStoreConfiguration;
+import org.apache.james.blob.objectstorage.aws.S3BlobStoreDAO;
+import org.apache.james.blob.objectstorage.aws.S3ClientFactory;
+
+import reactor.core.publisher.Mono;
+import software.amazon.awssdk.services.s3.model.GetObjectRequest;
+import software.amazon.awssdk.services.s3.model.PutObjectRequest;
+
+public class S3WithSSECBlobStoreDAO extends S3BlobStoreDAO {
Review Comment:
> How do we plan to use S3WithSSECBlobStoreDAO? Guice module chooser?
It was did in another ticket
https://github.com/apache/james-project/pull/2495
> I would prefer to add the SSE-C logic directly into S3BlobStoreDAO (we can not have x2 class every time we add an S3 option)
I initially considered adding more if else logic to `S3BlobStoreDAO`, but I'd prefer to avoid that (it quiet verbose). While we could implement the interface directly in `S3BlobStoreDAO`, it wouldn't differ much from what `S3WithSSECBlobStoreDAO` does.
IMO, SSE-C is an option, and it is not simple to inline, -> I prefer to keep the current S3BlobStoreDAO for simple.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] WIP - JAMES-4085 [S3 SSEC] S3 multitenancy: SSEC basic implementation [james-project]
Posted by "HoussemNasri (via GitHub)" <gi...@apache.org>.
HoussemNasri commented on code in PR #2488:
URL: https://github.com/apache/james-project/pull/2488#discussion_r1829610345
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/sse/S3SSECustomerKeyGenerator.java:
##########
@@ -0,0 +1,54 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws.sse;
+
+import java.security.NoSuchAlgorithmException;
+import java.security.spec.InvalidKeySpecException;
+import java.util.Base64;
+
+import javax.crypto.SecretKeyFactory;
+import javax.crypto.spec.PBEKeySpec;
+
+import com.google.common.hash.Hashing;
+
+public class S3SSECustomerKeyGenerator {
+
+ public static final int ITERATION_COUNT = 65536;
+ public static final int KEY_LENGTH = 256;
+ public static final SecretKeyFactory SECRET_KEY_FACTORY;
+
+ static {
+ try {
+ SECRET_KEY_FACTORY = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256");
+ } catch (NoSuchAlgorithmException e) {
+ throw new RuntimeException("Could not initialize SecretKeyFactory", e);
+ }
+ }
+
+ public static String generateCustomerKey(String masterPassword, String salt) throws InvalidKeySpecException {
+ PBEKeySpec spec = new PBEKeySpec(masterPassword.toCharArray(), salt.getBytes(), ITERATION_COUNT, KEY_LENGTH);
+ byte[] derivedKey = SECRET_KEY_FACTORY.generateSecret(spec).getEncoded();
Review Comment:
Can this code be called from multiple threads? Because SecretKeyFactory#generateSecret is not pure nor thread-safe.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-4085 [S3 SSEC] SSEC basic implementation [james-project]
Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #2488:
URL: https://github.com/apache/james-project/pull/2488#discussion_r1847566373
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreDAO.java:
##########
@@ -166,64 +177,79 @@ private static class FluxResponse {
}
private Mono<FluxResponse> getObject(BucketName bucketName, BlobId blobId) {
- return Mono.fromFuture(() ->
- client.getObject(
- builder -> builder.bucket(bucketName.asString()).key(blobId.asString()),
- new AsyncResponseTransformer<GetObjectResponse, FluxResponse>() {
-
- FluxResponse response;
-
- @Override
- public CompletableFuture<FluxResponse> prepare() {
- response = new FluxResponse();
- return response.supportingCompletableFuture;
- }
-
- @Override
- public void onResponse(GetObjectResponse response) {
- this.response.sdkResponse = response;
- }
-
- @Override
- public void exceptionOccurred(Throwable error) {
- this.response.supportingCompletableFuture.completeExceptionally(error);
- }
-
- @Override
- public void onStream(SdkPublisher<ByteBuffer> publisher) {
- response.flux = Flux.from(publisher);
- response.supportingCompletableFuture.complete(response);
- }
- }))
- .switchIfEmpty(Mono.error(() -> new ObjectStoreIOException("Request was unexpectedly canceled, no GetObjectResponse")));
+ return buildGetObjectRequestBuilder(bucketName, blobId)
+ .flatMap(getObjectRequestBuilder -> Mono.fromFuture(() ->
+ client.getObject(getObjectRequestBuilder.build(),
+ new AsyncResponseTransformer<GetObjectResponse, FluxResponse>() {
+
+ FluxResponse response;
+
+ @Override
+ public CompletableFuture<FluxResponse> prepare() {
+ response = new FluxResponse();
+ return response.supportingCompletableFuture;
+ }
+
+ @Override
+ public void onResponse(GetObjectResponse response) {
+ this.response.sdkResponse = response;
+ }
+
+ @Override
+ public void exceptionOccurred(Throwable error) {
+ this.response.supportingCompletableFuture.completeExceptionally(error);
+ }
+
+ @Override
+ public void onStream(SdkPublisher<ByteBuffer> publisher) {
+ response.flux = Flux.from(publisher);
+ response.supportingCompletableFuture.complete(response);
+ }
+ }))
+ .switchIfEmpty(Mono.error(() -> new ObjectStoreIOException("Request was unexpectedly canceled, no GetObjectResponse"))));
}
@Override
public Mono<byte[]> readBytes(BucketName bucketName, BlobId blobId) {
BucketName resolvedBucketName = bucketNameResolver.resolve(bucketName);
- return Mono.fromFuture(() ->
- client.getObject(
- builder -> builder.bucket(resolvedBucketName.asString()).key(blobId.asString()),
- new MinimalCopyBytesResponseTransformer(configuration, blobId)))
- .onErrorMap(NoSuchBucketException.class, e -> new ObjectNotFoundException("Bucket not found " + resolvedBucketName.asString(), e))
- .onErrorMap(NoSuchKeyException.class, e -> new ObjectNotFoundException("Blob not found " + blobId.asString() + " in bucket " + resolvedBucketName.asString(), e))
- .publishOn(Schedulers.parallel())
- .map(BytesWrapper::asByteArrayUnsafe)
- .onErrorMap(e -> e.getCause() instanceof OutOfMemoryError, Throwable::getCause);
+ return buildGetObjectRequestBuilder(resolvedBucketName, blobId)
+ .flatMap(putObjectRequest -> Mono.fromFuture(() ->
+ client.getObject(putObjectRequest.build(), new MinimalCopyBytesResponseTransformer(configuration, blobId)))
+ .onErrorMap(NoSuchBucketException.class, e -> new ObjectNotFoundException("Bucket not found " + resolvedBucketName.asString(), e))
+ .onErrorMap(NoSuchKeyException.class, e -> new ObjectNotFoundException("Blob not found " + blobId.asString() + " in bucket " + resolvedBucketName.asString(), e))
+ .publishOn(Schedulers.parallel())
+ .map(BytesWrapper::asByteArrayUnsafe)
+ .onErrorMap(e -> e.getCause() instanceof OutOfMemoryError, Throwable::getCause));
+ }
+
+ private Mono<GetObjectRequest.Builder> buildGetObjectRequestBuilder(BucketName bucketName, BlobId blobId) {
+ GetObjectRequest.Builder baseBuilder = GetObjectRequest.builder()
+ .bucket(bucketName.asString())
+ .key(blobId.asString());
+
+ if (s3RequestOption.ssec().enable()) {
+ return Mono.from(s3RequestOption.ssec().sseCustomerKeyFactory().get()
+ .generate(bucketName, blobId))
+ .map(sseCustomerKey -> baseBuilder
+ .sseCustomerAlgorithm(sseCustomerKey.ssecAlgorithm())
Review Comment:
> Consider this scenario: SSE-C is configured to encrypt objects with AES256, an object A is uploaded using that configuration, then the configured decryption algorihtm is changed to DES: What happends when we I try download object A?
IMO it is a failure. The admin needs to make sure to keep the same encryption/decryption configuration IMO. For example AES blob store https://github.com/apache/james-project/blob/master/docs/modules/servers/partials/configure/blobstore.adoc#encryption-choice.
> Maybe we can use [get-bucket-encryption](https://docs.aws.amazon.com/cli/latest/reference/s3api/get-bucket-encryption.html)?
BTW how would you resolve the issue with that? It seems to me that `get-bucket-encryption` is more at the bucket level while SSE-C is more at the object level.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-4085 [S3 SSEC] SSEC basic implementation [james-project]
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on PR #2488:
URL: https://github.com/apache/james-project/pull/2488#issuecomment-2472349507
I will squash fixup
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-4085 [S3 SSEC] SSEC basic implementation [james-project]
Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #2488:
URL: https://github.com/apache/james-project/pull/2488#discussion_r1834025643
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/sse/S3SSECConfiguration.java:
##########
@@ -0,0 +1,45 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws.sse;
+
+import java.util.List;
+
+import com.google.common.base.Preconditions;
+
+public interface S3SSECConfiguration {
+
+ List<String> SUPPORTED_ALGORITHMS = List.of("AES256");
+
+ String algorithm();
+
+ record Basic(String algorithm,
+ String masterPassword,
+ String salt) implements S3SSECConfiguration {
Review Comment:
There is no conf parsing + document yet?
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/sse/S3WithSSECBlobStoreDAO.java:
##########
@@ -0,0 +1,68 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws.sse;
+
+import org.apache.james.blob.api.BlobId;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.blob.objectstorage.aws.S3BlobStoreConfiguration;
+import org.apache.james.blob.objectstorage.aws.S3BlobStoreDAO;
+import org.apache.james.blob.objectstorage.aws.S3ClientFactory;
+
+import reactor.core.publisher.Mono;
+import software.amazon.awssdk.services.s3.model.GetObjectRequest;
+import software.amazon.awssdk.services.s3.model.PutObjectRequest;
+
+public class S3WithSSECBlobStoreDAO extends S3BlobStoreDAO {
Review Comment:
How do we plan to use `S3WithSSECBlobStoreDAO`? Guice module chooser?
I would prefer to add the SSE-C logic directly into `S3BlobStoreDAO` (we can not have x2 class every time we add an S3 option). WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-4085 [S3 SSEC] SSEC basic implementation [james-project]
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #2488:
URL: https://github.com/apache/james-project/pull/2488#discussion_r1833697703
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/sse/S3SSECConfiguration.java:
##########
@@ -23,20 +23,23 @@
import com.google.common.base.Preconditions;
-public record S3SSECustomerConfiguration(boolean enabled,
- String algorithm,
- String masterPassword,
- String salt) {
+public interface S3SSECConfiguration {
- public static final List<String> SUPPORTED_ALGORITHMS = List.of("AES256");
+ List<String> SUPPORTED_ALGORITHMS = List.of("AES256");
- public S3SSECustomerConfiguration(boolean enabled, String algorithm, String masterPassword, String salt) {
- this.enabled = enabled;
- if (enabled) {
- Preconditions.checkArgument(SUPPORTED_ALGORITHMS.contains(algorithm), "Unsupported algorithm: %s. The supported algorithms are: %s", algorithm, SUPPORTED_ALGORITHMS);
+ String algorithm();
+
+ record Basic(String algorithm,
+ String masterPassword,
+ String salt) implements S3SSECConfiguration {
+ public Basic {
+ Preconditions.checkArgument(SUPPORTED_ALGORITHMS.contains(algorithm), "Unsupported algorithm: " + algorithm + ". The supported algorithms are: " + SUPPORTED_ALGORITHMS);
+ }
+ }
+
+ record KeyDerivationEnabled(String algorithm) implements S3SSECConfiguration {
Review Comment:
Out of scope for this task?
##########
server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/sse/SingleCustomerKeyFactoryTest.java:
##########
@@ -0,0 +1,64 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws.sse;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+
+import java.security.spec.InvalidKeySpecException;
+
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.blob.api.TestBlobId;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import reactor.core.publisher.Mono;
+
+public class SingleCustomerKeyFactoryTest {
+ private static S3SSECustomerKeyFactory.SingleCustomerKeyFactory testee;
+
+ @BeforeAll
+ static void setup() throws InvalidKeySpecException {
+ testee = new S3SSECustomerKeyFactory.SingleCustomerKeyFactory(new S3SSECustomerConfiguration(true, "AES256", "masterPassword", "salt"));
+
+ }
+
+ @Test
+ void generateShouldReturnSSECustomerKey() {
+ // When
+ S3SSECustomerKeyFactory.SSECustomerKey sseCustomerKey = Mono.from(testee.generate(BucketName.of("bucket1"), new TestBlobId("blobId"))).block();
+
+ // Then
+ assertThat(sseCustomerKey.customerKey()).isEqualTo("tsY5n0n10fcpjm7aGArqbw231ptLGAF5ubxMxI2+QYw=");
+ assertThat(sseCustomerKey.md5()).isEqualTo("G6IB1YLHiY/9uCh3TJhxzQ==");
+ assertThat(sseCustomerKey.algorithm()).isEqualTo("AES256");
+ }
+
+ @Test
+ void generateShouldReturnSameSSECustomerKey() {
+ // When
+ S3SSECustomerKeyFactory.SSECustomerKey sseCustomerKey1 = Mono.from(testee.generate(BucketName.of("bucket1"), new TestBlobId("blobId"))).block();
+ S3SSECustomerKeyFactory.SSECustomerKey sseCustomerKey2 = Mono.from(testee.generate(BucketName.of("bucket1"), new TestBlobId("blobId"))).block();
Review Comment:
Is this intended to have same bucket name and blobid here or bad copy/paste?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] WIP - JAMES-4085 [S3 SSEC] S3 multitenancy: SSEC basic implementation [james-project]
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #2488:
URL: https://github.com/apache/james-project/pull/2488#discussion_r1830504348
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/sse/S3SSECustomerKeyGenerator.java:
##########
@@ -0,0 +1,54 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws.sse;
+
+import java.security.NoSuchAlgorithmException;
+import java.security.spec.InvalidKeySpecException;
+import java.util.Base64;
+
+import javax.crypto.SecretKeyFactory;
+import javax.crypto.spec.PBEKeySpec;
+
+import com.google.common.hash.Hashing;
+
+public class S3SSECustomerKeyGenerator {
+
+ public static final int ITERATION_COUNT = 65536;
+ public static final int KEY_LENGTH = 256;
+ public static final SecretKeyFactory SECRET_KEY_FACTORY;
+
+ static {
+ try {
+ SECRET_KEY_FACTORY = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256");
+ } catch (NoSuchAlgorithmException e) {
+ throw new RuntimeException("Could not initialize SecretKeyFactory", e);
+ }
+ }
+
+ public static String generateCustomerKey(String masterPassword, String salt) throws InvalidKeySpecException {
+ PBEKeySpec spec = new PBEKeySpec(masterPassword.toCharArray(), salt.getBytes(), ITERATION_COUNT, KEY_LENGTH);
+ byte[] derivedKey = SECRET_KEY_FACTORY.generateSecret(spec).getEncoded();
Review Comment:
> Can this code be called from multiple threads?
yes
> Because SecretKeyFactory#generateSecret is not pure and it does not guarantee thread-safety AFAIK.
I just write concurrent test case for it
see commit https://github.com/apache/james-project/pull/2488/commits/73fd56a0b807663bea924c24fce49d6db588edcc
It works well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] WIP - JAMES-4085 [S3 SSEC] S3 multitenancy: SSEC basic implementation [james-project]
Posted by "HoussemNasri (via GitHub)" <gi...@apache.org>.
HoussemNasri commented on code in PR #2488:
URL: https://github.com/apache/james-project/pull/2488#discussion_r1829610345
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/sse/S3SSECustomerKeyGenerator.java:
##########
@@ -0,0 +1,54 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws.sse;
+
+import java.security.NoSuchAlgorithmException;
+import java.security.spec.InvalidKeySpecException;
+import java.util.Base64;
+
+import javax.crypto.SecretKeyFactory;
+import javax.crypto.spec.PBEKeySpec;
+
+import com.google.common.hash.Hashing;
+
+public class S3SSECustomerKeyGenerator {
+
+ public static final int ITERATION_COUNT = 65536;
+ public static final int KEY_LENGTH = 256;
+ public static final SecretKeyFactory SECRET_KEY_FACTORY;
+
+ static {
+ try {
+ SECRET_KEY_FACTORY = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256");
+ } catch (NoSuchAlgorithmException e) {
+ throw new RuntimeException("Could not initialize SecretKeyFactory", e);
+ }
+ }
+
+ public static String generateCustomerKey(String masterPassword, String salt) throws InvalidKeySpecException {
+ PBEKeySpec spec = new PBEKeySpec(masterPassword.toCharArray(), salt.getBytes(), ITERATION_COUNT, KEY_LENGTH);
+ byte[] derivedKey = SECRET_KEY_FACTORY.generateSecret(spec).getEncoded();
Review Comment:
Can this code be called from multiple threads? Because SecretKeyFactory#generateSecret is not pure and it does not guarantee thread-safe AFAIK.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] WIP - JAMES-4085 [S3 SSEC] SSEC basic implementation [james-project]
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #2488:
URL: https://github.com/apache/james-project/pull/2488#discussion_r1830631088
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/sse/S3SSECustomerKeyGenerator.java:
##########
@@ -0,0 +1,54 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws.sse;
+
+import java.security.NoSuchAlgorithmException;
+import java.security.spec.InvalidKeySpecException;
+import java.util.Base64;
+
+import javax.crypto.SecretKeyFactory;
+import javax.crypto.spec.PBEKeySpec;
+
+import com.google.common.hash.Hashing;
+
+public class S3SSECustomerKeyGenerator {
+
+ public static final int ITERATION_COUNT = 65536;
+ public static final int KEY_LENGTH = 256;
+ public static final SecretKeyFactory SECRET_KEY_FACTORY;
+
+ static {
+ try {
+ SECRET_KEY_FACTORY = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256");
+ } catch (NoSuchAlgorithmException e) {
+ throw new RuntimeException("Could not initialize SecretKeyFactory", e);
+ }
+ }
+
+ public static String generateCustomerKey(String masterPassword, String salt) throws InvalidKeySpecException {
+ PBEKeySpec spec = new PBEKeySpec(masterPassword.toCharArray(), salt.getBytes(), ITERATION_COUNT, KEY_LENGTH);
+ byte[] derivedKey = SECRET_KEY_FACTORY.generateSecret(spec).getEncoded();
Review Comment:
Sorry for force push,
the test case for it: `S3SSECustomerKeyGeneratorTest#generateShouldWorkCorrectWhenConcurrent`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-4085 [S3 SSEC] SSEC basic implementation [james-project]
Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on PR #2488:
URL: https://github.com/apache/james-project/pull/2488#issuecomment-2464296396
read it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-4085 [S3 SSEC] SSEC basic implementation [james-project]
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #2488:
URL: https://github.com/apache/james-project/pull/2488#discussion_r1833841192
##########
server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/sse/SingleCustomerKeyFactoryTest.java:
##########
@@ -0,0 +1,64 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws.sse;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+
+import java.security.spec.InvalidKeySpecException;
+
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.blob.api.TestBlobId;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import reactor.core.publisher.Mono;
+
+public class SingleCustomerKeyFactoryTest {
+ private static S3SSECustomerKeyFactory.SingleCustomerKeyFactory testee;
+
+ @BeforeAll
+ static void setup() throws InvalidKeySpecException {
+ testee = new S3SSECustomerKeyFactory.SingleCustomerKeyFactory(new S3SSECustomerConfiguration(true, "AES256", "masterPassword", "salt"));
+
+ }
+
+ @Test
+ void generateShouldReturnSSECustomerKey() {
+ // When
+ S3SSECustomerKeyFactory.SSECustomerKey sseCustomerKey = Mono.from(testee.generate(BucketName.of("bucket1"), new TestBlobId("blobId"))).block();
+
+ // Then
+ assertThat(sseCustomerKey.customerKey()).isEqualTo("tsY5n0n10fcpjm7aGArqbw231ptLGAF5ubxMxI2+QYw=");
+ assertThat(sseCustomerKey.md5()).isEqualTo("G6IB1YLHiY/9uCh3TJhxzQ==");
+ assertThat(sseCustomerKey.algorithm()).isEqualTo("AES256");
+ }
+
+ @Test
+ void generateShouldReturnSameSSECustomerKey() {
+ // When
+ S3SSECustomerKeyFactory.SSECustomerKey sseCustomerKey1 = Mono.from(testee.generate(BucketName.of("bucket1"), new TestBlobId("blobId"))).block();
+ S3SSECustomerKeyFactory.SSECustomerKey sseCustomerKey2 = Mono.from(testee.generate(BucketName.of("bucket1"), new TestBlobId("blobId"))).block();
Review Comment:
this is my purpose
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] WIP - JAMES-4085 [S3 SSEC] S3 multitenancy: SSEC basic implementation [james-project]
Posted by "HoussemNasri (via GitHub)" <gi...@apache.org>.
HoussemNasri commented on code in PR #2488:
URL: https://github.com/apache/james-project/pull/2488#discussion_r1829610345
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/sse/S3SSECustomerKeyGenerator.java:
##########
@@ -0,0 +1,54 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws.sse;
+
+import java.security.NoSuchAlgorithmException;
+import java.security.spec.InvalidKeySpecException;
+import java.util.Base64;
+
+import javax.crypto.SecretKeyFactory;
+import javax.crypto.spec.PBEKeySpec;
+
+import com.google.common.hash.Hashing;
+
+public class S3SSECustomerKeyGenerator {
+
+ public static final int ITERATION_COUNT = 65536;
+ public static final int KEY_LENGTH = 256;
+ public static final SecretKeyFactory SECRET_KEY_FACTORY;
+
+ static {
+ try {
+ SECRET_KEY_FACTORY = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256");
+ } catch (NoSuchAlgorithmException e) {
+ throw new RuntimeException("Could not initialize SecretKeyFactory", e);
+ }
+ }
+
+ public static String generateCustomerKey(String masterPassword, String salt) throws InvalidKeySpecException {
+ PBEKeySpec spec = new PBEKeySpec(masterPassword.toCharArray(), salt.getBytes(), ITERATION_COUNT, KEY_LENGTH);
+ byte[] derivedKey = SECRET_KEY_FACTORY.generateSecret(spec).getEncoded();
Review Comment:
Can this code be called from multiple threads? Because SecretKeyFactory#generateSecret is not pure and it does not guarantee thread-safety AFAIK.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-4085 [S3 SSEC] SSEC basic implementation [james-project]
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #2488:
URL: https://github.com/apache/james-project/pull/2488#discussion_r1839338331
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/sse/S3SSECConfiguration.java:
##########
@@ -23,20 +23,23 @@
import com.google.common.base.Preconditions;
-public record S3SSECustomerConfiguration(boolean enabled,
- String algorithm,
- String masterPassword,
- String salt) {
+public interface S3SSECConfiguration {
- public static final List<String> SUPPORTED_ALGORITHMS = List.of("AES256");
+ List<String> SUPPORTED_ALGORITHMS = List.of("AES256");
- public S3SSECustomerConfiguration(boolean enabled, String algorithm, String masterPassword, String salt) {
- this.enabled = enabled;
- if (enabled) {
- Preconditions.checkArgument(SUPPORTED_ALGORITHMS.contains(algorithm), "Unsupported algorithm: %s. The supported algorithms are: %s", algorithm, SUPPORTED_ALGORITHMS);
+ String algorithm();
+
+ record Basic(String algorithm,
+ String masterPassword,
+ String salt) implements S3SSECConfiguration {
+ public Basic {
+ Preconditions.checkArgument(SUPPORTED_ALGORITHMS.contains(algorithm), "Unsupported algorithm: " + algorithm + ". The supported algorithms are: " + SUPPORTED_ALGORITHMS);
+ }
+ }
+
+ record KeyDerivationEnabled(String algorithm) implements S3SSECConfiguration {
Review Comment:
No answer to that remark?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-4085 [S3 SSEC] SSEC basic implementation [james-project]
Posted by "HoussemNasri (via GitHub)" <gi...@apache.org>.
HoussemNasri commented on code in PR #2488:
URL: https://github.com/apache/james-project/pull/2488#discussion_r1847031119
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreDAO.java:
##########
@@ -166,64 +177,79 @@ private static class FluxResponse {
}
private Mono<FluxResponse> getObject(BucketName bucketName, BlobId blobId) {
- return Mono.fromFuture(() ->
- client.getObject(
- builder -> builder.bucket(bucketName.asString()).key(blobId.asString()),
- new AsyncResponseTransformer<GetObjectResponse, FluxResponse>() {
-
- FluxResponse response;
-
- @Override
- public CompletableFuture<FluxResponse> prepare() {
- response = new FluxResponse();
- return response.supportingCompletableFuture;
- }
-
- @Override
- public void onResponse(GetObjectResponse response) {
- this.response.sdkResponse = response;
- }
-
- @Override
- public void exceptionOccurred(Throwable error) {
- this.response.supportingCompletableFuture.completeExceptionally(error);
- }
-
- @Override
- public void onStream(SdkPublisher<ByteBuffer> publisher) {
- response.flux = Flux.from(publisher);
- response.supportingCompletableFuture.complete(response);
- }
- }))
- .switchIfEmpty(Mono.error(() -> new ObjectStoreIOException("Request was unexpectedly canceled, no GetObjectResponse")));
+ return buildGetObjectRequestBuilder(bucketName, blobId)
+ .flatMap(getObjectRequestBuilder -> Mono.fromFuture(() ->
+ client.getObject(getObjectRequestBuilder.build(),
+ new AsyncResponseTransformer<GetObjectResponse, FluxResponse>() {
+
+ FluxResponse response;
+
+ @Override
+ public CompletableFuture<FluxResponse> prepare() {
+ response = new FluxResponse();
+ return response.supportingCompletableFuture;
+ }
+
+ @Override
+ public void onResponse(GetObjectResponse response) {
+ this.response.sdkResponse = response;
+ }
+
+ @Override
+ public void exceptionOccurred(Throwable error) {
+ this.response.supportingCompletableFuture.completeExceptionally(error);
+ }
+
+ @Override
+ public void onStream(SdkPublisher<ByteBuffer> publisher) {
+ response.flux = Flux.from(publisher);
+ response.supportingCompletableFuture.complete(response);
+ }
+ }))
+ .switchIfEmpty(Mono.error(() -> new ObjectStoreIOException("Request was unexpectedly canceled, no GetObjectResponse"))));
}
@Override
public Mono<byte[]> readBytes(BucketName bucketName, BlobId blobId) {
BucketName resolvedBucketName = bucketNameResolver.resolve(bucketName);
- return Mono.fromFuture(() ->
- client.getObject(
- builder -> builder.bucket(resolvedBucketName.asString()).key(blobId.asString()),
- new MinimalCopyBytesResponseTransformer(configuration, blobId)))
- .onErrorMap(NoSuchBucketException.class, e -> new ObjectNotFoundException("Bucket not found " + resolvedBucketName.asString(), e))
- .onErrorMap(NoSuchKeyException.class, e -> new ObjectNotFoundException("Blob not found " + blobId.asString() + " in bucket " + resolvedBucketName.asString(), e))
- .publishOn(Schedulers.parallel())
- .map(BytesWrapper::asByteArrayUnsafe)
- .onErrorMap(e -> e.getCause() instanceof OutOfMemoryError, Throwable::getCause);
+ return buildGetObjectRequestBuilder(resolvedBucketName, blobId)
+ .flatMap(putObjectRequest -> Mono.fromFuture(() ->
+ client.getObject(putObjectRequest.build(), new MinimalCopyBytesResponseTransformer(configuration, blobId)))
+ .onErrorMap(NoSuchBucketException.class, e -> new ObjectNotFoundException("Bucket not found " + resolvedBucketName.asString(), e))
+ .onErrorMap(NoSuchKeyException.class, e -> new ObjectNotFoundException("Blob not found " + blobId.asString() + " in bucket " + resolvedBucketName.asString(), e))
+ .publishOn(Schedulers.parallel())
+ .map(BytesWrapper::asByteArrayUnsafe)
+ .onErrorMap(e -> e.getCause() instanceof OutOfMemoryError, Throwable::getCause));
+ }
+
+ private Mono<GetObjectRequest.Builder> buildGetObjectRequestBuilder(BucketName bucketName, BlobId blobId) {
+ GetObjectRequest.Builder baseBuilder = GetObjectRequest.builder()
+ .bucket(bucketName.asString())
+ .key(blobId.asString());
+
+ if (s3RequestOption.ssec().enable()) {
+ return Mono.from(s3RequestOption.ssec().sseCustomerKeyFactory().get()
+ .generate(bucketName, blobId))
+ .map(sseCustomerKey -> baseBuilder
+ .sseCustomerAlgorithm(sseCustomerKey.ssecAlgorithm())
Review Comment:
Are we certain the ssecAlgorithm used to create the object is the same as the one currently configured? Consider this scenario: SSE-C is configured to encrypt objects with AES256, an object A is uploaded using that configuration, then the configured decryption algorihtm is changed to DES: What happends when we I try download object A? Is it correctly decrypted? Maybe we can use [`get-bucket-encryption`](https://docs.aws.amazon.com/cli/latest/reference/s3api/get-bucket-encryption.html)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-4085 [S3 SSEC] SSEC basic implementation [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2488:
URL: https://github.com/apache/james-project/pull/2488#discussion_r1843291371
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/sse/S3WithSSECBlobStoreDAO.java:
##########
@@ -0,0 +1,68 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws.sse;
+
+import org.apache.james.blob.api.BlobId;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.blob.objectstorage.aws.S3BlobStoreConfiguration;
+import org.apache.james.blob.objectstorage.aws.S3BlobStoreDAO;
+import org.apache.james.blob.objectstorage.aws.S3ClientFactory;
+
+import reactor.core.publisher.Mono;
+import software.amazon.awssdk.services.s3.model.GetObjectRequest;
+import software.amazon.awssdk.services.s3.model.PutObjectRequest;
+
+public class S3WithSSECBlobStoreDAO extends S3BlobStoreDAO {
Review Comment:
I think we should rather have SSE-C being an optional feature of the S3BlobStoreDAO rahter than it being yet-another BlobStoreDAO
I think we shall refactor this proposal so that it is the case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-4085 [S3 SSEC] SSEC basic implementation [james-project]
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #2488:
URL: https://github.com/apache/james-project/pull/2488#discussion_r1833842224
##########
server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/sse/SingleCustomerKeyFactoryTest.java:
##########
@@ -0,0 +1,64 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.objectstorage.aws.sse;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+
+import java.security.spec.InvalidKeySpecException;
+
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.blob.api.TestBlobId;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import reactor.core.publisher.Mono;
+
+public class SingleCustomerKeyFactoryTest {
+ private static S3SSECustomerKeyFactory.SingleCustomerKeyFactory testee;
+
+ @BeforeAll
+ static void setup() throws InvalidKeySpecException {
+ testee = new S3SSECustomerKeyFactory.SingleCustomerKeyFactory(new S3SSECustomerConfiguration(true, "AES256", "masterPassword", "salt"));
+
+ }
+
+ @Test
+ void generateShouldReturnSSECustomerKey() {
+ // When
+ S3SSECustomerKeyFactory.SSECustomerKey sseCustomerKey = Mono.from(testee.generate(BucketName.of("bucket1"), new TestBlobId("blobId"))).block();
+
+ // Then
+ assertThat(sseCustomerKey.customerKey()).isEqualTo("tsY5n0n10fcpjm7aGArqbw231ptLGAF5ubxMxI2+QYw=");
+ assertThat(sseCustomerKey.md5()).isEqualTo("G6IB1YLHiY/9uCh3TJhxzQ==");
+ assertThat(sseCustomerKey.algorithm()).isEqualTo("AES256");
+ }
+
+ @Test
+ void generateShouldReturnSameSSECustomerKey() {
+ // When
+ S3SSECustomerKeyFactory.SSECustomerKey sseCustomerKey1 = Mono.from(testee.generate(BucketName.of("bucket1"), new TestBlobId("blobId"))).block();
+ S3SSECustomerKeyFactory.SSECustomerKey sseCustomerKey2 = Mono.from(testee.generate(BucketName.of("bucket1"), new TestBlobId("blobId"))).block();
Review Comment:
it will return same customerKey, even when similar or different bucktname, blobId
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org