diff --git a/.github/workflows/auto-label-area.yml b/.github/workflows/auto-label-area.yml new file mode 100644 index 00000000..39dc5566 --- /dev/null +++ b/.github/workflows/auto-label-area.yml @@ -0,0 +1,26 @@ +name: Auto-label area + +on: + issues: + types: [opened] + +permissions: + issues: write + +jobs: + add-area-label: + runs-on: ubuntu-latest + steps: + - name: Add area label + uses: actions/github-script@v7 + with: + script: | + const AREA_LABEL = "area:java-functions"; + const labels = context.payload.issue.labels.map(l => l.name); + await github.rest.issues.addLabels({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.payload.issue.number, + labels: [AREA_LABEL], + }); + } diff --git a/.swarm/run/original-prompt.md b/.swarm/run/original-prompt.md deleted file mode 100644 index f3b08058..00000000 --- a/.swarm/run/original-prompt.md +++ /dev/null @@ -1,5 +0,0 @@ -There is a SFI (security) work item in this file - /src/main/java/com/microsoft/azure/functions/worker/JavaWorkerClient.java -Multiple language workers use plaintext/insecure gRPC channels, enabling MITM attacks. -Java worker: Uses plaintext gRPC channel enabling MITM tampering leading to potential code execution - -Can you fix it? It would be great to identify these things - - Is the fix necessary? - What was the historical context of designing the code as is? Was it intentional or a missed gap? - Can you confirm if there will be any regressions if the fix is made? - Will there be a contract or a breaking change for the customer? - How well is it tested? \ No newline at end of file diff --git a/.swarm/run/rounds/round-1/manifest.json b/.swarm/run/rounds/round-1/manifest.json deleted file mode 100644 index 1f292b0c..00000000 --- a/.swarm/run/rounds/round-1/manifest.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "roundNumber": 1, - "taskIds": [ - "expose-functions-uri", - "select-grpc-transport-from-uri", - "add-grpc-transport-tests" - ] -} \ No newline at end of file diff --git a/.swarm/run/rounds/round-1/planning/design-document.md b/.swarm/run/rounds/round-1/planning/design-document.md deleted file mode 100644 index ddca5f46..00000000 --- a/.swarm/run/rounds/round-1/planning/design-document.md +++ /dev/null @@ -1,15 +0,0 @@ -## Summary -`JavaWorkerClient` always builds `ManagedChannelBuilder.forAddress(...).usePlaintext()`, so the Java worker ignores the already-parsed `--functions-uri` and can never join a secure host/worker gRPC channel. - -## Findings -- **Fix necessary:** Yes. Today the worker guarantees plaintext and blocks any host-side TLS rollout, leaving any non-local transport exposed to MITM tampering. -- **Historical context:** Plaintext dates back to the early 2018 same-machine child-process design. In 2023 the worker added prefixed startup args, including `functions-uri`, plus fallback to legacy args, but `JavaWorkerClient` was not updated; this looks like an old assumption that became a gap once URI-based startup existed. -- **TLS behavior clarification:** The fix should honor TLS, not ignore it. When `functions-uri` uses `https`, the worker should build a TLS gRPC channel and fail if handshake, certificate, or hostname validation fails. Only `http` URIs, or legacy startup that supplies just host+port, should continue to use plaintext. -- **Regression / breaking change:** No new CLI or protocol contract. Existing `http` and legacy host+port launches keep their current behavior. The only observable behavior change is that a previously ignored or misconfigured `https` endpoint will stop connecting insecurely and instead fail closed. -- **Customer contract:** No new flags are required. Trust still comes from the JVM trust configuration already available via existing Java options, so hosts using private CAs do not need a new worker-specific switch. -- **Testing today:** `mvn test` currently passes (63 tests, 0 failures/errors/skips). Repo CI also runs build plus emulated, docker, and end-to-end matrices, but there is no direct secure gRPC transport coverage. - -## Plan -1. Expose the parsed `functions-uri` through `IApplication` in a compatibility-safe way. -2. Make `JavaWorkerClient` choose transport from the URI scheme: `https` => TLS with no plaintext downgrade, `http` or legacy host+port => plaintext. -3. Add focused plaintext/TLS transport tests and rerun `mvn test`. \ No newline at end of file diff --git a/.swarm/run/rounds/round-1/planning/plan.json b/.swarm/run/rounds/round-1/planning/plan.json deleted file mode 100644 index 8c14383f..00000000 --- a/.swarm/run/rounds/round-1/planning/plan.json +++ /dev/null @@ -1,34 +0,0 @@ -{ - "designDocument": "## Summary\n\u0060JavaWorkerClient\u0060 always builds \u0060ManagedChannelBuilder.forAddress(...).usePlaintext()\u0060, so the Java worker ignores the already-parsed \u0060--functions-uri\u0060 and can never join a secure host/worker gRPC channel.\n\n## Findings\n- **Fix necessary:** Yes. Today the worker guarantees plaintext and blocks any host-side TLS rollout, leaving any non-local transport exposed to MITM tampering.\n- **Historical context:** Plaintext dates back to the early 2018 same-machine child-process design. In 2023 the worker added prefixed startup args, including \u0060functions-uri\u0060, plus fallback to legacy args, but \u0060JavaWorkerClient\u0060 was not updated; this looks like an old assumption that became a gap once URI-based startup existed.\n- **TLS behavior clarification:** The fix should honor TLS, not ignore it. When \u0060functions-uri\u0060 uses \u0060https\u0060, the worker should build a TLS gRPC channel and fail if handshake, certificate, or hostname validation fails. Only \u0060http\u0060 URIs, or legacy startup that supplies just host\u002Bport, should continue to use plaintext.\n- **Regression / breaking change:** No new CLI or protocol contract. Existing \u0060http\u0060 and legacy host\u002Bport launches keep their current behavior. The only observable behavior change is that a previously ignored or misconfigured \u0060https\u0060 endpoint will stop connecting insecurely and instead fail closed.\n- **Customer contract:** No new flags are required. Trust still comes from the JVM trust configuration already available via existing Java options, so hosts using private CAs do not need a new worker-specific switch.\n- **Testing today:** \u0060mvn test\u0060 currently passes (63 tests, 0 failures/errors/skips). Repo CI also runs build plus emulated, docker, and end-to-end matrices, but there is no direct secure gRPC transport coverage.\n\n## Plan\n1. Expose the parsed \u0060functions-uri\u0060 through \u0060IApplication\u0060 in a compatibility-safe way.\n2. Make \u0060JavaWorkerClient\u0060 choose transport from the URI scheme: \u0060https\u0060 =\u003E TLS with no plaintext downgrade, \u0060http\u0060 or legacy host\u002Bport =\u003E plaintext.\n3. Add focused plaintext/TLS transport tests and rerun \u0060mvn test\u0060.", - "tasks": [ - { - "id": "expose-functions-uri", - "title": "Expose functions-uri to worker startup code", - "description": "Update \u0060src/main/java/com/microsoft/azure/functions/worker/IApplication.java\u0060 and \u0060Application.java\u0060 so the parsed \u0060--functions-uri\u0060 value is available through a compatibility-safe accessor. Use a Java 8 default interface method (or an equivalent non-breaking pattern) so existing implementers do not need source changes, while preserving the current precedence of prefixed args over legacy host/port/request/message-size arguments and keeping host/port extraction intact.", - "dependencies": [], - "roundNumber": 1, - "branchName": "worker/task-1" - }, - { - "id": "select-grpc-transport-from-uri", - "title": "Choose gRPC transport from endpoint scheme", - "description": "Refactor \u0060src/main/java/com/microsoft/azure/functions/worker/JavaWorkerClient.java\u0060 so channel construction reads the endpoint scheme from \u0060functions-uri\u0060 when present instead of hard-coding plaintext. If the URI is \u0060https\u0060, build the channel with transport security and let TLS or certificate failures surface; do not retry or downgrade to plaintext. If the URI is \u0060http\u0060, or no URI is available and startup fell back to legacy host\u002Bport args, keep the current plaintext behavior. Preserve the existing message-size behavior and fail fast with a clear error for unsupported schemes.", - "dependencies": [ - "expose-functions-uri" - ], - "roundNumber": 1, - "branchName": "worker/task-2" - }, - { - "id": "add-grpc-transport-tests", - "title": "Add plaintext and TLS transport coverage", - "description": "Add focused tests under \u0060src/test/java\u0060 (and \u0060src/test/resources\u0060 if needed) that cover: (1) the existing plaintext path still working with the current test host, (2) successful connection to a TLS-enabled gRPC test host when the worker is given a trusted \u0060https\u0060 \u0060functions-uri\u0060, and (3) a negative \u0060https\u0060 case proving TLS errors are not silently downgraded to plaintext. Extend or split the current test host utility as needed, then rerun \u0060mvn test\u0060 to keep the full suite green.", - "dependencies": [ - "select-grpc-transport-from-uri" - ], - "roundNumber": 1, - "branchName": "worker/task-3" - } - ], - "title": "Honor secure Functions host URIs in Java worker" -} \ No newline at end of file diff --git a/.swarm/tasks/add-grpc-transport-tests/task.md b/.swarm/tasks/add-grpc-transport-tests/task.md deleted file mode 100644 index 76119f45..00000000 --- a/.swarm/tasks/add-grpc-transport-tests/task.md +++ /dev/null @@ -1,10 +0,0 @@ -# Add plaintext and TLS transport coverage - -- Task ID: `add-grpc-transport-tests` -- Round: 1 -- Branch: worker/task-3 -- Dependencies: select-grpc-transport-from-uri - -## Description - -Add focused tests under `src/test/java` (and `src/test/resources` if needed) that cover: (1) the existing plaintext path still working with the current test host, (2) successful connection to a TLS-enabled gRPC test host when the worker is given a trusted `https` `functions-uri`, and (3) a negative `https` case proving TLS errors are not silently downgraded to plaintext. Extend or split the current test host utility as needed, then rerun `mvn test` to keep the full suite green. \ No newline at end of file diff --git a/.swarm/tasks/expose-functions-uri/result.json b/.swarm/tasks/expose-functions-uri/result.json deleted file mode 100644 index 9a8bbd2c..00000000 --- a/.swarm/tasks/expose-functions-uri/result.json +++ /dev/null @@ -1,21 +0,0 @@ -{ - "taskId": "expose-functions-uri", - "status": "Succeeded", - "summary": "Exposed functions-uri through a compatibility-safe IApplication default method, updated Application to override it while keeping legacy getUri/host/port behavior, and added startup-argument precedence coverage; validated with mvn test.", - "filesTouched": [ - "src/main/java/com/microsoft/azure/functions/worker/IApplication.java", - "src/main/java/com/microsoft/azure/functions/worker/Application.java", - "src/test/java/com/microsoft/azure/functions/worker/ApplicationTest.java" - ], - "testsRun": { - "executed": true, - "passed": 66, - "failed": 0, - "command": "mvn test" - }, - "failureExcerpt": null, - "designDeviations": null, - "followUps": [ - "Downstream gRPC transport selection can now read IApplication.getFunctionsUri() without breaking existing implementers." - ] -} \ No newline at end of file diff --git a/.swarm/tasks/expose-functions-uri/task.md b/.swarm/tasks/expose-functions-uri/task.md deleted file mode 100644 index b33568fd..00000000 --- a/.swarm/tasks/expose-functions-uri/task.md +++ /dev/null @@ -1,10 +0,0 @@ -# Expose functions-uri to worker startup code - -- Task ID: `expose-functions-uri` -- Round: 1 -- Branch: worker/task-1 -- Dependencies: (none) - -## Description - -Update `src/main/java/com/microsoft/azure/functions/worker/IApplication.java` and `Application.java` so the parsed `--functions-uri` value is available through a compatibility-safe accessor. Use a Java 8 default interface method (or an equivalent non-breaking pattern) so existing implementers do not need source changes, while preserving the current precedence of prefixed args over legacy host/port/request/message-size arguments and keeping host/port extraction intact. \ No newline at end of file diff --git a/.swarm/tasks/select-grpc-transport-from-uri/result.json b/.swarm/tasks/select-grpc-transport-from-uri/result.json deleted file mode 100644 index 426b88bf..00000000 --- a/.swarm/tasks/select-grpc-transport-from-uri/result.json +++ /dev/null @@ -1,20 +0,0 @@ -{ - "taskId": "select-grpc-transport-from-uri", - "status": "Succeeded", - "summary": "Updated JavaWorkerClient to select TLS or plaintext from functions-uri scheme, fail fast on unsupported schemes, and added transport-selection tests; validated with full Maven test suite.", - "filesTouched": [ - "src/main/java/com/microsoft/azure/functions/worker/JavaWorkerClient.java", - "src/test/java/com/microsoft/azure/functions/worker/JavaWorkerClientTest.java" - ], - "testsRun": { - "executed": true, - "passed": 70, - "failed": 0, - "command": "mvn -s /tmp/maven-settings.xml -Dmaven.wagon.http.ssl.insecure=true -Dmaven.wagon.http.ssl.allowall=true test" - }, - "failureExcerpt": null, - "designDeviations": null, - "followUps": [ - "Task add-grpc-transport-tests can extend coverage to end-to-end TLS handshake behavior with a secure test host." - ] -} \ No newline at end of file diff --git a/.swarm/tasks/select-grpc-transport-from-uri/task.md b/.swarm/tasks/select-grpc-transport-from-uri/task.md deleted file mode 100644 index 1e624c8c..00000000 --- a/.swarm/tasks/select-grpc-transport-from-uri/task.md +++ /dev/null @@ -1,10 +0,0 @@ -# Choose gRPC transport from endpoint scheme - -- Task ID: `select-grpc-transport-from-uri` -- Round: 1 -- Branch: worker/task-2 -- Dependencies: expose-functions-uri - -## Description - -Refactor `src/main/java/com/microsoft/azure/functions/worker/JavaWorkerClient.java` so channel construction reads the endpoint scheme from `functions-uri` when present instead of hard-coding plaintext. If the URI is `https`, build the channel with transport security and let TLS or certificate failures surface; do not retry or downgrade to plaintext. If the URI is `http`, or no URI is available and startup fell back to legacy host+port args, keep the current plaintext behavior. Preserve the existing message-size behavior and fail fast with a clear error for unsupported schemes. \ No newline at end of file diff --git a/eng/ci/templates/java-versions.yml b/eng/ci/templates/java-versions.yml index 301601dc..d2fbb759 100644 --- a/eng/ci/templates/java-versions.yml +++ b/eng/ci/templates/java-versions.yml @@ -1,16 +1,16 @@ variables: # Linux JDK Versions - JDK8_LINUX_VERSION: '482' - JDK8_LINUX_BUILD: '08' - JDK11_LINUX_VERSION: '11.0.30' - JDK17_LINUX_VERSION: '17.0.18' - JDK21_LINUX_VERSION: '21.0.10' - JDK25_LINUX_VERSION: '25.0.2' + JDK8_LINUX_VERSION: '492' + JDK8_LINUX_BUILD: '09' + JDK11_LINUX_VERSION: '11.0.31' + JDK17_LINUX_VERSION: '17.0.19' + JDK21_LINUX_VERSION: '21.0.11' + JDK25_LINUX_VERSION: '25.0.3' # Windows JDK Versions - JDK8_WINDOWS_VERSION: '482' - JDK8_WINDOWS_BUILD: '08' - JDK11_WINDOWS_VERSION: '11.0.30' - JDK17_WINDOWS_VERSION: '17.0.18' - JDK21_WINDOWS_VERSION: '21.0.10' - JDK25_WINDOWS_VERSION: '25.0.2' \ No newline at end of file + JDK8_WINDOWS_VERSION: '492' + JDK8_WINDOWS_BUILD: '09' + JDK11_WINDOWS_VERSION: '11.0.31' + JDK17_WINDOWS_VERSION: '17.0.19' + JDK21_WINDOWS_VERSION: '21.0.11' + JDK25_WINDOWS_VERSION: '25.0.3' \ No newline at end of file diff --git a/es-metadata.yml b/es-metadata.yml new file mode 100644 index 00000000..1dd26abe --- /dev/null +++ b/es-metadata.yml @@ -0,0 +1,12 @@ +schemaVersion: 1.0.0 +providers: +- provider: InventoryAsCode + version: 1.0.0 + metadata: + isProduction: true + accountableOwners: + service: e8fdf03b-44f7-48d3-8653-a744c922a342 + routing: + defaultAreaPath: + org: azfunc + path: internal\Azure Functions diff --git a/pom.xml b/pom.xml index a483d766..cf3253da 100644 --- a/pom.xml +++ b/pom.xml @@ -22,7 +22,7 @@ 5.9.1 4.11.0 3.7.6 - 1.75.0 + 1.81.0 @@ -75,7 +75,7 @@ com.google.protobuf protobuf-java - 3.25.5 + 3.25.8 io.grpc @@ -147,7 +147,7 @@ protobuf-maven-plugin 0.5.1 - com.google.protobuf:protoc:3.25.5:exe:${os.detected.classifier} + com.google.protobuf:protoc:3.25.8:exe:${os.detected.classifier} grpc-java io.grpc:protoc-gen-grpc-java:${grpc.version}:exe:${os.detected.classifier} ${basedir}/src/main/azure-functions-language-worker-protobuf/src/proto diff --git a/src/main/java/com/microsoft/azure/functions/worker/Application.java b/src/main/java/com/microsoft/azure/functions/worker/Application.java index 8bb4a494..554e0dbd 100644 --- a/src/main/java/com/microsoft/azure/functions/worker/Application.java +++ b/src/main/java/com/microsoft/azure/functions/worker/Application.java @@ -1,8 +1,8 @@ package com.microsoft.azure.functions.worker; -import java.net.MalformedURLException; +import java.net.URI; import java.net.URISyntaxException; -import java.net.URL; +import java.util.Locale; import java.util.logging.*; import javax.annotation.*; @@ -162,10 +162,6 @@ public String getFunctionsUri() { return this.uri; } - public String getUri() { - return this.getFunctionsUri(); - } - @Override public boolean logToConsole() { return this.logToConsole; @@ -190,17 +186,44 @@ private boolean isCommandlineValid() { private String parseUri(String uri) throws ParseException { try { - URL url = new URL(uri); - url.toURI(); - this.host = url.getHost(); - this.port = url.getPort(); + URI parsedUri = new URI(uri); + String host = parsedUri.getHost(); + String scheme = parsedUri.getScheme(); + int port = parsedUri.getPort(); + + if (scheme == null || scheme.isEmpty()) { + throw new IllegalArgumentException("URI scheme is missing"); + } + + switch (scheme.toLowerCase(Locale.ROOT)) { + case "http": + if (port == -1) { + port = 80; + } + break; + case "https": + if (port == -1) { + port = 443; + } + break; + default: + throw new IllegalArgumentException("Unsupported URI scheme"); + } + + if (host == null || host.isEmpty()) { + throw new IllegalArgumentException("URI host is missing"); + } + if (port < 1 || port > 65535) { throw new IndexOutOfBoundsException("port number out of range"); } + + this.host = host; + this.port = port; return uri; - } catch (MalformedURLException | URISyntaxException | IndexOutOfBoundsException e) { + } catch (URISyntaxException | IllegalArgumentException | IndexOutOfBoundsException e) { throw new ParseException(String.format( - "Error parsing URI \"%s\". Please provide a valid URI", uri)); + "Error parsing URI \"%s\". Please provide a valid http or https URI", uri)); } } diff --git a/src/main/java/com/microsoft/azure/functions/worker/JavaWorkerClient.java b/src/main/java/com/microsoft/azure/functions/worker/JavaWorkerClient.java index c0fbe8a6..478102b2 100644 --- a/src/main/java/com/microsoft/azure/functions/worker/JavaWorkerClient.java +++ b/src/main/java/com/microsoft/azure/functions/worker/JavaWorkerClient.java @@ -1,13 +1,11 @@ package com.microsoft.azure.functions.worker; -import java.net.URI; -import java.net.URISyntaxException; import java.util.*; import java.util.concurrent.*; import java.util.concurrent.atomic.*; import java.util.function.*; import java.util.logging.*; -import javax.annotation.*; +import javax.annotation.PostConstruct; import io.grpc.*; import io.grpc.stub.*; @@ -146,35 +144,11 @@ private synchronized void send(String requestId, MessageHandler marshaller private final Map>> handlerSuppliers; private final ClassLoaderProvider classPathProvider; - static boolean useTransportSecurity(@Nullable String functionsUri) { - if (functionsUri == null) { - return false; - } - - String scheme = parseFunctionsUriScheme(functionsUri); - switch (scheme.toLowerCase(Locale.ROOT)) { - case "http": - return false; - case "https": - return true; - default: - throw new IllegalArgumentException(String.format( - "Unsupported functions URI scheme \"%s\" in functions URI \"%s\". Only http and https are supported.", - scheme, functionsUri)); - } - } - - private static String parseFunctionsUriScheme(String functionsUri) { - try { - String scheme = new URI(functionsUri).getScheme(); - if (scheme == null || scheme.isEmpty()) { - throw new IllegalArgumentException(String.format( - "Unsupported functions URI \"%s\". Only http and https are supported.", functionsUri)); - } - return scheme; - } catch (URISyntaxException ex) { - throw new IllegalArgumentException(String.format( - "Error parsing functions URI \"%s\". Please provide a valid http or https URI.", functionsUri), ex); - } + /** + * @param functionsUri Host endpoint URI, or null for legacy startup args that only provide host and port. + */ + static boolean useTransportSecurity(String functionsUri) { + return functionsUri != null + && functionsUri.regionMatches(true, 0, "https://", 0, "https://".length()); } } diff --git a/src/test/java/com/microsoft/azure/functions/worker/ApplicationTest.java b/src/test/java/com/microsoft/azure/functions/worker/ApplicationTest.java index 8df09e2e..7f468e60 100644 --- a/src/test/java/com/microsoft/azure/functions/worker/ApplicationTest.java +++ b/src/test/java/com/microsoft/azure/functions/worker/ApplicationTest.java @@ -6,6 +6,7 @@ import java.lang.reflect.Method; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -32,7 +33,6 @@ public void prefixedStartupArgumentsTakePrecedenceOverLegacyValues() throws Exce assertTrue(isCommandLineValid(application)); assertEquals("https://functions.example:8443", application.getFunctionsUri()); - assertEquals("https://functions.example:8443", application.getUri()); assertEquals("functions.example", application.getHost()); assertEquals(8443, application.getPort()); assertEquals(Integer.valueOf(2048), application.getMaxMessageSize()); @@ -56,6 +56,43 @@ public void legacyHostAndPortRemainAvailableWhenFunctionsUriIsAbsent() throws Ex assertEquals(Integer.valueOf(4096), application.getMaxMessageSize()); } + @Test + public void httpsFunctionsUriDefaultsToPort443WhenPortIsMissing() throws Exception { + Application application = createApplication( + "--functions-uri", "https://functions.example/path", + "--functions-worker-id", "prefixed-worker", + "--functions-request-id", "prefixed-request", + "--functions-grpc-max-message-length", "2048"); + + assertTrue(isCommandLineValid(application)); + assertEquals("functions.example", application.getHost()); + assertEquals(443, application.getPort()); + } + + @Test + public void httpFunctionsUriDefaultsToPort80WhenPortIsMissing() throws Exception { + Application application = createApplication( + "--functions-uri", "http://functions.example/path", + "--functions-worker-id", "prefixed-worker", + "--functions-request-id", "prefixed-request", + "--functions-grpc-max-message-length", "2048"); + + assertTrue(isCommandLineValid(application)); + assertEquals("functions.example", application.getHost()); + assertEquals(80, application.getPort()); + } + + @Test + public void unsupportedFunctionsUriSchemeFailsCommandLineValidation() throws Exception { + Application application = createApplication( + "--functions-uri", "unix:///tmp/functions.sock", + "--functions-worker-id", "prefixed-worker", + "--functions-request-id", "prefixed-request", + "--functions-grpc-max-message-length", "2048"); + + assertFalse(isCommandLineValid(application)); + } + private static Application createApplication(String... args) throws Exception { Constructor constructor = Application.class.getDeclaredConstructor(String[].class); constructor.setAccessible(true); diff --git a/src/test/java/com/microsoft/azure/functions/worker/JavaWorkerClientTest.java b/src/test/java/com/microsoft/azure/functions/worker/JavaWorkerClientTest.java index c3437334..7309a567 100644 --- a/src/test/java/com/microsoft/azure/functions/worker/JavaWorkerClientTest.java +++ b/src/test/java/com/microsoft/azure/functions/worker/JavaWorkerClientTest.java @@ -3,7 +3,6 @@ import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; public class JavaWorkerClientTest { @@ -23,10 +22,7 @@ public void httpsFunctionsUriUsesTransportSecurity() { } @Test - public void unsupportedFunctionsUriSchemeFailsFast() { - IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, - () -> JavaWorkerClient.useTransportSecurity("unix:///tmp/functions.sock")); - - assertTrue(exception.getMessage().contains("Only http and https are supported.")); + public void httpsFunctionsUriSchemeIsCaseInsensitive() { + assertTrue(JavaWorkerClient.useTransportSecurity("HTTPS://functions.example:8443")); } } diff --git a/src/test/java/com/microsoft/azure/functions/worker/functional/tests/GrpcTransportTest.java b/src/test/java/com/microsoft/azure/functions/worker/functional/tests/GrpcTransportTest.java new file mode 100644 index 00000000..95c6d813 --- /dev/null +++ b/src/test/java/com/microsoft/azure/functions/worker/functional/tests/GrpcTransportTest.java @@ -0,0 +1,141 @@ +package com.microsoft.azure.functions.worker.functional.tests; + +import java.net.*; +import java.util.concurrent.*; +import javax.net.ssl.*; + +import com.microsoft.azure.functions.rpc.messages.*; +import com.microsoft.azure.functions.worker.test.utilities.*; +import org.junit.jupiter.api.*; + +import static org.junit.jupiter.api.Assertions.*; + +public class GrpcTransportTest extends FunctionsTestBase { + private static final String RETURN_VALUE = "transport-ok"; + private static final String TRUSTSTORE_RESOURCE = "grpc-tls/localhost-truststore.p12"; + private static final String TRUSTSTORE_PASSWORD = "changeit"; + + public String ReturnStringFunction() { + return RETURN_VALUE; + } + + @Test + public void legacyPlaintextTransportStillWorks() throws Exception { + try (SkipTestingScope ignored = SkipTestingScope.enable(); + FunctionsTestHost host = new FunctionsTestHost()) { + InvocationResponse response = this.invokeReturnString(host, "plaintext-function", "plaintext-request"); + + assertEquals(TypedData.DataCase.STRING, response.getReturnValue().getDataCase()); + assertEquals(RETURN_VALUE, response.getReturnValue().getString()); + } + } + + @Test + public void trustedHttpsFunctionsUriConnectsToTlsHost() throws Exception { + try (SkipTestingScope ignored = SkipTestingScope.enable(); + TrustStoreScope ignoredTrustStore = TrustStoreScope.use(TRUSTSTORE_RESOURCE, TRUSTSTORE_PASSWORD, "PKCS12"); + FunctionsTestHost host = new FunctionsTestHost(FunctionsTestHost.ServerTransport.TLS, FunctionsTestHost.ClientTransport.HTTPS)) { + InvocationResponse response = this.invokeReturnString(host, "tls-function", "tls-request"); + + assertEquals(TypedData.DataCase.STRING, response.getReturnValue().getDataCase()); + assertEquals(RETURN_VALUE, response.getReturnValue().getString()); + } + } + + @Test + public void httpsFunctionsUriDoesNotDowngradeToPlaintextWhenTlsFails() { + ExecutionException exception = assertThrows(ExecutionException.class, () -> { + try (SkipTestingScope ignored = SkipTestingScope.enable(); + FunctionsTestHost ignoredHost = new FunctionsTestHost(FunctionsTestHost.ServerTransport.PLAINTEXT, FunctionsTestHost.ClientTransport.HTTPS)) { + } + }); + + assertTrue(hasCause(exception, SSLException.class), "Expected TLS failure but got: " + exception); + } + + private InvocationResponse invokeReturnString(FunctionsTestHost host, String functionId, String requestId) throws Exception { + this.loadFunction(host, functionId, "ReturnStringFunction"); + return host.call(requestId, functionId); + } + + private static boolean hasCause(Throwable throwable, Class type) { + Throwable current = throwable; + while (current != null) { + if (type.isInstance(current)) { + return true; + } + current = current.getCause(); + } + return false; + } + + private static final class TrustStoreScope implements AutoCloseable { + private final String originalTrustStore; + private final String originalTrustStorePassword; + private final String originalTrustStoreType; + + private TrustStoreScope(String originalTrustStore, String originalTrustStorePassword, String originalTrustStoreType) { + this.originalTrustStore = originalTrustStore; + this.originalTrustStorePassword = originalTrustStorePassword; + this.originalTrustStoreType = originalTrustStoreType; + } + + static TrustStoreScope use(String resourceName, String password, String storeType) { + String originalTrustStore = System.getProperty("javax.net.ssl.trustStore"); + String originalTrustStorePassword = System.getProperty("javax.net.ssl.trustStorePassword"); + String originalTrustStoreType = System.getProperty("javax.net.ssl.trustStoreType"); + URL resource = GrpcTransportTest.class.getClassLoader().getResource(resourceName); + if (resource == null) { + throw new IllegalStateException("Missing TLS truststore resource: " + resourceName); + } + + try { + System.setProperty("javax.net.ssl.trustStore", new java.io.File(resource.toURI()).getAbsolutePath()); + } catch (URISyntaxException ex) { + throw new IllegalStateException("Invalid TLS truststore resource path: " + resourceName, ex); + } + System.setProperty("javax.net.ssl.trustStorePassword", password); + System.setProperty("javax.net.ssl.trustStoreType", storeType); + return new TrustStoreScope(originalTrustStore, originalTrustStorePassword, originalTrustStoreType); + } + + @Override + public void close() { + restore("javax.net.ssl.trustStore", this.originalTrustStore); + restore("javax.net.ssl.trustStorePassword", this.originalTrustStorePassword); + restore("javax.net.ssl.trustStoreType", this.originalTrustStoreType); + } + + private static void restore(String key, String value) { + if (value == null) { + System.clearProperty(key); + } else { + System.setProperty(key, value); + } + } + } + + private static final class SkipTestingScope implements AutoCloseable { + private static final String SKIP_TESTING_PROPERTY = "azure.functions.worker.java.skip.testing"; + private final String originalValue; + + private SkipTestingScope(String originalValue) { + this.originalValue = originalValue; + } + + static SkipTestingScope enable() { + String originalValue = System.getProperty(SKIP_TESTING_PROPERTY); + System.setProperty(SKIP_TESTING_PROPERTY, "true"); + return new SkipTestingScope(originalValue); + } + + @Override + public void close() { + if (this.originalValue == null) { + System.clearProperty(SKIP_TESTING_PROPERTY); + } else { + System.setProperty(SKIP_TESTING_PROPERTY, this.originalValue); + } + } + } +} diff --git a/src/test/java/com/microsoft/azure/functions/worker/test/utilities/FunctionsTestHost.java b/src/test/java/com/microsoft/azure/functions/worker/test/utilities/FunctionsTestHost.java index 207f0634..78d73a03 100644 --- a/src/test/java/com/microsoft/azure/functions/worker/test/utilities/FunctionsTestHost.java +++ b/src/test/java/com/microsoft/azure/functions/worker/test/utilities/FunctionsTestHost.java @@ -1,8 +1,10 @@ package com.microsoft.azure.functions.worker.test.utilities; import java.io.*; -import java.net.ServerSocket; +import java.net.*; +import java.nio.file.*; import java.util.*; +import java.util.concurrent.*; import java.util.concurrent.atomic.*; import java.util.concurrent.locks.*; import java.util.function.*; @@ -14,15 +16,44 @@ import com.microsoft.azure.functions.worker.*; import com.microsoft.azure.functions.rpc.messages.*; import io.grpc.*; +import io.grpc.netty.shaded.io.grpc.netty.*; import io.grpc.stub.*; import org.apache.commons.lang3.tuple.*; public final class FunctionsTestHost implements AutoCloseable, IApplication { + public enum ClientTransport { + LEGACY, + HTTP, + HTTPS + } + + public enum ServerTransport { + PLAINTEXT, + TLS + } + + private static final int RESPONSE_TIMEOUT_SECONDS = 10; + private static final long RESPONSE_POLL_MILLIS = 100L; + private static final String TLS_RESOURCE_ROOT = "grpc-tls/"; + private static final String TLS_CERTIFICATE_RESOURCE = TLS_RESOURCE_ROOT + "localhost-cert.pem"; + private static final String TLS_PRIVATE_KEY_RESOURCE = TLS_RESOURCE_ROOT + "localhost-key.pem"; + private int port; public FunctionsTestHost() throws Exception { + this(ServerTransport.PLAINTEXT, ClientTransport.LEGACY); + } + + public FunctionsTestHost(ServerTransport serverTransport, ClientTransport clientTransport) throws Exception { + this.serverTransport = serverTransport; + this.clientTransport = clientTransport; this.port = populatePort(); - this.initializeServer(); - this.initializeClient(); + try { + this.initializeServer(); + this.initializeClient(); + } catch (Exception ex) { + this.closeQuietly(); + throw ex; + } } private final List list = Arrays.asList(55005, 5005); @@ -37,23 +68,42 @@ private int populatePort() { @PostConstruct private void initializeServer() throws IOException { - ServerBuilder builder = ServerBuilder.forPort(this.getPort()); + ServerBuilder builder = this.serverTransport == ServerTransport.TLS + ? NettyServerBuilder.forPort(this.getPort()) + .sslContext(GrpcSslContexts.forServer(getTlsResource(TLS_CERTIFICATE_RESOURCE), getTlsResource(TLS_PRIVATE_KEY_RESOURCE)).build()) + : ServerBuilder.forPort(this.getPort()); this.grpcHost = new HostGrpcImplementation(); this.server = builder.addService(this.grpcHost).build(); this.server.start(); } @PostConstruct - private void initializeClient() throws InterruptedException { + private void initializeClient() throws Exception { this.client = new JavaWorkerClient(this); - this.client.listen("java-worker-test", HostGrpcImplementation.ESTABLISH_REQID); - this.grpcHost.handleMessage(HostGrpcImplementation.ESTABLISH_REQID, m -> this.grpcHost.initWorker()); + this.listeningTask = this.client.listen("java-worker-test", HostGrpcImplementation.ESTABLISH_REQID); + this.grpcHost.handleMessageWithTimeout( + HostGrpcImplementation.ESTABLISH_REQID, + m -> this.grpcHost.initWorker(), + RESPONSE_TIMEOUT_SECONDS, + TimeUnit.SECONDS); } @Override public void close() throws Exception { - this.client.close(); - this.server.shutdownNow().awaitTermination(); + Exception closeException = null; + if (this.client != null) { + try { + this.client.close(); + } catch (Exception ex) { + closeException = ex; + } + } + if (this.server != null) { + this.server.shutdownNow().awaitTermination(15, TimeUnit.SECONDS); + } + if (closeException != null) { + throw closeException; + } } public void loadFunction(String id, String reflectionName, Map bindings) throws Exception { @@ -80,13 +130,52 @@ public final InvocationResponse call(String reqId, String funcId, Triple listeningTask; private String lastCallReqId = HostGrpcImplementation.LOADFUNC_REQID; + private void closeQuietly() { + try { + this.close(); + } catch (Exception ignored) { + } + } + + private static File getTlsResource(String resourcePath) { + URL resource = FunctionsTestHost.class.getClassLoader().getResource(resourcePath); + if (resource == null) { + throw new IllegalStateException("Missing test TLS resource: " + resourcePath); + } + try { + return Paths.get(resource.toURI()).toFile(); + } catch (URISyntaxException ex) { + throw new IllegalStateException("Invalid test TLS resource path: " + resourcePath, ex); + } + } + + private void throwIfListeningFailed() throws ExecutionException, InterruptedException { + if (this.listeningTask != null && this.listeningTask.isDone()) { + this.listeningTask.get(); + } + } + @ThreadSafe private class HostGrpcImplementation extends FunctionRpcGrpc.FunctionRpcImplBase { @@ -107,25 +196,50 @@ private void setResponse(String requestId, StreamingMessage value, StreamObserve this.getResponseCondition(requestId).signal(); } - void handleMessage(String requestId, Function handler) throws InterruptedException { + void handleMessage(String requestId, Function handler) throws Exception { this.lock.lock(); try { - if (this.responder.get(requestId) == null) { - this.getResponseCondition(requestId).await(); - } - StreamingMessage message = this.respValue.get(requestId); - StreamingMessage response = null; - if (handler != null) { - response = handler.apply(message); + while (this.responder.get(requestId) == null) { + this.getResponseCondition(requestId).await(RESPONSE_POLL_MILLIS, TimeUnit.MILLISECONDS); + FunctionsTestHost.this.throwIfListeningFailed(); } - if (response != null) { - this.responder.get(requestId).onNext(response); + this.respondToMessage(requestId, handler); + } finally { + this.lock.unlock(); + } + } + + void handleMessageWithTimeout(String requestId, Function handler, + long timeout, TimeUnit unit) throws Exception { + this.lock.lock(); + try { + long deadlineNanos = System.nanoTime() + unit.toNanos(timeout); + while (this.responder.get(requestId) == null) { + FunctionsTestHost.this.throwIfListeningFailed(); + long remainingNanos = deadlineNanos - System.nanoTime(); + if (remainingNanos <= 0) { + throw new TimeoutException("Timed out waiting for gRPC request " + requestId); + } + long waitMillis = Math.max(1L, Math.min(TimeUnit.NANOSECONDS.toMillis(remainingNanos), RESPONSE_POLL_MILLIS)); + this.getResponseCondition(requestId).await(waitMillis, TimeUnit.MILLISECONDS); } + this.respondToMessage(requestId, handler); } finally { this.lock.unlock(); } } + private void respondToMessage(String requestId, Function handler) { + StreamingMessage message = this.respValue.get(requestId); + StreamingMessage response = null; + if (handler != null) { + response = handler.apply(message); + } + if (response != null) { + this.responder.get(requestId).onNext(response); + } + } + private StreamingMessage initWorker() { WorkerInitRequest.Builder request = WorkerInitRequest.newBuilder().setHostVersion("2.0.0"); return StreamingMessage.newBuilder().setRequestId(INITWORKER_REQID).setWorkerInitRequest(request).build(); diff --git a/src/test/resources/grpc-tls/localhost-cert.pem b/src/test/resources/grpc-tls/localhost-cert.pem new file mode 100644 index 00000000..4d9d070b --- /dev/null +++ b/src/test/resources/grpc-tls/localhost-cert.pem @@ -0,0 +1,19 @@ +-----BEGIN CERTIFICATE----- +MIIDHzCCAgegAwIBAgIUUT0dnmqskpKcIS3wt+Fq+obG5GEwDQYJKoZIhvcNAQEL +BQAwFDESMBAGA1UEAwwJbG9jYWxob3N0MB4XDTI2MDUxMjE3MjExMFoXDTM2MDUw +OTE3MjExMFowFDESMBAGA1UEAwwJbG9jYWxob3N0MIIBIjANBgkqhkiG9w0BAQEF +AAOCAQ8AMIIBCgKCAQEAmRbV1NPyunri/+EN14nfIQhyY7grIRmUS5XTvsJhVQMg +/vP0rZ1H1I8f7mzkmyfDsB0i5putmz01lAHD5/Wfa0JEhVgmnwK20RsufjsWhwpY +tVpbfzgY5LJILSwz5Qw//Q2VDFiFXK7plVkfnmwrutUQ4AXuk55IuLDHlHxAI7vt +v/MxxlCri+EeoZcmTTVkLeMvtTf7Pw3upc8PBNNDYJOJvld4DXxz4e93ZhFmQGck +QMVeGGPsL+jmhu78pdfW+TcwlEJTvKBZ8xYTGaQbks6ZwXslYvuL0mI7XEajTSrM +Pw7vJeaJH85fETtZNfz3/IR4or88pyQQN9Cv0fY3xQIDAQABo2kwZzAdBgNVHQ4E +FgQUQgk8tVR0g3rgA6MRktL39pScY9UwHwYDVR0jBBgwFoAUQgk8tVR0g3rgA6MR +ktL39pScY9UwDwYDVR0TAQH/BAUwAwEB/zAUBgNVHREEDTALgglsb2NhbGhvc3Qw +DQYJKoZIhvcNAQELBQADggEBAJg1IQoiIV8qoLpVLbY5pvgkeClmFQsVwFTB0rLS +lU4ZkVkoIiYowQnb10WFpGdyhfI+WhZcLev2Dmm7PJ9Jp0H0vC5U0ebNLzMJdwjr +HjJcIuueYJDJ3YelEIzO2qa4R977PCvBPLHEi7/KRdSbAOz6lf6Yp+OPe46gJHP1 +XwaKt628oPd7A4FdBTF7lUfcLdPUnW9Glhg7VfB6aX6o5+Mup+QEt96xlXg6Ua2L +xYw8AJ75rAmQLwu05uzyaTGlHE7BXtsP+bEBAz4CXVv1z5i1UJfB6N/aWB1VOHKN +2r1x1fMUCNLpsoTdtmkwAmkmzx4bfyWtIIK+hAxTm7bmjGk= +-----END CERTIFICATE----- diff --git a/src/test/resources/grpc-tls/localhost-key.pem b/src/test/resources/grpc-tls/localhost-key.pem new file mode 100644 index 00000000..976e744b --- /dev/null +++ b/src/test/resources/grpc-tls/localhost-key.pem @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCZFtXU0/K6euL/ +4Q3Xid8hCHJjuCshGZRLldO+wmFVAyD+8/StnUfUjx/ubOSbJ8OwHSLmm62bPTWU +AcPn9Z9rQkSFWCafArbRGy5+OxaHCli1Wlt/OBjkskgtLDPlDD/9DZUMWIVcrumV +WR+ebCu61RDgBe6Tnki4sMeUfEAju+2/8zHGUKuL4R6hlyZNNWQt4y+1N/s/De6l +zw8E00Ngk4m+V3gNfHPh73dmEWZAZyRAxV4YY+wv6OaG7vyl19b5NzCUQlO8oFnz +FhMZpBuSzpnBeyVi+4vSYjtcRqNNKsw/Du8l5okfzl8RO1k1/Pf8hHiivzynJBA3 +0K/R9jfFAgMBAAECggEANvM/Zdl+MvmRKY+6zDcs5EqH5Mtij8sCs+7fxoU3MrCg +02L13Kur8Nw+9fIYTKkFUN3kfSo8MpDR/oJzs3sy8ekjd0mg80qiHITJN342I9rO +5Km+Vffo14424iAPsJOpFEgfzAKqPA58waLv+omRWMrJ99+pN0uFhuXNfbrruudX +zgCMWtw7gqciTEOrcy2GLyKOClfg6AIn/9F1Z3yrQBrHW7TZjPhyHZi3PeIeXAz4 +hVjj5zHP8GI8P4O/kP6gFfB5x/1PXGErGOSl3HOieKse7JTMu7NCtgLEEshFAjOd +z4EsAGzJb0FdyH1eX9+WhsvVkbTTnek3fhBCMIJ/MQKBgQDLvpbTbRLdbxH6pDIx +I53jBpDU1x+QU5j1lvLZJtvdMnsJFA43zamSqwmIzuIAs9POzEGIP586TCr5PXBi +NrAz9mYZC1ye59ZFq0JVundbCWEO+YxuR5glgqjubWdfwFzFynCyuuZTqmPozGnY +wSOjIE7g0Vw6aJVXJq0eXG1EAwKBgQDAWlUSyXSVZ7INXFr+Dh0gd12HhQMYmxEz +twtXEfZTGwHqlxZOo0/G/LYVWPdIqkUw86aaKorlfgg5yzbJ6tfgxiE71W7VPiBR +X9Cefz16th2rQfM6Y1fszimV1Dpf3HU5TuN/ZK4tlEqsfK01yfjofPCjNL1c2BQr +sHYkUi5elwKBgBy8LZN2D7IRRyzdWYLarhrlwylxia8WSz1f47JCq8GfrACUxoiS +Rfc8jiSwYOmOczH4Vsm7h152fZ0XUDFZ2zII709a7d4vfmXnCH0Exm6dfQXapjar +fEbWDbNK1MiJXcw7h/d9KpzkLCEaK1d5regE13sXq/VE6MMY3lOo33Q3AoGAamsP +mh8+ktIV3fJ0nQ3t62JeqnVaayiPcc8ZRQi5AO12N/Vy7/rGTk7N5i2cUeVx9k02 +pSBYS/NYVbEqFLgKy16SUGoasXt3oc2iu62ls9hBvdf02x7PLEI7G5uY2CQ97oDI +uFhZTPo3/gnUQmgFf4pwD7tD8LPTJQCxvBKDeO0CgYEApeC2lxH4FekJEA4iIBG+ +InRXX20QCLqI2d4xMOHHDEWaulhDEkjs9HEErA+3HpkK5hOTsEIU6GENrpc3P2JY +iFaO/gJiFEErkXsSaDRRaROTKI27qA0JPWzrfDHH4G59qpVVH6KWIYJvWbsHHo49 +MSFfMNCXhE9polVRU6EyWb0= +-----END PRIVATE KEY----- diff --git a/src/test/resources/grpc-tls/localhost-truststore.p12 b/src/test/resources/grpc-tls/localhost-truststore.p12 new file mode 100644 index 00000000..cb4c21c1 Binary files /dev/null and b/src/test/resources/grpc-tls/localhost-truststore.p12 differ