From 1f638e1a491d333f5fa22e958c0b35607b49a5c1 Mon Sep 17 00:00:00 2001
From: Chris Ridd <chris.ridd@forgerock.com>
Date: Tue, 09 Jun 2015 13:00:07 +0000
Subject: [PATCH] FR-731 OPENDJ-2071 Evaluate proxy auth controls first

---
 opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendBindOperation.java     |    9 +-
 opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendDeleteOperation.java   |    3 
 opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java   |    7 +-
 opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java   |    5 +
 opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendCompareOperation.java  |    7 +-
 opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendModifyDNOperation.java |    9 +-
 opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendWorkflowElement.java   |   89 ++++++++++++++++++++++-------
 opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java      |    5 +
 8 files changed, 92 insertions(+), 42 deletions(-)

diff --git a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java
index 8874250..10e86c5 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java
@@ -1031,6 +1031,7 @@
    */
   private void processControls(DN parentDN) throws DirectoryException
   {
+    LocalBackendWorkflowElement.evaluateProxyAuthControls(this);
     LocalBackendWorkflowElement.removeAllDisallowedControls(parentDN, this);
 
     List<Control> requestControls = getRequestControls();
@@ -1038,7 +1039,7 @@
     {
       for (Control c : requestControls)
       {
-        String  oid = c.getOID();
+        final String  oid = c.getOID();
 
         if (OID_LDAP_ASSERTION.equals(oid))
         {
@@ -1103,7 +1104,7 @@
           postReadRequest =
                 getRequestControl(LDAPPostReadRequestControl.DECODER);
         }
-        else if (LocalBackendWorkflowElement.processProxyAuthControls(this, oid))
+        else if (LocalBackendWorkflowElement.isProxyAuthzControl(oid))
         {
           continue;
         }
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendBindOperation.java b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendBindOperation.java
index 08f43a7..724d8a1 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendBindOperation.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendBindOperation.java
@@ -380,16 +380,15 @@
     List<Control> requestControls = getRequestControls();
     if (requestControls != null && !requestControls.isEmpty())
     {
-      for (int i=0; i < requestControls.size(); i++)
+      for (Control c : requestControls)
       {
-        Control c   = requestControls.get(i);
-        String  oid = c.getOID();
+        final String  oid = c.getOID();
 
-        if (oid.equals(OID_AUTHZID_REQUEST))
+        if (OID_AUTHZID_REQUEST.equals(oid))
         {
           returnAuthzID = true;
         }
-        else if (oid.equals(OID_PASSWORD_POLICY_CONTROL))
+        else if (OID_PASSWORD_POLICY_CONTROL.equals(oid))
         {
           pwPolicyControlRequested = true;
         }
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendCompareOperation.java b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendCompareOperation.java
index 80008b5..91ee30c 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendCompareOperation.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendCompareOperation.java
@@ -354,6 +354,7 @@
    */
   private void handleRequestControls() throws DirectoryException
   {
+    LocalBackendWorkflowElement.evaluateProxyAuthControls(this);
     LocalBackendWorkflowElement.removeAllDisallowedControls(entryDN, this);
 
     List<Control> requestControls = getRequestControls();
@@ -361,9 +362,9 @@
     {
       for (Control c : requestControls)
       {
-        String  oid = c.getOID();
+        final String  oid = c.getOID();
 
-        if (oid.equals(OID_LDAP_ASSERTION))
+        if (OID_LDAP_ASSERTION.equals(oid))
         {
           LDAPAssertionRequestControl assertControl =
                 getRequestControl(LDAPAssertionRequestControl.DECODER);
@@ -412,7 +413,7 @@
                 ERR_COMPARE_CANNOT_PROCESS_ASSERTION_FILTER.get(entryDN, de.getMessageObject()));
           }
         }
-        else if (LocalBackendWorkflowElement.processProxyAuthControls(this, oid))
+        else if (LocalBackendWorkflowElement.isProxyAuthzControl(oid))
         {
           continue;
         }
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendDeleteOperation.java b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendDeleteOperation.java
index df7673e..bf1e5ee 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendDeleteOperation.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendDeleteOperation.java
@@ -400,6 +400,7 @@
    */
   private void handleRequestControls() throws DirectoryException
   {
+    LocalBackendWorkflowElement.evaluateProxyAuthControls(this);
     LocalBackendWorkflowElement.removeAllDisallowedControls(entryDN, this);
 
     List<Control> requestControls = getRequestControls();
@@ -466,7 +467,7 @@
           preReadRequest =
                 getRequestControl(LDAPPreReadRequestControl.DECODER);
         }
-        else if (LocalBackendWorkflowElement.processProxyAuthControls(this, oid))
+        else if (LocalBackendWorkflowElement.isProxyAuthzControl(oid))
         {
           continue;
         }
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendModifyDNOperation.java b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendModifyDNOperation.java
index ae33640..5fa241f 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendModifyDNOperation.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendModifyDNOperation.java
@@ -540,15 +540,16 @@
    */
   private void handleRequestControls() throws DirectoryException
   {
+    LocalBackendWorkflowElement.evaluateProxyAuthControls(this);
     LocalBackendWorkflowElement.removeAllDisallowedControls(entryDN, this);
 
-    List<Control> requestControls = getRequestControls();
+    final List<Control> requestControls = getRequestControls();
     if (requestControls != null && !requestControls.isEmpty())
     {
       for (ListIterator<Control> iter = requestControls.listIterator(); iter.hasNext();)
       {
-        Control c = iter.next();
-        String  oid = c.getOID();
+        final Control c = iter.next();
+        final String  oid = c.getOID();
 
         if (OID_LDAP_ASSERTION.equals(oid))
         {
@@ -621,7 +622,7 @@
             iter.set(postReadRequest);
           }
         }
-        else if (LocalBackendWorkflowElement.processProxyAuthControls(this, oid))
+        else if (LocalBackendWorkflowElement.isProxyAuthzControl(oid))
         {
           continue;
         }
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java
index cbbaa9c..2df63de 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java
@@ -652,6 +652,7 @@
    */
   private void processRequestControls() throws DirectoryException
   {
+    LocalBackendWorkflowElement.evaluateProxyAuthControls(this);
     LocalBackendWorkflowElement.removeAllDisallowedControls(entryDN, this);
 
     List<Control> requestControls = getRequestControls();
@@ -659,8 +660,8 @@
     {
       for (ListIterator<Control> iter = requestControls.listIterator(); iter.hasNext();)
       {
-        Control c = iter.next();
-        String  oid = c.getOID();
+        final Control c = iter.next();
+        final String  oid = c.getOID();
 
         if (OID_LDAP_ASSERTION.equals(oid))
         {
@@ -738,7 +739,7 @@
             iter.set(postReadRequest);
           }
         }
-        else if (LocalBackendWorkflowElement.processProxyAuthControls(this, oid))
+        else if (LocalBackendWorkflowElement.isProxyAuthzControl(oid))
         {
           continue;
         }
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java
index d3c2504..19b3b9a 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java
@@ -298,6 +298,7 @@
    */
   private void handleRequestControls() throws DirectoryException
   {
+    LocalBackendWorkflowElement.evaluateProxyAuthControls(this);
     LocalBackendWorkflowElement.removeAllDisallowedControls(baseDN, this);
 
     List<Control> requestControls  = getRequestControls();
@@ -305,7 +306,7 @@
     {
       for (Control c : requestControls)
       {
-        String  oid = c.getOID();
+        final String  oid = c.getOID();
 
         if (OID_LDAP_ASSERTION.equals(oid))
         {
@@ -377,7 +378,7 @@
                                 de.getMessageObject()), de);
           }
         }
-        else if (LocalBackendWorkflowElement.processProxyAuthControls(this, oid))
+        else if (LocalBackendWorkflowElement.isProxyAuthzControl(oid))
         {
           continue;
         }
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendWorkflowElement.java b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendWorkflowElement.java
index 5d6b17e..2e40158 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendWorkflowElement.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/workflowelement/localbackend/LocalBackendWorkflowElement.java
@@ -289,6 +289,22 @@
   }
 
   /**
+   * Check if an OID is for a proxy authorization control.
+   *
+   * @param oid The OID to check
+   * @return <code>true</code> if the OID is for a proxy auth v1 or v2 control,
+   * <code>false</code> otherwise.
+   */
+  public static boolean isProxyAuthzControl(String oid)
+  {
+    if (OID_PROXIED_AUTH_V1.equals(oid) || OID_PROXIED_AUTH_V2.equals(oid))
+    {
+      return true;
+    }
+    return false;
+  }
+
+  /**
    * Removes all the disallowed request controls from the provided operation.
    * <p>
    * As per RFC 4511 4.1.11, if a disallowed request control is critical, then a
@@ -296,9 +312,7 @@
    * if the disallowed request control is non critical, it is removed because we
    * do not want the backend to process it.
    *
-   * @param targetDN
-   *          the target DN on which the operation applies
-   * @param op
+   * @param operation
    *          the operation currently processed
    * @throws DirectoryException
    *           If a disallowed request control is critical, thrown with
@@ -307,29 +321,21 @@
    *           could not be decoded. Care must be taken not to expose any
    *           potentially sensitive information in the exception.
    */
-  static void removeAllDisallowedControls(DN targetDN, Operation op)
+  static void removeAllDisallowedControls(DN targetDN, Operation operation)
       throws DirectoryException
   {
-    final List<Control> requestControls = op.getRequestControls();
+    List<Control> requestControls = operation.getRequestControls();
     if (requestControls != null && !requestControls.isEmpty())
     {
       for (Iterator<Control> iter = requestControls.iterator(); iter.hasNext();)
       {
         final Control control = iter.next();
-        // The aci check for the proxy auth controls needs to check the authentication DN,
-        // not the operation target.
-        // TODO should we check the authentication DN for all controls?
-        final DN aciTarget;
-        if (OID_PROXIED_AUTH_V1.equals(control.getOID()) || OID_PROXIED_AUTH_V2.equals(control.getOID()))
+        if (isProxyAuthzControl(control.getOID()))
         {
-          aciTarget= op.getClientConnection().getAuthenticationInfo().getAuthenticationDN();
-        }
-        else
-        {
-          aciTarget = targetDN;
+          continue;
         }
 
-        if (!getAccessControlHandler().isAllowed(aciTarget, op, control))
+        if (!getAccessControlHandler().isAllowed(targetDN, operation, control))
         {
           // As per RFC 4511 4.1.11.
           if (control.isCritical())
@@ -348,12 +354,52 @@
   }
 
   /**
+   * Evaluate all aci and privilege checks for any proxy auth controls.
+   * This must be done before evaluating all other controls so that their aci
+   * can then be checked correctly.
+   *
+   * @param operation  The operation containing the controls
+   * @throws DirectoryException if a proxy auth control is found but cannot
+   * be used.
+   */
+  public static void evaluateProxyAuthControls(Operation operation)
+      throws DirectoryException
+  {
+    final List<Control> requestControls = operation.getRequestControls();
+    if (requestControls != null && !requestControls.isEmpty())
+    {
+      for (Control control : requestControls)
+      {
+        final String oid = control.getOID();
+        if (isProxyAuthzControl(oid))
+        {
+          if (getAccessControlHandler().isAllowed(operation.getClientConnection()
+                  .getAuthenticationInfo().getAuthenticationDN(), operation, control))
+          {
+            processProxyAuthControls(operation, oid);
+          }
+          else
+          {
+            // As per RFC 4511 4.1.11.
+            if (control.isCritical())
+            {
+              throw new DirectoryException(
+                      ResultCode.UNAVAILABLE_CRITICAL_EXTENSION,
+                      ERR_CONTROL_INSUFFICIENT_ACCESS_RIGHTS.get(control.getOID()));
+            }
+          }
+        }
+      }
+    }
+  }
+
+  /**
    * Check the requester has the PROXIED_AUTH privilege in order to be able to use a proxy auth control.
    *
    * @param operation  The operation being checked
    * @throws DirectoryException  If insufficient privileges are detected
    */
-  static void checkPrivilegeForProxyAuthControl(Operation operation) throws DirectoryException
+  private static void checkPrivilegeForProxyAuthControl(Operation operation) throws DirectoryException
   {
     if (! operation.getClientConnection().hasPrivilege(Privilege.PROXIED_AUTH, operation))
     {
@@ -369,7 +415,8 @@
    * @param authorizationEntry  The entry being authorized as (e.g. from a proxy auth control)
    * @throws DirectoryException  If no proxy permission is allowed
    */
-  static void checkAciForProxyAuthControl(Operation operation, Entry authorizationEntry) throws DirectoryException
+  private static void checkAciForProxyAuthControl(Operation operation, Entry authorizationEntry)
+      throws DirectoryException
   {
     if (! AccessControlConfigManager.getInstance().getAccessControlHandler()
             .mayProxy(operation.getClientConnection().getAuthenticationInfo().getAuthenticationEntry(),
@@ -388,10 +435,9 @@
    *
    * @param operation  The operation containing the control(s)
    * @param oid  The OID of the detected proxy auth control
-   * @return <code>true</code> if the control has been processed, <code>false</code> if not
    * @throws DirectoryException
    */
-  static boolean processProxyAuthControls(Operation operation, String oid)
+  private static void processProxyAuthControls(Operation operation, String oid)
           throws DirectoryException
   {
     final Entry authorizationEntry;
@@ -413,7 +459,7 @@
     }
     else
     {
-      return false;
+      return;
     }
 
     checkAciForProxyAuthControl(operation, authorizationEntry);
@@ -427,7 +473,6 @@
     {
       operation.setProxiedAuthorizationDN(authorizationEntry.getName());
     }
-    return true;
   }
 
   /**

--
Gitblit v1.10.0