From 2dc72cacae8a7d12e995bab73540bf2f4df113b2 Mon Sep 17 00:00:00 2001 From: joshvanl Date: Tue, 19 Sep 2023 14:32:12 +0100 Subject: [PATCH 1/3] Change `DAPR_GRPC_ENDPOINT` to infer TLS based on query parameter Signed-off-by: joshvanl --- 0008-S-sidecar-endpoint-tls.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/0008-S-sidecar-endpoint-tls.md b/0008-S-sidecar-endpoint-tls.md index 621f8f6..04d39bf 100644 --- a/0008-S-sidecar-endpoint-tls.md +++ b/0008-S-sidecar-endpoint-tls.md @@ -67,12 +67,12 @@ Cons: ### Design -* `DAPR_GRPC_ENDPOINT` defines entire endpoit for gRPC, not just host: `https://dapr-grpc.mycompany.com` -* `DAPR_HTTP_ENDPOINT` defines entire endpoit for HTTP, not just host: `https://dapr-http.mycompany.com` -* Port is parsed from the URL (`https://dapr.mycompany.com:8080`) or via the default port of the protocol used in the URL (80 for `http` and 443 for `https`) +* `DAPR_GRPC_ENDPOINT` defines entire endpoint for gRPC, not just host: `dapr-grpc.mycompany.com`. No port in the URL defaults to 443. +* `DAPR_HTTP_ENDPOINT` defines entire endpoint for HTTP, not just host: `https://dapr-http.mycompany.com` +* Port is parsed from the hostport string (`dapr.mycompany.com:8080`) or via the default port of the protocol used in the URL (80 for `plaintext` and 443 for `TLS`) * `DAPR_GRPC_ENDPOINT` and `DAPR_HTTP_ENDPOINT` can be set at the same time since some SDKs (Java, as of now) supports both protocols at the same time and app can pick which one to use. -* `DAPR_GRPC_ENDPOINT` and `DAPR_HTTP_ENDPOINT` must be parsed and the protocol will be used for SDK to determine if communication is over TLS (if not done automatically). In summary, `https` means secure channel. -* Initially, only `http` and `https` protocols should be supported. Other protocols can be added in the future depending on each language support. +* `DAPR_HTTP_ENDPOINT` must be parsed and the protocol will be used by SDK to determine if communication is over TLS (if not done automatically). In summary, `https` means secure channel. +* `DAPR_GRPC_ENDPOINT` must be parsed and the query parameter will be used to determine whether the endpoint uses TLS. In summary, `?tls=true` means to use TLS. An empty query parameter defaults TLS based on port (`TLS` when port is 443, `plaintext` otherwise). SDKs should error on unrecognised or invalid query parameters. * `DAPR_GRPC_ENDPOINT` and `DAPR_HTTP_ENDPOINT` have priority over existing `DAPR_HOST` and `DAPR_HTTP_PORT` or `DAPR_GRPC_PORT` environment variables. Application's hardcoded values passed via constructor takes priority over any environment variable. In summary, this is the priority list (highest on top): 1. Values passed via constructor or builder method. 2. Properties or any other language specific configuration framework. @@ -88,6 +88,9 @@ https://github.com/dapr/java-sdk/blob/76aec01e9aa4af7a72b910d77685ddd3f0bf86f3/s * Compatability guarantees This feature should allow localhost definition too `http://127.0.0.1:3500`, for example. +this feature should continue to allow using other resolvers other than DNS (e.g. +`unix://`). + * Deprecation / co-existence with existing functionality This feature takes priority over existing (inconsistent) environment variables from each SDK. If app provides a hardcoded value for Dapr endpoint (via constructor, for example), it takes priority. Use of existing `DAPR_API_TOKEN` environment variables is highly encouraged for remote API but not required. From 403c597e102a1c810abd3ea120972895ac5a477c Mon Sep 17 00:00:00 2001 From: joshvanl Date: Tue, 26 Sep 2023 21:33:36 +0100 Subject: [PATCH 2/3] Update 0008-S-sidecar-endpoint-tls.md with example results of parsing various DAPR_GRPC_ENDPOINT environment variable values Signed-off-by: joshvanl --- 0008-S-sidecar-endpoint-tls.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/0008-S-sidecar-endpoint-tls.md b/0008-S-sidecar-endpoint-tls.md index 04d39bf..0c60300 100644 --- a/0008-S-sidecar-endpoint-tls.md +++ b/0008-S-sidecar-endpoint-tls.md @@ -79,6 +79,19 @@ Cons: 3. `DAPR_GRPC_ENDPOINT` and `DAPR_HTTP_ENDPOINT` 4. Existing `DAPR_HOST` (or equivalent, defaulting to `127.0.0.1`) + `DAPR_HTTP_PORT` or `DAPR_GRPC_PORT` +`DAPR_GRPC_ENDPOINT` host port parsing example: + +``` +myhost => port=443 tls=true resolver=dns +myhost?tls=false => port=443 tls=false resolver=dns +myhost:443 => port=443 tls=true resolver=dns +myhost:1003 => port=1003 tls=false resolver=dns +myhost:1003?tls=true => port=1003 tls=true resolver=dns +dns://myhost:1003?tls=true => port=1003 tls=true resolver=dns +unix://my.sock => port= tls=false resolver=unix +unix://my.sock?tls=true => port= tls=true resolver=unix +``` + #### Example of implementation https://github.com/dapr/java-sdk/blob/76aec01e9aa4af7a72b910d77685ddd3f0bf86f3/sdk/src/main/java/io/dapr/client/DaprClientBuilder.java#L172C3-L192 From a065789ef692e201d8c0c19414b8e3610ce181c2 Mon Sep 17 00:00:00 2001 From: joshvanl Date: Wed, 4 Oct 2023 10:48:00 +0100 Subject: [PATCH 3/3] Adds backwards compatibility to `https` scheme, and don't use TLS on port 443 by default. Signed-off-by: joshvanl --- 0008-S-sidecar-endpoint-tls.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/0008-S-sidecar-endpoint-tls.md b/0008-S-sidecar-endpoint-tls.md index 0c60300..100f4a2 100644 --- a/0008-S-sidecar-endpoint-tls.md +++ b/0008-S-sidecar-endpoint-tls.md @@ -72,7 +72,7 @@ Cons: * Port is parsed from the hostport string (`dapr.mycompany.com:8080`) or via the default port of the protocol used in the URL (80 for `plaintext` and 443 for `TLS`) * `DAPR_GRPC_ENDPOINT` and `DAPR_HTTP_ENDPOINT` can be set at the same time since some SDKs (Java, as of now) supports both protocols at the same time and app can pick which one to use. * `DAPR_HTTP_ENDPOINT` must be parsed and the protocol will be used by SDK to determine if communication is over TLS (if not done automatically). In summary, `https` means secure channel. -* `DAPR_GRPC_ENDPOINT` must be parsed and the query parameter will be used to determine whether the endpoint uses TLS. In summary, `?tls=true` means to use TLS. An empty query parameter defaults TLS based on port (`TLS` when port is 443, `plaintext` otherwise). SDKs should error on unrecognised or invalid query parameters. +* `DAPR_GRPC_ENDPOINT` must be parsed and the query parameter will be used to determine whether the endpoint uses TLS. In summary, `?tls=true` means to use TLS. An empty query parameter defaults TLS to false. SDKs should error on unrecognised or invalid query parameters. * `DAPR_GRPC_ENDPOINT` and `DAPR_HTTP_ENDPOINT` have priority over existing `DAPR_HOST` and `DAPR_HTTP_PORT` or `DAPR_GRPC_PORT` environment variables. Application's hardcoded values passed via constructor takes priority over any environment variable. In summary, this is the priority list (highest on top): 1. Values passed via constructor or builder method. 2. Properties or any other language specific configuration framework. @@ -82,9 +82,9 @@ Cons: `DAPR_GRPC_ENDPOINT` host port parsing example: ``` -myhost => port=443 tls=true resolver=dns +myhost => port=443 tls=false resolver=dns myhost?tls=false => port=443 tls=false resolver=dns -myhost:443 => port=443 tls=true resolver=dns +myhost:443 => port=443 tls=false resolver=dns myhost:1003 => port=1003 tls=false resolver=dns myhost:1003?tls=true => port=1003 tls=true resolver=dns dns://myhost:1003?tls=true => port=1003 tls=true resolver=dns @@ -101,13 +101,17 @@ https://github.com/dapr/java-sdk/blob/76aec01e9aa4af7a72b910d77685ddd3f0bf86f3/s * Compatability guarantees This feature should allow localhost definition too `http://127.0.0.1:3500`, for example. -this feature should continue to allow using other resolvers other than DNS (e.g. +* This feature should continue to allow using other resolvers other than DNS (e.g. `unix://`). * Deprecation / co-existence with existing functionality This feature takes priority over existing (inconsistent) environment variables from each SDK. If app provides a hardcoded value for Dapr endpoint (via constructor, for example), it takes priority. Use of existing `DAPR_API_TOKEN` environment variables is highly encouraged for remote API but not required. +* SDKs will continue to accept the old behaviour of DAPR_GRPC_ENPOINT` with + the scheme value `https` to signal to use TLS. Where a value contains both the + `https` scheme and `?tls=false` query, SDKs will error and refuse to connect. + * Feature flags N/A