Fixes to local and remote cryptography in Key Vault Keys (Azure#19485)

* Fixed issue that caused an NPE when attempting to use a CryptographyClient for GCM encryption.

* Fixed issue where properties of responses received when using a CryptographyClient for encryption/decryption were not serialized/deserialized and populated properly into the EncryptResult and DecryptResult classes.

* Fixed local GCM encryption/decryption, as it was implemented incorrectly. Also made sure that EncryptResult and DecryptResult produced by these operations contain the right data.

* Fixed checkstyle issues.

* Applied PR feedback.

* Applied PR feedback.

Author: vcolin7

sdk/keyvault/azure-security-keyvault-keys/src/main/java/com/azure/security/keyvault/keys/cryptography/AesGcm.java

@@ -16,6 +16,8 @@
 import java.util.Arrays;
 import java.util.Objects;
 
+import static com.azure.security.keyvault.keys.cryptography.SymmetricKeyCryptographyClient.AES_BLOCK_SIZE;
+
 abstract class AesGcm extends SymmetricEncryptionAlgorithm {
     final int keySizeInBytes;
     final int keySize;
@@ -42,8 +44,14 @@ static class AesGcmEncryptor implements ICryptoTransform {
                 cipher = Cipher.getInstance("AES/GCM/NoPadding", provider);
             }
 
+            int tagLength = (authenticationTag == null) ? AES_BLOCK_SIZE : authenticationTag.length;
+
             cipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(key, "AES"),
-                new GCMParameterSpec(authenticationTag.length << 3, iv));
+                new GCMParameterSpec(tagLength << 3, iv));
+
+            if (additionalAuthenticatedData != null) {
+                cipher.updateAAD(additionalAuthenticatedData);
+            }
         }
 
         @Override
@@ -67,11 +75,14 @@ static class AesGcmDecryptor implements ICryptoTransform {
                 cipher = Cipher.getInstance("AES/GCM/NoPadding", provider);
             }
 
-
             Objects.requireNonNull(authenticationTag, "'authenticationTag' cannot be null");
 
             cipher.init(Cipher.DECRYPT_MODE, new SecretKeySpec(key, "AES"),
                 new GCMParameterSpec(authenticationTag.length << 3, iv));
+
+            if (additionalAuthenticatedData != null) {
+                cipher.updateAAD(additionalAuthenticatedData);
+            }
         }
 
         @Override

sdk/keyvault/azure-security-keyvault-keys/src/main/java/com/azure/security/keyvault/keys/cryptography/CryptographyAsyncClient.java

@@ -714,8 +714,9 @@ private boolean checkKeyPermissions(List<KeyOperation> operations, KeyOperation
 
     private Mono<Boolean> ensureValidKeyAvailable() {
         boolean keyNotAvailable = (key == null && keyCollection != null);
+        boolean keyNotValid = (key != null && !key.isValid());
 
-        if (keyNotAvailable) {
+        if (keyNotAvailable || keyNotValid) {
             if (keyCollection.equals(SECRETS_COLLECTION)) {
                 return getSecretKey().map(jsonWebKey -> {
                     key = (jsonWebKey);

sdk/keyvault/azure-security-keyvault-keys/src/main/java/com/azure/security/keyvault/keys/cryptography/CryptographyServiceClient.java

@@ -129,48 +129,44 @@ Mono<EncryptResult> encrypt(EncryptOptions encryptOptions, Context context) {
         Objects.requireNonNull(encryptOptions, "'encryptOptions' cannot be null.");
 
         EncryptionAlgorithm algorithm = encryptOptions.getAlgorithm();
-        byte[] iv = encryptOptions.getIv();
-        byte[] authenticatedData = encryptOptions.getAdditionalAuthenticatedData();
         KeyOperationParameters parameters = new KeyOperationParameters()
             .setAlgorithm(algorithm)
             .setValue(encryptOptions.getPlainText())
-            .setIv(iv)
-            .setAdditionalAuthenticatedData(authenticatedData);
+            .setIv(encryptOptions.getIv())
+            .setAdditionalAuthenticatedData(encryptOptions.getAdditionalAuthenticatedData());
         context = context == null ? Context.NONE : context;
 
         return service.encrypt(vaultUrl, keyName, version, apiVersion, ACCEPT_LANGUAGE, parameters,
             CONTENT_TYPE_HEADER_VALUE, context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
-            .doOnRequest(ignored -> logger.info("Encrypting content with algorithm - {}", algorithm.toString()))
-            .doOnSuccess(response -> logger.info("Retrieved encrypted content with algorithm- {}",
-                algorithm.toString()))
-            .doOnError(error -> logger.warning("Failed to encrypt content with algorithm - {}", algorithm.toString(),
-                error))
-            .flatMap(keyOperationResultResponse ->
-                Mono.just(new EncryptResult(keyOperationResultResponse.getValue().getResult(), algorithm, keyId)));
+            .doOnRequest(ignored -> logger.info("Encrypting content with algorithm - {}", algorithm))
+            .doOnSuccess(response -> logger.info("Retrieved encrypted content with algorithm - {}", algorithm))
+            .doOnError(error -> logger.warning("Failed to encrypt content with algorithm - {}", algorithm, error))
+            .map(keyOperationResultResponse -> {
+                KeyOperationResult keyOperationResult = keyOperationResultResponse.getValue();
+
+                return new EncryptResult(keyOperationResult.getResult(), algorithm, keyId,
+                    keyOperationResult.getIv(), keyOperationResult.getAdditionalAuthenticatedData(),
+                    keyOperationResult.getAuthenticationTag());
+            });
     }
 
     Mono<DecryptResult> decrypt(DecryptOptions decryptOptions, Context context) {
         Objects.requireNonNull(decryptOptions, "'decryptOptions' cannot be null.");
 
         EncryptionAlgorithm algorithm = decryptOptions.getAlgorithm();
-        byte[] iv = decryptOptions.getIv();
-        byte[] additionalAuthenticatedData = decryptOptions.getAdditionalAuthenticatedData();
-        byte[] authenticationTag = decryptOptions.getAuthenticationTag();
         KeyOperationParameters parameters = new KeyOperationParameters()
             .setAlgorithm(algorithm)
             .setValue(decryptOptions.getCipherText())
-            .setIv(iv)
-            .setAdditionalAuthenticatedData(additionalAuthenticatedData)
-            .setAuthenticationTag(authenticationTag);
+            .setIv(decryptOptions.getIv())
+            .setAdditionalAuthenticatedData(decryptOptions.getAdditionalAuthenticatedData())
+            .setAuthenticationTag(decryptOptions.getAuthenticationTag());
         context = context == null ? Context.NONE : context;
 
         return service.decrypt(vaultUrl, keyName, version, apiVersion, ACCEPT_LANGUAGE, parameters,
             CONTENT_TYPE_HEADER_VALUE, context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
-            .doOnRequest(ignored -> logger.info("Decrypting content with algorithm - {}", algorithm.toString()))
-            .doOnSuccess(response -> logger.info("Retrieved decrypted content with algorithm- {}",
-                algorithm.toString()))
-            .doOnError(error -> logger.warning("Failed to decrypt content with algorithm - {}", algorithm.toString(),
-                error))
+            .doOnRequest(ignored -> logger.info("Decrypting content with algorithm - {}", algorithm))
+            .doOnSuccess(response -> logger.info("Retrieved decrypted content with algorithm - {}", algorithm))
+            .doOnError(error -> logger.warning("Failed to decrypt content with algorithm - {}", algorithm, error))
             .flatMap(keyOperationResultResponse -> Mono.just(
                 new DecryptResult(keyOperationResultResponse.getValue().getResult(), algorithm, keyId)));
     }
@@ -180,27 +176,25 @@ Mono<SignResult> sign(SignatureAlgorithm algorithm, byte[] digest, Context conte
         context = context == null ? Context.NONE : context;
         return service.sign(vaultUrl, keyName, version, apiVersion, ACCEPT_LANGUAGE, parameters,
             CONTENT_TYPE_HEADER_VALUE, context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
-            .doOnRequest(ignored -> logger.info("Signing content with algorithm - {}", algorithm.toString()))
-            .doOnSuccess(response -> logger.info("Retrieved signed content with algorithm- {}", algorithm.toString()))
-            .doOnError(error -> logger.warning("Failed to sign content with algorithm - {}", algorithm.toString(),
-                error))
+            .doOnRequest(ignored -> logger.info("Signing content with algorithm - {}", algorithm))
+            .doOnSuccess(response -> logger.info("Retrieved signed content with algorithm - {}", algorithm))
+            .doOnError(error -> logger.warning("Failed to sign content with algorithm - {}", algorithm, error))
             .flatMap(keyOperationResultResponse ->
                 Mono.just(new SignResult(keyOperationResultResponse.getValue().getResult(), algorithm, keyId)));
     }
 
     Mono<VerifyResult> verify(SignatureAlgorithm algorithm, byte[] digest, byte[] signature, Context context) {
 
-        KeyVerifyRequest parameters = new KeyVerifyRequest().setAlgorithm(algorithm).setDigest(digest).setSignature(signature);
+        KeyVerifyRequest parameters =
+            new KeyVerifyRequest().setAlgorithm(algorithm).setDigest(digest).setSignature(signature);
         context = context == null ? Context.NONE : context;
 
         return service.verify(vaultUrl, keyName, version, apiVersion, ACCEPT_LANGUAGE, parameters,
             CONTENT_TYPE_HEADER_VALUE, context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
-            .doOnRequest(ignored -> logger.info("Verifying content with algorithm - {}", algorithm.toString()))
-            .doOnSuccess(response -> logger.info("Retrieved verified content with algorithm- {}", algorithm.toString()))
-            .doOnError(error -> logger.warning("Failed to verify content with algorithm - {}", algorithm.toString(),
-                error))
-            .flatMap(response ->
-                Mono.just(new VerifyResult(response.getValue().getValue(), algorithm, keyId)));
+            .doOnRequest(ignored -> logger.info("Verifying content with algorithm - {}", algorithm))
+            .doOnSuccess(response -> logger.info("Retrieved verified content with algorithm - {}", algorithm))
+            .doOnError(error -> logger.warning("Failed to verify content with algorithm - {}", algorithm, error))
+            .flatMap(response -> Mono.just(new VerifyResult(response.getValue().getValue(), algorithm, keyId)));
     }
 
     Mono<WrapResult> wrapKey(KeyWrapAlgorithm algorithm, byte[] key, Context context) {
@@ -211,31 +205,25 @@ Mono<WrapResult> wrapKey(KeyWrapAlgorithm algorithm, byte[] key, Context context
 
         return service.wrapKey(vaultUrl, keyName, version, apiVersion, ACCEPT_LANGUAGE, parameters,
             CONTENT_TYPE_HEADER_VALUE, context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
-            .doOnRequest(ignored -> logger.info("Wrapping key content with algorithm - {}", algorithm.toString()))
-            .doOnSuccess(response -> logger.info("Retrieved wrapped key content with algorithm- {}",
-                algorithm.toString()))
-            .doOnError(error -> logger.warning("Failed to verify content with algorithm - {}", algorithm.toString(),
-                error))
+            .doOnRequest(ignored -> logger.info("Wrapping key content with algorithm - {}", algorithm))
+            .doOnSuccess(response -> logger.info("Retrieved wrapped key content with algorithm - {}", algorithm))
+            .doOnError(error -> logger.warning("Failed to verify content with algorithm - {}", algorithm, error))
             .flatMap(keyOperationResultResponse ->
                 Mono.just(new WrapResult(keyOperationResultResponse.getValue().getResult(), algorithm, keyId)));
     }
 
     Mono<UnwrapResult> unwrapKey(KeyWrapAlgorithm algorithm, byte[] encryptedKey, Context context) {
-
         KeyWrapUnwrapRequest parameters = new KeyWrapUnwrapRequest()
             .setAlgorithm(algorithm)
             .setValue(encryptedKey);
         context = context == null ? Context.NONE : context;
 
         return service.unwrapKey(vaultUrl, keyName, version, apiVersion, ACCEPT_LANGUAGE, parameters,
             CONTENT_TYPE_HEADER_VALUE, context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
-            .doOnRequest(ignored -> logger.info("Unwrapping key content with algorithm - {}", algorithm.toString()))
-            .doOnSuccess(response -> logger.info("Retrieved unwrapped key content with algorithm- {}",
-                algorithm.toString()))
-            .doOnError(error -> logger.warning("Failed to unwrap key content with algorithm - {}",
-                algorithm.toString(), error))
-            .flatMap(response ->
-                Mono.just(new UnwrapResult(response.getValue().getResult(), algorithm, keyId)));
+            .doOnRequest(ignored -> logger.info("Unwrapping key content with algorithm - {}", algorithm))
+            .doOnSuccess(response -> logger.info("Retrieved unwrapped key content with algorithm - {}", algorithm))
+            .doOnError(error -> logger.warning("Failed to unwrap key content with algorithm - {}", algorithm, error))
+            .flatMap(response -> Mono.just(new UnwrapResult(response.getValue().getResult(), algorithm, keyId)));
     }
 
 

sdk/keyvault/azure-security-keyvault-keys/src/main/java/com/azure/security/keyvault/keys/cryptography/KeyOperationParameters.java

@@ -11,8 +11,10 @@
  * The key operations parameters.
  */
 class KeyOperationParameters {
+    private static final byte[] EMPTY_ARRAY = new byte[0];
+
     /**
-     * algorithm identifier. Possible values include: 'RSA-OAEP',
+     * Algorithm identifier. Possible values include: 'RSA-OAEP',
      * 'RSA-OAEP-256', 'RSA1_5'.
      */
     @JsonProperty(value = "alg", required = true)
@@ -28,19 +30,19 @@ class KeyOperationParameters {
      * Initialization vector for symmetric algorithms.
      */
     @JsonProperty(value = "iv")
-    private byte[] iv;
+    private Base64Url iv;
 
     /**
      * Additional data to authenticate but not encrypt/decrypt when using authenticated crypto algorithms.
      */
     @JsonProperty(value = "aad")
-    private byte[] additionalAuthenticatedData;
+    private Base64Url additionalAuthenticatedData;
 
     /**
      * The tag to authenticate when performing decryption with an authenticated algorithm.
      */
     @JsonProperty(value = "tag")
-    private byte[] authenticationTag;
+    private Base64Url authenticationTag;
 
     /**
      * Get the algorithm value.
@@ -59,6 +61,7 @@ public EncryptionAlgorithm getAlgorithm() {
      */
     public KeyOperationParameters setAlgorithm(EncryptionAlgorithm algorithm) {
         this.algorithm = algorithm;
+
         return this;
     }
 
@@ -69,8 +72,9 @@ public KeyOperationParameters setAlgorithm(EncryptionAlgorithm algorithm) {
      */
     public byte[] getValue() {
         if (this.value == null) {
-            return new byte[0];
+            return EMPTY_ARRAY;
         }
+
         return this.value.decodedBytes();
     }
 
@@ -81,11 +85,8 @@ public byte[] getValue() {
      * @return the KeyOperationsParameters object itself.
      */
     public KeyOperationParameters setValue(byte[] value) {
-        if (value == null) {
-            this.value = null;
-        } else {
-            this.value = Base64Url.encode(value);
-        }
+        this.value = value != null ? Base64Url.encode(value) : null;
+
         return this;
     }
 
@@ -95,7 +96,11 @@ public KeyOperationParameters setValue(byte[] value) {
      * @return The initialization vector.
      */
     public byte[] getIv() {
-        return iv;
+        if (this.iv == null) {
+            return EMPTY_ARRAY;
+        }
+
+        return this.iv.decodedBytes();
     }
 
     /**
@@ -105,7 +110,8 @@ public byte[] getIv() {
      * @return The updated {@link KeyOperationParameters} object.
      */
     public KeyOperationParameters setIv(byte[] iv) {
-        this.iv = iv;
+        this.iv = iv != null ? Base64Url.encode(iv) : null;
+
         return this;
     }
 
@@ -115,7 +121,11 @@ public KeyOperationParameters setIv(byte[] iv) {
      * @return The additional authenticated data.
      */
     public byte[] getAdditionalAuthenticatedData() {
-        return additionalAuthenticatedData;
+        if (this.additionalAuthenticatedData == null) {
+            return EMPTY_ARRAY;
+        }
+
+        return this.additionalAuthenticatedData.decodedBytes();
     }
 
     /**
@@ -125,7 +135,9 @@ public byte[] getAdditionalAuthenticatedData() {
      * @return The updated {@link KeyOperationParameters} object.
      */
     public KeyOperationParameters setAdditionalAuthenticatedData(byte[] additionalAuthenticatedData) {
-        this.additionalAuthenticatedData = additionalAuthenticatedData;
+        this.additionalAuthenticatedData =
+            additionalAuthenticatedData != null ? Base64Url.encode(additionalAuthenticatedData) : null;
+
         return this;
     }
 
@@ -135,7 +147,11 @@ public KeyOperationParameters setAdditionalAuthenticatedData(byte[] additionalAu
      * @return The authentication tag.
      */
     public byte[] getAuthenticationTag() {
-        return authenticationTag;
+        if (this.authenticationTag == null) {
+            return EMPTY_ARRAY;
+        }
+
+        return this.authenticationTag.decodedBytes();
     }
 
     /**
@@ -145,7 +161,8 @@ public byte[] getAuthenticationTag() {
      * @return The updated {@link KeyOperationParameters} object.
      */
     public KeyOperationParameters setAuthenticationTag(byte[] authenticationTag) {
-        this.authenticationTag = authenticationTag;
+        this.authenticationTag = authenticationTag != null ? Base64Url.encode(authenticationTag) : null;
+
         return this;
     }
 }