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

PasswordProvider abstraction could be abused for races #6666

Open
leventov opened this issue Nov 27, 2018 · 6 comments
Open

PasswordProvider abstraction could be abused for races #6666

leventov opened this issue Nov 27, 2018 · 6 comments

Comments

@leventov
Copy link
Member

Some classes that have multiple PasswordProvider fields could inherently be subject to races:

  • SSLClientConfig
  • MySQLConnectorConfig
  • ClientSSLContextBuilder
  • TLSServerConfig
  • BasicHTTPAuthenticator
  • BasicAuthDBConfig
  • AWSCredentialsConfig
@leventov
Copy link
Member Author

AWSCredentialsConfig is confirmed to be a subject for a race.

@gianm
Copy link
Contributor

gianm commented Nov 28, 2018

What's the race that could be abused?

@leventov
Copy link
Member Author

PasswordProvider abstraction allows (and actually is designed for) to be able to return different values over time. So in AWSCredentialsConfig and ConfigDrivenAwsCredentialsConfigProvider, accessKey and secreteKey are managed as separate PasswordProviders => allows the same kind of race as #6665.

@leventov
Copy link
Member Author

leventov commented Nov 28, 2018

I would suggest to replace it with an abstraction like this:

interface AbstractCredentialsProvider {
  List<String> getCredentials();
}

Where the implementation is responsible for making the values returned in the list to be consistent. On the user side, list.get(0) could be interpreted as "accessKey" or "username" or whatever, list.get(1) as "secretKey" or "password", etc.

@himanshug
Copy link
Contributor

#7400 is good use case

I would prefer to remove "Abstract" from interface name and return map instead of list so as not to depend on ordering of elements for their meaning... something like (if that works)

interface CredentialsProvider {
  Map<String, String> getCredentials();
}

@github-actions
Copy link

This issue has been marked as stale due to 280 days of inactivity.
It will be closed in 4 weeks if no further activity occurs. If this issue is still
relevant, please simply write any comment. Even if closed, you can still revive the
issue at any time or discuss it on the dev@druid.apache.org list.
Thank you for your contributions.

@github-actions github-actions bot added the stale label Jul 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants