OAuth2: Dynamic Secrets #697

mbana · 2022-09-05T21:17:33Z

Support defining the secrets dynamically, i.e., they should appear as dynamic_active_secrets in the admin /config_dump page:

$ kubectl port-forward --namespace default deployment/default 19000:19000 &
$ curl -s 'http://localhost:19000/stats' | grep sds
sds.client_secret_test_1.version_text: "690"
sds.hmac_secret_test_1.version_text: "690"
cluster.kubeshop-kusk-gateway-oauth2.eu.auth0.com.client_ssl_socket_factory.ssl_context_update_by_sds: 0
sds.client_secret_test_1.init_fetch_timeout: 0
sds.client_secret_test_1.key_rotation_failed: 0
sds.client_secret_test_1.update_attempt: 2
sds.client_secret_test_1.update_failure: 0
sds.client_secret_test_1.update_rejected: 0
sds.client_secret_test_1.update_success: 1
sds.client_secret_test_1.update_time: 1662408207693
sds.client_secret_test_1.version: 18219575566587264222
sds.hmac_secret_test_1.init_fetch_timeout: 0
sds.hmac_secret_test_1.key_rotation_failed: 0
sds.hmac_secret_test_1.update_attempt: 2
sds.hmac_secret_test_1.update_failure: 0
sds.hmac_secret_test_1.update_rejected: 0
sds.hmac_secret_test_1.update_success: 1
sds.hmac_secret_test_1.update_time: 1662408207693
sds.hmac_secret_test_1.version: 18219575566587264222
sds.client_secret_test_1.update_duration: P0(nan,0) P25(nan,0) P50(nan,0) P75(nan,0) P90(nan,0) P95(nan,0) P99(nan,0) P99.5(nan,0) P99.9(nan,0) P100(nan,0)
sds.hmac_secret_test_1.update_duration: P0(nan,0) P25(nan,0) P50(nan,0) P75(nan,0) P90(nan,0) P95(nan,0) P99(nan,0) P99.5(nan,0) P99.9(nan,0) P100(nan,0)
$ curl -s 'http://localhost:19000/config_dump' | jq '.[] | .[-1].dynamic_active_secrets'
[
  {
    "name": "client_secret_test_1",
    "version_info": "690",
    "last_updated": "2022-09-05T20:03:27.693Z",
    "secret": {
      "@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret",
      "name": "client_secret_test_1",
      "generic_secret": {
        "secret": {
          "inline_string": "[redacted]"
        }
      }
    }
  },
  {
    "name": "hmac_secret_test_1",
    "version_info": "690",
    "last_updated": "2022-09-05T20:03:27.693Z",
    "secret": {
      "@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret",
      "name": "hmac_secret_test_1",
      "generic_secret": {
        "secret": {
          "inline_bytes": "W3JlZGFjdGVkXQ=="
        }
      }
    }
  }
]

Things to note from the above output are:

The secrets are in the dynamic_active_secrets object not in a dynamic_warming_secrets object, the later of which would mean that the secrets have not been sent to envoy. See https://www.envoyproxy.io/docs/envoy/v1.23.1/api-v3/admin/v3/config_dump.proto#admin-v3-secretsconfigdump for more information. As per the documentation dynamic_active_secrets:

"The dynamically loaded active secrets. These are secrets that are available to service clusters or listeners."

The output from the /stats endpoint shows no failures.

Signed-off-by: Mohamed Bana mohamed@bana.io

Summary

The main changes are in:

EnvoyConfiguration.GenerateSnapshot.
internal/envoy/auth/oauth2_filter.go - use ADS as the config source.
internal/envoy/config/hcm.go - the removal of SetNodeOnFirstMessageOnly.
It's important to wait for the log line below to appear before applying changes otherwise it seems things remain in dynamic_warming_secrets and a restart of Envoy is required by doing curl -X POST 'http://localhost:19000/quitquitquit (given the port forward to the admin port is active):

kusk-system kusk-gateway-manager-778dbbbb65-jwzn8 manager {"level":"info","ts":1662412460.1216035,"logger":"CacheManager","caller":"manager/envoy_callbacks.go:65","msg":"OnStreamRequest","id":1,"request.TypeUrl":"type.googleapis.com/envoy.config.listener.v3.Listener","request.Node.Cluster":"default.default","request.Node.Id":"default-646f9677f7-kxtbw"}

TODO

Update smoketests to tests that secrets are in dynamic_active_secrets and not in dynamic_warming_secrets. This just requires a GET on the endpoint and checking that they are indeed dynamic_active_secrets. Partially complete in that the structure of the test has been written.
Update OAuth2 documentation.
Managements of secrets when an API is deleted. The secrets defined needed to be deleted as well.
Could someone please have a look at internal/envoy/config/hcm.go and tell me why SetNodeOnFirstMessageOnly: true is there in the first place? It seems like when it is set to true, the secrets remain in the dynamic_warming_secrets object which won't allow dynamic secrets to be defined.
Document how a developer could quickly test out OAuth2.

netlify · 2022-09-05T21:18:03Z

✅ Deploy Preview for kusk-docs-preview ready!

Name	Link
🔨 Latest commit	`16230a2`
🔍 Latest deploy log	https://app.netlify.com/sites/kusk-docs-preview/deploys/633d57f87e91d30008149240
😎 Deploy Preview	https://deploy-preview-697--kusk-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

jasmingacic · 2022-09-05T21:19:28Z

Is this still work in progress?

cmd/manager/main.go

internal/controllers/config_manager.go

internal/controllers/envoy.yaml

internal/envoy/auth/oauth2_filter.go

internal/envoy/config/envoy_configuration.go

kylehodgetts · 2022-09-06T07:59:21Z

internal/envoy/config/hcm.go

+				ApiType:             envoy_core_v3.ApiConfigSource_GRPC,
+				RequestTimeout:      durationpb.New(defaultRequestTimeout),
+				GrpcServices:        []*envoy_core_v3.GrpcService{makeGrpcService(cluster, "", defaultResponseTimeout)},
+				//SetNodeOnFirstMessageOnly: true, // Could someone please justify why this is set to `true`?


What does this setting even do?

I'm not entirely sure myself, but it seems to make a difference in secrets being discovered or not, what I mean is, if I leave this field in place, the secrets stays in dynamic_warming_secrets object instead of the dynamic_active_secrets object.

@jasmingacic and @povilasv, any idea?

internal/envoy/manager/cache_manager.go

internal/envoy/manager/envoy_callbacks.go

jasmingacic

Other than those minor tweak the code looks alright.

Is this still work in progress @mbana ?

mbana · 2022-09-07T19:21:12Z

Unfortunately I am seeing very weird behaviour. After I apply my API spec on a freshly deployed cluster, I see the secrets appearing as dynamic_warming_secrets (sometimes):

$ kubectl apply -f examples/auth/oauth2/authorization-code-grant
$ kubectl port-forward --namespace default deployment/default 19000:19000
$ curl -s 'http://localhost:19000/config_dump' | jq '.configs | last'
{
  "@type": "type.googleapis.com/envoy.admin.v3.SecretsConfigDump",
  "dynamic_warming_secrets": [
    {
      "name": "hmac-secret-606c6600-1a3e-4bb9-9d30-948526b80a74",
      "version_info": "uninitialized",
      "last_updated": "2022-09-07T19:07:52Z",
      "secret": {
        "@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret",
        "name": "hmac-secret-606c6600-1a3e-4bb9-9d30-948526b80a74"
      }
    },
    {
      "name": "token-secret-2ae05632-00c3-4bea-bc4f-4eabbe4443ae",
      "version_info": "uninitialized",
      "last_updated": "2022-09-07T19:07:52Z",
      "secret": {
        "@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret",
        "name": "token-secret-2ae05632-00c3-4bea-bc4f-4eabbe4443ae"
      }
    }
  ]
}

After I restart the Envoy instance, I see them appearing in a different state, i.e., in dynamic_active_secrets and usable because I've gone through OAuth2 flow and it works:

$ kubectl port-forward --namespace default deployment/default 19000:19000 &
$ curl -X POST 'http://localhost:19000/quitquitquit' 
$ kubectl port-forward --namespace default deployment/default 19000:19000 &
$ curl -s 'http://localhost:19000/config_dump' | jq '.configs | last'
{
  "@type": "type.googleapis.com/envoy.admin.v3.SecretsConfigDump",
  "dynamic_active_secrets": [
    {
      "name": "token-secret-1741f35b-afdf-457b-905d-38561415e871",
      "version_info": "d99e6e0e-2ee1-11ed-9c03-0242ac110004",
      "last_updated": "2022-09-07T19:19:15.176Z",
      "secret": {
        "@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret",
        "name": "token-secret-1741f35b-afdf-457b-905d-38561415e871",
        "generic_secret": {
          "secret": {
            "inline_string": "[redacted]"
          }
        }
      }
    },
    {
      "name": "hmac-secret-f4506605-759d-4cb8-a23d-4bd5b4086f86",
      "version_info": "d99e6e0e-2ee1-11ed-9c03-0242ac110004",
      "last_updated": "2022-09-07T19:19:15.176Z",
      "secret": {
        "@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret",
        "name": "hmac-secret-f4506605-759d-4cb8-a23d-4bd5b4086f86",
        "generic_secret": {
          "secret": {
            "inline_bytes": "W3JlZGFjdGVkXQ=="
          }
        }
      }
    }
  ]
}

Due to all of this I've intentionally skipped the TestSecretsAreDynamicActiveSecrets test. In the future, if this all works as intended, we can re-enable and complete writing the entire test for dynamic_active_secrets.

@jasmingacic and @aabedraba, I don't know what a user would prefer, editing the ConfigMap that used to contain the static secrets, as documented on https://docs.kusk.io/guides/authentication/oauth2#4-update-envoyfleet-configmap, or issuing a curl -X POST 'http://localhost:19000/quitquitquit'. Personally I think restarting Envoy is far easier.

aabedraba · 2022-09-07T21:05:39Z

Personally I think restarting Envoy is far easier.

If that's our only option to avoid manual configMap update, let's go for it.

jasmingacic · 2022-09-07T21:06:51Z

Personally I think restarting Envoy is far easier.

I agree. Let's do it

jasmingacic · 2022-10-04T14:22:44Z

@mbana what's the status on this?

It is becoming stale we might have troubles merging it soon

@type

Support defining the secrets dynamically, i.e., they should appear as `dynamic_active_secrets` in the admin `/config_dump` page: ```sh $ kubectl port-forward --namespace default deployment/default 19000:19000 & $ curl -s 'http://localhost:19000/stats' | grep sds sds.client_secret_test_1.version_text: "690" sds.hmac_secret_test_1.version_text: "690" cluster.kubeshop-kusk-gateway-oauth2.eu.auth0.com.client_ssl_socket_factory.ssl_context_update_by_sds: 0 sds.client_secret_test_1.init_fetch_timeout: 0 sds.client_secret_test_1.key_rotation_failed: 0 sds.client_secret_test_1.update_attempt: 2 sds.client_secret_test_1.update_failure: 0 sds.client_secret_test_1.update_rejected: 0 sds.client_secret_test_1.update_success: 1 sds.client_secret_test_1.update_time: 1662408207693 sds.client_secret_test_1.version: 18219575566587264222 sds.hmac_secret_test_1.init_fetch_timeout: 0 sds.hmac_secret_test_1.key_rotation_failed: 0 sds.hmac_secret_test_1.update_attempt: 2 sds.hmac_secret_test_1.update_failure: 0 sds.hmac_secret_test_1.update_rejected: 0 sds.hmac_secret_test_1.update_success: 1 sds.hmac_secret_test_1.update_time: 1662408207693 sds.hmac_secret_test_1.version: 18219575566587264222 sds.client_secret_test_1.update_duration: P0(nan,0) P25(nan,0) P50(nan,0) P75(nan,0) P90(nan,0) P95(nan,0) P99(nan,0) P99.5(nan,0) P99.9(nan,0) P100(nan,0) sds.hmac_secret_test_1.update_duration: P0(nan,0) P25(nan,0) P50(nan,0) P75(nan,0) P90(nan,0) P95(nan,0) P99(nan,0) P99.5(nan,0) P99.9(nan,0) P100(nan,0) $ curl -s 'http://localhost:19000/config_dump' | jq '.[] | .[-1].dynamic_active_secrets' [ { "name": "client_secret_test_1", "version_info": "690", "last_updated": "2022-09-05T20:03:27.693Z", "secret": { "@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret", "name": "client_secret_test_1", "generic_secret": { "secret": { "inline_string": "[redacted]" } } } }, { "name": "hmac_secret_test_1", "version_info": "690", "last_updated": "2022-09-05T20:03:27.693Z", "secret": { "@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.Secret", "name": "hmac_secret_test_1", "generic_secret": { "secret": { "inline_bytes": "W3JlZGFjdGVkXQ==" } } } } ] ``` Things to note from the above output are: 1. The secrets are in the `dynamic_active_secrets` object not in a `dynamic_warming_secrets` object, the later of which would mean that the secrets have not been sent to envoy. See <https://www.envoyproxy.io/docs/envoy/v1.23.1/api-v3/admin/v3/config_dump.proto#admin-v3-secretsconfigdump> for more information. As per the documentation `dynamic_active_secrets`: > "The dynamically loaded active secrets. These are secrets that are available to service clusters or listeners." 2. The output from the `/stats` endpoint shows no failures. Signed-off-by: Mohamed Bana <mohamed@bana.io>

mbana · 2022-10-05T10:09:48Z

@mbana what's the status on this?

It is becoming stale we might have troubles merging it soon

I've made a local build with the changes in from envoyproxy/envoy#23356 (comment). It's available at:

ttl.sh/kubeshop/envoy-fix-oauth-sds:24h
docker.io/banaioltd/envoy:fix-oauth-sds

I'll test it out and see if it works.

mbana · 2022-10-28T10:01:39Z

The SDS fix has been merged into main, see envoyproxy/envoy#23356, but not into
envoyproxy/envoy@main...release/v1.24.

I suspect we will need to wait until v1.25.0.

mbana · 2022-10-31T14:32:40Z

Also, consider #743 once Envoy upstream issue is fixed.

mbana self-assigned this Sep 5, 2022

mbana requested review from jasmingacic, kylehodgetts and povilasv as code owners September 5, 2022 21:17

kylehodgetts suggested changes Sep 6, 2022

View reviewed changes

aabedraba linked an issue Sep 6, 2022 that may be closed by this pull request

Allow to automatically add client_secret to ConfigMap in OAuth2 #703

Open

jasmingacic suggested changes Sep 6, 2022

View reviewed changes

mbana force-pushed the mbana-oauth-issue-401-secrets-ads branch 2 times, most recently from feb1c8d to 79ab348 Compare September 7, 2022 19:00

mbana force-pushed the mbana-oauth-issue-401-secrets-ads branch 2 times, most recently from ef1925c to dcea0fa Compare September 7, 2022 19:30

mbana force-pushed the mbana-oauth-issue-401-secrets-ads branch from dcea0fa to db27a80 Compare September 8, 2022 16:19

mbana requested review from kylehodgetts and jasmingacic September 8, 2022 16:20

mbana added the auth label Sep 14, 2022

mbana force-pushed the mbana-oauth-issue-401-secrets-ads branch from db27a80 to 16230a2 Compare October 5, 2022 10:09

mbana marked this pull request as draft October 11, 2022 10:35

mbana mentioned this pull request Oct 12, 2022

Pass correct init manager to the secret manager envoyproxy/envoy#23356

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OAuth2: Dynamic Secrets #697

OAuth2: Dynamic Secrets #697

mbana commented Sep 5, 2022 •

edited

Loading

netlify bot commented Sep 5, 2022 •

edited

Loading

jasmingacic commented Sep 5, 2022

kylehodgetts Sep 6, 2022

mbana Sep 6, 2022

mbana Sep 8, 2022

jasmingacic left a comment

mbana commented Sep 7, 2022 •

edited

Loading

aabedraba commented Sep 7, 2022

jasmingacic commented Sep 7, 2022

jasmingacic commented Oct 4, 2022

mbana commented Oct 5, 2022 •

edited

Loading

mbana commented Oct 28, 2022

mbana commented Oct 31, 2022

OAuth2: Dynamic Secrets #697

Are you sure you want to change the base?

OAuth2: Dynamic Secrets #697

Conversation

mbana commented Sep 5, 2022 • edited Loading

Summary

TODO

netlify bot commented Sep 5, 2022 • edited Loading

✅ Deploy Preview for kusk-docs-preview ready!

jasmingacic commented Sep 5, 2022

kylehodgetts Sep 6, 2022

Choose a reason for hiding this comment

mbana Sep 6, 2022

Choose a reason for hiding this comment

mbana Sep 8, 2022

Choose a reason for hiding this comment

jasmingacic left a comment

Choose a reason for hiding this comment

mbana commented Sep 7, 2022 • edited Loading

aabedraba commented Sep 7, 2022

jasmingacic commented Sep 7, 2022

jasmingacic commented Oct 4, 2022

mbana commented Oct 5, 2022 • edited Loading

mbana commented Oct 28, 2022

mbana commented Oct 31, 2022

mbana commented Sep 5, 2022 •

edited

Loading

netlify bot commented Sep 5, 2022 •

edited

Loading

mbana commented Sep 7, 2022 •

edited

Loading

mbana commented Oct 5, 2022 •

edited

Loading