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

Issues/430 custom serializers: Allow for custom serialization on all things stored in cache backend to mitigate security and performance problems #431

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
<parent>
<groupId>de.javakaffee.msm</groupId>
<artifactId>memcached-session-manager-project</artifactId>
<version>2.3.3-SNAPSHOT</version>
<version>2.4.0-SNAPSHOT</version>
</parent>

<groupId>de.javakaffee.msm</groupId>
<artifactId>memcached-session-manager</artifactId>
<version>2.3.3-SNAPSHOT</version>
<version>2.4.0-SNAPSHOT</version>
<packaging>jar</packaging>
<name>memcached-session-manager core</name>
<description>The msm core, provides java serialization strategy.</description>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package de.javakaffee.web.msm;

import de.javakaffee.web.msm.MemcachedSessionService.SessionManager;

public class DefaultObjectIOFactory implements ObjectIOFactory {
@Override
public ObjectIOStrategy createObjectIOStrategy(final SessionManager _manager) {
return new DefaultObjectIOStrategy();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package de.javakaffee.web.msm;

import java.io.IOException;
import java.io.InputStream;
import java.io.ObjectInput;
import java.io.ObjectInputStream;
import java.io.ObjectOutput;
import java.io.ObjectOutputStream;
import java.io.OutputStream;

public class DefaultObjectIOStrategy implements ObjectIOStrategy {
@Override
public ObjectInput createObjectInput(final InputStream is) throws IOException {
return new ObjectInputStream(is);
}

@Override
public ObjectOutput createObjectOutput(final OutputStream os) throws IOException {
return new ObjectOutputStream(os);
}
}
123 changes: 85 additions & 38 deletions core/src/main/java/de/javakaffee/web/msm/MemcachedSessionService.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
import static de.javakaffee.web.msm.Statistics.StatsType.SESSION_DESERIALIZATION;

import java.io.IOException;
import java.io.ObjectInput;
import java.io.ObjectInputStream;
import java.io.ObjectOutput;
import java.io.ObjectOutputStream;
import java.security.Principal;
import java.util.List;
Expand Down Expand Up @@ -162,6 +164,21 @@ static enum LockStatus {
private String _transcoderFactoryClassName = JavaSerializationTranscoderFactory.class.getName();

/**
* The class name of the factory for
* {@link de.javakaffee.web.msm.ObjectIOFactory}s. Default class name is
* {@link de.javakaffee.web.msm.DefaultObjectIOFactory}.
*/
private String _objectIOFactoryClassName = DefaultObjectIOFactory.class.getName();

public String getObjectIOFactoryClassName() {
return _objectIOFactoryClassName;
}

public void setObjectIOFactoryClassName(final String objectIOFactoryClassName) {
this._objectIOFactoryClassName = objectIOFactoryClassName;
}

/**
* Specifies, if iterating over collection elements shall be done on a copy
* of the collection or on the collection itself.
* <p>
Expand Down Expand Up @@ -222,6 +239,7 @@ static enum LockStatus {
protected TranscoderService _transcoderService;

private TranscoderFactory _transcoderFactory;
private ObjectIOFactory _objectIOFactory;

private BackupSessionService _backupSessionService;

Expand Down Expand Up @@ -295,8 +313,10 @@ public static interface SessionManager extends Manager {
String getString(final String key, final Object... args);

boolean isMaxInactiveIntervalSet();
int getMaxInactiveInterval();
void setMaxInactiveInterval(int interval);
@Override
int getMaxInactiveInterval();
@Override
void setMaxInactiveInterval(int interval);

int getMaxActiveSessions();
void incrementSessionCounter();
Expand Down Expand Up @@ -339,7 +359,7 @@ public static interface SessionManager extends Manager {
* @param oos the output stream
* @throws IOException expected to be declared by the implementation.
*/
void writePrincipal( @Nonnull Principal principal, @Nonnull ObjectOutputStream oos) throws IOException;
void writePrincipal( @Nonnull Principal principal, @Nonnull ObjectOutput oos) throws IOException;

/**
* Reads the Principal from the given OIS.
Expand All @@ -349,7 +369,7 @@ public static interface SessionManager extends Manager {
* @throws IOException expected to be declared by the implementation.
*/
@Nonnull
Principal readPrincipal( @Nonnull ObjectInputStream ois ) throws ClassNotFoundException, IOException;
Principal readPrincipal( @Nonnull ObjectInput ois ) throws ClassNotFoundException, IOException;

/**
* Determines if the context has a security contraint with form based login.
Expand Down Expand Up @@ -417,11 +437,12 @@ public void shutdown() {
* @param storage the storage client to use, for normal operations this should be <code>null</code>.
*/
void startInternal( final StorageClient storage ) throws LifecycleException {
if (storage == null)
_storage = null;
else
_storage = storage;

if (storage == null) {
_storage = null;
} else {
_storage = storage;
}

startInternal();
}

Expand Down Expand Up @@ -496,7 +517,7 @@ protected MemcachedNodesManager createMemcachedNodesManager(final String memcach
}

private TranscoderService createTranscoderService( final Statistics statistics ) {
return new TranscoderService( getTranscoderFactory().createTranscoder( _manager ) );
return new TranscoderService( getTranscoderFactory().createTranscoder( _manager ), getObjectIOFactory().createObjectIOStrategy( _manager ) );
}

protected TranscoderFactory getTranscoderFactory() {
Expand All @@ -510,6 +531,17 @@ protected TranscoderFactory getTranscoderFactory() {
return _transcoderFactory;
}

protected ObjectIOFactory getObjectIOFactory() {
if ( _objectIOFactory == null ) {
try {
_objectIOFactory = createObjectIOFactory();
} catch ( final Exception e ) {
throw new RuntimeException( "Could not create transcoder factory.", e );
}
}
return _objectIOFactory;
}

protected StorageClient createStorageClient(final MemcachedNodesManager memcachedNodesManager,
final Statistics statistics ) {
if ( ! _enabled.get() ) {
Expand All @@ -523,7 +555,7 @@ protected StorageClient createStorageClient(final MemcachedNodesManager memcache

private TranscoderFactory createTranscoderFactory() throws InstantiationException, IllegalAccessException, ClassNotFoundException {
_log.info( "Creating transcoder factory " + _transcoderFactoryClassName );
final Class<? extends TranscoderFactory> transcoderFactoryClass = loadTranscoderFactoryClass();
final Class<? extends TranscoderFactory> transcoderFactoryClass = loadFactoryClass(_transcoderFactoryClassName, _manager.getContainerClassLoader(), TranscoderFactory.class);
final TranscoderFactory transcoderFactory = transcoderFactoryClass.newInstance();
transcoderFactory.setCopyCollectionsForSerialization( _copyCollectionsForSerialization );
if ( _customConverterClassNames != null ) {
Expand All @@ -533,17 +565,24 @@ private TranscoderFactory createTranscoderFactory() throws InstantiationExceptio
return transcoderFactory;
}

private Class<? extends TranscoderFactory> loadTranscoderFactoryClass() throws ClassNotFoundException {
Class<? extends TranscoderFactory> transcoderFactoryClass;
final ClassLoader classLoader = _manager.getContainerClassLoader();
private ObjectIOFactory createObjectIOFactory() throws InstantiationException, IllegalAccessException, ClassNotFoundException {
_log.info( "Creating objectIO factory " + _objectIOFactoryClassName );
final Class<? extends ObjectIOFactory> objectIOFactoryClass = loadFactoryClass(_objectIOFactoryClassName, _manager.getContainerClassLoader(), ObjectIOFactory.class);
final ObjectIOFactory objectIOFactory = objectIOFactoryClass.newInstance();
return objectIOFactory;
}


private <T> Class<? extends T> loadFactoryClass(final String className, final ClassLoader classLoader, final Class<T> factoryInterface) throws ClassNotFoundException {
Class<? extends T> factoryClass;
try {
_log.debug( "Loading transcoder factory class " + _transcoderFactoryClassName + " using classloader " + classLoader );
transcoderFactoryClass = Class.forName( _transcoderFactoryClassName, false, classLoader ).asSubclass( TranscoderFactory.class );
_log.debug( "Loading transcoder factory class " + className + " using classloader " + classLoader );
factoryClass = Class.forName( className, false, classLoader ).asSubclass(factoryInterface);
} catch ( final ClassNotFoundException e ) {
_log.info( "Could not load transcoderfactory class with classloader "+ classLoader +", trying " + getClass().getClassLoader() );
transcoderFactoryClass = Class.forName( _transcoderFactoryClassName, false, getClass().getClassLoader() ).asSubclass( TranscoderFactory.class );
factoryClass = Class.forName( className, false, getClass().getClassLoader() ).asSubclass( factoryInterface );
}
return transcoderFactoryClass;
return factoryClass;
}

/**
Expand Down Expand Up @@ -723,7 +762,7 @@ public MemcachedBackupSession createSession( String sessionId ) {
if ( _log.isDebugEnabled() ) {
_log.debug( "Remove session id " + session.getId() + " from _invalidSessionsCache, marking new session valid" );
}
_invalidSessionsCache.remove(session.getId());
_invalidSessionsCache.remove(session.getId());
}
return session;

Expand Down Expand Up @@ -970,15 +1009,17 @@ private MemcachedBackupSession loadBackupSession(final String requestedSessionId
try {
final SessionValidityInfo validityInfo = _lockingStrategy.loadBackupSessionValidityInfo( requestedSessionId );
if ( validityInfo == null || !validityInfo.isValid() ) {
if(_log.isDebugEnabled())
_log.debug( "No validity info (or no valid one) found for sessionId " + requestedSessionId );
if(_log.isDebugEnabled()) {
_log.debug( "No validity info (or no valid one) found for sessionId " + requestedSessionId );
}
return null;
}

final byte[] obj = _storage.get( getSessionIdFormat().createBackupKey( requestedSessionId ) );
if ( obj == null ) {
if(_log.isDebugEnabled())
_log.debug( "No backup found for sessionId " + requestedSessionId );
if(_log.isDebugEnabled()) {
_log.debug( "No backup found for sessionId " + requestedSessionId );
}
return null;
}

Expand Down Expand Up @@ -1008,14 +1049,16 @@ public void requestFinished(final String sessionId, final String requestId) {
if(!_sticky) {
final MemcachedBackupSession msmSession = _manager.getSessionInternal( sessionId );
if ( msmSession == null ) {
if(_log.isDebugEnabled())
_log.debug( "No session found in session map for " + sessionId );
if(_log.isDebugEnabled()) {
_log.debug( "No session found in session map for " + sessionId );
}
return;
}

if ( !msmSession.isValidInternal() ) {
if(_log.isDebugEnabled())
_log.debug( "Non valid session found in session map for " + sessionId );
if(_log.isDebugEnabled()) {
_log.debug( "Non valid session found in session map for " + sessionId );
}
return;
}

Expand All @@ -1024,8 +1067,9 @@ public void requestFinished(final String sessionId, final String requestId) {
// we must not remove it as this would case session data loss
// for the other request
if ( msmSession.releaseReference() > 0 ) {
if(_log.isDebugEnabled())
_log.debug( "Session " + sessionId + " is still used by another request, skipping backup and (optional) lock handling/release." );
if(_log.isDebugEnabled()) {
_log.debug( "Session " + sessionId + " is still used by another request, skipping backup and (optional) lock handling/release." );
}
return;
}
msmSession.passivate();
Expand Down Expand Up @@ -1062,8 +1106,9 @@ public Future<BackupResult> backupSession( final String sessionId, final boolean

final MemcachedBackupSession msmSession = _manager.getSessionInternal( sessionId );
if ( msmSession == null ) {
if(_log.isDebugEnabled())
_log.debug( "No session found in session map for " + sessionId );
if(_log.isDebugEnabled()) {
_log.debug( "No session found in session map for " + sessionId );
}
if ( !_sticky ) {
// Issue 116/137: Only notify the lockingStrategy if the session was loaded and has not been removed/invalidated
if(!_invalidSessionsCache.containsKey(sessionId)) {
Expand All @@ -1074,8 +1119,9 @@ public Future<BackupResult> backupSession( final String sessionId, final boolean
}

if ( !msmSession.isValidInternal() ) {
if(_log.isDebugEnabled())
_log.debug( "Non valid session found in session map for " + sessionId );
if(_log.isDebugEnabled()) {
_log.debug( "Non valid session found in session map for " + sessionId );
}
return new SimpleFuture<BackupResult>( BackupResult.SKIPPED );
}

Expand All @@ -1085,16 +1131,17 @@ public Future<BackupResult> backupSession( final String sessionId, final boolean
// we must not remove it as this would case session data loss
// for the other request
if ( msmSession.releaseReference() > 0 ) {
if(_log.isDebugEnabled())
_log.debug( "Session " + sessionId + " is still used by another request, skipping backup and (optional) lock handling/release." );
if(_log.isDebugEnabled()) {
_log.debug( "Session " + sessionId + " is still used by another request, skipping backup and (optional) lock handling/release." );
}
return new SimpleFuture<BackupResult>( BackupResult.SKIPPED );
}
msmSession.passivate();
_manager.removeInternal( msmSession, false );
}
}

final boolean force = sessionIdChanged || msmSession.isSessionIdChanged() || !_sticky && (msmSession.getSecondsSinceLastBackup() >= msmSession.getMaxInactiveInterval());
final boolean force = sessionIdChanged || msmSession.isSessionIdChanged() || !_sticky && msmSession.getSecondsSinceLastBackup() >= msmSession.getMaxInactiveInterval();
final Future<BackupResult> result = _backupSessionService.backupSession( msmSession, force );

if ( !_sticky ) {
Expand Down Expand Up @@ -1228,7 +1275,7 @@ public void setMemcachedNodes( final String memcachedNodes ) {
public String getMemcachedNodes() {
return _memcachedNodes;
}

private MemcachedNodesManager reloadMemcachedConfig( final String memcachedNodes, final String failoverNodes ) {

/* first create all dependent services
Expand Down Expand Up @@ -1638,7 +1685,7 @@ protected void updateExpirationInMemcached() {
public void setSessionBackupAsync( final boolean sessionBackupAsync ) {
final boolean oldSessionBackupAsync = _sessionBackupAsync;
_sessionBackupAsync = sessionBackupAsync;
if ( ( oldSessionBackupAsync != sessionBackupAsync ) && _manager.isInitialized() ) {
if ( oldSessionBackupAsync != sessionBackupAsync && _manager.isInitialized() ) {
_log.info( "SessionBackupAsync was changed to " + sessionBackupAsync + ", creating new BackupSessionService with new configuration." );
_backupSessionService = new BackupSessionService( _transcoderService, _sessionBackupAsync, _sessionBackupTimeout,
_backupThreadCount, _storage, _memcachedNodesManager, _statistics );
Expand Down
7 changes: 7 additions & 0 deletions core/src/main/java/de/javakaffee/web/msm/ObjectIOFactory.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package de.javakaffee.web.msm;

import de.javakaffee.web.msm.MemcachedSessionService.SessionManager;

public interface ObjectIOFactory {
ObjectIOStrategy createObjectIOStrategy(SessionManager sessionManager);
}
13 changes: 13 additions & 0 deletions core/src/main/java/de/javakaffee/web/msm/ObjectIOStrategy.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package de.javakaffee.web.msm;

import java.io.IOException;
import java.io.InputStream;
import java.io.ObjectInput;
import java.io.ObjectOutput;
import java.io.OutputStream;

public interface ObjectIOStrategy {
ObjectInput createObjectInput( InputStream is ) throws IOException;

ObjectOutput createObjectOutput( OutputStream os ) throws IOException;
}
Loading