ServiceBusSharedKeyCredential: Create a new HMAC object for each toke…

…n acquisition, since sharing an HMAC is not thread-safe. (Azure#42353) * ServiceBusSharedKeyCredential: Create a new HMAC object for each token acquisition, since sharing an HMAC is not thread-safe. * define error messages as static
billwert · Oct 16, 2024 · 0cf801c · 0cf801c
1 parent c01cff1
commit 0cf801c
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 16 deletions.
diff --git a/sdk/servicebus/azure-messaging-servicebus/CHANGELOG.md b/sdk/servicebus/azure-messaging-servicebus/CHANGELOG.md
@@ -8,6 +8,8 @@
 
 ### Bugs Fixed
 
+- Fixes the thread unsafe use of javax.crypto.Mac instance in ServiceBusSharedKeyCredential. ([42353](https://github.com/Azure/azure-sdk-for-java/pull/42353))
+
 ### Other Changes
 
 ## 7.17.4 (2024-09-27)

diff --git a/...ain/java/com/azure/messaging/servicebus/implementation/ServiceBusSharedKeyCredential.java b/...ain/java/com/azure/messaging/servicebus/implementation/ServiceBusSharedKeyCredential.java
@@ -48,12 +48,14 @@
 public class ServiceBusSharedKeyCredential implements TokenCredential {
     private static final String SHARED_ACCESS_SIGNATURE_FORMAT = "SharedAccessSignature sr=%s&sig=%s&se=%s&skn=%s";
     private static final String HASH_ALGORITHM = "HMACSHA256";
+    private static final String NO_HASH_ALGORITHM_ERROR_MESSAGE = "Unable to create hashing algorithm '" + HASH_ALGORITHM + "'";
+    private static final String INVALID_SHARED_ACCESS_KEY = "'sharedAccessKey' is an invalid value for the hashing algorithm.";
 
     private static final ClientLogger LOGGER = new ClientLogger(ServiceBusSharedKeyCredential.class);
 
     private final String policyName;
-    private final Mac hmac;
     private final Duration tokenValidity;
+    private final SecretKeySpec secretKeySpec;
     private final String sharedAccessSignature;
 
     /**
@@ -100,21 +102,8 @@ public ServiceBusSharedKeyCredential(String policyName, String sharedAccessKey,
             throw new IllegalArgumentException("'tokenTimeToLive' has to positive and in the order-of seconds");
         }
 
-        try {
-            hmac = Mac.getInstance(HASH_ALGORITHM);
-        } catch (NoSuchAlgorithmException e) {
-            throw LOGGER.logExceptionAsError(new UnsupportedOperationException(
-                String.format("Unable to create hashing algorithm '%s'", HASH_ALGORITHM), e));
-        }
-
         final byte[] sasKeyBytes = sharedAccessKey.getBytes(UTF_8);
-        final SecretKeySpec finalKey = new SecretKeySpec(sasKeyBytes, HASH_ALGORITHM);
-        try {
-            hmac.init(finalKey);
-        } catch (InvalidKeyException e) {
-            throw LOGGER.logExceptionAsError(new IllegalArgumentException(
-                "'sharedAccessKey' is an invalid value for the hashing algorithm.", e));
-        }
+        this.secretKeySpec = new SecretKeySpec(sasKeyBytes, HASH_ALGORITHM);
         this.sharedAccessSignature = null;
     }
 
@@ -133,7 +122,7 @@ public ServiceBusSharedKeyCredential(String sharedAccessSignature) {
         this.sharedAccessSignature = Objects.requireNonNull(sharedAccessSignature,
             "'sharedAccessSignature' cannot be null");
         this.policyName = null;
-        this.hmac = null;
+        this.secretKeySpec = null;
         this.tokenValidity = null;
     }
 
@@ -165,6 +154,16 @@ private AccessToken generateSharedAccessSignature(final String resource) throws
             return new AccessToken(sharedAccessSignature, getExpirationTime(sharedAccessSignature));
         }
 
+        final Mac hmac;
+        try {
+            hmac = Mac.getInstance(HASH_ALGORITHM);
+            hmac.init(secretKeySpec);
+        } catch (NoSuchAlgorithmException e) {
+            throw LOGGER.logExceptionAsError(new UnsupportedOperationException(NO_HASH_ALGORITHM_ERROR_MESSAGE, e));
+        } catch (InvalidKeyException e) {
+            throw LOGGER.logExceptionAsError(new IllegalArgumentException(INVALID_SHARED_ACCESS_KEY, e));
+        }
+
         final String utf8Encoding = UTF_8.name();
         final OffsetDateTime expiresOn = OffsetDateTime.now(ZoneOffset.UTC).plus(tokenValidity);
         final String expiresOnEpochSeconds = Long.toString(expiresOn.toEpochSecond());