-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
S3 client side encryption #16843
S3 client side encryption #16843
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,6 +31,11 @@ | |
import com.amazonaws.internal.StaticCredentialsProvider; | ||
import com.amazonaws.services.s3.AmazonS3; | ||
import com.amazonaws.services.s3.AmazonS3Client; | ||
import com.amazonaws.services.s3.AmazonS3EncryptionClient; | ||
import com.amazonaws.services.s3.model.CryptoConfiguration; | ||
import com.amazonaws.services.s3.model.EncryptionMaterials; | ||
import com.amazonaws.services.s3.model.EncryptionMaterialsProvider; | ||
import com.amazonaws.services.s3.model.StaticEncryptionMaterialsProvider; | ||
import org.elasticsearch.ElasticsearchException; | ||
import org.elasticsearch.common.Strings; | ||
import org.elasticsearch.common.collect.Tuple; | ||
|
@@ -49,15 +54,17 @@ public class InternalAwsS3Service extends AbstractLifecycleComponent<AwsS3Servic | |
/** | ||
* (acceskey, endpoint) -> client | ||
*/ | ||
private Map<Tuple<String, String>, AmazonS3Client> clients = new HashMap<>(); | ||
private Map<Tuple<String, Tuple<String, EncryptionMaterials>>, AmazonS3Client> clients = new HashMap<>(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand why we need to add the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thx David, I was confused too, I think this is make sense, this allows you to use a different key for each snapshot repository. For example you can have two repos, with two different keys :
Please let me know if I'm doing something wrong here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok that makes sense to me. |
||
|
||
@Inject | ||
public InternalAwsS3Service(Settings settings) { | ||
super(settings); | ||
} | ||
|
||
@Override | ||
public synchronized AmazonS3 client(String endpoint, Protocol protocol, String region, String account, String key, Integer maxRetries) { | ||
public synchronized AmazonS3 client(String endpoint, Protocol protocol, String region, String account, String key, Integer maxRetries, | ||
EncryptionMaterials clientSideEncryptionMaterials) { | ||
|
||
if (Strings.isNullOrEmpty(endpoint)) { | ||
// We need to set the endpoint based on the region | ||
if (region != null) { | ||
|
@@ -69,11 +76,14 @@ public synchronized AmazonS3 client(String endpoint, Protocol protocol, String r | |
} | ||
} | ||
|
||
return getClient(endpoint, protocol, account, key, maxRetries); | ||
return getClient(endpoint, protocol, account, key, maxRetries, clientSideEncryptionMaterials); | ||
} | ||
|
||
private synchronized AmazonS3 getClient(String endpoint, Protocol protocol, String account, String key, Integer maxRetries) { | ||
Tuple<String, String> clientDescriptor = new Tuple<>(endpoint, account); | ||
private synchronized AmazonS3 getClient(String endpoint, Protocol protocol, String account, String key, Integer maxRetries, | ||
EncryptionMaterials clientSideEncryptionMaterials) { | ||
|
||
Tuple<String, EncryptionMaterials> tempTuple = new Tuple<>(account, clientSideEncryptionMaterials); | ||
Tuple<String, Tuple<String, EncryptionMaterials>> clientDescriptor = new Tuple<>(endpoint, tempTuple); | ||
AmazonS3Client client = clients.get(clientDescriptor); | ||
if (client != null) { | ||
return client; | ||
|
@@ -123,7 +133,18 @@ private synchronized AmazonS3 getClient(String endpoint, Protocol protocol, Stri | |
new StaticCredentialsProvider(new BasicAWSCredentials(account, key)) | ||
); | ||
} | ||
client = new AmazonS3Client(credentials, clientConfiguration); | ||
|
||
if (clientSideEncryptionMaterials != null) { | ||
EncryptionMaterialsProvider encryptionMaterialsProvider = new StaticEncryptionMaterialsProvider(clientSideEncryptionMaterials); | ||
CryptoConfiguration cryptoConfiguration = new CryptoConfiguration(); | ||
client = new AmazonS3EncryptionClient( | ||
credentials, | ||
encryptionMaterialsProvider, | ||
clientConfiguration, | ||
cryptoConfiguration); | ||
} else { | ||
client = new AmazonS3Client(credentials, clientConfiguration); | ||
} | ||
|
||
if (endpoint != null) { | ||
client.setEndpoint(endpoint); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering about this. It's all about encryption.
We already have
server_side_encryption
. I wonder if we should at some point rename the settings (may be within another PR) to:Just thinking out loud here. Might be too much engineering though...