From b7f4d70e93767d9c5a9d1de4b2e025d39954408d Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Sun, 9 Jun 2024 13:00:46 -0700 Subject: [PATCH] Managed identity bug fix, pick up msal patch --- eng/versioning/external_dependencies.txt | 2 +- .../microsoft-azure-eventhubs-eph/pom.xml | 2 +- .../pom.xml | 2 +- .../microsoft-azure-eventhubs/pom.xml | 2 +- .../azure-identity-broker/CHANGELOG.md | 1 + sdk/identity/azure-identity-broker/pom.xml | 4 +- sdk/identity/azure-identity/CHANGELOG.md | 1 + sdk/identity/azure-identity/pom.xml | 4 +- .../implementation/IdentityClient.java | 10 +- .../implementation/IdentityClientBase.java | 6 +- .../implementation/util/IdentityUtil.java | 9 ++ .../implementation/util/ValidationUtil.java | 49 ++++++++++ .../implementation/ValidationUtilTests.java | 97 +++++++++++++++++++ 13 files changed, 175 insertions(+), 14 deletions(-) create mode 100644 sdk/identity/azure-identity/src/test/java/com/azure/identity/implementation/ValidationUtilTests.java diff --git a/eng/versioning/external_dependencies.txt b/eng/versioning/external_dependencies.txt index 875c02b54a7ad..0804f5b5a3b49 100644 --- a/eng/versioning/external_dependencies.txt +++ b/eng/versioning/external_dependencies.txt @@ -201,7 +201,7 @@ com.microsoft.azure:azure-mgmt-resources;1.3.0 com.microsoft.azure:azure-mgmt-search;1.24.1 com.microsoft.azure:azure-mgmt-storage;1.3.0 com.microsoft.azure:azure-storage;8.0.0 -com.microsoft.azure:msal4j;1.15.0 +com.microsoft.azure:msal4j;1.15.1 com.microsoft.azure:msal4j-brokers;1.0.0 com.microsoft.azure:msal4j-persistence-extension;1.3.0 com.sun.activation:jakarta.activation;1.2.2 diff --git a/sdk/eventhubs/microsoft-azure-eventhubs-eph/pom.xml b/sdk/eventhubs/microsoft-azure-eventhubs-eph/pom.xml index ec764aabf1fe7..eb45151f18e98 100644 --- a/sdk/eventhubs/microsoft-azure-eventhubs-eph/pom.xml +++ b/sdk/eventhubs/microsoft-azure-eventhubs-eph/pom.xml @@ -64,7 +64,7 @@ com.microsoft.azure msal4j - 1.15.0 + 1.15.1 test diff --git a/sdk/eventhubs/microsoft-azure-eventhubs-extensions/pom.xml b/sdk/eventhubs/microsoft-azure-eventhubs-extensions/pom.xml index b6192f94ebe8a..3d60d1d7f8100 100644 --- a/sdk/eventhubs/microsoft-azure-eventhubs-extensions/pom.xml +++ b/sdk/eventhubs/microsoft-azure-eventhubs-extensions/pom.xml @@ -68,7 +68,7 @@ com.microsoft.azure msal4j - 1.15.0 + 1.15.1 test diff --git a/sdk/eventhubs/microsoft-azure-eventhubs/pom.xml b/sdk/eventhubs/microsoft-azure-eventhubs/pom.xml index e7ea08541cd5f..f7eb35d52cb29 100644 --- a/sdk/eventhubs/microsoft-azure-eventhubs/pom.xml +++ b/sdk/eventhubs/microsoft-azure-eventhubs/pom.xml @@ -77,7 +77,7 @@ com.microsoft.azure msal4j - 1.15.0 + 1.15.1 test diff --git a/sdk/identity/azure-identity-broker/CHANGELOG.md b/sdk/identity/azure-identity-broker/CHANGELOG.md index 35b1f523c1c7c..2a869196c972a 100644 --- a/sdk/identity/azure-identity-broker/CHANGELOG.md +++ b/sdk/identity/azure-identity-broker/CHANGELOG.md @@ -7,6 +7,7 @@ #### Dependency Updates - Upgraded `azure-identity` from `1.12.1` to version `1.12.2`. +- Upgraded `msal4j` from `1.15.0` to version `1.15.1`. ## 1.1.1 (2024-05-02) diff --git a/sdk/identity/azure-identity-broker/pom.xml b/sdk/identity/azure-identity-broker/pom.xml index dd7c8dacf92d6..1d9b850954247 100644 --- a/sdk/identity/azure-identity-broker/pom.xml +++ b/sdk/identity/azure-identity-broker/pom.xml @@ -37,7 +37,7 @@ com.microsoft.azure msal4j - 1.15.0 + 1.15.1 com.microsoft.azure @@ -62,7 +62,7 @@ - com.microsoft.azure:msal4j:[1.15.0] + com.microsoft.azure:msal4j:[1.15.1] com.microsoft.azure:msal4j-brokers:[1.0.0] diff --git a/sdk/identity/azure-identity/CHANGELOG.md b/sdk/identity/azure-identity/CHANGELOG.md index 70a6b83776e46..41cdd322dac85 100644 --- a/sdk/identity/azure-identity/CHANGELOG.md +++ b/sdk/identity/azure-identity/CHANGELOG.md @@ -11,6 +11,7 @@ - Upgraded `azure-core` from `1.49.0` to version `1.49.1`. - Upgraded `azure-core-http-netty` from `1.15.0` to version `1.15.1`. +- Upgraded `msal4j` from `1.15.0` to version `1.15.1`. ## 1.12.1 (2024-05-02) diff --git a/sdk/identity/azure-identity/pom.xml b/sdk/identity/azure-identity/pom.xml index 33eb57cd122ab..1c68184ffefb1 100644 --- a/sdk/identity/azure-identity/pom.xml +++ b/sdk/identity/azure-identity/pom.xml @@ -46,7 +46,7 @@ com.microsoft.azure msal4j - 1.15.0 + 1.15.1 com.microsoft.azure @@ -151,7 +151,7 @@ - com.microsoft.azure:msal4j:[1.15.0] + com.microsoft.azure:msal4j:[1.15.1] com.microsoft.azure:msal4j-persistence-extension:[1.3.0] net.java.dev.jna:jna-platform:[5.6.0] org.linguafranca.pwdb:KeePassJava2:[2.1.4] diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClient.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClient.java index 9e53576f52e0b..5228a75fc22e4 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClient.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClient.java @@ -39,6 +39,7 @@ import reactor.core.publisher.Mono; import javax.net.ssl.HttpsURLConnection; +import java.io.File; import java.io.IOException; import java.net.HttpURLConnection; import java.net.MalformedURLException; @@ -50,6 +51,7 @@ import java.net.URLEncoder; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.Path; import java.nio.file.Paths; import java.time.Duration; import java.time.OffsetDateTime; @@ -65,6 +67,8 @@ import java.util.function.Function; import java.util.function.Supplier; +import static com.azure.identity.implementation.util.ValidationUtil.validateSecretFile; + /** * The identity client that contains APIs to retrieve access tokens * from various configurations. @@ -956,8 +960,10 @@ private Mono authenticateToArcManagedIdentityEndpoint(String identi null)); } - String secretKeyPath = realm.substring(separatorIndex + 1); - secretKey = new String(Files.readAllBytes(Paths.get(secretKeyPath)), StandardCharsets.UTF_8); + String secretKeyPathHeaderValue = realm.substring(separatorIndex + 1); + Path secretKeyPath = validateSecretFile(new File(secretKeyPathHeaderValue), LOGGER); + + secretKey = new String(Files.readAllBytes(secretKeyPath), StandardCharsets.UTF_8); if (connection != null) { diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientBase.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientBase.java index de647a175e4fe..9ab0f98ab01b2 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientBase.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientBase.java @@ -97,6 +97,8 @@ import java.util.function.Supplier; import java.util.regex.Pattern; +import static com.azure.identity.implementation.util.IdentityUtil.isWindowsPlatform; + public abstract class IdentityClientBase { static final SerializerAdapter SERIALIZER_ADAPTER = JacksonAdapter.createDefaultSerializerAdapter(); static final String WINDOWS_STARTER = "cmd.exe"; @@ -811,10 +813,6 @@ String getSafeWorkingDirectory() { } } - boolean isWindowsPlatform() { - return System.getProperty("os.name").contains("Windows"); - } - String redactInfo(String input) { return ACCESS_TOKEN_PATTERN.matcher(input).replaceAll("****"); } diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/util/IdentityUtil.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/util/IdentityUtil.java index 3d61b2489a9d9..ce667e31d8111 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/util/IdentityUtil.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/util/IdentityUtil.java @@ -111,4 +111,13 @@ public static byte[] convertInputStreamToByteArray(InputStream inputStream) { } return outputStream.toByteArray(); } + + + public static boolean isWindowsPlatform() { + return System.getProperty("os.name").contains("Windows"); + } + + public static boolean isLinuxPlatform() { + return System.getProperty("os.name").contains("Linux"); + } } diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/util/ValidationUtil.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/util/ValidationUtil.java index 6493e027f5f40..fc90fec98774a 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/util/ValidationUtil.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/util/ValidationUtil.java @@ -3,10 +3,18 @@ package com.azure.identity.implementation.util; +import com.azure.core.exception.ClientAuthenticationException; +import com.azure.core.util.CoreUtils; import com.azure.core.util.logging.ClientLogger; +import java.io.File; import java.net.URI; import java.net.URISyntaxException; +import java.nio.file.Path; +import java.nio.file.Paths; + +import static com.azure.identity.implementation.util.IdentityUtil.isLinuxPlatform; +import static com.azure.identity.implementation.util.IdentityUtil.isWindowsPlatform; /** * Utility class for validating parameters. @@ -91,4 +99,45 @@ public static void validateInteractiveBrowserRedirectUrlSetup(Integer port, Stri private static boolean isValidTenantCharacter(char c) { return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || (c == '.') || (c == '-'); } + + + public static Path validateSecretFile(File file, ClientLogger logger) { + + Path path = file.toPath(); + if (isWindowsPlatform()) { + String programData = System.getenv("ProgramData"); + if (CoreUtils.isNullOrEmpty(programData)) { + throw logger.logExceptionAsError(new ClientAuthenticationException("The ProgramData environment" + + " variable is not set.", null)); + } + String target = Paths.get(programData, "AzureConnectedMachineAgent", "Tokens").toString(); + if (!path.getParent().toString().equals(target)) { + throw logger.logExceptionAsError(new ClientAuthenticationException("The secret key file is not" + + " located in the expected directory.", null)); + } + } else if (isLinuxPlatform()) { + Path target = Paths.get("/", "var", "opt", "azcmagent", "tokens"); + if (!path.getParent().equals(target)) { + throw logger.logExceptionAsError(new ClientAuthenticationException("The secret key file is not" + + " located in the expected directory.", null)); + } + } else { + throw logger.logExceptionAsError(new ClientAuthenticationException("The platform is not supported" + + " for Azure Arc Managed Identity Endpoint", null)); + } + + if (!path.toString().endsWith(".key")) { + throw logger.logExceptionAsError(new ClientAuthenticationException("The secret key file does not" + + " have the expected file extension", null)); + } + + + + if (file.length() > 4096) { + throw logger.logExceptionAsError(new ClientAuthenticationException("The secret key file is too large" + + " to be read from Azure Arc Managed Identity Endpoint", null)); + } + + return path; + } } diff --git a/sdk/identity/azure-identity/src/test/java/com/azure/identity/implementation/ValidationUtilTests.java b/sdk/identity/azure-identity/src/test/java/com/azure/identity/implementation/ValidationUtilTests.java new file mode 100644 index 0000000000000..550ac10b0389a --- /dev/null +++ b/sdk/identity/azure-identity/src/test/java/com/azure/identity/implementation/ValidationUtilTests.java @@ -0,0 +1,97 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.identity.implementation; + +import com.azure.core.exception.ClientAuthenticationException; +import com.azure.core.util.logging.ClientLogger; +import com.azure.identity.implementation.util.ValidationUtil; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledOnOs; +import org.junit.jupiter.api.condition.OS; + +import java.io.File; +import java.nio.file.Path; +import java.nio.file.Paths; + +import static com.azure.identity.implementation.util.IdentityUtil.isLinuxPlatform; +import static com.azure.identity.implementation.util.IdentityUtil.isWindowsPlatform; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +@DisabledOnOs({OS.MAC}) +public class ValidationUtilTests { + private static final ClientLogger LOGGER = new ClientLogger(ValidationUtilTests.class); + + private static File good; + private static File fileTooLong; + private static File wrongPrefix; + private static File wrongExtension; + private static File fileWithRelativeSegments; + + + @BeforeAll + public static void setupClass() { + Path beginning = null; + if (isWindowsPlatform()) { + beginning = Paths.get(System.getenv("ProgramData"), "AzureConnectedMachineAgent", "Tokens"); + } else if (isLinuxPlatform()) { + + beginning = Paths.get("/", "var", "opt", "azcmagent", "tokens"); + } + + good = new TestFile(Paths.get(beginning.toString(), "good.key").toString()); + fileTooLong = new TestFile(Paths.get(beginning.toString(), "fileTooLong.key").toString(), 4097); + wrongPrefix = new TestFile(Paths.get("wrongPrefix", ".key").toString()); + wrongExtension = new TestFile(Paths.get(beginning.toString(), "wrongExtension.txt").toString()); + fileWithRelativeSegments = new TestFile(Paths.get(beginning.toString(), "..", "file.key").toString()); + + } + @Test + public void testValidPath() { + assertDoesNotThrow(() -> ValidationUtil.validateSecretFile(good, LOGGER)); + } + + @Test + public void testInvalidTooLong() { + Throwable thrown = assertThrows(ClientAuthenticationException.class, () -> ValidationUtil.validateSecretFile(fileTooLong, LOGGER)); + assertTrue(thrown.getMessage().contains("The secret key file is too large")); + } + + @Test + public void testInvalidWrongPrefix() { + Throwable thrown = assertThrows(ClientAuthenticationException.class, () -> ValidationUtil.validateSecretFile(wrongPrefix, LOGGER)); + assertTrue(thrown.getMessage().contains("The secret key file is not located in the expected directory")); + } + + @Test + public void testInvalidWrongExtension() { + Throwable thrown = assertThrows(ClientAuthenticationException.class, () -> ValidationUtil.validateSecretFile(wrongExtension, LOGGER)); + assertTrue(thrown.getMessage().contains("The secret key file does not have the expected file extension")); + } + + @Test + public void testInvalidRelativeSegments() { + Throwable thrown = assertThrows(ClientAuthenticationException.class, () -> ValidationUtil.validateSecretFile(fileWithRelativeSegments, LOGGER)); + assertTrue(thrown.getMessage().contains("The secret key file is not located in the expected directory")); + } + + static class TestFile extends File { + long length = 4096; + TestFile(String pathname) { + super(pathname); + } + + TestFile(String pathName, long length) { + super(pathName); + this.length = length; + } + + @Override + public long length() { + return length; + } + } +}