-
Notifications
You must be signed in to change notification settings - Fork 30
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
With IAM Authentication enabled, Connector does duplicate refresh #771
Labels
priority: p2
Moderately-important priority. Fix may not be included in next release.
type: bug
Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Comments
This will be fixed with #731 most likely. |
enocom
added a commit
that referenced
this issue
May 20, 2024
enocom
added a commit
that referenced
this issue
May 20, 2024
enocom
added a commit
that referenced
this issue
May 20, 2024
enocom
added a commit
that referenced
this issue
May 20, 2024
enocom
added a commit
that referenced
this issue
May 20, 2024
enocom
added a commit
that referenced
this issue
May 20, 2024
When callers (like the Cloud SQL Proxy) warmup the background refresh with EngineVersion, the initial refresh operation starts with IAM Authn disabled. Then when a caller connects with IAM authentication, the existing refresh is invalidated (because it doesn't include the client's OAuth2 token). In effect, two refresh operations are completed when the dialer could have just run one initially. This commit ensures calls to EngineVersion respect the dialer's global IAM authentication setting. If IAM authentication is enabled at the dialer level, then EngineVersion will ensure the refresh operation uses IAM authentication. And only one refresh operation occurs between warmup and first connection. In cases where a dialer is initialized without IAM authentication, but then a call to dial requests IAM authentication, a second refresh is unavoidable. This seems to be an uncommon enough use case that it is an acceptable tradeoff given how IAM authentication is tightly coupled with the client certificate refresh. Separately, when Cloud SQL Proxy invocations start the Proxy with the --token flag in combination with IAM authentication, the underlying token does not have a corresponding refresh token and so cannot be refreshed. As a result, when the double calls occur (as described above), there is a third refresh attempt started because the token has a missing expiration field (there is only AccessToken, no RefreshToken, or Expiry). The dialer sees the missing expiration as an expired client certificate and immediately starts a new refresh. But because the dialer has already consumed two attempts, the rate limiter (2 initial attempts, then 30s/attempt) forces the client to wait 30s before connecting. This commit ensures that situation will not happen by using the correct client correct expiration and not the invalid token expiration. For cases where the Cloud SQL Proxy configures the dialer with the --token flag (and no refresh token), the dialer will always default to using the client certificate's expiration. This means the refresh cycle will fail once the token expires. Fixes #771
enocom
added a commit
that referenced
this issue
May 20, 2024
When callers (like the Cloud SQL Proxy) warmup the background refresh with EngineVersion, the initial refresh operation starts with IAM Authn disabled. Then when a caller connects with IAM authentication, the existing refresh is invalidated (because it doesn't include the client's OAuth2 token). In effect, two refresh operations are completed when the dialer could have just run one initially. This commit ensures calls to EngineVersion respect the dialer's global IAM authentication setting. If IAM authentication is enabled at the dialer level, then EngineVersion will ensure the refresh operation uses IAM authentication. And only one refresh operation occurs between warmup and first connection. In cases where a dialer is initialized without IAM authentication, but then a call to dial requests IAM authentication, a second refresh is unavoidable. This seems to be an uncommon enough use case that it is an acceptable tradeoff given how IAM authentication is tightly coupled with the client certificate refresh. Separately, when Cloud SQL Proxy invocations start the Proxy with the --token flag in combination with IAM authentication, the underlying token does not have a corresponding refresh token and so cannot be refreshed. As a result, when the double calls occur (as described above), there is a third refresh attempt started because the token has a missing expiration field (there is only AccessToken, no RefreshToken, or Expiry). The dialer sees the missing expiration as an expired client certificate and immediately starts a new refresh. But because the dialer has already consumed two attempts, the rate limiter (2 initial attempts, then 30s/attempt) forces the client to wait 30s before connecting. This commit ensures that situation will not happen by using the correct client certificate expiration and not the invalid token expiration. For cases where the Cloud SQL Proxy configures the dialer with the --token flag (and no refresh token), the dialer will always default to using the client certificate's expiration. This means the refresh cycle will fail once the token expires. Fixes #771
enocom
added a commit
that referenced
this issue
May 21, 2024
When callers (like the Cloud SQL Proxy) warmup the background refresh with EngineVersion, the initial refresh operation starts with IAM Authn disabled. Then when a caller connects with IAM authentication, the existing refresh is invalidated (because it doesn't include the client's OAuth2 token). In effect, two refresh operations are completed when the dialer could have just run one initially. This commit ensures calls to EngineVersion respect the dialer's global IAM authentication setting. If IAM authentication is enabled at the dialer level, then EngineVersion will ensure the refresh operation uses IAM authentication. And only one refresh operation occurs between warmup and first connection. In cases where a dialer is initialized without IAM authentication, but then a call to dial requests IAM authentication, a second refresh is unavoidable. This seems to be an uncommon enough use case that it is an acceptable tradeoff given how IAM authentication is tightly coupled with the client certificate refresh. Separately, when Cloud SQL Proxy invocations start the Proxy with the --token flag in combination with IAM authentication, the underlying token does not have a corresponding refresh token and so cannot be refreshed. As a result, when the double calls occur (as described above), there is a third refresh attempt started because the token has a missing expiration field (there is only AccessToken, no RefreshToken, or Expiry). The dialer sees the missing expiration as an expired client certificate and immediately starts a new refresh. But because the dialer has already consumed two attempts, the rate limiter (2 initial attempts, then 30s/attempt) forces the client to wait 30s before connecting. This commit ensures that situation will not happen by using the correct client certificate expiration and not the invalid token expiration. For cases where the Cloud SQL Proxy configures the dialer with the --token flag (and no refresh token), the dialer will always default to using the client certificate's expiration. This means the refresh cycle will fail once the token expires. Fixes #771
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
priority: p2
Moderately-important priority. Fix may not be included in next release.
type: bug
Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Using the Cloud SQL Proxy with debug logging enabled, you can see that the Go Connector makes two refresh operations for one connection attempt:
This is happening because the Proxy calls EngineVersion to warm the cache, but EngineVersion doesn't support a way to specify if IAM Authn is used or not, and so assumes it's not in use (the nil argument).
When the call to connection info occurs, it enables IAM auth, and causes the existing refresh to be canceled.
The text was updated successfully, but these errors were encountered: