Skip to content

Commit 8d2a969

Browse files
authored
[core-amqp] Removing connection string from leaking connection information being logged (Azure#19306)
* Removing connection string information from logging as it will leak secrets present in it.
1 parent 7b3c119 commit 8d2a969

File tree

2 files changed

+49
-14
lines changed

2 files changed

+49
-14
lines changed

sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/implementation/ConnectionStringProperties.java

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,9 @@ public class ConnectionStringProperties {
3232
+ "SharedAccessKeyName={sharedAccessKeyName};SharedAccessKey={sharedAccessKey};EntityPath={entityPath}";
3333
private static final String CONNECTION_STRING_WITH_SAS = "Endpoint={endpoint};SharedAccessSignature="
3434
+ "SharedAccessSignature {sharedAccessSignature};EntityPath={entityPath}";
35-
private static final String ERROR_MESSAGE_FORMAT = "Could not parse 'connectionString'. Expected format: "
36-
+ CONNECTION_STRING_WITH_ACCESS_KEY + " or " + CONNECTION_STRING_WITH_SAS + ". Actual: %s";
37-
private static final String ERROR_MESSAGE_ENDPOINT_FORMAT = "'Endpoint' must be provided in 'connectionString'."
38-
+ " Actual: %s";
35+
private static final String ERROR_MESSAGE_FORMAT = String.format(Locale.US,
36+
"Could not parse 'connectionString'. Expected format: %s or %s.", CONNECTION_STRING_WITH_ACCESS_KEY,
37+
CONNECTION_STRING_WITH_SAS);
3938

4039
private final URI endpoint;
4140
private final String entityPath;
@@ -77,7 +76,7 @@ public ConnectionStringProperties(String connectionString) {
7776
final String value = pair[1].trim();
7877

7978
if (key.equalsIgnoreCase(ENDPOINT)) {
80-
final String endpointUri = validateAndUpdateDefaultScheme(value, connectionString);
79+
final String endpointUri = validateAndUpdateDefaultScheme(value);
8180
try {
8281
endpoint = new URI(endpointUri);
8382
} catch (URISyntaxException e) {
@@ -106,7 +105,7 @@ public ConnectionStringProperties(String connectionString) {
106105
if (endpoint == null
107106
|| (includesSharedKey && includesSharedAccessSignature) // includes both SAS and key or value
108107
|| (!hasSharedKeyAndValue && !includesSharedAccessSignature)) { // invalid key, value and SAS
109-
throw new IllegalArgumentException(String.format(Locale.US, ERROR_MESSAGE_FORMAT, connectionString));
108+
throw logger.logExceptionAsError(new IllegalArgumentException(ERROR_MESSAGE_FORMAT));
110109
}
111110

112111
this.endpoint = endpoint;
@@ -161,20 +160,19 @@ public String getSharedAccessSignature() {
161160
* The function checks for pre existing scheme of "sb://" , "http://" or "https://". If the scheme is not provided
162161
* in endpoint, it will set the default scheme to "sb://".
163162
*/
164-
private String validateAndUpdateDefaultScheme(final String endpoint, final String connectionString) {
165-
String updatedEndpoint = endpoint.trim();
163+
private String validateAndUpdateDefaultScheme(final String endpoint) {
166164

167165
if (CoreUtils.isNullOrEmpty(endpoint)) {
168-
throw logger.logExceptionAsError(new IllegalArgumentException(String.format(Locale.US,
169-
ERROR_MESSAGE_ENDPOINT_FORMAT, connectionString)));
170-
166+
throw logger.logExceptionAsError(new IllegalArgumentException(
167+
"'Endpoint' must be provided in 'connectionString'."));
171168
}
172-
final String endpointLowerCase = endpoint.toLowerCase(Locale.getDefault());
169+
170+
final String endpointLowerCase = endpoint.trim().toLowerCase(Locale.ROOT);
173171
if (!endpointLowerCase.startsWith(ENDPOINT_SCHEME_SB_PREFIX)
174172
&& !endpointLowerCase.startsWith(ENDPOINT_SCHEME_HTTP_PREFIX)
175173
&& !endpointLowerCase.startsWith(ENDPOINT_SCHEME_HTTPS_PREFIX)) {
176-
updatedEndpoint = ENDPOINT_SCHEME_SB_PREFIX + endpoint;
174+
return ENDPOINT_SCHEME_SB_PREFIX + endpoint;
177175
}
178-
return updatedEndpoint;
176+
return endpointLowerCase;
179177
}
180178
}

sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/implementation/ConnectionStringPropertiesTest.java

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import java.util.Locale;
1212
import java.util.stream.Stream;
1313

14+
import static org.junit.jupiter.api.Assertions.assertFalse;
1415
import static org.junit.jupiter.api.Assertions.assertThrows;
1516

1617
public class ConnectionStringPropertiesTest {
@@ -62,6 +63,42 @@ public void invalidSasKeyValue() {
6263
assertThrows(IllegalArgumentException.class, () -> new ConnectionStringProperties(connectionString));
6364
}
6465

66+
/**
67+
* Verifies sdk do not expose secret in exception string when SharedAccessKeyName is not present.
68+
*/
69+
@Test
70+
public void invalidConnectionStringNoSecretExposed() {
71+
// Arrange
72+
final String invalidConnectionString = getConnectionString(HOSTNAME_URI, EVENT_HUB, SAS_KEY, SAS_VALUE)
73+
.replace(String.format(Locale.US, "SharedAccessKeyName=%s;", SAS_KEY), "");
74+
75+
// Act & Assert
76+
final Exception exception = assertThrows(IllegalArgumentException.class, () -> new ConnectionStringProperties(invalidConnectionString));
77+
78+
final String actualMessage = exception.getMessage();
79+
80+
assertFalse(actualMessage.contains(SAS_VALUE));
81+
assertFalse(actualMessage.contains(HOSTNAME_URI));
82+
}
83+
84+
/**
85+
* Verifies sdk do not expose secret in exception string when 'Endpoint' is not present.
86+
*/
87+
@Test
88+
public void invalidEndpointNoSecretExposed() {
89+
// Arrange
90+
final String invalidConnectionString = getConnectionString(HOSTNAME_URI, EVENT_HUB, SAS_KEY, SAS_VALUE)
91+
.replace(String.format(Locale.US, "Endpoint=%s;", HOSTNAME_URI), "");
92+
93+
// Act & Assert
94+
final Exception exception = assertThrows(IllegalArgumentException.class, () -> new ConnectionStringProperties(invalidConnectionString));
95+
96+
final String actualMessage = exception.getMessage();
97+
98+
assertFalse(actualMessage.contains(SAS_VALUE));
99+
assertFalse(actualMessage.contains(SAS_KEY));
100+
}
101+
65102
@Test
66103
public void differentEndpointScheme() {
67104
// Arrange

0 commit comments

Comments
 (0)