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

ERR unknown command 'CONFIG' when using Secured Redis #124

Closed
danveloper opened this Issue Jan 25, 2015 · 21 comments

Comments

Projects
None yet
7 participants
@danveloper
Contributor

danveloper commented Jan 25, 2015

Redis security recommends disabling the CONFIG command so that remote users cannot reconfigure an instance. The RedisHttpSessionConfiguration requires access to this during its initialization. Hosted Redis services, like AWS ElastiCache disable this command by default, with no option to re-enable it.

@rwinch rwinch added the bug label Jan 26, 2015

@rwinch rwinch added this to the 1.0.1 milestone Jan 26, 2015

@rwinch

This comment has been minimized.

Member

rwinch commented Jan 26, 2015

Thanks for the report @danveloper! This indeed seems to be a bug with the RedisHttpSessionConfiguration and thus the @EnableRedisHttpSession annotation.

UPDATE Fixing in 1.0.1

As of Spring Session 1.0.1 this can be disabled by exposing ConfigureRedisAction.NO_OP as a bean.

An XML Configuration example

<util:constant
        static-field="org.springframework.session.data.redis.config.ConfigureRedisAction.NO_OP"/>

A Java Configuration example

@Bean
public static ConfigureRedisAction configureRedisAction() {
    return ConfigureRedisAction.NO_OP;
}

Fixing the Issue

I'm debating what the best approach to fixing this would be though and wondering what your thoughts were @danveloper.

There is certainly a need for a fix, so I'm not debating that we need to fix something. However, I like the fact that it updates the Redis configuration by default for two reasons:

  • It makes it very easy to get things up and working in a development environment
  • Making it enabled by default means that users would have to explicitly disable for production. Since there is an explicit step to disable the configuration, they should be aware that it is necessary to configure Redis to send the namespace notifications. This is critical for applications that require SessionDestroyedEvent to be fired to clean up resources. In particular, this is important for WebSocket applications to ensure open WebSockets are closed when the HttpSession expires.

My initial thoughts on how we should update the configuration is:

  • RedisHttpSessionConfiguration should by default update the Redis configuration only if Spring WebSocket support is enabled.
  • RedisHttpSessionConfiguration should allow disabling updating the Redis configuration
  • RedisHttpSessionConfiguration should by default try to subscribe to keyspace notifications only if Spring WebSocket support is enabled. This will help increase performance for applications simply using Spring Session for HttpSession which typically does not need to receive the SessionDestroyedEvent
  • RedisHttpSessionConfiguration should allow explicitly configuring if the application should subscribe to keyspace notifications
  • We should update the documentation to discuss the changes

Workaround

In the meantime, a workaround is to remove @EnableRedisHttpSession from your configuration and then include a configuration with a fix. For example:

import java.util.Arrays;
import java.util.List;
import java.util.Map;

import org.springframework.beans.factory.BeanClassLoaderAware;
import org.springframework.beans.factory.InitializingBean;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.ImportAware;
import org.springframework.core.annotation.AnnotationAttributes;
import org.springframework.core.annotation.AnnotationUtils;
import org.springframework.core.type.AnnotationMetadata;
import org.springframework.data.redis.connection.RedisConnection;
import org.springframework.data.redis.connection.RedisConnectionFactory;
import org.springframework.data.redis.core.RedisTemplate;
import org.springframework.data.redis.listener.PatternTopic;
import org.springframework.data.redis.listener.RedisMessageListenerContainer;
import org.springframework.data.redis.serializer.StringRedisSerializer;
import org.springframework.scheduling.annotation.EnableScheduling;
import org.springframework.session.ExpiringSession;
import org.springframework.session.SessionRepository;
import org.springframework.session.data.redis.RedisOperationsSessionRepository;
import org.springframework.session.data.redis.SessionMessageListener;
import org.springframework.session.web.http.HttpSessionStrategy;
import org.springframework.session.web.http.SessionRepositoryFilter;
import org.springframework.util.ClassUtils;

@Configuration
@EnableScheduling
public class RedisHttpSessionConfiguration {
    @Value("${spring.session.maxInactive ?: 1800}")
    private Integer maxInactiveIntervalInSeconds;

    private HttpSessionStrategy httpSessionStrategy;

    @Autowired
    private ApplicationEventPublisher eventPublisher;

    @Bean
    public RedisMessageListenerContainer redisMessageListenerContainer(
            RedisConnectionFactory connectionFactory) {
        RedisMessageListenerContainer container = new RedisMessageListenerContainer();
        container.setConnectionFactory(connectionFactory);
        container.addMessageListener(redisSessionMessageListener(),
                Arrays.asList(new PatternTopic("__keyevent@*:del"),new PatternTopic("__keyevent@*:expired")));
        return container;
    }

    @Bean
    public SessionMessageListener redisSessionMessageListener() {
        return new SessionMessageListener(eventPublisher);
    }

    @Bean
    public RedisTemplate<String,ExpiringSession> sessionRedisTemplate(RedisConnectionFactory connectionFactory) {
        RedisTemplate<String, ExpiringSession> template = new RedisTemplate<String, ExpiringSession>();
        template.setKeySerializer(new StringRedisSerializer());
        template.setHashKeySerializer(new StringRedisSerializer());
        template.setConnectionFactory(connectionFactory);
        return template;
    }

    @Bean
    public RedisOperationsSessionRepository sessionRepository(RedisTemplate<String, ExpiringSession> sessionRedisTemplate) {
        RedisOperationsSessionRepository sessionRepository = new RedisOperationsSessionRepository(sessionRedisTemplate);
        sessionRepository.setDefaultMaxInactiveInterval(maxInactiveIntervalInSeconds);
        return sessionRepository;
    }

    @Bean
    public <S extends ExpiringSession> SessionRepositoryFilter<? extends ExpiringSession> springSessionRepositoryFilter(SessionRepository<S> sessionRepository) {
        SessionRepositoryFilter<S> sessionRepositoryFilter = new SessionRepositoryFilter<S>(sessionRepository);
        if(httpSessionStrategy != null) {
            sessionRepositoryFilter.setHttpSessionStrategy(httpSessionStrategy);
        }
        return sessionRepositoryFilter;
    }

    @Autowired(required = false)
    public void setHttpSessionStrategy(HttpSessionStrategy httpSessionStrategy) {
        this.httpSessionStrategy = httpSessionStrategy;
    }
}

If you are not using the SessionDestroyedEvent you can also disable subscribing to the notifications which should improve performance. For example:

import java.util.Arrays;
import java.util.List;
import java.util.Map;

import org.springframework.beans.factory.BeanClassLoaderAware;
import org.springframework.beans.factory.InitializingBean;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.ImportAware;
import org.springframework.core.annotation.AnnotationAttributes;
import org.springframework.core.annotation.AnnotationUtils;
import org.springframework.core.type.AnnotationMetadata;
import org.springframework.data.redis.connection.RedisConnection;
import org.springframework.data.redis.connection.RedisConnectionFactory;
import org.springframework.data.redis.core.RedisTemplate;
import org.springframework.data.redis.listener.PatternTopic;
import org.springframework.data.redis.listener.RedisMessageListenerContainer;
import org.springframework.data.redis.serializer.StringRedisSerializer;
import org.springframework.scheduling.annotation.EnableScheduling;
import org.springframework.session.ExpiringSession;
import org.springframework.session.SessionRepository;
import org.springframework.session.data.redis.RedisOperationsSessionRepository;
import org.springframework.session.data.redis.SessionMessageListener;
import org.springframework.session.web.http.HttpSessionStrategy;
import org.springframework.session.web.http.SessionRepositoryFilter;
import org.springframework.util.ClassUtils;

@Configuration
public class RedisHttpSessionConfiguration {
    @Value("${spring.session.maxInactive ?: 1800}")
    private Integer maxInactiveIntervalInSeconds;

    private HttpSessionStrategy httpSessionStrategy;

    @Bean
    public RedisTemplate<String,ExpiringSession> sessionRedisTemplate(RedisConnectionFactory connectionFactory) {
        RedisTemplate<String, ExpiringSession> template = new RedisTemplate<String, ExpiringSession>();
        template.setKeySerializer(new StringRedisSerializer());
        template.setHashKeySerializer(new StringRedisSerializer());
        template.setConnectionFactory(connectionFactory);
        return template;
    }

    @Bean
    public RedisOperationsSessionRepository sessionRepository(RedisTemplate<String, ExpiringSession> sessionRedisTemplate) {
        RedisOperationsSessionRepository sessionRepository = new RedisOperationsSessionRepository(sessionRedisTemplate);
        sessionRepository.setDefaultMaxInactiveInterval(maxInactiveIntervalInSeconds);
        return sessionRepository;
    }

    @Bean
    public <S extends ExpiringSession> SessionRepositoryFilter<? extends ExpiringSession> springSessionRepositoryFilter(SessionRepository<S> sessionRepository) {
        SessionRepositoryFilter<S> sessionRepositoryFilter = new SessionRepositoryFilter<S>(sessionRepository);
        if(httpSessionStrategy != null) {
            sessionRepositoryFilter.setHttpSessionStrategy(httpSessionStrategy);
        }
        return sessionRepositoryFilter;
    }

    @Autowired(required = false)
    public void setHttpSessionStrategy(HttpSessionStrategy httpSessionStrategy) {
        this.httpSessionStrategy = httpSessionStrategy;
    }
}
@danveloper

This comment has been minimized.

Contributor

danveloper commented Jan 26, 2015

I think it should be enabled by default, but fail gracefully with a warning. This would allow the same configuration to be used between dev and prod, where dev would JustWork(tm) and prod would require some manual intervention (which would be obvious from the warning).

I was able to work around the problem by subclassing the RedisHttpSessionConfiguration with an implementation that disables the keyspace notifications initializer, and bringing it in through normal configuration means:

package org.springframework.session.data.redis.config.annotation.web.http

import org.springframework.beans.factory.annotation.Value
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration
import org.springframework.core.type.AnnotationMetadata
import org.springframework.data.redis.connection.RedisConnectionFactory
import org.springframework.data.redis.core.RedisTemplate
import org.springframework.session.ExpiringSession
import org.springframework.session.data.redis.RedisOperationsSessionRepository

@Configuration
class GateRedisHttpSessionConfiguration extends RedisHttpSessionConfiguration {

  @Value('${session.expiration:1800}')
  int expiration

  public void setImportMetadata(AnnotationMetadata importMetadata) {
  }

  @Bean
  public RedisOperationsSessionRepository sessionRepository(RedisTemplate<String, ExpiringSession> sessionRedisTemplate) {
    RedisOperationsSessionRepository sessionRepository = new RedisOperationsSessionRepository(sessionRedisTemplate);
    sessionRepository.setDefaultMaxInactiveInterval(expiration);
    return sessionRepository;
  }

  @Override
  public RedisHttpSessionConfiguration.EnableRedisKeyspaceNotificationsInitializer enableRedisKeyspaceNotificationsInitializer(RedisConnectionFactory connectionFactory) {
    null
  }
}

For posterity, here are the steps for enabling the keyspace notifications on AWS:

Log into the AWS console and choose the ElastiCache service

image

Choose the Cache Parameter Groups and click Create Parameter Group

image

Give the new group and name and description and click Create

image

With the new parameter group created, select it and click Edit Parameters

image

Page through the parameters until you find notify-keyspace-events and enter "eA" in the Value field and click Save Changes

image

Choose Cache Clusters from the context navigation and create a new Redis cache cluster

When specifying your cluster detail, choose the newly created parameter group

image

@rwinch

This comment has been minimized.

Member

rwinch commented Jan 26, 2015

@danveloper I'm glad you were able to work around the issue. Thank you for your feedback and for the useful directions on enabling keyspace notifications in AWS.

I have concerns with the configuration not failing when it tries to update the Redis configuration.

Developers likely did thorough testing within another environment. For example, they might be integrating with WebSocket support and validate that when an HttpSession is terminated the WebSocket is indeed closing properly.

This means they are confident that everything is working by the time the get to production. Many users may not even notice a warning being logged. Without Redis keyspace notifications being enabled, the applications WebSocket connections will not properly closed when the HttpSession is destroyed.

Obviously developers should read the instructions completely, look for warnings, and perform some smoke tests in production to avoid such issues. However, I fear that by not failing we are setting up users for failure.

A compromise might be to require users to explicitly set a property to allow "fall back". This means users would be aware that they need to do something in production. Of course this is one more piece of configuration users would need. Thoughts?

@danveloper

This comment has been minimized.

Contributor

danveloper commented Jan 26, 2015

Could it provide a different strategy for ascertaining the database's configuration? Maybe in the absence of the CONFIG command, the event notification initializer could run a test to determine if expirations were working? This might introduce some slowness during startup, but might be favorable to total failure.

I'd probably prefer a slower startup over managing feature flags between dev & prod. If this isn't possible or is otherwise infeasible, then feature flag might be the only way to go.

@rwinch

This comment has been minimized.

Member

rwinch commented Jan 26, 2015

Great idea! I think this may be doable, but we will see when I get into the details. Thanks for your feedback!

@njariwala

This comment has been minimized.

njariwala commented Jan 30, 2015

RedisHttpSessionConfiguration.EnableRedisKeyspaceNotificationsInitializer is not visible for outsiders extending RedisHttpSessionConfiguration. The workaround doesn't work. Can you help me with this?

@rwinch

This comment has been minimized.

Member

rwinch commented Feb 3, 2015

@njariwala You would need to place it in the same package as illustrated in the example above.

@chrylis

This comment has been minimized.

Contributor

chrylis commented Feb 4, 2015

If the class is semantically usable for non-core extensions, it seems like a bad practice to trespass on the distribution package, especially since that can fail in interesting ways with particular classloader hierarchies.

@rwinch

This comment has been minimized.

Member

rwinch commented Feb 4, 2015

@chrylis Yes this is considered a workaround until we can get a fix out. The alternative is to use the more verbose workaround I provided.

@andirdju

This comment has been minimized.

andirdju commented Mar 31, 2015

@rwinch , @danveloper ,
I think calling the config commands is a bit like hibernate.hbm2ddl.auto option, its better to make it disabled by default.

what do you think?

@rwinch

This comment has been minimized.

Member

rwinch commented Mar 31, 2015

@andirdju Thanks for your thoughts. I think this is quite a bit different:

  • Hibernate would fail to store any data if the schema is not created. The failure would be obvious to anyone who tested the application
  • If Redis is not configured correctly, the sessions would still be created. The failure is that session timeouts would not be handled properly (causing a security vulnerability) The failure would not be obvious unless someone waited until the session timed out and verified it cleaned up the related resources (i.e. websocket connections)
@rwinch

This comment has been minimized.

Member

rwinch commented Apr 10, 2015

I've been giving this quite a bit of thought and I think the best route is to:

  • Add a flag that disables the automatic configuration
  • Update the error message to include more details about why it isn't working

The reason being:

  • This approach allows secure us to be secure by default (we know session expiration events will be fired)
  • The approach works by default out of the box. If there is a problem, then users are informed and know what steps they need to do.
  • Adding a check to startup time can have a significant impact to startup time. It also means that the Redis cluster must be up at the same time the application is (a disable flag would prevent this)
  • If users want the same configuration between production and dev, they should setup their Redis instance the same. It is not that difficult and a onetime thing to setup the dev instance to broadcast events)
  • We could also allow for the flag to be set with the Spring Environment which means we could disable the check via a System Property.

@danveloper I know you disagreed with this approach, so I'd like to encourage a response from you about this.

Anyone else following this issue, I'd love to hear your feedback as well.

@rwinch rwinch closed this in d2e5005 Apr 15, 2015

rwinch added a commit that referenced this issue Apr 15, 2015

@rwinch rwinch changed the title from Unable to use with secured Redis to ERR unknown command 'CONFIG' when using Secured Redis Apr 15, 2015

@ccit-spence

This comment has been minimized.

ccit-spence commented Jul 21, 2015

@rwinch Is there anything wrong with adding @Profile(value = "production") to the beans so that local Redis testing is still allowed?

@rwinch

This comment has been minimized.

Member

rwinch commented Jul 21, 2015

@ccit-spence I generally do not like to use a different setup for production than development. The problem is that you are testing your production setup throughout development which means you are almost sure to run into "unforeseen" problems. Given Redis is so easy to setup, I would highly recommend using the same setup in development as production.

With all that said, you are obviously free to do as you choose. Using a profile will work, but I wouldn't recommend it for the reasons above.

@ccit-spence

This comment has been minimized.

ccit-spence commented Jul 21, 2015

@rwinch We basically have 2 sets of testing. One: local for making sure nothing obvious is broken, Two: Test ran on CI servers i.e. production. A problem good or bad is from a dev machine you can't access Elasticache. Elasticache is only accessible via EC2 instances.

But yes, I do agree having 2 sets of tests is not something I like to do. In this case since Elasticache is only accessible from EC2 I don't see any other way for a dev to test locally.

@rwinch

This comment has been minimized.

Member

rwinch commented Jul 21, 2015

@ccit-spence Are you just using Redis within Elasticache or do you have a custom implementation of Spring Sessions' SessionRepository? If you are just using Redis within Elasticache, you can easily install Redis locally.

@ccit-spence

This comment has been minimized.

ccit-spence commented Jul 21, 2015

@rwinch Right now it is pretty basic. Just using Spring Session defaults. Main motivation for Elasticache is not having to maintain the cluster/nodes.

@rwinch

This comment has been minimized.

Member

rwinch commented Jul 21, 2015

@ccit-spence So you could run a local Redis instance right? Not saying you have to, but it seems like an option that would make the development and production much more similar. Anyways, the choice is totally up to you :)

@oak-tree

This comment has been minimized.

oak-tree commented Jul 19, 2016

I'm reopen this issue because if facing the same issue on azure with their redis server. Any idea how to over come this as I cannot change some of the config. According to the doc:

https://azure.microsoft.com/en-us/documentation/articles/cache-configure/

Important: Because configuration and management of Azure Redis Cache instances is managed by Microsoft the following commands are disabled. If you try to invoke them you will receive an error message similar to "(error) ERR unknown command".

@oak-tree

This comment has been minimized.

oak-tree commented Jul 19, 2016

Thanks!

zombozo12 added a commit to zombozo12/Redis-Server-Exploit that referenced this issue Jul 10, 2018

delete 'config' command in redis-cli
spring-projects/spring-session#124

Redis security recommends disabling the CONFIG command so that remote users cannot reconfigure an instance. The RedisHttpSessionConfiguration requires access to this during its initialization. Hosted Redis services, like AWS ElastiCache disable this command by default, with no option to re-enable it.

correct me if I'm wrong ;)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment