kubelogin icon indicating copy to clipboard operation
kubelogin copied to clipboard

Error when storing large token in keyring on macOS

Open Gerrit-K opened this issue 10 months ago • 15 comments

Describe the issue

kubelogin repeatedly fails with:

error: get-token: could not write the token cache: keyring write kubelogin/tokencache/8f08accac[...]: data passed to Set was too big

To reproduce

Not sure how to reproduce this, as it's probably caused by a large token from our IdP. We do have a lot of groups that are mapped in the token, which significantly increases the token size.

Your environment

  • OS: macOS Sequoia 15.2
  • kubelogin version: v1.32.0
  • kubectl version: v1.30.6
  • OpenID Connect provider: Keycloak

Additional context

I verified that this was not happening in version 1.31.1, so I'm relatively confident this is related to #973. The description of the PR states that

The code prefers the OS keyring, if supported. Falls back to file based cache

However, with every kubectl command, I noticed consecutive failures and wasn't able to obtain a token* to interact with the cluster. There was no fallback to the file-based cache. For a reason I don't understand yet, the new option --no-keyring also doesn't seem to be available (and neither is --force-keyring), so I can't work around this by any other means than downgrading to 1.31.1:

$ kubectl oidc-login --help | grep keyring
$ kubectl oidc-login --version
kubelogin version 1.32.0

*in fact, I did get a token (which I could verify with the verbose logging), but kubelogin wasn't able to persist it

Gerrit-K avatar Jan 20 '25 15:01 Gerrit-K

I can confirm the issue to be related to kubelogin version 1.32.0. Downgrading to 1.31.1 fixes the issue as a workaround.

Not working:

$ kubectl oidc-login --version
kubelogin version 1.32.0

Error message:

error: get-token: could not write the token cache: keyring write kubelogin/tokencache/4b405edc3945a844f98861a1fb987a4297184c85f01d6bf4ebef54fe15b5cb70: data passed to Set was too big

Working version:

kubectl oidc-login --version
kubelogin version 1.31.1

mhanc avatar Jan 20 '25 15:01 mhanc

I'm also seeing this issue since upgrading from 1.31.0 to 1.32.0.

❯ kubelogin version
kubelogin version v1.32.0 (go1.23.5 darwin_arm64)

I was able to workaround it by setting the --token-cache-storage=disk option.

Our IdP is AWS Cognito and token length is around ~1,300 characters.

jamesrwhite avatar Jan 20 '25 16:01 jamesrwhite

Same same :) using keycloak/microsoft entraid

bbrala avatar Jan 21 '25 09:01 bbrala

Same issue. I can confirm that jamesrwhite's workaround works for me - i.e. add the --token-cache-storage=disk option to the options for get-token in my kube config file.

Technical info:

  • kubelogin version: 1.32.0
  • Hardware: Apple M2 Pro
  • OS: Sonoma 14.6.1

So, it looks like possibly an integration issue with the OS keyring on MacOS perhaps?

kelveden avatar Jan 21 '25 11:01 kelveden

In the underlying library (go-keyring) it claims the limit for the keychain is around 3,000 bytes (source) but if you look at how it's actually implemented it's the combination of the service, username and password that you pass to Set() that cannot exceed 4096 bytes (source).

Worth noting this is also once those values have been base64 encoded and shell escaped.

A potential fix could be to calculate the length in the same way before calling Set() in the logic that selects which storage to use or alternatively to catch the ErrSetDataTooBig error here and fallback to using disk storage.

Catching the error and falling back to disk storage seems simpler and less likely to break if the underlying library changes it's implementation in my opinion.

I'd be happy to open a PR for this.

jamesrwhite avatar Jan 21 '25 15:01 jamesrwhite

Hi, I have the same issue on a Windows-WSL machine. I have no keyring installend and the fallback to disk is not happening. --token-cache-storage=auto should automatically fallback to disk, if keyring is not available.

As already mentioned --token-cache-storage=disk fix the problem, but yould force us to update all our kubeconfigs

OS: Win11 kubelogin version: v1.32.0 kubectl version: v1.32.1 Ubuntu: 24.04 on WSL OpenID Connect provider: EntraID

WSL-Version: 2.3.26.0 Kernelversion: 5.15.167.4-1 WSLg-Version: 1.0.65 MSRDC-Version: 1.2.5620 Direct3D-Version: 1.611.1-81528511 DXCore-Version: 10.0.26100.1-240331-1435.ge-release Windows-Version: 10.0.22631.4751

minimax75 avatar Jan 22 '25 15:01 minimax75

v1.32.1 is available with the fix of https://github.com/int128/kubelogin/pull/1257.

I think it is difficult to cover all edge cases in the fallback mode. I tested the keyring on Linux and it causes an error if dbus is not running.

I'd like to change the default mode to --token-cache-storage=disk. Any opinions?

int128 avatar Jan 25 '25 07:01 int128

@int128 thanks for merging and releasing my hotfix, I can confirm I'm no longer getting the same issue with v1.32.1 🥳

jamesrwhite avatar Jan 27 '25 09:01 jamesrwhite

<3

bbrala avatar Jan 27 '25 12:01 bbrala

I'd like to change the default mode to --token-cache-storage=disk. Any opinions?

I had a use case where I was running oidc-login from a container which became problematic with the new keyring changes as it required dbus to be running inside the container causing this error:

error: get-token: could not write the token cache: keyring write kubelogin/tokencache/...: dbus: couldn't determine address of session bus
Unable to connect to the server: getting credentials: exec: executable kubectl failed with exit code 1

Setting the --token-cache-storage=disk option in kubeconfigs or downgrading kubelogin to v1.31.1 solved the issue, so if the old behavior could be the default for kubelogin would be nice, or kubelogin falling back to disk if needed.

anders-elastisys avatar Jan 28 '25 08:01 anders-elastisys

I'd like to change the default mode to --token-cache-storage=disk. Any opinions?

It does make sense to use the OS keyring when available, since it is arguably more secure than just a file somewhere on disk. Having a fallback to disk when all else fails is good enough IMHO. If there are other errors that need to be exempted then those should be addressed separately, just like in #1257.

ergonab avatar Jan 28 '25 08:01 ergonab

I think the fallback mode will cause the complicated problem.

It needs to handle a case if both keyring and disk have the credentials. For example,

  1. It stores the credentials to keyring.
  2. It loads the credentials from keyring.
  3. It possibly stores the credentials to the disk, for example, when the provider returns a large ID token.
  4. It loads the credentials from keyring but it is expired. It always requests re-login.

It is possible to try to load the credentials from keyring and then disk, but I think it brings the other problems:

  • It causes a flaky behavior if the credentials size is approximately 3,000 bytes.
  • Credentials may be written to the disk. For the security, it is nice to enforce the keyring usage to prevent it.

int128 avatar Jan 28 '25 09:01 int128

The decision could also be offloaded to the user. The problem with the original behaviour in 1.32.0 was that you just got an error from the Go library without any indication what went wrong. If the keyring was the default, the user could be given the choice to switch to disk storage, if an error with the keyring occurs. Then, being made aware of the security implications, the user could opt-in to storing it in a less secure place (on disk) by passing the --token-cache-storage=disk flag. But the error message needs to indicate this and explain to the user what to do.

Of course this is suboptimal long-term when the Go library is (hopefully) fixed to support longer tokens, but at least the user knows what happens with his token data and we tried the more secure option first.

ergonab avatar Jan 28 '25 10:01 ergonab

If the token is too large, it maybe makes sense to switch to envelope encryption?

Generate a secret, encrypt the token with it, and store the key in the keyring. The key size is easy to control.


Another idea, that might sound a bit silly but should be possible: Technically speaking only the signature of the token is security relevant. So storing just the signature of the token in the keyring is enough for security.

However when people use access tokens instead of login tokens, there is no requirement for a JWT and therefore one can't just split the signature away.


All in all, I want to emphasise: It's good we have these problems now, and we should solve them without offloading them to users, because offloading security to users is the worst security choice.

SISheogorath avatar Jan 30 '25 17:01 SISheogorath

That's right. I think we will be able to enforce keyring usage by default when the implementation is enough mature.

int128 avatar Jan 31 '25 03:01 int128