From 24f179799e2823bf6762baa2c8ca422e971b7111 Mon Sep 17 00:00:00 2001
From: Matthew Swift <matthew.swift@forgerock.com>
Date: Thu, 10 May 2012 11:28:13 +0000
Subject: [PATCH] Fix OPENDJ-475: Incorrect behaviour/result code regarding non-critical controls
---
opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java | 9 +-
opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java | 8 +-
opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java | 10 +-
opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendBindOperation.java | 9 +-
opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendWorkflowElement.java | 42 ++++++++++++++
opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyDNOperation.java | 7 +-
opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/authorization/dseecompat/TargetControlTestCase.java | 19 +++---
opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendCompareOperation.java | 9 +-
opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendDeleteOperation.java | 9 +-
opendj-sdk/opends/src/server/org/opends/server/workflowelement/externalchangelog/ECLSearchOperation.java | 20 +++++-
opendj-sdk/opends/src/server/org/opends/server/core/ExtendedOperationBasis.java | 24 +++++--
11 files changed, 112 insertions(+), 54 deletions(-)
diff --git a/opendj-sdk/opends/src/server/org/opends/server/core/ExtendedOperationBasis.java b/opendj-sdk/opends/src/server/org/opends/server/core/ExtendedOperationBasis.java
index 0aeb4ae..d0d0c50 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/core/ExtendedOperationBasis.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/core/ExtendedOperationBasis.java
@@ -23,7 +23,7 @@
*
*
* Copyright 2006-2010 Sun Microsystems, Inc.
- * Portions copyright 2011 ForgeRock AS.
+ * Portions copyright 2011-2012 ForgeRock AS.
*/
package org.opends.server.core;
import org.opends.messages.MessageBuilder;
@@ -419,13 +419,23 @@
try
{
if (!AccessControlConfigManager.getInstance()
- .getAccessControlHandler().isAllowed(
- this.getAuthorizationDN(), this, c))
+ .getAccessControlHandler()
+ .isAllowed(getAuthorizationDN(), this, c))
{
- setResultCode(ResultCode.INSUFFICIENT_ACCESS_RIGHTS);
- appendErrorMessage(ERR_CONTROL_INSUFFICIENT_ACCESS_RIGHTS
- .get(c.getOID()));
- return;
+ // As per RFC 4511 4.1.11.
+ if (c.isCritical())
+ {
+ setResultCode(ResultCode.UNAVAILABLE_CRITICAL_EXTENSION);
+ appendErrorMessage(ERR_CONTROL_INSUFFICIENT_ACCESS_RIGHTS
+ .get(c.getOID()));
+ }
+ else
+ {
+ // We don't want to process this non-critical control, so
+ // remove it.
+ removeRequestControl(c);
+ continue;
+ }
}
}
catch (DirectoryException e)
diff --git a/opendj-sdk/opends/src/server/org/opends/server/workflowelement/externalchangelog/ECLSearchOperation.java b/opendj-sdk/opends/src/server/org/opends/server/workflowelement/externalchangelog/ECLSearchOperation.java
index 36dc59f..b54689f 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/workflowelement/externalchangelog/ECLSearchOperation.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/workflowelement/externalchangelog/ECLSearchOperation.java
@@ -405,11 +405,23 @@
{
Control c = requestControls.get(i);
String oid = c.getOID();
- if (! AccessControlConfigManager.getInstance().
- getAccessControlHandler().isAllowed(baseDN, this, c))
+
+ if (!AccessControlConfigManager.getInstance().getAccessControlHandler()
+ .isAllowed(baseDN, this, c))
{
- throw new DirectoryException(ResultCode.INSUFFICIENT_ACCESS_RIGHTS,
- ERR_CONTROL_INSUFFICIENT_ACCESS_RIGHTS.get(oid));
+ // As per RFC 4511 4.1.11.
+ if (c.isCritical())
+ {
+ throw new DirectoryException(
+ ResultCode.UNAVAILABLE_CRITICAL_EXTENSION,
+ ERR_CONTROL_INSUFFICIENT_ACCESS_RIGHTS.get(oid));
+ }
+ else
+ {
+ // We don't want to process this non-critical control, so remove it.
+ removeRequestControl(c);
+ continue;
+ }
}
if (oid.equals(OID_ECL_COOKIE_EXCHANGE_CONTROL))
diff --git a/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java b/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java
index e1c6457..d358780 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java
@@ -23,7 +23,7 @@
*
*
* Copyright 2008-2010 Sun Microsystems, Inc.
- * Portions Copyright 2011 ForgeRock AS
+ * Portions Copyright 2011-2012 ForgeRock AS
*/
package org.opends.server.workflowelement.localbackend;
@@ -1415,11 +1415,10 @@
Control c = requestControls.get(i);
String oid = c.getOID();
- if (!AccessControlConfigManager.getInstance().
- getAccessControlHandler().isAllowed(parentDN, this, c))
+ if (!LocalBackendWorkflowElement.isControlAllowed(parentDN, this, c))
{
- throw new DirectoryException(ResultCode.INSUFFICIENT_ACCESS_RIGHTS,
- ERR_CONTROL_INSUFFICIENT_ACCESS_RIGHTS.get(oid));
+ // Skip disallowed non-critical controls.
+ continue;
}
if (oid.equals(OID_LDAP_ASSERTION))
diff --git a/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendBindOperation.java b/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendBindOperation.java
index 7fce649..6ca0485 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendBindOperation.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendBindOperation.java
@@ -23,7 +23,7 @@
*
*
* Copyright 2008-2010 Sun Microsystems, Inc.
- * Portions copyright 2011 ForgeRock AS.
+ * Portions copyright 2011-2012 ForgeRock AS.
*/
package org.opends.server.workflowelement.localbackend;
@@ -439,11 +439,10 @@
Control c = requestControls.get(i);
String oid = c.getOID();
- if (! AccessControlConfigManager.getInstance().
- getAccessControlHandler(). isAllowed(bindDN, this, c))
+ if (!LocalBackendWorkflowElement.isControlAllowed(bindDN, this, c))
{
- throw new DirectoryException(ResultCode.INSUFFICIENT_ACCESS_RIGHTS,
- ERR_CONTROL_INSUFFICIENT_ACCESS_RIGHTS.get(oid));
+ // Skip disallowed non-critical controls.
+ continue;
}
if (oid.equals(OID_AUTHZID_REQUEST))
diff --git a/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendCompareOperation.java b/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendCompareOperation.java
index f663e1b..8124bff 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendCompareOperation.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendCompareOperation.java
@@ -23,7 +23,7 @@
*
*
* Copyright 2008-2009 Sun Microsystems, Inc.
- * Portions Copyright 2011 ForgeRock AS
+ * Portions Copyright 2011-2012 ForgeRock AS
*/
package org.opends.server.workflowelement.localbackend;
@@ -407,11 +407,10 @@
Control c = requestControls.get(i);
String oid = c.getOID();
- if (! AccessControlConfigManager.getInstance().
- getAccessControlHandler().isAllowed(entryDN, this, c))
+ if (!LocalBackendWorkflowElement.isControlAllowed(entryDN, this, c))
{
- throw new DirectoryException(ResultCode.INSUFFICIENT_ACCESS_RIGHTS,
- ERR_CONTROL_INSUFFICIENT_ACCESS_RIGHTS.get(oid));
+ // Skip disallowed non-critical controls.
+ continue;
}
if (oid.equals(OID_LDAP_ASSERTION))
diff --git a/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendDeleteOperation.java b/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendDeleteOperation.java
index 846aeba..de3d6f4 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendDeleteOperation.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendDeleteOperation.java
@@ -23,7 +23,7 @@
*
*
* Copyright 2008-2009 Sun Microsystems, Inc.
- * Portions Copyright 2011 ForgeRock AS
+ * Portions Copyright 2011-2012 ForgeRock AS
*/
package org.opends.server.workflowelement.localbackend;
@@ -507,11 +507,10 @@
Control c = requestControls.get(i);
String oid = c.getOID();
- if (!AccessControlConfigManager.getInstance().
- getAccessControlHandler().isAllowed(entryDN, this, c))
+ if (!LocalBackendWorkflowElement.isControlAllowed(entryDN, this, c))
{
- throw new DirectoryException(ResultCode.INSUFFICIENT_ACCESS_RIGHTS,
- ERR_CONTROL_INSUFFICIENT_ACCESS_RIGHTS.get(oid));
+ // Skip disallowed non-critical controls.
+ continue;
}
if (oid.equals(OID_LDAP_ASSERTION))
diff --git a/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyDNOperation.java b/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyDNOperation.java
index eb93e30..9f1d87c 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyDNOperation.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyDNOperation.java
@@ -724,11 +724,10 @@
Control c = requestControls.get(i);
String oid = c.getOID();
- if (! AccessControlConfigManager.getInstance().
- getAccessControlHandler().isAllowed(entryDN, this, c))
+ if (!LocalBackendWorkflowElement.isControlAllowed(entryDN, this, c))
{
- throw new DirectoryException(ResultCode.INSUFFICIENT_ACCESS_RIGHTS,
- ERR_CONTROL_INSUFFICIENT_ACCESS_RIGHTS.get(oid));
+ // Skip disallowed non-critical controls.
+ continue;
}
if (oid.equals(OID_LDAP_ASSERTION))
diff --git a/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java b/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java
index 609c59b..fd779b0 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java
@@ -763,14 +763,12 @@
Control c = requestControls.get(i);
String oid = c.getOID();
- if (! AccessControlConfigManager.getInstance().
- getAccessControlHandler().isAllowed(entryDN, this, c))
+ if (!LocalBackendWorkflowElement.isControlAllowed(entryDN, this, c))
{
- throw new DirectoryException(ResultCode.INSUFFICIENT_ACCESS_RIGHTS,
- ERR_CONTROL_INSUFFICIENT_ACCESS_RIGHTS.get(oid));
+ // Skip disallowed non-critical controls.
+ continue;
}
-
if (oid.equals(OID_LDAP_ASSERTION))
{
LDAPAssertionRequestControl assertControl =
diff --git a/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java b/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java
index 7cfc1eb..52b155c 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java
@@ -23,7 +23,7 @@
*
*
* Copyright 2008-2010 Sun Microsystems, Inc.
- * Portions copyright 2011 ForgeRock AS
+ * Portions copyright 2011-2012 ForgeRock AS
*/
package org.opends.server.workflowelement.localbackend;
@@ -350,11 +350,11 @@
{
Control c = requestControls.get(i);
String oid = c.getOID();
- if (! AccessControlConfigManager.getInstance().
- getAccessControlHandler().isAllowed(baseDN, this, c))
+
+ if (!LocalBackendWorkflowElement.isControlAllowed(baseDN, this, c))
{
- throw new DirectoryException(ResultCode.INSUFFICIENT_ACCESS_RIGHTS,
- ERR_CONTROL_INSUFFICIENT_ACCESS_RIGHTS.get(oid));
+ // Skip disallowed non-critical controls.
+ continue;
}
if (oid.equals(OID_LDAP_ASSERTION))
diff --git a/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendWorkflowElement.java b/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendWorkflowElement.java
index 774b34a..3d70947 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendWorkflowElement.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendWorkflowElement.java
@@ -51,6 +51,7 @@
import org.opends.server.types.*;
import org.opends.server.workflowelement.LeafWorkflowElement;
+import static org.opends.messages.CoreMessages.*;
import static org.opends.server.config.ConfigConstants.*;
@@ -325,6 +326,47 @@
/**
+ * Determine whether or not the provided request control is permitted by the
+ * access control policy. If it is not allowed, then abort the operation if
+ * the control was critical, otherwise ignore it.
+ *
+ * @param targetDN
+ * The operation target DN.
+ * @param op
+ * The operation.
+ * @param control
+ * The request control.
+ * @return {@code true} if access is allowed, or {@code false} if access is
+ * not allowed, but the control is non-critical and should be ignored.
+ * @throws DirectoryException
+ * If access is not allowed and the control is critical.
+ */
+ static boolean isControlAllowed(DN targetDN, Operation op, Control control)
+ throws DirectoryException
+ {
+ if (!AccessControlConfigManager.getInstance().getAccessControlHandler()
+ .isAllowed(targetDN, op, control))
+ {
+ // 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()));
+ }
+ else
+ {
+ // We don't want the backend to process this non-critical control, so
+ // remove it.
+ op.removeRequestControl(control);
+ return false;
+ }
+ }
+ return true;
+ }
+
+
+
+ /**
* Adds the post-read response control to the response if requested.
*
* @param operation
diff --git a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/authorization/dseecompat/TargetControlTestCase.java b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/authorization/dseecompat/TargetControlTestCase.java
index 7b1b8ca..a97a965 100644
--- a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/authorization/dseecompat/TargetControlTestCase.java
+++ b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/authorization/dseecompat/TargetControlTestCase.java
@@ -23,6 +23,7 @@
*
*
* Copyright 2008-2009 Sun Microsystems, Inc.
+ * Portions copyright 2012 ForgeRock AS.
*/
@@ -276,11 +277,11 @@
LDAPSearchParams(level3User, PWD, null, null, null,
superUser, filter, "aclRights mail description", true,
true, 0);
- //This should fail since the both controls are not allowed for the
- //ou=admins, o=test suffix.
+ //This should succeed since both controls are not allowed for the
+ //ou=admins, o=test suffix, but both are critical.
LDAPSearchParams(superUser, PWD, null, null, null,
superUser, filter, "aclRights mail description", true,
- true, LDAPResultCode.INSUFFICIENT_ACCESS_RIGHTS);
+ true, 0);
deleteAttrFromEntry(peopleBase, "aci");
}
@@ -299,12 +300,12 @@
makeAddLDIF(ATTR_AUTHZ_GLOBAL_ACI, ACCESS_HANDLER_DN,
controlAdmin, controlPeople);
LDIFAdminModify(globalControlAcis, DIR_MGR_DN, PWD);
- //Fails because geteffectiverights control not allowed on
- //ou=people, o=test
+ //Succeeds because geteffectiverights control is not allowed on
+ //ou=people, o=test, but it is non-critical.
LDAPSearchParams(level3User, PWD, null,
"dn: " + level1User, null,
level1User, filter, "aclRights mail description",
- false, false, LDAPResultCode.INSUFFICIENT_ACCESS_RIGHTS);
+ false, false, 0);
//Ok because geteffectiverights control is allowed on
//ou=admin, o=test
LDAPSearchParams(level3User, PWD, null,
@@ -318,11 +319,11 @@
String addEntryLDIF=makeAddEntryLDIF(newPeopleDN, newEntry);
LDIFAdd(addEntryLDIF, superUser, PWD, controlStr,
LDAPResultCode.PROTOCOL_ERROR);
- //Test add to ou=admin, o=test with assertion control,
+ //Test add to ou=admin, o=test with assertion control, and critical
//should get access denied since this control is not allowed.
String addEntryLDIF1=makeAddEntryLDIF(newAdminDN, newEntry);
LDIFAdd(addEntryLDIF1, superUser, PWD, controlStr,
- LDAPResultCode.INSUFFICIENT_ACCESS_RIGHTS);
+ LDAPResultCode.UNAVAILABLE_CRITICAL_EXTENSION);
deleteAttrFromAdminEntry(ACCESS_HANDLER_DN, ATTR_AUTHZ_GLOBAL_ACI);
}
@@ -344,7 +345,7 @@
LDAPSearchParams(superUser, PWD, null,
"dn: " + superUser, null,
base, filter, "aclRights mail description", false, false,
- LDAPResultCode.INSUFFICIENT_ACCESS_RIGHTS);
+ 0 /* disallowed but non-critical */);
LDIFModify(aciRight, superUser, PWD, OID_LDAP_READENTRY_PREREAD,
LDAPResultCode.INSUFFICIENT_ACCESS_RIGHTS);
deleteAttrFromEntry (base, "aci");
--
Gitblit v1.10.0