Fix OPENDJ-293: InternalClientConnection memory leak when performing passwo...

Matthew Swift

22.51.2011 05d4affb7296e157581880544fd0fb49b9d98325

Fix OPENDJ-293: InternalClientConnection memory leak when performing password modify/state extended operations or SASL binds

No longer register internal connections in the authenticated users table.

 opends/src/server/org/opends/server/api/ClientConnection.java

@@ -23,6 +23,7 @@
 *
 *
 *      Copyright 2006-2009 Sun Microsystems, Inc.
 *      Portions copyright 2011 ForgeRock AS
 */
package org.opends.server.api;

@@ -89,8 +90,10 @@
   */
  private static final DebugTracer TRACER = getTracer();

  // The set of authentication information for this client connection.
  private AuthenticationInfo authenticationInfo;
  /**
   * The set of authentication information for this client connection.
   */
  protected AuthenticationInfo authenticationInfo;

  /**
   * Indicates whether a multistage SASL bind is currently in progress
@@ -185,8 +188,7 @@

  /**
   * Performs any internal cleanup that may be necessary when this
   * client connection is disconnected, or if not on disconnec, then
   * ultimately whenever it is reaped by the garbage collector.  In
   * client connection is disconnected.  In
   * this case, it will be used to ensure that the connection is
   * deregistered with the {@code AuthenticatedUsers} manager, and
   * will then invoke the {@code finalizeClientConnection} method.
@@ -234,39 +236,6 @@
    }

    networkGroup.removeConnection(this);

    try
    {
      finalizeClientConnection();
    }
    catch (Exception e)
    {
      if (debugEnabled())
      {
        TRACER.debugCaught(DebugLogLevel.ERROR, e);
      }
    }
  }



  /**
   * Performs any cleanup work that may be necessary when this client
   * connection is terminated.  By default, no action is taken.
   * <BR><BR>
   * If possible, this method will be invoked when the client
   * connection is disconnected.  If it isn't invoked at that time,
   * then it will be called when the client connection object is
   * finalized by the garbage collector.
   */
 @org.opends.server.types.PublicAPI(
      stability=org.opends.server.types.StabilityLevel.VOLATILE,
      mayInstantiate=false,
      mayExtend=true,
      mayInvoke=false)
  protected void finalizeClientConnection()
  {
    // No implementation is required by default.
  }


@@ -1316,7 +1285,7 @@
   *                 user and should automatically inherit the root
   *                 privilege set.
   */
  private void updatePrivileges(Entry entry, boolean isRoot)
  protected void updatePrivileges(Entry entry, boolean isRoot)
  {
    privileges = getPrivileges(entry, isRoot);
  }
@@ -1592,7 +1561,7 @@
   * @throws  DirectoryException  If a problem occurs while attempting
   *                              to make the determination.
   */
  public Set<Group> getGroups(Operation operation)
  public Set<Group<?>> getGroups(Operation operation)
         throws DirectoryException
  {
    // FIXME -- This probably isn't the most efficient implementation.
@@ -1616,17 +1585,17 @@

    if ((authzDN == null) || authzDN.isNullDN())
    {
      return Collections.<Group>emptySet();
      return Collections.<Group<?>>emptySet();
    }

    Entry userEntry = DirectoryServer.getEntry(authzDN);
    if (userEntry == null)
    {
      return Collections.<Group>emptySet();
      return Collections.<Group<?>>emptySet();
    }

    HashSet<Group> groupSet = new HashSet<Group>();
    for (Group g :
    HashSet<Group<?>> groupSet = new HashSet<Group<?>>();
    for (Group<?> g :
         DirectoryServer.getGroupManager().getGroupInstances())
    {
      if (g.isMember(userEntry))

 opends/src/server/org/opends/server/core/AuthenticatedUsers.java

@@ -167,7 +167,7 @@
   * @return  The set of client connections authenticated as the specified user,
   *          or {@code null} if there are none.
   */
  synchronized CopyOnWriteArraySet<ClientConnection> get(DN userDN)
  public CopyOnWriteArraySet<ClientConnection> get(DN userDN)
  {
    lock.readLock().lock();
    try

 opends/src/server/org/opends/server/protocols/internal/InternalClientConnection.java

@@ -23,6 +23,7 @@
 *
 *
 *      Copyright 2006-2009 Sun Microsystems, Inc.
 *      Portions copyright 2011 ForgeRock AS
 */
package org.opends.server.protocols.internal;

@@ -99,9 +100,6 @@
  // The static connection for root-based connections.
  private static InternalClientConnection rootConnection;

  // The authentication info for this connection.
  private AuthenticationInfo authenticationInfo;

  // The empty operation list for this connection.
  private LinkedList<Operation> operationList;

@@ -236,11 +234,23 @@
  {
    super();


    this.setNetworkGroup(NetworkGroup.getInternalNetworkGroup());

    this.authenticationInfo = authInfo;
    super.setAuthenticationInfo(authInfo);
    // Don't call super.setAuthenticationInfo() since this will register this
    // connection in the authenticated users table, which is unnecessary and
    // will also cause the connection to be leaked since internal connections
    // are never closed/disconnected.
    if (authInfo == null)
    {
      this.authenticationInfo = new AuthenticationInfo();
      updatePrivileges(null, false);
    }
    else
    {
      this.authenticationInfo = authInfo;
      updatePrivileges(authInfo.getAuthorizationEntry(), authInfo.isRoot());
    }

    super.setSizeLimit(0);
    super.setTimeLimit(0);
    super.setIdleTimeLimit(0);
@@ -655,21 +665,6 @@


  /**
   * Retrieves information about the authentication that has been
   * performed for this connection.
   *
   * @return  Information about the user that is currently
   *          authenticated on this connection.
   */
  @Override()
  public AuthenticationInfo getAuthenticationInfo()
  {
    return authenticationInfo;
  }



  /**
   * This method has no effect, as the authentication info for
   * internal client connections is set when the connection is created
   * and cannot be changed after the fact.

 opends/tests/unit-tests-testng/src/server/org/opends/server/core/GroupManagerTestCase.java

@@ -23,6 +23,7 @@
 *
 *
 *      Copyright 2008-2010 Sun Microsystems, Inc.
 *      Portions copyright 2011 ForgeRock AS
 */
package org.opends.server.core;

@@ -1430,7 +1431,7 @@
    assertFalse(conn0.isMemberOf(group2, searchOperation));
    assertFalse(conn0.isMemberOf(group3, searchOperation));

    Set<Group> groupSet = conn0.getGroups(null);
    Set<Group<?>> groupSet = conn0.getGroups(null);
    assertTrue(groupSet.isEmpty());

    groupSet = conn0.getGroups(searchOperation);

 opends/tests/unit-tests-testng/src/server/org/opends/server/types/PrivilegeTestCase.java

@@ -37,13 +37,16 @@
import java.io.BufferedWriter;
import java.io.File;
import java.io.FileWriter;
import java.net.Socket;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.UUID;
import java.util.concurrent.CopyOnWriteArraySet;

import org.opends.server.TestCaseUtils;
import org.opends.server.admin.std.meta.GlobalCfgDefn;
import org.opends.server.admin.std.meta.RootDNCfgDefn;
import org.opends.server.api.ClientConnection;
import org.opends.server.backends.task.Task;
import org.opends.server.backends.task.TaskBackend;
import org.opends.server.backends.task.TaskState;
@@ -63,9 +66,10 @@
import org.opends.server.core.SchemaConfigManager;
import org.opends.server.protocols.internal.InternalClientConnection;
import org.opends.server.protocols.internal.InternalSearchOperation;
import org.opends.server.tools.LDAPModify;
import org.opends.server.tools.LDAPPasswordModify;
import org.opends.server.tools.LDAPSearch;
import org.opends.server.protocols.ldap.BindRequestProtocolOp;
import org.opends.server.protocols.ldap.BindResponseProtocolOp;
import org.opends.server.protocols.ldap.LDAPMessage;
import org.opends.server.tools.*;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.DataProvider;
@@ -115,7 +119,7 @@
  {
    TestCaseUtils.startServer();
    TestCaseUtils.enableBackend("unindexedRoot");
            

    TestCaseUtils.dsconfig(
            "set-sasl-mechanism-handler-prop",
            "--handler-name", "DIGEST-MD5",
@@ -2563,45 +2567,66 @@
      "sn: User",
      "userPassword: password");

    Entry testEntry =
               DirectoryServer.getEntry(DN.decode("cn=Test User,o=test"));
    AuthenticationInfo authInfo = new AuthenticationInfo(testEntry, false);
    InternalClientConnection testConnection =
         new InternalClientConnection(authInfo);


    // Make sure the user starts out without any privileges.
    for (Privilege p : Privilege.values())
    // We won't use an internal connection here because these are not notified
    // of dynamic changes to authentication info.
    Socket s = new Socket("127.0.0.1", TestCaseUtils.getServerLdapPort());
    try
    {
      assertFalse(testConnection.hasPrivilege(p, null));
      TestCaseUtils.configureSocket(s);
      LDAPReader r = new LDAPReader(s);
      LDAPWriter w = new LDAPWriter(s);

      BindRequestProtocolOp bindRequest = new BindRequestProtocolOp(
          ByteString.valueOf("cn=Test User,o=test"), 3,
          ByteString.valueOf("password"));
      LDAPMessage message = new LDAPMessage(1, bindRequest);
      w.writeMessage(message);

      message = r.readMessage();
      BindResponseProtocolOp bindResponse = message.getBindResponseProtocolOp();
      assertEquals(bindResponse.getResultCode(), 0);

      CopyOnWriteArraySet<ClientConnection> connections = DirectoryServer
          .getAuthenticatedUsers().get(DN.decode("cn=Test User,o=test"));

      assertNotNull(connections);
      assertEquals(connections.size(), 1);
      ClientConnection testConnection = connections.iterator().next();

      // Make sure the user starts out without any privileges.
      for (Privilege p : Privilege.values())
      {
        assertFalse(testConnection.hasPrivilege(p, null));
      }

      // Modify the user entry to add the CONFIG_READ privilege and verify that
      // the client connection reflects that.
      ArrayList<Modification> mods = new ArrayList<Modification>();
      mods.add(new Modification(ModificationType.ADD, Attributes.create(
          "ds-privilege-name", "config-read")));
      ModifyOperation modifyOperation = rootConnection.processModify(
          DN.decode("cn=Test User,o=test"), mods);
      assertEquals(modifyOperation.getResultCode(), ResultCode.SUCCESS);
      assertTrue(testConnection.hasPrivilege(Privilege.CONFIG_READ, null));

      // Take the privilege away from the user and verify that it is recognized
      // immediately.
      mods.clear();
      mods.add(new Modification(ModificationType.DELETE, Attributes.create(
          "ds-privilege-name", "config-read")));
      modifyOperation = rootConnection.processModify(
          DN.decode("cn=Test User,o=test"), mods);
      assertEquals(modifyOperation.getResultCode(), ResultCode.SUCCESS);
      assertFalse(testConnection.hasPrivilege(Privilege.CONFIG_READ, null));

      DeleteOperation deleteOperation = rootConnection.processDelete(DN
          .decode("cn=Test User,o=test"));
      assertEquals(deleteOperation.getResultCode(), ResultCode.SUCCESS);
    }


    // Modify the user entry to add the CONFIG_READ privilege and verify that
    // the client connection reflects that.
    ArrayList<Modification> mods = new ArrayList<Modification>();
    mods.add(new Modification(ModificationType.ADD,
        Attributes.create("ds-privilege-name", "config-read")));
    ModifyOperation modifyOperation =
         rootConnection.processModify(DN.decode("cn=Test User,o=test"), mods);
    assertEquals(modifyOperation.getResultCode(), ResultCode.SUCCESS);
    assertTrue(testConnection.hasPrivilege(Privilege.CONFIG_READ, null));


    // Take the privilege away from the user and verify that it is recognized
    // immediately.
    mods.clear();
    mods.add(new Modification(ModificationType.DELETE,
        Attributes.create("ds-privilege-name", "config-read")));
    modifyOperation =
         rootConnection.processModify(DN.decode("cn=Test User,o=test"), mods);
    assertEquals(modifyOperation.getResultCode(), ResultCode.SUCCESS);
    assertFalse(testConnection.hasPrivilege(Privilege.CONFIG_READ, null));


    DeleteOperation deleteOperation =
         rootConnection.processDelete(DN.decode("cn=Test User,o=test"));
    assertEquals(deleteOperation.getResultCode(), ResultCode.SUCCESS);
    finally
    {
      s.close();
    }
  }