-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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
Explicitly disallow default credential file loading in repository-gcs #23992
Comments
Is it possible to catch the SMException and throw in that case a better exception? |
That may be possible. I will experiment. |
Would granting In general, what do you think about forbidding (un-scopped cc @tlrx |
@albertzaharovits I don't think we should add any permissions for this. Catching the security exception and "peeking" at it is one solution we might be able to use in order to give a better error message here, as suggested by @dadoonet. We should at minimum document the limitation (my last solution in the original description). |
@rjernst understood, thanks for rephrasing it, looks more actionable now! |
This has been resurrected in today's FixItFriday. Indeed, documenting the limitation, as proposed by @rjernst is the way forward. Adding security privileges only for the sake of a friendlier run time error is not convincing anyone. I will follow-up with the docs PR. |
Google cloud storage has 2 ways to authenticate: by detection of the cloud environment, or by a plaintext file containing credential information. Support for credential file is being added to the elasticsearch keystore, but the way in which the cloud environment is loaded is problematic for completely disallowing the plaintext credential file. The google code to load the default credentials first looks for an environment variable which points to the file, and then looks in "known locations", before trying to do detection of the cloud environment.
We should do just as we did with s3 and not allow using a plaintext file. One idea is to look for the environment variable and known locations, and error up front. The tricky part about this is it would require adding security permissions just to check if the env var or files exist. Another idea is to leave it as it is currently, which would both fail, due to lack of security permissions to read the env var or the file locations (in the user home directory). At minimum, we should document that this is not supported.
The text was updated successfully, but these errors were encountered: