Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

security: Using k8s idp instead of providing console-sa #2166

Merged

Conversation

cniackz
Copy link
Contributor

@cniackz cniackz commented Jun 18, 2024

Objective:

To let Kubernetes and the IdP decide whether access is valid or not, and to remove MinIO from the equation, as shown in the documentation:

https://kubernetes.io/docs/reference/access-authn-authz/authentication/#openid-connect-tokens

Advantage:

We no longer use the console-sa token. Instead, we validate the token directly via Kubernetes, which is more secure.

Examples on how to configure k8s idp:

Other related docs for testing and development:

Note:

This change wasn't easy for me, as I had to fully understand how the IdP works with Kubernetes and our Operator. However, once I grasped it, the change became straightforward to review and test with proper understanding. I hope you find it satisfactory, and I am open to feedback!

How to test:

https://github.com/cniackz/public/wiki/How-to-test-2166-from-scratch

@cniackz cniackz force-pushed the use_id_token_for_authentication branch 2 times, most recently from 87c4ae0 to 63b5be2 Compare June 18, 2024 17:44
@cniackz cniackz self-assigned this Jun 18, 2024
Copy link
Contributor

@ramondeklein ramondeklein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Test_getK8sSAToken raises a panic on my machine:

--- FAIL: Test_getK8sSAToken (0.00s)
    --- FAIL: Test_getK8sSAToken/Missing_file,_empty (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x48 pc=0x15a0777]

goroutine 23 [running]:
testing.tRunner.func1.2({0x21dd4e0, 0x3c3e830})
        /usr/local/go/src/testing/testing.go:1631 +0x24a
testing.tRunner.func1()
        /usr/local/go/src/testing/testing.go:1634 +0x377
panic({0x21dd4e0?, 0x3c3e830?})
        /usr/local/go/src/runtime/panic.go:770 +0x132
golang.org/x/oauth2.(*Token).Extra(0x24?, {0x2551a59?, 0x603?})
        /home/ramon/go/pkg/mod/golang.org/x/[email protected]/token.go:102 +0x17
github.com/minio/operator/api.Test_getK8sSAToken.func1(0xc000672820)
        /home/ramon/minio/operator/api/config_test.go:53 +0xa6
testing.tRunner(0xc000672820, 0xc000a04130)
        /usr/local/go/src/testing/testing.go:1689 +0xfb
created by testing.(*T).Run in goroutine 22
        /usr/local/go/src/testing/testing.go:1742 +0x390
FAIL    github.com/minio/operator/api   0.042s
FAIL

api/login.go Outdated Show resolved Hide resolved
api/config.go Outdated Show resolved Hide resolved
@cniackz cniackz force-pushed the use_id_token_for_authentication branch 3 times, most recently from 4d2399c to d55d566 Compare June 18, 2024 19:27
@cniackz cniackz force-pushed the use_id_token_for_authentication branch from d55d566 to 6a8760d Compare June 18, 2024 19:29
@cniackz cniackz changed the title Using k8s idp instead of providing console-sa security: Using k8s idp instead of providing console-sa Jun 18, 2024
Copy link
Contributor

@ramondeklein ramondeklein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm no expert in Kubernetes IDP, but I do have significant experience with OIDC (Open ID Connect). In OIDC, there are three important tokens:

  • The Access token is a token to authorize requests and is a token to provide the client access to a certain service. In most cases the sub claim also specifies who you are. But this is often, because the user identity is required to know if you're allowed to execute the request. It's not actually meant for identification. Access token are often scoped for a specific functionality to reduce the blast radius if the token leaks.
  • The Identity token is a token that can be used for identification. The claims in this token often specify who you are. More detailed user information can typically be obtained via userinfo_endpoint (from the OIDC configuration). Because the identity can often be used to determine if a user is allowed to do something, it's not the purpose of this token.
  • The Refresh token (optional) can be used to obtain a new set of tokens (refresh) without needing to authorize again. This token is only issues if the offline_access scope is specified when requesting the tokens.

There is often confusion about when to use an access token and when to use an identity token. A rule-of-thumb is:

  • Identity token: Who are you?
  • Access token: What are you allowed to do?

A very simple example could be when authorizing to Google. The identity token would include your full-name, e-mail address, profile URL, ... and is not related to any specific service. The access token to allow access to Google Mail would also include your e-mail address, but the token would be scoped to be only valid for e-mail. The token cannot be used to access Google Photos, because that would have its own access-token.

I guess that in this scenario, we need access so the access token is probably the better choice. Maybe @aead can also comment...

I could dive into Kubernetes OIDC or OAuth2 authentication if you like...

@harshavardhana
Copy link
Member

  • The Identity token is a token that can be used for identification. The claims in this token often specify who you are. More detailed user information can typically be obtained via userinfo_endpoint (from the OIDC configuration). Because the identity can often be used to determine if a user is allowed to do something, it's not the purpose of this token.

We want this from K8s to validate if the "authentication" part is covered; as far as I know, we do not do any authz for Operator access where the access token can be used.

@harshavardhana harshavardhana merged commit fd7ede7 into minio:master Jun 19, 2024
31 checks passed
@pjuarezd
Copy link
Member

pjuarezd commented Jun 20, 2024

with this is kubernetes IDP setup is required? if so this is a breaking change that should be documented @cniackz , please add it to the release notes in #2141.

@ravindk89 would be better to have documentation on this available before we publish next operator release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants