upthewaterspout commented on a change in pull request #6844:
URL: https://github.com/apache/geode/pull/6844#discussion_r704654640
##########
File path: geode-apis-compatible-with-redis/README.md
##########
@@ -91,68 +94,139 @@ To confirm that everything shut down correctly, if you
execute a Redis command i
Could not connect to Redis at 127.0.0.1:6379: Connection refused
not connected>
```
-### <a name="redis-commands"></a>Redis Commands
+
+## <a name="security"></a>Security
+
+Security is implemented slightly differently to OSS Redis. Redis stores
password information in plain text in the redis.conf file.
+
+When using Apache Geode, to enable security, a Security Manager needs to be
configured on the server(s). This Security Manager will authenticate `AUTH
<password>` commands and `AUTH <username> <password>` commands. Users can set a
custom `default` username using the `geode-compatible-with-redis-username`
parameter. This username will be used when `AUTH <password>` commands are sent
without a `<username>`.
+
+The following gfsh command will configure a `SimpleSecurityManager`:
Review comment:
Maybe we should demo the `ExampleSecurityManager` instead - which at
least lets you set the password?
We should also mention what permission the user needs to have given to them
- eg DATA:WRITE:REDIS_DATA.
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/connection/AuthExecutor.java
##########
@@ -15,35 +15,55 @@
*/
package org.apache.geode.redis.internal.executor.connection;
-import java.util.Arrays;
+import static
org.apache.geode.redis.internal.RedisConstants.ERROR_AUTH_CALLED_WITHOUT_PASSWORD_CONFIGURED;
+import static
org.apache.geode.redis.internal.RedisConstants.ERROR_INVALID_USERNAME_OR_PASSWORD;
+import static org.apache.geode.redis.internal.netty.Coder.bytesToString;
+
import java.util.List;
+import java.util.Properties;
-import org.apache.geode.redis.internal.RedisConstants;
import org.apache.geode.redis.internal.executor.Executor;
import org.apache.geode.redis.internal.executor.RedisResponse;
import org.apache.geode.redis.internal.netty.Command;
import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
+import org.apache.geode.security.AuthenticationFailedException;
+import org.apache.geode.security.SecurityManager;
public class AuthExecutor implements Executor {
@Override
- public RedisResponse executeCommand(Command command,
- ExecutionHandlerContext context) {
+ public RedisResponse executeCommand(Command command, ExecutionHandlerContext
context) {
List<byte[]> commandElems = command.getProcessedCommand();
- byte[] password = context.getAuthPassword();
- if (password == null) {
- return RedisResponse.error(RedisConstants.ERROR_NO_PASS);
+ SecurityManager securityManager = context.getSecurityManager();
+ Properties props = new Properties();
+ if (commandElems.size() == 2) {
+ if (securityManager == null) {
+ return
RedisResponse.error(ERROR_AUTH_CALLED_WITHOUT_PASSWORD_CONFIGURED);
+ }
+ props.setProperty("security-username", context.getRedisUsername());
+ props.setProperty("security-password",
bytesToString(commandElems.get(1)));
+ } else {
+ if (securityManager == null) {
+ String receivedUsername = new String(commandElems.get(1));
+ if (receivedUsername.equalsIgnoreCase(context.getRedisUsername())) {
+ return RedisResponse.ok();
Review comment:
Yesterday we discussed that we think we ought to deviate for the native
redis behavior here. If there is no security manager, *any* attempt to call
auth should fail. Hopefully that can make the logic in this method a bit
simpler.
##########
File path:
geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
##########
@@ -1317,7 +1317,7 @@ static Class _getAttributeType(String attName) {
"Specifies the address on which the Redis API for Geode is listening.
If set to the empty string or this property is not specified, localhost is
requested from the operating system.");
m.put(REDIS_ENABLED,
"When the default value of false, the Redis API for Geode is not
available. Set to true to enable the Redis API for Geode.");
- m.put(REDIS_PASSWORD,
+ m.put(REDIS_USERNAME,
"Specifies the password that the server uses when a client attempts to
authenticate. The default is none and no authentication will be required.");
Review comment:
I think you need to change the description here as well - it still
refers to a password.
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/connection/AuthExecutor.java
##########
@@ -15,35 +15,55 @@
*/
package org.apache.geode.redis.internal.executor.connection;
-import java.util.Arrays;
+import static
org.apache.geode.redis.internal.RedisConstants.ERROR_AUTH_CALLED_WITHOUT_PASSWORD_CONFIGURED;
+import static
org.apache.geode.redis.internal.RedisConstants.ERROR_INVALID_USERNAME_OR_PASSWORD;
+import static org.apache.geode.redis.internal.netty.Coder.bytesToString;
+
import java.util.List;
+import java.util.Properties;
-import org.apache.geode.redis.internal.RedisConstants;
import org.apache.geode.redis.internal.executor.Executor;
import org.apache.geode.redis.internal.executor.RedisResponse;
import org.apache.geode.redis.internal.netty.Command;
import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
+import org.apache.geode.security.AuthenticationFailedException;
+import org.apache.geode.security.SecurityManager;
public class AuthExecutor implements Executor {
@Override
- public RedisResponse executeCommand(Command command,
- ExecutionHandlerContext context) {
+ public RedisResponse executeCommand(Command command, ExecutionHandlerContext
context) {
List<byte[]> commandElems = command.getProcessedCommand();
- byte[] password = context.getAuthPassword();
- if (password == null) {
- return RedisResponse.error(RedisConstants.ERROR_NO_PASS);
+ SecurityManager securityManager = context.getSecurityManager();
+ Properties props = new Properties();
+ if (commandElems.size() == 2) {
+ if (securityManager == null) {
+ return
RedisResponse.error(ERROR_AUTH_CALLED_WITHOUT_PASSWORD_CONFIGURED);
+ }
+ props.setProperty("security-username", context.getRedisUsername());
+ props.setProperty("security-password",
bytesToString(commandElems.get(1)));
Review comment:
I think you can use the constants from `SecurityManager` for strings
like "security-username"
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
