design: add initial external authorization proposal #2643

jpeach · 2020-06-30T10:01:18Z

This updates #432.
This updates #2459.

Signed-off-by: James Peach jpeach@vmware.com

codecov · 2020-06-30T10:05:11Z

Codecov Report

Merging #2643 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2643   +/-   ##
=======================================
  Coverage   76.88%   76.88%           
=======================================
  Files          72       72           
  Lines        5702     5702           
=======================================
  Hits         4384     4384           
  Misses       1227     1227           
  Partials       91       91

Impacted Files	Coverage Δ
internal/dag/cache.go	`98.09% <0.00%> (-0.96%)`	⬇️
internal/dag/status.go	`97.43% <0.00%> (ø)`
internal/fixture/meta.go	`100.00% <0.00%> (ø)`
cmd/contour/servecontext.go	`93.54% <0.00%> (ø)`
internal/contour/handler.go	`78.94% <0.00%> (ø)`
internal/contour/metrics.go	`65.85% <0.00%> (ø)`
internal/k8s/statusupdater.go	`20.40% <0.00%> (ø)`
internal/featuretests/featuretests.go	`88.50% <0.00%> (ø)`
internal/dag/builder.go	`92.06% <0.00%> (+0.13%)`	⬆️
internal/k8s/objectmeta.go	`100.00% <0.00%> (+6.89%)`	⬆️

stevesloka

How does the actual auth flow work? You mentioned OIDC, but not how we might configure that?
You mentioned not allowing external auth servers, so this proposal requires all auth to exist inside the k8s cluster where Contour is running?

design/external-authorization-design.md

stevesloka · 2020-06-30T20:46:44Z

Another thought I had was adding auth to Conditions somehow, so you could allow operators to tell a child set of Proxies should have specific auth, but then not allow them to configure from there. I think some operators might not want each route to be disabled for example.

jpeach · 2020-06-30T23:17:41Z

Another thought I had was adding auth to Conditions somehow, so you could allow operators to tell a child set of Proxies should have specific auth, but then not allow them to configure from there. I think some operators might not want each route to be disabled for example.

I'm not really following this, can you explain this a bit more?

jpeach · 2020-07-01T00:03:09Z

How does the actual auth flow work?

There's some relevant sequence diagrams in #2459. I can reproduce them here for context.

You mentioned OIDC, but not how we might configure that?

You would create a SupportService CRD that references a service that implements OIDC. That will need to be configured with all the necessary OIDC parameters. There's still a bunch of work needed to explore and document how to do this.

You mentioned not allowing external auth servers, so this proposal requires all auth to exist inside the k8s cluster where Contour is running?

If you used an ExternalName Service to back the SupportService CRD, then you could have an auth server outside the cluster. Not sure if that will need special code to support, or whether we would get it for free from Contour's existing Service support.

jpeach · 2020-07-01T02:59:38Z

xref #1922

design/external-authorization-design.md

jpeach · 2020-07-02T22:03:06Z

FYI, @pims convinced me that being able to toggle the fail-open or fail-closed behavior is needed when migrating from internal application auth to external auth, so I'm planning to add that flag to the API.

jpeach · 2020-07-09T01:06:16Z

@jpeach Note to self ... consider what happens to headers injected by the external auth server on routes that have auth disabled. A client request could still present those headers, causing an application to become confused (a request could accidentally be performed as an authorized identity).

So perhaps we need to add API to ensure that any injected auth headers are scrubbed from the incoming client request. Envoy has the concept of internal headers, which aligns fairly closely, but that (and the API for removing headers) requires listing specific headers (would be more robust to pattern match IMHO). The Envoy concept on internal requests is also a bit hard to understand and it's not obvious that it would be the right choice in all deployments.

youngnick

This is great work, and I think it's really close. Just a few small questions and nits.

design/external-authorization-design.md

youngnick

I think at this point any concerns I have are squarely in 'implementation detail' territory. This LGTM, but I'd like to see @stevesloka's thoughts as well.

stevesloka

Looking good @jpeach, just some more questions on bits that aren't clear to me.

design/external-authorization-design.md

stevesloka · 2020-07-15T14:55:07Z

Also don't want to loose context from a Slack conversation about renaming s/SupportService/ExtensionService: https://kubernetes.slack.com/archives/C8XRH2R4J/p1594707008255100

skriss

still paging in all the context, but added some initial questions & comments.

design/external-authorization-design.md

jpeach · 2020-07-17T05:19:14Z

I'm pretty sure I addressed all the comments, but I'll leave this until next week to give a change for additional feedback. I added a new "Compatibility" section to make it more obvious how auth will interact with existing features.

Also SupportService is now ExtensionService.

youngnick

Just approving again to say that I prefer extensionRef.

skriss

no other comments, LGTM!

stevesloka · 2020-07-20T21:25:57Z

This CRD is relatively generic, so that it can be re-used for Envoy rate limiting and logging services.

If the goal of this CRD is to re-use for other bits other than just auth. How does Contour understand that context? For instance, if we use this for rate limiting and features of the spec aren't valid to be set, how could that be implemented?

design/external-authorization-design.md

jpeach · 2020-07-20T21:31:50Z

This CRD is relatively generic, so that it can be re-used for Envoy rate limiting and logging services.

If the goal of this CRD is to re-use for other bits other than just auth. How does Contour understand that context?

The context can be understood from where the CRD is referenced. In this design, Contour can do the right thing because the CRD is referenced from an authorization policy. In logging and rate limiting, there would be a similar context in the HTTPProxy resource.

For instance, if we use this for rate limiting and features of the spec aren't valid to be set, how could that be implemented?

Hard to answer this hypothetical, but since the extension service CRD is basically scoped to generate an Envoy cluster, that's sufficient for the rate limiting and logging cases that we know about. Maybe there could be conflicting use cases in the future, but I expect we could address them by revving the CRD, adding another CRD, or in some other way.

design/external-authorization-design.md

stevesloka · 2020-07-20T21:43:39Z

The context can be understood from where the CRD is referenced. In this design, Contour can do the right thing because the CRD is referenced from an authorization policy. In logging and rate limiting, there would be a similar context in the HTTPProxy resource.

Ahh ok that could work looking at where it's referenced from. =)

stevesloka

I've made a bunch of comments in the design that have questions and things I'm not sure how they might work, but I'm not going to block any further, but still think they are important.

jpeach · 2020-07-21T01:56:11Z

added ingress to non-goals
moved timeout to ExtensionService CRD
made ExtensionService services field a slice

design/external-authorization-design.md

tsaarni · 2020-07-21T08:32:53Z

design/external-authorization-design.md

+- Most HTTP authorization protocols depend on HTTPS to keep headers and URL query parameters private.
+  Supporting only authorization on HTTPS, Contour reduces the changes of misconfiguring this.
+- HTTPS sessions between Envoy and the authorization server are required.
+  TLS validation can be configured if necessary (should be recommended),


Just a note for future: preferably the Envoy <-> Authorization server connection would be mutually authenticated (related to #2338).

Yeh that should be doable eventually.

design/external-authorization-design.md

This updates projectcontour#432. This updates projectcontour#2459. This updates projectcontour#2325. Signed-off-by: James Peach <jpeach@vmware.com>

glerchundi · 2020-07-22T04:46:00Z

Thinking out loud but as this design doc i ntroduces the concept of ExtensionService, have you considered the idea of using WASM based extension services instead of just those based on gRPC? It’s problably premature to take even into consideration but I would like to put it on the table to see if the new spec definitions can absorb them in the future.

…

On Wed, 22 Jul 2020 at 02:23, James Peach ***@***.***> wrote: Merged #2643 <#2643> into master. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2643 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AARA7FREL5OJQJVWTIFSIYTR4YWQPANCNFSM4OMCA73A> .

jpeach · 2020-07-22T04:56:43Z

Thinking out loud but as this design doc i ntroduces the concept of ExtensionService, have you considered the idea of using WASM based extension services instead of just those based on gRPC? It’s problably premature to take even into consideration but I would like to put it on the table to see if the new spec definitions can absorb them in the future.

In principle the extensionRef field in the HTTPProxy resource could reference something that is not an ExtensionService CRD.

jpeach added the kind/design Categorizes issue or PR as related to design. label Jun 30, 2020

jpeach requested review from stevesloka, skriss, michmike and youngnick June 30, 2020 10:01

stevesloka suggested changes Jun 30, 2020

View reviewed changes

pims reviewed Jul 1, 2020

View reviewed changes

design/external-authorization-design.md Outdated Show resolved Hide resolved

pims reviewed Jul 1, 2020

View reviewed changes

design/external-authorization-design.md Outdated Show resolved Hide resolved

jpeach force-pushed the external-auth-design branch 2 times, most recently from 37697bb to 49dc967 Compare July 1, 2020 03:46

jpeach force-pushed the external-auth-design branch 2 times, most recently from 5f782c3 to 7c613d1 Compare July 7, 2020 02:20

jpeach mentioned this pull request Jul 8, 2020

Epic: Oauth2 integration #2664

Open

youngnick suggested changes Jul 14, 2020

View reviewed changes

jpeach force-pushed the external-auth-design branch from 7c613d1 to 9c62844 Compare July 14, 2020 03:32

youngnick approved these changes Jul 14, 2020

View reviewed changes

stevesloka reviewed Jul 15, 2020

View reviewed changes

skriss reviewed Jul 15, 2020

View reviewed changes

jpeach force-pushed the external-auth-design branch from 9c62844 to 8110819 Compare July 16, 2020 01:28

jpeach mentioned this pull request Jul 17, 2020

Provide support for integrating with an external auth service. #432

Closed

jpeach force-pushed the external-auth-design branch from 8110819 to b641eae Compare July 17, 2020 05:09

youngnick approved these changes Jul 20, 2020

View reviewed changes

jpeach force-pushed the external-auth-design branch from b641eae to 81d02a4 Compare July 20, 2020 21:05

skriss approved these changes Jul 20, 2020

View reviewed changes

stevesloka reviewed Jul 20, 2020

View reviewed changes

design/external-authorization-design.md Outdated Show resolved Hide resolved

stevesloka reviewed Jul 20, 2020

View reviewed changes

design/external-authorization-design.md Show resolved Hide resolved

stevesloka approved these changes Jul 20, 2020

View reviewed changes

jpeach force-pushed the external-auth-design branch from 81d02a4 to 13ee1f7 Compare July 21, 2020 01:53

tsaarni reviewed Jul 21, 2020

View reviewed changes

design: add initial external authorization proposal

3ed4de8

This updates projectcontour#432. This updates projectcontour#2459. This updates projectcontour#2325. Signed-off-by: James Peach <jpeach@vmware.com>

jpeach force-pushed the external-auth-design branch from 13ee1f7 to 3ed4de8 Compare July 22, 2020 00:16

jpeach merged commit e02ca4c into projectcontour:master Jul 22, 2020

jpeach deleted the external-auth-design branch July 22, 2020 00:23

youngnick mentioned this pull request Sep 18, 2020

[feature] Add rate limiting support #370

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

design: add initial external authorization proposal #2643

design: add initial external authorization proposal #2643

jpeach commented Jun 30, 2020

codecov bot commented Jun 30, 2020 •

edited

Loading

stevesloka left a comment

stevesloka commented Jun 30, 2020

jpeach commented Jun 30, 2020

jpeach commented Jul 1, 2020

jpeach commented Jul 1, 2020

jpeach commented Jul 2, 2020

jpeach commented Jul 9, 2020

youngnick left a comment

youngnick left a comment

stevesloka left a comment

stevesloka commented Jul 15, 2020

skriss left a comment

jpeach commented Jul 17, 2020 •

edited

Loading

youngnick left a comment

skriss left a comment

stevesloka commented Jul 20, 2020

jpeach commented Jul 20, 2020

stevesloka commented Jul 20, 2020

stevesloka left a comment •

edited

Loading

jpeach commented Jul 21, 2020

tsaarni Jul 21, 2020

jpeach Jul 21, 2020

glerchundi commented Jul 22, 2020 via email

jpeach commented Jul 22, 2020

design: add initial external authorization proposal #2643

design: add initial external authorization proposal #2643

Conversation

jpeach commented Jun 30, 2020

codecov bot commented Jun 30, 2020 • edited Loading

Codecov Report

stevesloka left a comment

Choose a reason for hiding this comment

stevesloka commented Jun 30, 2020

jpeach commented Jun 30, 2020

jpeach commented Jul 1, 2020

jpeach commented Jul 1, 2020

jpeach commented Jul 2, 2020

jpeach commented Jul 9, 2020

youngnick left a comment

Choose a reason for hiding this comment

youngnick left a comment

Choose a reason for hiding this comment

stevesloka left a comment

Choose a reason for hiding this comment

stevesloka commented Jul 15, 2020

skriss left a comment

Choose a reason for hiding this comment

jpeach commented Jul 17, 2020 • edited Loading

youngnick left a comment

Choose a reason for hiding this comment

skriss left a comment

Choose a reason for hiding this comment

stevesloka commented Jul 20, 2020

jpeach commented Jul 20, 2020

stevesloka commented Jul 20, 2020

stevesloka left a comment • edited Loading

Choose a reason for hiding this comment

jpeach commented Jul 21, 2020

tsaarni Jul 21, 2020

Choose a reason for hiding this comment

jpeach Jul 21, 2020

Choose a reason for hiding this comment

glerchundi commented Jul 22, 2020 via email

jpeach commented Jul 22, 2020

codecov bot commented Jun 30, 2020 •

edited

Loading

jpeach commented Jul 17, 2020 •

edited

Loading

stevesloka left a comment •

edited

Loading