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.

Sign up for GitHub

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

Jump to bottom

Kube ignores expirationTimestamp when exec returns a client certificate #1675

Open
goenning opened this issue Jan 1, 2025 · 6 comments · May be fixed by #1676
Open

Kube ignores expirationTimestamp when exec returns a client certificate #1675

goenning opened this issue Jan 1, 2025 · 6 comments · May be fixed by #1676
Labels
bug Something isn't working

Comments

@goenning
Copy link
Contributor

goenning commented Jan 1, 2025 •
edited
Loading

Current and expected behavior

gardenlogin returns a client certificate with expirationTimestamp, but kube does not use it, which means it's not able to refresh the certs.

Here's an example of the response:

 kubectl gardenlogin get-client-certificate --name local --namespace garden-local --garden-cluster-identity gardener-local
{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","spec":{"interactive":false},"status":{"expirationTimestamp":"2025-01-01T18:20:20Z","clientCertificateData":"-----BEGIN CERTIFICATE-----\nMIIEMTCCApmgAwIBAgIQVBZZJvTJ964WKGdxRCOsQDANBgkqhkiG9w0BAQsFADAd\nMRswGQYDVQQDExJjYS1jbGllbnQtNThmOGU5YWMwHhcNMjUwMTAxMTgwNDIwWhcN\nMjUwMTAxMTgyMDIwWjA0MRcwFQYDVQQKEw5zeXN0ZW06bWFzdGVyczEZMBcGA1UE\nAxMQa3ViZXJuZXRlcy1hZG1pbjCCAaIwDQYJKoZIhvcNAQEBBQADggGPADCCAYoC\nggGBANubtCbumr4HNqObBsWgtySJJaFJyMBZbFqseIbgjfLYiGjYd2Xht7IwdcDM\nkhhH3UoOtofsUHo7suEyKYhUr1Wvjvw2mq/usukji5ahYdgjoiJY8rBZMHlHp0Tl\nB8pH2WD3J/iDRWtJDEjMnD0uuhqXAAOFrbt6gdBeY9g7SqxQIQuCc1RQdcTIsda6\nvfqn2QWbL2Xm6vRzSg/TRPoHCPWhzv1hLsLsmGjZAzuwV/F6TsZveEyEKOFVfuPE\nZ8YVZYm7fHpZUEhBPzBzuNkh6mVcF39AAF5NX6Qc6btmI7UBXX6LTEEU3rSguLlq\n2dqJsW3wQRLRQ8KZFBGv5kXN3UGjZVWSwxFGtkMU5PT/BPCRptbYTV0+Wpy0xbeo\njgNTSKIQHA09Tj0iFhx6EQIYVnusl0d8/1M9jh1k4CJRRMd1/YEooO8I+uKrEu25\nBhy1dFBxc/4cllwDGcJ/v7hDAveh8sttegs7IFwfsP89l6CE94hMXUz0qAVOz02J\n/As4GQIDAQABo1YwVDAOBgNVHQ8BAf8EBAMCBaAwEwYDVR0lBAwwCgYIKwYBBQUH\nAwIwDAYDVR0TAQH/BAIwADAfBgNVHSMEGDAWgBQPOaAVARxxRj+5fazPH1Qb7y1B\ngTANBgkqhkiG9w0BAQsFAAOCAYEASO9O1P8ItfmkDFtalm0InzaJnD4yL/M4jzdd\nL4zFAmH2hrOZUaO76cscx8f2PWaY5uyZsMJYo194qCvuG8klF1D7HwbccU9eKiaG\nPj229wkke4cJVYZftuLN+2K/pF0N2UmmwFS5RrnWnos3czC0IAzHNqbPZw8kWoVI\nnyYEUeAcww+cG4fhco+xOI+lED6XdgsBbKun4n+Hk2awR06EZAVZXh5XBpx0Wtpq\nJqwpyNAs6qhnYnpYD83OPTR1QbaXdzPzd+vUAND78cDhUfIBTDeunAdrDIZKhNS0\nSpR0OSu5o/+ouMdJ7vlFadKJFRJMz6OD2oalk7e7vRiJYXUy1jOP7KzNMPBYkPfO\nNa9we3Swdtrf783n6nB5IQWyNz0O6YkfKA9V3nGYr1WOfwDyGHGZe0Tl9KhwHvbn\nHTqt3SXcq4cNlfgEe52Sxvapv2VWHBE2hSGhwtXYtKTUxt/5561dz95/bdFYJaST\nMZkq9c7MoI0P0AiSmhzPiUqRJjip\n-----END CERTIFICATE-----\n","clientKeyData":"-----BEGIN RSA PRIVATE KEY-----\nMIIG4wIBAAKCAYEA25u0Ju6avgc2o5sGxaC3JIkloUnIwFlsWqx4huCN8tiIaNh3\nZeG3sjB1wMySGEfdSg62h+xQejuy4TIpiFSvVa+O/Daar+6y6SOLlqFh2COiIljy\nsFkweUenROUHykfZYPcn+INFa0kMSMycPS66GpcAA4Wtu3qB0F5j2DtKrFAhC4Jz\nVFB1xMix1rq9+qfZBZsvZebq9HNKD9NE+gcI9aHO/WEuwuyYaNkDO7BX8XpOxm94\nTIQo4VV+48RnxhVlibt8ellQSEE/MHO42SHqZVwXf0AAXk1fpBzpu2YjtQFdfotM\nQRTetKC4uWrZ2omxbfBBEtFDwpkUEa/mRc3dQaNlVZLDEUa2QxTk9P8E8JGm1thN\nXT5anLTFt6iOA1NIohAcDT1OPSIWHHoRAhhWe6yXR3z/Uz2OHWTgIlFEx3X9gSig\n7wj64qsS7bkGHLV0UHFz/hyWXAMZwn+/uEMC96Hyy216CzsgXB+w/z2XoIT3iExd\nTPSoBU7PTYn8CzgZAgMBAAECggGANaUGT9BPWoYWkcaH0/TbaABcgdatRFdTwZ6J\n3qCx7BV61Omz2N5Xah6uVQdj1KyRYMrRScivK/yzSJFhVd0h63N0ISYRiVJsv8ig\nC46mgQ7LR0qlFfDjh2y6bnjn+0Cr2CTnzav7ZCa26n2Y2NU2PtKN/U5FTxuw5D56\nc1KgfhEg0VfvZjQ1YHmJ/6MkTzt7JwRugpzy3f3fpJf+LHlVdfMtxfVcX989p7cD\nNCxyfxlabtzB7ScHeWvSED9k/h0xealpgtJqA3ZSHP2BQas4AFXxo7/+iNO4plkq\nX9Fug318HxshiA5wUhObfucQZS6jeuUaShnPOd0Rx9URRniZnXrUBOpnQHfstntt\nG0YTq0x63mLdJUCleGitmcYHTiD2f1fe7BVGrgFjj+7i9PluxAUvOA+bhWy7mz9C\nc7jGW3EFZvYO1IeBQ645Lo7qtpQwHwgMtb14r4Yra68yyMipJS4A2VsCgFauixju\nMBxojfo82K68APrYADoFz/eErDORAoHBAO3z4cEIHxzVanNnk6L9+UhnPr/zoNfU\nrC8i2ICKjBfELNx9JdzXvKL1Hi05P0ico8N2PQBr2y2ChW2qGNmWPoZ9J7jk37jE\n04YsARJ3Qb2BYRZKGBEEeAsEPZMK+PONBaF4pQ4MzkJWvPMLYeTKym3mQHKhOkTb\nTs4g7KjYbj6J6+7xXzFozAUiilZcso6VWAUGXsZVQoKSpyH6ZyHT4utOfsgOlaLf\nU3T8XYDQMMyplpQB+x0DyEJCu2FPxj7rlQKBwQDsQ6TTk1l4C3XDTlgbqu1cwuxT\nPKpZnNVCxJ0irrdXdE0agvAlpoYVoyKjkEw55Rx7JmFHuXvnnOtXlo0hWXCMOYSR\ndnn/wSSpjvKFfeGHRRJ0hcW+HgyTmyrsmuDniWwDrAp/pFvJmtq/b9I3ibWbcJuK\neJmn9ROCtex8mkYh8zQABMri3Qg6ZrsUDpLjOywO59U1kaeohmuX5E8jIb7v9KBF\nQYDHMsT1eW7iwukyMPMMorT2FoBPetS14nkoGXUCgcEAlqZVQ3T03bhwUOg+nG7X\nKHPVUonYR00qRh11nMwareFR1baWBh/AGhu+IzufSUWCmPl+3YlrloCQIUITwY5R\nv6v1PiYJTl2u3b92Xk6KnIQRB5+ZZQzDQ5ZHtZS40Nlzuys6tI5+95kav3VxYPzS\n9IlWPDy/O2Dtn6tQqgV8prODYCAI+Ks0n/uiQdMmaQc8VtIVptPL7AcVMXnrWYtg\nm0FzGw3AxwFFQUeVmzwz6R9lagdnCJOgcfL110I+bQ+RAoHAQk7Fvp93F6rXtSWS\nssko7sTCAKZhBN9wNtnpY+d3CzNXxF4FOfvfW8k6Gn+P9Ruo/6MI4FFdReaTYjSd\njUOFw8UMwKpomO/C81dsFyz6E7W99TfqBG54hyCgTmE1R6YYy3sIQ0SEgjNUuy+N\n5wpeDq7u6FDqhunQPPoD0gCWOEcCTBMMQdlYytM0I+/nJ6Dev5wvCWbEkBaRscJg\nt3JsHPoh1O4KT12fS2l2Rmv/eJemTuCHOx2bizLk6dsSt79VAoHADt+qLlyodtT6\nynfKu4Uv2e6pY1vunO421t+o4FtwsAj4ageHSAYJ1vUkPUr+pShXGcSRG5iZamv1\ncqfbdi5Kua7PpAYS7rVPk+t4kIgicBsT28M9IFnzvYlcGfXpf7pc5Qe1gGOVaPG4\nbfOA825xNvi0rl11YzCq4+X/IWSJATmq/Dqsx7W9PpgOhl6MoD5If562zEry5JU7\nU0+kVuRjzuzj7327TMhHYGad8wOT8BA4y6wZ+f2gYFGSMokRVygR\n-----END RSA PRIVATE KEY-----\n"}}

Possible solution

I'd like to contribute but I'm struggling to find the solution for it. The credentials are used to build a rustls::ClientConfig and added as a connector to the tower Stack, which means that if the credentials change, we should probably rebuild the stack?

Is that even possible?

Any suggestions on how to approach this one?

Additional context

No response

Environment

macOS on a Gardener cluster

Configuration and features

No response

Affected crates

kube-client

Would you like to work on fixing this bug?

maybe
@goenning goenning added the bug Something isn't working label Jan 1, 2025
@goenning
Copy link
Contributor Author

goenning commented Jan 1, 2025

related #1088

@goenning
Copy link
Contributor Author

goenning commented Jan 1, 2025

This might be terrible solution, but I'll just mention it anyway :)

Add a valid_until field as Option<DateTime<Utc>> to the kube::Client which in most cases would be set to None, but for an expirable certifcate this would be set, and the owner of the Client would have to check this property before each use, and in case the client is no longer valid, they'd need to recreate the client.

@clux
Copy link
Member

clux commented Jan 2, 2025

Yeah, I don't think recreating the stack inside the client builder is at all practical here. We've already thrown away the Config once converting it to a Client so we don't have that data anymore to be able to do a recreate. Even if we copied that inside, it feels sketchy on many levels; like what happens with all existing connections in other async calls. New mutex boundaries..

Is garden actually setting cert expiry that low that this is a problem? I expected auth refresh stuff like this (which happens in the oob client created specifically in the auth modules via the RefreshableToken) to only affect minor transient stuff, and there's already a mechanism to do regular token refresh there...

@clux
Copy link
Member

clux commented Jan 2, 2025

It might be possible to do something with enforce_revocation_expiration and bubble up an error, but it's awkward with how people effectively run multiple watchers with errors discarded and people can't realistially always catch this (we expect errors to be eventually fixable without restarting the app - particularly watcher Errors).

Add a valid_until field as Option<DateTime<Utc>> to the kube::Client which in most cases would be set to None, but for an expirable certifcate this would be set, and the owner of the Client would have to check this property before each use, and in case the client is no longer valid, they'd need to recreate the client.

Hah. Yeah, a variant of this would work. Not ideal, but people that truly need this (local development against garden / tooling or GUIs with wide support) could grab the property out of Client (or maybe Config before creating the Client) and set a timer to trigger a teardown (if process::exit is ok) / replace (if you arc mutex the client 😬).

..long term though, it would be nice if there were ways to bubble up information like this to connectors, but that's a lot of layers and libraries to go through, particularly when the end result people work with is an amorphous BoxService..

@goenning
Copy link
Contributor Author

goenning commented Jan 2, 2025

Is garden actually setting cert expiry that low that this is a problem? I expected auth refresh stuff like this (which happens in the oob client created specifically in the auth modules via the RefreshableToken) to only affect minor transient stuff, and there's already a mechanism to do regular token refresh there...

It's 15min by default, which in my case is an issue as it's a GUI app which usually has very long sessions. RefreshableToken works fine as it's done via the AuthLayer + Header

Hah. Yeah, a variant of this would work. Not ideal, but people that truly need this (local development against garden / tooling or GUIs with wide support) could grab the property out of Client (or maybe Config before creating the Client) and set a timer to trigger a teardown (if process::exit is ok) / replace (if you arc mutex the client 😬).

I thought about getting it from the Config, but the auth_exec function is not public (neither is Auth), so I can't really fetch it from there, and even if it was possible, we'd be invoking the exec command 3 times: 1 on my side, 1 when creating the Client, and one last time on the AuthLayer (where the credentials are simply discarded).

I have a working version of this solution, I can send a PR and let you decide if this is something you want to add or not. Is that OK?

@clux
Copy link
Member

clux commented Jan 2, 2025

I thought about getting it from the Config, but the auth_exec function is not public (neither is Auth), so I can't really fetch it from there, and even if it was possible, we'd be invoking the exec command 3 times: 1 on my side, 1 when creating the Client, and one last time on the AuthLayer (where the credentials are simply discarded).

Ah, ok. That makes sense.

I can send a PR and let you decide if this is something you want to add or not

Please! Sounds completely fine to expose a timestamp if it can help solve it on the user side (even if it's a bit ugly there).

@goenning goenning linked a pull request Jan 2, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

add valid_until to Client goenning/kube-rs
2 participants
@goenning @clux