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

service/s3/s3crypto: Add support for client-side S3 encryption using Asymmetric key #1241

Closed
alethenorio opened this issue May 2, 2017 · 17 comments
Labels
blocked Work is blocked on this issue for this codebase. Other labels or comments may indicate why. closed-for-staleness feature-request A feature should be added or improved. third-party This issue is related to third-party libraries or applications.

Comments

@alethenorio
Copy link

Please fill out the sections below to help us address your issue.

Version of AWS SDK for Go?

master 984e0dc

Version of Go (go version)?

1.8

What issue did you see?

Attempting to perform client side s3 encryption as explained here

http://docs.aws.amazon.com/AmazonS3/latest/dev/encrypt-client-side-asymmetric-master-key.html

However it seems the library only supports kms keys and not any client side keys. Is this just not implemented yet or am I missing something?

Steps to reproduce

Looked at s3crypto.NewEncryptionClient API and it seems to only support kms keys.

@xibz xibz added the feature-request A feature should be added or improved. label May 2, 2017
@xibz
Copy link
Contributor

xibz commented May 2, 2017

Hello @ByteFlinger, currently only kms is supported. We have this in our backlog, but I'll mark this as a feature request. You can also implement your own key wrap handler, if needed. I will bring this up in our next planning meeting. Cheers!

@alethenorio
Copy link
Author

alethenorio commented May 2, 2017 via email

@xibz
Copy link
Contributor

xibz commented May 2, 2017

@ByteFlinger, yea that is a good high level explanation. The RSA algorithm would be RSA/ECB/OAEPWithSHA-256AndMGF1Padding. As long as you implement the key wrapping interface, it shouldn't be any different to when we decide to support it.

The EncryptedKey should be the encrypted symmetric key that was randomly generated and shouldn't be ignored. What I would do is write a wrapper for RSA that supports the key wrapping interface. I'd then write unit tests to ensure everything is being properly encrypted and decrypted. I would even ensure cross compatibility between SDKs by encrypting with one SDK and decrypting with another.

@alethenorio
Copy link
Author

alethenorio commented May 9, 2017

@xibz I have taken a look at the whole thing and am left wondering one thing. Do you guys have any specification on what you usually as the label when calling DecryptOAEP/EncryptOAEP?

Also, how does one upload encrypted data using the s3Manager.NewUploaderWithClient?

It seems that the Encryption and Decryption client do not embedding of the S3Client therefore they do not qualify as a client for the NewUploaderWithClient function

@xibz
Copy link
Contributor

xibz commented May 9, 2017

@ByteFlinger - The label that is supported for RSA is RSA/ECB/OAEPWithSHA-256AndMGF1Padding. That algorithm, RSA/ECB/OAEPWithSHA-256AndMGF1Padding, is a Java cipher, and I believe it is apart of one of their crypto libraries. You will need to mimic their implementation to ensure you have cross compatibility for our future implementation.

Currently the crypto client does not support being used in s3manager. However, we do have it in our backlog along with implementing RSA for a key wrap algorithm.

@alethenorio
Copy link
Author

alethenorio commented May 9, 2017

I was not able to find any label information on the Java SDK so I am uncertain whether this is a Go implementation or the algorithm itself. Can you confirm that when you mention RSA/ECB/OAEPWithSHA-256AndMGF1Padding, you mean the label that is later required to decrypt the key itself (because apparently according to Go documentation an encrypted value using a certain label can only be decrypted using the same label)?

I have had some success implementing the whole thing and will be verifying it against the Java SDK. One thing which I am uncertain of if the MGF1 padding. While Go has sha256 which can be used with OAEP, I am not sure where MGF1 fits in the whole picture. I should add that while familiar with PKI I am by no means a crypto expert. I believe that MGF1 padding is already used by Go when using OAEP but uncertain so any guidelines on the matter are welcome (and I suppose any issues will arise when testing against the Java SDK).

Regarding the s3manager support, I was able to basically wrap the Encryption and Decryption clients in a standard s3.S3 client (since they implement some of the APIs of the s3.S3API) and use that with the s3manager upload and download and it seemed to work although I have not performed any extensive verification. Should I refrain from doing that? Do you think it might be an issue?

@xibz
Copy link
Contributor

xibz commented May 9, 2017

For paragraph 1, yes, that is the label that is used for decryption. Here is the Java SDK label.

Paragraph 2: Yes, Golang's OAEP will use MGF1 padding.

Paragraph 3: Yea, I don't see any issues with using the crypto client with s3manager.

@alethenorio
Copy link
Author

alethenorio commented May 17, 2017

Hi

I spend some time going through everything and digging into the JDK SDK. The example I linked in the original post ends up encrypting using ENCRYPTION-ONLY which seems to use no wrapping algo (Thus RSA/ECB/OAEPWithSHA-256AndMGF1Padding is not used) which the Go SDK cannot decrypt because it complains about not supporting V1 headers.

Setting encryption mode in the Java SDK to Authenticated solves the issue and it uses RSA/ECB/OAEPWithSHA-256AndMGF1Padding for encryption including V2 headers.

Having done all that I cannot seem to verify encryption/decrpytion using the JDK SDK.

I get the following error when trying to decrypt a go encrypted value in Java

Exception in thread "main" com.amazonaws.AmazonClientException: Unable to decrypt symmetric key from object metadata
	at com.amazonaws.util.Throwables.failure(Throwables.java:74)
	at com.amazonaws.services.s3.internal.crypto.ContentCryptoMaterial.cek(ContentCryptoMaterial.java:297)
	at com.amazonaws.services.s3.internal.crypto.ContentCryptoMaterial.fromObjectMetadata0(ContentCryptoMaterial.java:422)
	at com.amazonaws.services.s3.internal.crypto.ContentCryptoMaterial.fromObjectMetadata(ContentCryptoMaterial.java:345)
	at com.amazonaws.services.s3.internal.crypto.S3CryptoModuleAE.decipherWithMetadata(S3CryptoModuleAE.java:249)
	at com.amazonaws.services.s3.internal.crypto.S3CryptoModuleAE.decipher(S3CryptoModuleAE.java:150)
	at com.amazonaws.services.s3.internal.crypto.S3CryptoModuleAE.getObjectSecurely(S3CryptoModuleAE.java:128)
	at com.amazonaws.services.s3.internal.crypto.CryptoModuleDispatcher.getObjectSecurely(CryptoModuleDispatcher.java:116)
	at com.amazonaws.services.s3.AmazonS3EncryptionClient.getObject(AmazonS3EncryptionClient.java:575)
	at com.amazonaws.services.s3.AmazonS3Client.getObject(AmazonS3Client.java:1263)
	at com.company.Application.main(Application.java:88)
Caused by: java.security.InvalidKeyException: Unwrapping failed
	at com.sun.crypto.provider.RSACipher.engineUnwrap(RSACipher.java:445)
	at javax.crypto.Cipher.unwrap(Cipher.java:2550)
	at com.amazonaws.services.s3.internal.crypto.ContentCryptoMaterial.cek(ContentCryptoMaterial.java:281)
	... 9 more
Caused by: javax.crypto.BadPaddingException: Decryption error
	at sun.security.rsa.RSAPadding.unpadOAEP(RSAPadding.java:499)
	at sun.security.rsa.RSAPadding.unpad(RSAPadding.java:293)
	at com.sun.crypto.provider.RSACipher.doFinal(RSACipher.java:363)
	at com.sun.crypto.provider.RSACipher.engineUnwrap(RSACipher.java:440)
	... 11 more

And doing the reverse (decrypting Java encrypted value in Go)

crypto/rsa: decryption error

The key handler implementation Go code is rather simple so I don't quite see where I could be maknig a mistake. Here are some snippets

// Generate random aesKey
aesKey := make([]byte, 32)
rand.Read(aesKey)

// Encrypt with RSA key
cipherText, err := rsa.EncryptOAEP(sha256.New(), rand.Reader, publicKey, aesKey, nil)

// Generate random IV.
iv := make([]byte, ivSize)
rand.Read(iv)

// s3crypto.CipherData
CipherData{
		Key:                 aesKey
		IV:                  iv,
		WrapAlgorithm:       "RSA/ECB/OAEPWithSHA-256AndMGF1Padding",
		MaterialDescription: MaterialDescription{},
		EncryptedKey:        cipherText,
}

// Decrypt Key
rsa.DecryptOAEP(sha256.New(), rand.Reader, privateKey, key, nil)

// Build encryption/decryption clients
s3crypto.NewDecryptionClient(session, func(c *s3crypto.DecryptionClient) {
			c.WrapRegistry = map[string]s3crypto.WrapEntry{
				RSAWrap: (rsaKeyHandler{}).decryptHandler,
			}
}

s3crypto.NewEncryptionClient(session, s3crypto.AESGCMContentCipherBuilder(rsaKeyGenerator))

I also tried setting the label to RSA/ECB/OAEPWithSHA-256AndMGF1Padding rather than nil as we talked above but the issues were the same.
Comparing the metadata headers in S3 indicates that they are pretty much the same (Go sets x-amz-meta-x-amz-unencrypted-content-length and x-amz-meta-x-amz-unencrypted-content-md5 which Java does not).

The java error seems to tell me that there might be some issue witht the Padding which I also noticed that it is not actually PKCS5Padding as per comments in the code.

Before I give this up, would there be any chance you can take a quick look at the above and see if you can spot any issue?

@xibz
Copy link
Contributor

xibz commented May 17, 2017

@ByteFlinger - With encryption and decryption it is going to be pretty difficult to see whether or not this is encrypting/decrypting properly. Do you have some tests in place to see if it is encrypting and decrypting properly? I would also run those test vectors in Java to ensure you are getting the appropriate values as expected. Have you pushed this code up? I want to take a look at the overall implementation to see if I can see anything.

@alethenorio
Copy link
Author

alethenorio commented May 17, 2017 via email

@jasdel jasdel changed the title Support for client-side S3 encryption using Asymmetric key missing? service/s3/s3crypto: Add support for client-side S3 encryption using Asymmetric key Nov 9, 2017
@tfeng
Copy link

tfeng commented Dec 23, 2017

@ByteFlinger In my project, I experienced the exact same problem as yours. Thanks to your code skeleton, I was able to encrypt/decrypt files in S3 with RSA key pairs. But my Java program gave the exact same error. Did you figure out the reason?

@alethenorio
Copy link
Author

Unfortunately not.

I did not have the time to spend on this a lot more and layed the issue aside for the moment.

@rogaha
Copy link

rogaha commented Apr 23, 2018

Any updates here? Is java the only official SDK available supporting the Client-Side Master Key method?

@xibz xibz added blocked Work is blocked on this issue for this codebase. Other labels or comments may indicate why. third-party This issue is related to third-party libraries or applications. labels Apr 24, 2018
@xibz
Copy link
Contributor

xibz commented Apr 24, 2018

@rogaha - Currently there is a limitation in Go's RSA crypto library which doesn't allow specification of the MGF1 hash separate from the cipher hash. See here. The cleanest solution would be to wait until Go's standard library has support for the distinction of which hashes to use for the padding and cipher.

@ByteFlinger @tfeng - This is the reason to why you guys were receiving that Java error. Java uses SHA-1 for its MGF1 padding when specifying RSA/ECB/OAEPWithSHA-256AndMGF1Padding, but SHA-256 for the cipher. When you pass the SHA-256 hash.Hash to the DecryptOAEP, it will use SHA-256 for both the cipher and the MGF1 padder.

@alethenorio
Copy link
Author

@xibz An update on the matter is much appreciated and explains the issue.

Here's hoping it does not take another year for this feature to be added in Go

@diehlaws diehlaws added 3rd-party and removed third-party This issue is related to third-party libraries or applications. labels Jan 4, 2019
@ArcticSnowman
Copy link

And another year has gone by...

@diehlaws diehlaws added third-party This issue is related to third-party libraries or applications. and removed 3rd-party labels Feb 25, 2020
@github-actions
Copy link

We have noticed this issue has not received attention in 1 year. We will close this issue for now. If you think this is in error, please feel free to comment and reopen the issue.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Feb 25, 2021
@github-actions github-actions bot closed this as completed Mar 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Work is blocked on this issue for this codebase. Other labels or comments may indicate why. closed-for-staleness feature-request A feature should be added or improved. third-party This issue is related to third-party libraries or applications.
Projects
None yet
Development

No branches or pull requests

6 participants