Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[FEATURE REQ] Provide an implementation of 'AsyncKeyEncryptionKey' that uses a local symmetric key #6569

Closed
2 tasks done
SukruthKS opened this issue Nov 26, 2019 · 14 comments
Closed
2 tasks done
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. feature-request This issue requires a new behavior in the product in order be resolved. KeyVault

Comments

@SukruthKS
Copy link

Is your feature request related to a problem? Please describe.
With version 8 of the storage SDK, we are using SymmetricKey for client-side encryption of Azure storage blobs. This implementation performs encryption locally on the machine and accepts an in-memory encryption key. The new version 12 of the storage SDK (azure-storage-blob-cryptography) uses a new interface AsyncKeyEncryptionKey for the encryption key and I couldn't find an implementation of it like the SymmetricKey class. There is one implementation that Rick from storage sdk team pointed me to KeyEncryptionKeyClient but it requires the encryption key to be present in key vault. More details in this query - #6536.

Describe the solution you'd like
An implementation of AsyncKeyEncryptionKey that mimics the functionality provided by SymmetricKey.

Describe alternatives you've considered
No other alternatives have been considered yet.

Additional context
NA

Information Checklist
Kindly make sure that you have added all the following information above and checkoff the required fields otherwise we will treat the issuer as an incomplete report

  • Description Added
  • Expected solution specified
@rickle-msft rickle-msft added Client This issue points to a problem in the data-plane of the library. feature-request This issue requires a new behavior in the product in order be resolved. KeyVault labels Nov 27, 2019
@joshfree joshfree assigned g2vinay and 04diiguyi and unassigned 04diiguyi Dec 10, 2019
@SukruthKS
Copy link
Author

@g2vinay what is the ETA for this?

@g2vinay
Copy link
Member

g2vinay commented Jan 14, 2020

@SukruthKS,
Thanks for following up.
I am currently working on this feature.
It will be feature complete in January and will be available as part of our next monthly release in early February.

@g2vinay
Copy link
Member

g2vinay commented Feb 17, 2020

@SukruthKS
The Febburay KV-keys release, supports this feature.
Here is the package link: https://search.maven.org/artifact/com.azure/azure-security-keyvault-keys/4.2.0-beta.1/jar

Sample Code to build AsyncKeyEncryptionKey using local symmetric key:
JsonWebKey localKey = JsonWebKey.fromAes(new SecretKeySpec(localSymmeticKeyBytes, "AES"), Arrays.asList(KeyOperation.WRAP_KEY, KeyOperation.UNWRAP_KEY));
KeyVaultKey kvKey = KeyVaultKey.fromName("localSymmeticKey", localKey);

AsyncKeyEncryptionKey akek = new KeyEncryptionKeyClientBuilder()
.credential(credentialReal)
.buildAsyncKeyEncryptionKey(kvKey).block();

@SukruthKS
Copy link
Author

@g2vinay in the sample code you provided above, why should I pass any credentials? I'm not using a key vault and this is just a local symmetric key. If I don't provide the credentials, I get the below error when building the key object.

ERROR com.azure.security.keyvault.keys.cryptography.KeyEncryptionKeyClientBuilder - Key Vault credentials cannot be null and are required to build the key encryption key async client
AsyncKeyEncryptionKey akek = new KeyEncryptionKeyClientBuilder()
.credential(credentialReal)
.buildAsyncKeyEncryptionKey(kvKey).block();

@g2vinay
Copy link
Member

g2vinay commented Feb 28, 2020

@SukruthKS
Thank you for the follow up.
Currently, KeyEncryptionKeyClient is designed to support both Key Vault keys and local keys, and it uses the Cryptography Client underneath for crypto operations. The cryptography client is not enabled for local cryptography operations yet, so that's why the credential is needed.
For Local keys, you can pass an empty credential as follows:

AsyncKeyEncryptionKey akek = new KeyEncryptionKeyClientBuilder()
                    .credential(trc -> Mono.empty())
                   .buildAsyncKeyEncryptionKey(kvKey).block();

In the upcoming releases, we will be enabling local crypto on Cryptography client as well, it is captured in this issue #8006
So, your feedback is definitely helpful and will be taken into account for local crypto flow on Cryptography client before it releases.
I will update you once it is out.

@SukruthKS
Copy link
Author

Thanks!

@SukruthKS
Copy link
Author

Thanks, Vinay! I ran into another issue when using the local symmetric key with storage client. It appears that KeyId is not getting set which causes a null pointer exception.

Code:

JsonWebKey jsonWebKey = JsonWebKey.fromAes(
    new SecretKeySpec(encryptionKeyBytes, "AES"),
    Arrays.asList(KeyOperation.WRAP_KEY, KeyOperation.UNWRAP_KEY));

KeyVaultKey localKek = KeyVaultKey.fromName(
    "localSymmetricKey",
    jsonWebKey);

AsyncKeyEncryptionKey akek = new KeyEncryptionKeyClientBuilder()
    .credential(new BasicAuthenticationCredential("dummy", "dummy"))
    .buildAsyncKeyEncryptionKey(localKek)
    .block();

EncryptedBlobClient encryptedBlobClient = new EncryptedBlobClientBuilder()
    .endpoint(serviceClient.getAccountUrl())
    .sasToken("<SAS token>")
    .containerName(containerName)
    .blobName(blobName)
    .key(akek, KeyWrapAlgorithm.A256KW.toString())
    .buildEncryptedBlobClient();

encryptedBlobClient.uploadFromFile(filepath);

Stack trace:

Exception in thread "main" java.lang.NullPointerException: value
	at java.base/java.util.Objects.requireNonNull(Objects.java:246)
	at reactor.core.publisher.MonoJust.<init>(MonoJust.java:34)
	at reactor.core.publisher.Mono.just(Mono.java:560)
	at com.azure.security.keyvault.keys.cryptography.CryptographyAsyncClient.lambda$getKeyId$0(CryptographyAsyncClient.java:131)
	at reactor.core.publisher.MonoDefer.subscribe(MonoDefer.java:44)
	at reactor.core.publisher.Mono.subscribe(Mono.java:4087)
	at reactor.core.publisher.Mono.block(Mono.java:1662)
	at com.azure.storage.blob.specialized.cryptography.EncryptedBlobAsyncClient.lambda$encryptBlob$8(EncryptedBlobAsyncClient.java:373)
	at reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.onNext(FluxMapFuseable.java:107)
	at reactor.core.publisher.Operators$MonoSubscriber.complete(Operators.java:1592)
	at reactor.core.publisher.MonoFlatMap$FlatMapMain.onNext(MonoFlatMap.java:144)
	at reactor.core.publisher.Operators$MonoSubscriber.complete(Operators.java:1592)
	at reactor.core.publisher.MonoFlatMap$FlatMapInner.onNext(MonoFlatMap.java:241)
	at reactor.core.publisher.Operators$MonoSubscriber.complete(Operators.java:1592)
	at reactor.core.publisher.MonoFlatMap$FlatMapMain.onNext(MonoFlatMap.java:144)
	at reactor.core.publisher.Operators$ScalarSubscription.request(Operators.java:2148)
	at reactor.core.publisher.MonoFlatMap$FlatMapMain.onSubscribe(MonoFlatMap.java:103)
	at reactor.core.publisher.MonoJust.subscribe(MonoJust.java:54)
	at reactor.core.publisher.MonoDefer.subscribe(MonoDefer.java:52)
	at reactor.core.publisher.InternalMonoOperator.subscribe(InternalMonoOperator.java:55)
	at reactor.core.publisher.MonoFlatMap$FlatMapMain.onNext(MonoFlatMap.java:150)
	at reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.onNext(FluxMapFuseable.java:121)
	at reactor.core.publisher.Operators$ScalarSubscription.request(Operators.java:2148)
	at reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.request(FluxMapFuseable.java:162)
	at reactor.core.publisher.MonoFlatMap$FlatMapMain.onSubscribe(MonoFlatMap.java:103)
	at reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.onSubscribe(FluxMapFuseable.java:90)
	at reactor.core.publisher.MonoCurrentContext.subscribe(MonoCurrentContext.java:35)
	at reactor.core.publisher.InternalMonoOperator.subscribe(InternalMonoOperator.java:55)
	at reactor.core.publisher.MonoUsing.subscribe(MonoUsing.java:102)
	at reactor.core.publisher.Mono.subscribe(Mono.java:4087)
	at reactor.core.publisher.Mono.block(Mono.java:1662)
	at com.azure.storage.common.implementation.StorageImplUtils.blockWithOptionalTimeout(StorageImplUtils.java:99)
	at com.azure.storage.blob.specialized.cryptography.EncryptedBlobClient.uploadFromFile(EncryptedBlobClient.java:166)
	at com.azure.storage.blob.specialized.cryptography.EncryptedBlobClient.uploadFromFile(EncryptedBlobClient.java:139)
	at com.azure.storage.blob.specialized.cryptography.EncryptedBlobClient.uploadFromFile(EncryptedBlobClient.java:122)
	at com.testfactory.apps.App.testPutObject(App.java:716)
	at com.testfactory.apps.App.main(App.java:70)

@g2vinay
Copy link
Member

g2vinay commented Mar 2, 2020

@SukruthKS
It looks like encrypted blob client expects the key to always have an id (its assuming it will be a key living in key vault)
But for local keys there is no id as they're not living in key vault.
So, to satisfy the encrypted blob client, you can pass a dummy key id for your local key as follows:

JsonWebKey jsonWebKey = JsonWebKey.fromAes(
    new SecretKeySpec(encryptionKeyBytes, "AES"),
    Arrays.asList(KeyOperation.WRAP_KEY, KeyOperation.UNWRAP_KEY));

KeyVaultKey localKek = KeyVaultKey.fromKeyId("https://dummyvault.vault.azure.net/keys/localSymmetricKey/dummy-version", jsonWebKey);

AsyncKeyEncryptionKey akek = new KeyEncryptionKeyClientBuilder()
    .credential(new BasicAuthenticationCredential("dummy", "dummy"))
    .buildAsyncKeyEncryptionKey(localKek)
    .block();

@rickle-msft , since the keys can be local too now, is it possible to relax the requirement of key id in encrypted blob client ?

@rickle-msft
Copy link
Contributor

Sorry, removed my old comment after processing this a little more. We actually do need a key Id because we put that in the encryption metadata to ensure that the key we later use to decrypt is the same key we used to encrypt.

@rickle-msft
Copy link
Contributor

@SukruthK Do you guys plan on using only non URI looking strings e.g. “key-name” as key ids for local keys ? Or should we expect you may pass any URI looking strings as key id for local keys too?

@SukruthKS
Copy link
Author

@rickle-msft we plan to use only non-URI strings for local keys.

@g2vinay
Copy link
Member

g2vinay commented Apr 16, 2020

@SukruthKS
The April beta release contains the features:

  1. Set custom key ids.
  2. No requirement for credentials for local keys.
    Package link: https://search.maven.org/artifact/com.azure/azure-security-keyvault-keys/4.2.0-beta.3/jar

Here is the code sample that you can use:

JsonWebKey localKey = JsonWebKey.fromAes(new SecretKeySpec(encryptionKeyBytes, "AES"), Arrays.asList(KeyOperation.WRAP_KEY, KeyOperation.UNWRAP_KEY))
                    .setId("my-id");
AsyncKeyEncryptionKey akek = new LocalKeyEncryptionKeyClientBuilder()
                                                 .buildAsyncKeyEncryptionKey(localKey).block();

@SukruthKS
Copy link
Author

Thanks @g2vinay! will try this out soon.

@azure-sdk azure-sdk added the customer-reported Issues that are reported by GitHub users external to the Azure organization. label Sep 24, 2020
@vcolin7 vcolin7 added this to the [2020] October milestone Sep 28, 2020
@vcolin7 vcolin7 removed the customer-reported Issues that are reported by GitHub users external to the Azure organization. label Sep 28, 2020
@vcolin7
Copy link
Member

vcolin7 commented Oct 7, 2020

Marking this as resolved as this feature is now available in Beta. Will update once it's made generally available (GA).

@vcolin7 vcolin7 closed this as completed Oct 7, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. feature-request This issue requires a new behavior in the product in order be resolved. KeyVault
Projects
None yet
Development

No branches or pull requests

6 participants