Fix issue 3928: Wrong error message sent to access log (while correct one i...

matthew_swift

24.52.2009 5067760c866efc66b933457bd399affa47c9e9a4

Fix issue 3928: Wrong error message sent to access log (while correct one is in error log)

Allow access control handler implementations to throw DirectoryExceptions encountered when attempting to perform an access control decision. Previously, this was not possible and implementations were forced to deny the operation and log an error to the error log. The DSEE compat access control handler has been updated so that it now throws DirectoryExceptions when:

    * it fails to decode an access control related LDAP control (result code PROTOCOL_ERROR)

    * it fails to decode a modified ACI (result code INVALID_ATTRIBUTE_SYNTAX)

These errors will now be logged in the access log and returned to the client as expected.

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

@@ -129,9 +129,15 @@
   *          The operation for which to make the determination.
   * @return {@code true} if the operation should be allowed by the
   *         access control configuration, or {@code false} if not.
   * @throws DirectoryException
   *           If an error occurred while performing the access
   *           control check. For example, if an attribute could not
   *           be decoded. Care must be taken not to expose any
   *           potentially sensitive information in the exception.
   */
  public abstract boolean isAllowed(
      LocalBackendAddOperation addOperation);
      LocalBackendAddOperation addOperation)
    throws DirectoryException;



@@ -148,9 +154,17 @@
   *          The control for which to make the determination.
   * @return {@code true} if the control should be allowed by the
   *         access control configuration, or {@code false} if not.
   * @throws DirectoryException
   *           If an error occurred while performing the access
   *           control check. For example, if an attribute could not
   *           be decoded. Care must be taken not to expose any
   *           potentially sensitive information in the exception.
   */
  public abstract boolean isAllowed(DN dn, Operation op,
                                    Control control);
  public abstract boolean isAllowed(
      DN dn,
      Operation op,
      Control control)
    throws DirectoryException;



@@ -163,9 +177,15 @@
   *          The operation for which to make the determination.
   * @return {@code true} if the operation should be allowed by the
   *         access control configuration, or {@code false} if not.
   * @throws DirectoryException
   *           If an error occurred while performing the access
   *           control check. For example, if an attribute could not
   *           be decoded. Care must be taken not to expose any
   *           potentially sensitive information in the exception.
   */
  public abstract boolean isAllowed(
      LocalBackendBindOperation bindOperation);
      LocalBackendBindOperation bindOperation)
    throws DirectoryException;



@@ -178,9 +198,15 @@
   *          The operation for which to make the determination.
   * @return {@code true} if the operation should be allowed by the
   *         access control configuration, or {@code false} if not.
   * @throws DirectoryException
   *           If an error occurred while performing the access
   *           control check. For example, if an attribute could not
   *           be decoded. Care must be taken not to expose any
   *           potentially sensitive information in the exception.
   */
  public abstract boolean isAllowed(
      LocalBackendCompareOperation compareOperation);
      LocalBackendCompareOperation compareOperation)
    throws DirectoryException;



@@ -193,9 +219,15 @@
   *          The operation for which to make the determination.
   * @return {@code true} if the operation should be allowed by the
   *         access control configuration, or {@code false} if not.
   * @throws DirectoryException
   *           If an error occurred while performing the access
   *           control check. For example, if an attribute could not
   *           be decoded. Care must be taken not to expose any
   *           potentially sensitive information in the exception.
   */
  public abstract boolean isAllowed(
      LocalBackendDeleteOperation deleteOperation);
      LocalBackendDeleteOperation deleteOperation)
    throws DirectoryException;



@@ -208,9 +240,15 @@
   *          The operation for which to make the determination.
   * @return {@code true} if the operation should be allowed by the
   *         access control configuration, or {@code false} if not.
   * @throws DirectoryException
   *           If an error occurred while performing the access
   *           control check. For example, if an attribute could not
   *           be decoded. Care must be taken not to expose any
   *           potentially sensitive information in the exception.
   */
  public abstract boolean isAllowed(
      ExtendedOperation extendedOperation);
      ExtendedOperation extendedOperation)
    throws DirectoryException;



@@ -223,9 +261,15 @@
   *          The operation for which to make the determination.
   * @return {@code true} if the operation should be allowed by the
   *         access control configuration, or {@code false} if not.
   * @throws DirectoryException
   *           If an error occurred while performing the access
   *           control check. For example, if an attribute could not
   *           be decoded. Care must be taken not to expose any
   *           potentially sensitive information in the exception.
   */
  public abstract boolean isAllowed(
      LocalBackendModifyOperation modifyOperation);
      LocalBackendModifyOperation modifyOperation)
    throws DirectoryException;



@@ -238,9 +282,15 @@
   *          The operation for which to make the determination.
   * @return {@code true} if the operation should be allowed by the
   *         access control configuration, or {@code false} if not.
   * @throws DirectoryException
   *           If an error occurred while performing the access
   *           control check. For example, if an attribute could not
   *           be decoded. Care must be taken not to expose any
   *           potentially sensitive information in the exception.
   */
  public abstract boolean isAllowed(
      LocalBackendModifyDNOperation modifyDNOperation);
      LocalBackendModifyDNOperation modifyDNOperation)
    throws DirectoryException;



@@ -256,9 +306,15 @@
   *          The operation for which to make the determination.
   * @return {@code true} if the operation should be allowed by the
   *         access control configuration, or {@code false} if not.
   * @throws DirectoryException
   *           If an error occurred while performing the access
   *           control check. For example, if an attribute could not
   *           be decoded. Care must be taken not to expose any
   *           potentially sensitive information in the exception.
   */
  public abstract boolean isAllowed(
      LocalBackendSearchOperation searchOperation);
      LocalBackendSearchOperation searchOperation)
    throws DirectoryException;




 opends/src/server/org/opends/server/authorization/dseecompat/AciHandler.java

@@ -77,6 +77,7 @@
import org.opends.server.types.Operation;
import org.opends.server.types.Privilege;
import org.opends.server.types.RDN;
import org.opends.server.types.ResultCode;
import org.opends.server.types.SearchFilter;
import org.opends.server.types.SearchResultEntry;
import org.opends.server.types.SearchResultReference;
@@ -89,7 +90,7 @@
 * The AciHandler class performs the main processing for the dseecompat
 * package.
 */
public class AciHandler extends
public final class AciHandler extends
    AccessControlHandler<DseeCompatAccessControlHandlerCfg>
{
  /**
@@ -318,6 +319,7 @@
   */
  @Override
  public boolean isAllowed(DN entryDN, Operation op, Control control)
      throws DirectoryException
  {
    boolean ret;
    if (!(ret = skipAccessCheck(op)))
@@ -335,31 +337,20 @@
    }
    else if (control.getOID().equals(OID_GET_EFFECTIVE_RIGHTS))
    {
      try
      GetEffectiveRightsRequestControl getEffectiveRightsControl;
      if (control instanceof LDAPControl)
      {
        GetEffectiveRightsRequestControl getEffectiveRightsControl;
        if (control instanceof LDAPControl)
        {
          getEffectiveRightsControl =
              GetEffectiveRightsRequestControl.DECODER.decode(control
                  .isCritical(), ((LDAPControl) control).getValue());
        }
        else
        {
          getEffectiveRightsControl =
              (GetEffectiveRightsRequestControl) control;
        }
        op.setAttachment(OID_GET_EFFECTIVE_RIGHTS,
            getEffectiveRightsControl);
        getEffectiveRightsControl =
            GetEffectiveRightsRequestControl.DECODER.decode(control
                .isCritical(), ((LDAPControl) control).getValue());
      }
      catch (DirectoryException de)
      else
      {
        Message message =
            WARN_ACI_SYNTAX_DECODE_EFFECTIVERIGHTS_FAIL.get(de
                .getMessage());
        logError(message);
        ret = false;
        getEffectiveRightsControl =
            (GetEffectiveRightsRequestControl) control;
      }
      op.setAttachment(OID_GET_EFFECTIVE_RIGHTS,
          getEffectiveRightsControl);
    }
    return ret;
  }
@@ -388,14 +379,11 @@


  /**
   * Check access on add operations.
   *
   * @param operation
   *          The add operation to check access on.
   * @return True if access is allowed.
   * {@inheritDoc}
   */
  @Override
  public boolean isAllowed(LocalBackendAddOperation operation)
      throws DirectoryException
  {
    AciLDAPOperationContainer operationContainer =
        new AciLDAPOperationContainer(operation, ACI_ADD);
@@ -543,15 +531,11 @@


  /**
   * Check access on modify operations.
   *
   * @param operation
   *          The modify operation to check access on.
   * @return True if access is allowed.
   * {@inheritDoc}
   */

  @Override
  public boolean isAllowed(LocalBackendModifyOperation operation)
      throws DirectoryException
  {
    AciLDAPOperationContainer operationContainer =
        new AciLDAPOperationContainer(operation, ACI_NULL);
@@ -910,9 +894,12 @@
   * @param skipAccessCheck
   *          True if access checking should be skipped.
   * @return True if access is allowed.
   * @throws DirectoryException
   *           If a modified ACI could not be decoded.
   */
  private boolean aciCheckMods(AciLDAPOperationContainer container,
      LocalBackendModifyOperation operation, boolean skipAccessCheck)
      throws DirectoryException
  {
    Entry resourceEntry = container.getResourceEntry();
    DN dn = resourceEntry.getDN();
@@ -1044,8 +1031,8 @@
              Message message =
                  WARN_ACI_MODIFY_FAILED_DECODE.get(String.valueOf(dn),
                      ex.getMessage());
              logError(message);
              return false;
              throw new DirectoryException(
                  ResultCode.INVALID_ATTRIBUTE_SYNTAX, message);
            }
          }
        }
@@ -1598,9 +1585,11 @@
   *          The authorization DN.
   * @return True if the entry has no ACI attributes or if all of the
   *         "aci" attributes values pass ACI syntax checking.
   * @throws DirectoryException
   *           If a modified ACI could not be decoded.
   */
  private boolean verifySyntax(Entry entry, Operation operation,
      DN clientDN)
      DN clientDN) throws DirectoryException
  {
    if (entry.hasOperationalAttribute(aciType))
    {
@@ -1633,8 +1622,8 @@
            Message message =
                WARN_ACI_ADD_FAILED_DECODE.get(String.valueOf(entry
                    .getDN()), ex.getMessage());
            logError(message);
            return false;
            throw new DirectoryException(
                ResultCode.INVALID_ATTRIBUTE_SYNTAX, message);
          }
        }
      }

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

@@ -22,7 +22,7 @@
 * CDDL HEADER END
 *
 *
 *      Copyright 2006-2008 Sun Microsystems, Inc.
 *      Copyright 2006-2009 Sun Microsystems, Inc.
 */
package org.opends.server.core;
import org.opends.messages.MessageBuilder;
@@ -47,6 +47,7 @@
import org.opends.server.loggers.debug.DebugLogger;
import org.opends.server.loggers.debug.DebugTracer;
import org.opends.server.types.*;

import static org.opends.messages.CoreMessages.*;
import static org.opends.server.util.ServerConstants.*;

@@ -362,6 +363,7 @@
   * managing synchronization, and any other work that might need to
   * be done in the course of processing.
   */
  @Override
  public final void run()
  {
    setResultCode(ResultCode.UNDEFINED);
@@ -417,15 +419,25 @@
      {
        for (Control c : requestControls)
        {
          if (!AccessControlConfigManager.getInstance().
                  getAccessControlHandler().
                  isAllowed(this.getAuthorizationDN(), this, c)) {
            setResultCode(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);

            appendErrorMessage(ERR_CONTROL_INSUFFICIENT_ACCESS_RIGHTS.get(
                    c.getOID()));
          try
          {
            if (!AccessControlConfigManager.getInstance()
                .getAccessControlHandler().isAllowed(
                    this.getAuthorizationDN(), this, c))
            {
              setResultCode(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);
              appendErrorMessage(ERR_CONTROL_INSUFFICIENT_ACCESS_RIGHTS
                  .get(c.getOID()));
              return;
            }
          }
          catch (DirectoryException e)
          {
            setResultCode(e.getResultCode());
            appendErrorMessage(e.getMessageObject());
            return;
          }

          if (! c.isCritical())
          {
            // The control isn't critical, so we don't care if it's supported
@@ -451,13 +463,21 @@
      // FIXME: for now assume that this will check all permission
      // pertinent to the operation. This includes proxy authorization
      // and any other controls specified.
      if (AccessControlConfigManager.getInstance()
          .getAccessControlHandler().isAllowed(this) == false) {
        setResultCode(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);

        appendErrorMessage(ERR_EXTENDED_AUTHZ_INSUFFICIENT_ACCESS_RIGHTS.get(
                String.valueOf(requestOID)));

      try
      {
        if (AccessControlConfigManager.getInstance()
            .getAccessControlHandler().isAllowed(this) == false)
        {
          setResultCode(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);
          appendErrorMessage(ERR_EXTENDED_AUTHZ_INSUFFICIENT_ACCESS_RIGHTS
              .get(String.valueOf(requestOID)));
          return;
        }
      }
      catch (DirectoryException e)
      {
        setResultCode(e.getResultCode());
        appendErrorMessage(e.getMessageObject());
        return;
      }


 opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java

@@ -550,12 +550,21 @@
        // FIXME: earlier checks to see if the entry already exists or
        // if the parent entry does not exist may have already exposed
        // sensitive information to the client.
        if (AccessControlConfigManager.getInstance().getAccessControlHandler().
                 isAllowed(this) == false)
        try
        {
          setResultCode(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);
          appendErrorMessage(ERR_ADD_AUTHZ_INSUFFICIENT_ACCESS_RIGHTS.get(
                                  String.valueOf(entryDN)));
          if (AccessControlConfigManager.getInstance()
              .getAccessControlHandler().isAllowed(this) == false)
          {
            setResultCode(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);
            appendErrorMessage(ERR_ADD_AUTHZ_INSUFFICIENT_ACCESS_RIGHTS
                .get(String.valueOf(entryDN)));
            break addProcessing;
          }
        }
        catch (DirectoryException e)
        {
          setResultCode(e.getResultCode());
          appendErrorMessage(e.getMessageObject());
          break addProcessing;
        }


 opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendBindOperation.java

@@ -233,12 +233,21 @@
      // FIXME: for now assume that this will check all permission
      // pertinent to the operation. This includes any controls
      // specified.
      if (! AccessControlConfigManager.getInstance().getAccessControlHandler().
                 isAllowed(this))
      try
      {
        setResultCode(ResultCode.INVALID_CREDENTIALS);
        setAuthFailureReason(ERR_BIND_AUTHZ_INSUFFICIENT_ACCESS_RIGHTS.get(
                                  String.valueOf(bindDN)));
        if (!AccessControlConfigManager.getInstance()
            .getAccessControlHandler().isAllowed(this))
        {
          setResultCode(ResultCode.INVALID_CREDENTIALS);
          setAuthFailureReason(ERR_BIND_AUTHZ_INSUFFICIENT_ACCESS_RIGHTS
              .get(String.valueOf(bindDN)));
          break bindProcessing;
        }
      }
      catch (DirectoryException e)
      {
        setResultCode(e.getResultCode());
        setAuthFailureReason(e.getMessageObject());
        break bindProcessing;
      }


 opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendCompareOperation.java

@@ -278,12 +278,21 @@

        // FIXME: earlier checks to see if the entry already exists may
        // have already exposed sensitive information to the client.
        if (! AccessControlConfigManager.getInstance().
                   getAccessControlHandler().isAllowed(this))
        try
        {
          setResultCode(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);
          appendErrorMessage(ERR_COMPARE_AUTHZ_INSUFFICIENT_ACCESS_RIGHTS.get(
                                  String.valueOf(entryDN)));
          if (!AccessControlConfigManager.getInstance()
              .getAccessControlHandler().isAllowed(this))
          {
            setResultCode(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);
            appendErrorMessage(ERR_COMPARE_AUTHZ_INSUFFICIENT_ACCESS_RIGHTS
                .get(String.valueOf(entryDN)));
            break compareProcessing;
          }
        }
        catch (DirectoryException e)
        {
          setResultCode(e.getResultCode());
          appendErrorMessage(e.getMessageObject());
          break compareProcessing;
        }


 opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendDeleteOperation.java

@@ -278,12 +278,21 @@

        // FIXME: earlier checks to see if the entry already exists may
        // have already exposed sensitive information to the client.
        if (! AccessControlConfigManager.getInstance().
                   getAccessControlHandler().isAllowed(this))
        try
        {
          setResultCode(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);
          appendErrorMessage(ERR_DELETE_AUTHZ_INSUFFICIENT_ACCESS_RIGHTS.get(
                                  String.valueOf(entryDN)));
          if (!AccessControlConfigManager.getInstance()
              .getAccessControlHandler().isAllowed(this))
          {
            setResultCode(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);
            appendErrorMessage(ERR_DELETE_AUTHZ_INSUFFICIENT_ACCESS_RIGHTS
                .get(String.valueOf(entryDN)));
            break deleteProcessing;
          }
        }
        catch (DirectoryException e)
        {
          setResultCode(e.getResultCode());
          appendErrorMessage(e.getMessageObject());
          break deleteProcessing;
        }


 opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyDNOperation.java

@@ -450,12 +450,21 @@
        // FIXME: earlier checks to see if the entry or new superior
        // already exists may have already exposed sensitive information
        // to the client.
        if (! AccessControlConfigManager.getInstance().
                   getAccessControlHandler().isAllowed(this))
        try
        {
          setResultCode(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);
          appendErrorMessage(ERR_MODDN_AUTHZ_INSUFFICIENT_ACCESS_RIGHTS.get(
                                  String.valueOf(entryDN)));
          if (!AccessControlConfigManager.getInstance()
              .getAccessControlHandler().isAllowed(this))
          {
            setResultCode(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);
            appendErrorMessage(ERR_MODDN_AUTHZ_INSUFFICIENT_ACCESS_RIGHTS
                .get(String.valueOf(entryDN)));
            break modifyDNProcessing;
          }
        }
        catch (DirectoryException e)
        {
          setResultCode(e.getResultCode());
          appendErrorMessage(e.getMessageObject());
          break modifyDNProcessing;
        }


 opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java

@@ -499,12 +499,21 @@

        // FIXME: earlier checks to see if the entry already exists may have
        // already exposed sensitive information to the client.
        if (! AccessControlConfigManager.getInstance().
                   getAccessControlHandler().isAllowed(this))
        try
        {
          setResultCode(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);
          appendErrorMessage(ERR_MODIFY_AUTHZ_INSUFFICIENT_ACCESS_RIGHTS.get(
                                  String.valueOf(entryDN)));
          if (!AccessControlConfigManager.getInstance()
              .getAccessControlHandler().isAllowed(this))
          {
            setResultCode(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);
            appendErrorMessage(ERR_MODIFY_AUTHZ_INSUFFICIENT_ACCESS_RIGHTS
                .get(String.valueOf(entryDN)));
            break modifyProcessing;
          }
        }
        catch (DirectoryException e)
        {
          setResultCode(e.getResultCode());
          appendErrorMessage(e.getMessageObject());
          break modifyProcessing;
        }


 opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java

@@ -191,12 +191,21 @@
      // FIXME: for now assume that this will check all permission
      // pertinent to the operation. This includes proxy authorization
      // and any other controls specified.
      if (! AccessControlConfigManager.getInstance().getAccessControlHandler().
                 isAllowed(this))
      try
      {
        setResultCode(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);
        appendErrorMessage(ERR_SEARCH_AUTHZ_INSUFFICIENT_ACCESS_RIGHTS.get(
                                String.valueOf(baseDN)));
        if (!AccessControlConfigManager.getInstance()
            .getAccessControlHandler().isAllowed(this))
        {
          setResultCode(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);
          appendErrorMessage(ERR_SEARCH_AUTHZ_INSUFFICIENT_ACCESS_RIGHTS
              .get(String.valueOf(baseDN)));
          break searchProcessing;
        }
      }
      catch (DirectoryException e)
      {
        setResultCode(e.getResultCode());
        appendErrorMessage(e.getMessageObject());
        break searchProcessing;
      }