You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "gwynlionhart (via GitHub)" <gi...@apache.org> on 2024/05/17 19:27:34 UTC
[I] CredentialProviderToken in accumulo-client.properties stores the serialized token which contains the password, defeating the purpose of the CredentialProviderToken [accumulo]
gwynlionhart opened a new issue, #4573:
URL: https://github.com/apache/accumulo/issues/4573
**Describe the bug**
Upgrading from 1.10.2 to 2.1.2. The accumulo-client.properties requires the auth.token to be a base64-encoded serialization of the Token class. I created a token for the CredentialProviderToken class using the create-token (which doesn't support CredentialProviderToken correctly in 2.1 btw, I'll elaborate on that later).
If someone has read access to that properties file, they can get the token and paste it into an online base64 decoder (that supports the ISO-8859-1 character set) in order to get the password. For CredentialProviderToken that defeats the purpose because it's supposed to be keeping the password "safe" (yes I know, not ideally safe) by keeping it inside the JCEKS. Right now I believe it only works because it's deserializing the token, kind of bypassing the point of the CredentialProviderToken.
In fact, I just tested by commenting out the "general.security.credential.provider.paths" and it initialized just fine by deserializing the CredentialProviderToken, so I it doesn't look like it's treating CredentialProviderToken any differently than a Password token? I think the fix would be that when auth.type=CredentialProviderToken then the auth.token value should be the JCEKS alias name and then the token is initialized by calling the `public CredentialProviderToken(String name, String credentialProviders)`
Or if it must be a serialized instance of the token, then CredentialProviderToken should override the serialization method so that it doesn't return the password but rather the alias name ("name" field of the class).
Ideally I'd be able to continue using this authentication method, especially because it is still supported on the server side. It was actually kind of hard to find documentation about CredentialProviderToken in the context of accumulo-client.properties. Is it intended to still be supported? Or should I move away from accumulo-client.properties and just use the newClient fluent builder and regular java properties?
As for the create-token shell command: In this section of the source code for CreateToken.java, you can see that it attempts to run the token.init() _within_ the loop on TokenProperties rather than outside the loop. CredentialProviderToken requires two properties, so this ends up throwing an exception because it was init too soon.
```
for (TokenProperty tp : token.getProperties()) {
String input;
if (pass != null && tp.getKey().equals("password")) {
input = pass;
} else {
if (tp.getMask()) {
input = getConsoleReader().readLine(tp.getDescription() + ": ", '*');
} else {
input = getConsoleReader().readLine(tp.getDescription() + ": ");
}
}
props.put(tp.getKey(), input);
token.init(props); //This is inside the for loop, so only one of the two properties of CredentialProviderToken has been looped on
}
//token.init(props) should go here
```
**Versions (OS, Maven, Java, and others, as appropriate):**
- Affected version(s) of this project: 2.1.2
- OS: Testing the create-token on Ubuntu, running Accumulo on centos
- Others:
**To Reproduce**
Steps to reproduce the behavior (or a link to an example repository that reproduces the problem):
1. Try to run `accumulo create-token -tc org.apache.accumulo.core.client.security.tokens.CredentialProviderToken -u user`
2. Prompted for the alias inside the JCEKS. Enter anything (or whatever alias you have in your example JCEKS
3. Press enter and see error
```
Alias to extract from CredentialProvider: instance.password
Thread 'create-token' died.
java.lang.IllegalArgumentException: Expected name and credentialProviders properties.
at org.apache.accumulo.core.client.security.tokens.CredentialProviderToken.init(CredentialProviderToken.java:99)
at org.apache.accumulo.core.util.CreateToken.execute(CreateToken.java:110)
at org.apache.accumulo.start.Main.lambda$execKeyword$0(Main.java:122)
at java.base/java.lang.Thread.run(Thread.java:829)
2024-05-17T19:15:18,991 [start.Main] ERROR: Thread 'create-token' died.
java.lang.IllegalArgumentException: Expected name and credentialProviders properties.
at org.apache.accumulo.core.client.security.tokens.CredentialProviderToken.init(CredentialProviderToken.java:99) ~[accumulo-core-2.1.2.jar:2.1.2]
at org.apache.accumulo.core.util.CreateToken.execute(CreateToken.java:110) ~[accumulo-core-2.1.2.jar:2.1.2]
at org.apache.accumulo.start.Main.lambda$execKeyword$0(Main.java:122) ~[accumulo-start-2.1.2.jar:2.1.2]
at java.lang.Thread.run(Thread.java:829) [?:?]
```
**Expected behavior**
A clear and concise description of what you expected to happen.
**Screenshots**
If applicable, add screenshots to help explain your problem.
**Additional context**
Add any other context about the problem here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] CredentialProviderToken in accumulo-client.properties stores the serialized token which contains the password, exposing the password out of the JCEKS file [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on issue #4573:
URL: https://github.com/apache/accumulo/issues/4573#issuecomment-2247762511
@gwynlionhart - just curious, did you happen to test the code in the PR, or just review it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] CredentialProviderToken in accumulo-client.properties stores the serialized token which contains the password, exposing the password out of the JCEKS file [accumulo]
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on issue #4573:
URL: https://github.com/apache/accumulo/issues/4573#issuecomment-2247769325
> @gwynlionhart - just curious, did you happen to test the code in the PR, or just review it?
@dlmarion They actually provided test code to verify it works as expected. I adapted that code into a unit test which I've now added to my PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] CredentialProviderToken in accumulo-client.properties stores the serialized token which contains the password, exposing the password out of the JCEKS file [accumulo]
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on issue #4573:
URL: https://github.com/apache/accumulo/issues/4573#issuecomment-2121331080
It looks like this might have been broken by #508 in an attempt to remove some special case handling of the CredentialProviderToken with dedicated client properties, and replace it with the generic handling with AuthenticationToken serialization. The problem with that is that the CredentialProviderToken extends PasswordToken, and was not intended to be serialized, but instead was taking advantage of the PasswordToken's methods for retrieving the secret after retrieval of the secret from the provider during the CredentialProviderToken construction. With serialization, it does store the password, because that's how PasswordToken serialization works.
In the spirit of preserving the intent of #508, to remove the special properties that handle CredentialProviderToken serialization in a special way, I think the best path forward is to fix the CredentialProviderToken to make it safe for serialization.
The other issue, about the init method being called inside the loop looks like an easy fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] CredentialProviderToken in accumulo-client.properties stores the serialized token which contains the password, exposing the password out of the JCEKS file [accumulo]
Posted by "gwynlionhart (via GitHub)" <gi...@apache.org>.
gwynlionhart commented on issue #4573:
URL: https://github.com/apache/accumulo/issues/4573#issuecomment-2127051872
Checked out the PR and it looks good!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] CredentialProviderToken in accumulo-client.properties stores the serialized token which contains the password, exposing the password out of the JCEKS file [accumulo]
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on issue #4573:
URL: https://github.com/apache/accumulo/issues/4573#issuecomment-2121362382
I created a PR to attempt to fix these issues in #4580 . Please take a look and see if it fixes the issues you're seeing. Note: the new serialization is incompatible with any earlier serialization for CredentialProviderToken (that's kind of the point, of course, since the problem is the current serialization, but still, I'm pointing it out to make it clear).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] CredentialProviderToken in accumulo-client.properties stores the serialized token which contains the password, exposing the password out of the JCEKS file [accumulo]
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii closed issue #4573: CredentialProviderToken in accumulo-client.properties stores the serialized token which contains the password, exposing the password out of the JCEKS file
URL: https://github.com/apache/accumulo/issues/4573
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org