From 212c466106004fd6723dfe4f6cd574ab51a8fba3 Mon Sep 17 00:00:00 2001
From: neil_a_wilson <neil_a_wilson@localhost>
Date: Fri, 01 Jun 2007 19:43:05 +0000
Subject: [PATCH] Update the way that privileges are evaluated by the server. Previously, they were always based on the authentication identity rather than the authorization identity. This means that when the two are different, the result could be incorrect. One key example of this is the use of the proxied authorization control by a root user. In this case, the proxied authorization would not be subject to access control because the authenticated user (but not the authorized user) had the bypass-acl privilege.
---
opendj-sdk/opends/src/server/org/opends/server/api/ClientConnection.java | 110 ++++++++++++++++++++------
opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/core/TestModifyDNOperation.java | 16 ++-
opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/core/CompareOperationTestCase.java | 10 +
opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciContainer.java | 5 +
opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/types/PrivilegeTestCase.java | 40 +++++++--
5 files changed, 133 insertions(+), 48 deletions(-)
diff --git a/opendj-sdk/opends/src/server/org/opends/server/api/ClientConnection.java b/opendj-sdk/opends/src/server/org/opends/server/api/ClientConnection.java
index bc8b7e4..46475d5 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/api/ClientConnection.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/api/ClientConnection.java
@@ -837,7 +837,7 @@
if (authenticationInfo == null)
{
this.authenticationInfo = new AuthenticationInfo();
- privileges = new HashSet<Privilege>();
+ updatePrivileges(null, false);
}
else
{
@@ -861,8 +861,6 @@
DirectoryServer.getAuthenticatedUsers().put(
authZEntry.getDN(), this);
}
-
- updatePrivileges(authNEntry, authenticationInfo.isRoot());
}
else
{
@@ -871,9 +869,9 @@
DirectoryServer.getAuthenticatedUsers().put(
authZEntry.getDN(), this);
}
-
- privileges = new HashSet<Privilege>();
}
+
+ updatePrivileges(authZEntry, authenticationInfo.isRoot());
}
}
@@ -954,11 +952,23 @@
public boolean hasPrivilege(Privilege privilege,
Operation operation)
{
- boolean result = privileges.contains(privilege);
+ boolean result;
- if (debugEnabled())
+ if (privilege == Privilege.PROXIED_AUTH)
{
- if (operation == null)
+ // This determination should always be made against the
+ // authentication identity rather than the authorization
+ // identity.
+ Entry authEntry = authenticationInfo.getAuthenticationEntry();
+ boolean isRoot = authenticationInfo.isRoot();
+ return getPrivileges(authEntry,
+ isRoot).contains(Privilege.PROXIED_AUTH);
+ }
+
+ if (operation == null)
+ {
+ result = privileges.contains(privilege);
+ if (debugEnabled())
{
DN authDN = authenticationInfo.getAuthenticationDN();
@@ -968,16 +978,39 @@
privilege.getName(), result);
TRACER.debugMessage(DebugLogLevel.INFO, message);
}
+ }
+ else
+ {
+ if (operation.getAuthorizationDN().equals(
+ authenticationInfo.getAuthorizationDN()))
+ {
+ result = privileges.contains(privilege);
+ if (debugEnabled())
+ {
+ DN authDN = authenticationInfo.getAuthenticationDN();
+
+ int msgID = MSGID_CLIENTCONNECTION_AUDIT_HASPRIVILEGE;
+ String message = getMessage(msgID, getConnectionID(),
+ operation.getOperationID(),
+ String.valueOf(authDN),
+ privilege.getName(), result);
+ TRACER.debugMessage(DebugLogLevel.INFO, message);
+ }
+ }
else
{
- DN authDN = authenticationInfo.getAuthenticationDN();
-
- int msgID = MSGID_CLIENTCONNECTION_AUDIT_HASPRIVILEGE;
- String message = getMessage(msgID, getConnectionID(),
- operation.getOperationID(),
- String.valueOf(authDN),
- privilege.getName(), result);
- TRACER.debugMessage(DebugLogLevel.INFO, message);
+ Entry authorizationEntry = operation.getAuthorizationEntry();
+ if (authorizationEntry == null)
+ {
+ result = false;
+ }
+ else
+ {
+ boolean isRoot =
+ DirectoryServer.isRootDN(authorizationEntry.getDN());
+ result = getPrivileges(authorizationEntry,
+ isRoot).contains(privilege);
+ }
}
}
@@ -1069,18 +1102,24 @@
/**
- * Updates the privileges associated with this client connection
- * object based on the provided entry for the authentication
- * identity.
+ * Retrieves the set of privileges encoded in the provided entry.
*
- * @param entry The entry for the authentication identity
- * associated with this client connection.
- * @param isRoot Indicates whether the associated user is a root
- * user and should automatically inherit the root
+ * @param entry The entry to use to obtain the privilege
+ * information.
+ * @param isRoot Indicates whether the set of root privileges
+ * should be automatically included in the
* privilege set.
+ *
+ * @return A set of the privileges that should be assigned.
*/
- private void updatePrivileges(Entry entry, boolean isRoot)
+ private HashSet<Privilege> getPrivileges(Entry entry,
+ boolean isRoot)
{
+ if (entry == null)
+ {
+ return new HashSet<Privilege>(0);
+ }
+
HashSet<Privilege> newPrivileges = new HashSet<Privilege>();
HashSet<Privilege> removePrivileges = new HashSet<Privilege>();
@@ -1115,8 +1154,7 @@
// We don't know what privilege to remove, so we'll
// remove all of them.
newPrivileges.clear();
- privileges = newPrivileges;
- return;
+ return newPrivileges;
}
else
{
@@ -1144,7 +1182,25 @@
newPrivileges.remove(p);
}
- privileges = newPrivileges;
+ return newPrivileges;
+ }
+
+
+
+ /**
+ * Updates the privileges associated with this client connection
+ * object based on the provided entry for the authentication
+ * identity.
+ *
+ * @param entry The entry for the authentication identity
+ * associated with this client connection.
+ * @param isRoot Indicates whether the associated user is a root
+ * user and should automatically inherit the root
+ * privilege set.
+ */
+ private void updatePrivileges(Entry entry, boolean isRoot)
+ {
+ privileges = getPrivileges(entry, isRoot);
}
diff --git a/opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciContainer.java b/opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciContainer.java
index 52537f4..ff7da54 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciContainer.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/authorization/dseecompat/AciContainer.java
@@ -654,7 +654,10 @@
if(this.useAuthzid)
return this.authzid;
else
- return this.authorizationEntry.getDN();
+ if (this.authorizationEntry == null)
+ return DN.nullDN();
+ else
+ return this.authorizationEntry.getDN();
}
/**
diff --git a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/core/CompareOperationTestCase.java b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/core/CompareOperationTestCase.java
index 6664ae4..77b150f 100644
--- a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/core/CompareOperationTestCase.java
+++ b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/core/CompareOperationTestCase.java
@@ -97,7 +97,9 @@
"givenname;lang-en: Rodney",
"sn;lang-en: Ogasawara",
"cn;lang-en: Rodney Ogasawara",
- "title;lang-en: Sales, Director"
+ "title;lang-en: Sales, Director",
+ "aci: (targetattr=\"*\")(version 3.0; acl \"Proxy Rights\"; " +
+ "allow(proxy) userdn=\"ldap:///uid=proxy.user,o=test\";)"
);
AddOperation addOperation =
connection.processAdd(entry.getDN(),
@@ -483,7 +485,8 @@
InvocationCounterPlugin.resetAllCounters();
ProxiedAuthV1Control authV1Control =
- new ProxiedAuthV1Control(new ASN1OctetString());
+ new ProxiedAuthV1Control(new ASN1OctetString(
+ "cn=Directory Manager,cn=Root DNs,cn=config"));
List<Control> controls = new ArrayList<Control>();
controls.add(authV1Control);
@@ -536,7 +539,8 @@
InvocationCounterPlugin.resetAllCounters();
ProxiedAuthV2Control authV2Control =
- new ProxiedAuthV2Control(new ASN1OctetString("dn:"));
+ new ProxiedAuthV2Control(new ASN1OctetString(
+ "dn:cn=Directory Manager,cn=Root DNs,cn=config"));
List<Control> controls = new ArrayList<Control>();
controls.add(authV2Control);
diff --git a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/core/TestModifyDNOperation.java b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/core/TestModifyDNOperation.java
index 1e1eb42..57aec3b 100644
--- a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/core/TestModifyDNOperation.java
+++ b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/core/TestModifyDNOperation.java
@@ -63,6 +63,7 @@
public void setUp() throws Exception
{
TestCaseUtils.startServer();
+ TestCaseUtils.initializeTestBackend(true);
TestCaseUtils.clearJEBackend(false, "userRoot", "dc=example,dc=com");
InternalClientConnection connection =
@@ -73,7 +74,9 @@
"dn: dc=example,dc=com",
"objectclass: top",
"objectclass: domain",
- "dc: example"
+ "dc: example",
+ "aci: (targetattr=\"*\")(version 3.0; acl \"Proxy Rights\"; " +
+ "allow(proxy) userdn=\"ldap:///uid=proxy.user,o=test\";)"
);
// Add the people entry
@@ -825,8 +828,8 @@
@Test
public void testRawProxyAuthV1Modify() throws Exception
{
- ProxiedAuthV1Control authV1Control =
- new ProxiedAuthV1Control(new ASN1OctetString());
+ ProxiedAuthV1Control authV1Control = new ProxiedAuthV1Control(
+ new ASN1OctetString("cn=Directory Manager,cn=Root DNs,cn=config"));
List<Control> controls = new ArrayList<Control>();
controls.add(authV1Control);
InvocationCounterPlugin.resetAllCounters();
@@ -884,8 +887,8 @@
@Test
public void testProcessedProxyAuthV1Modify() throws Exception
{
- ProxiedAuthV1Control authV1Control =
- new ProxiedAuthV1Control(new ASN1OctetString());
+ ProxiedAuthV1Control authV1Control = new ProxiedAuthV1Control(new ASN1OctetString(
+ "cn=Directory Manager,cn=Root DNs,cn=config"));
List<Control> controls = new ArrayList<Control>();
controls.add(authV1Control);
InvocationCounterPlugin.resetAllCounters();
@@ -969,7 +972,8 @@
public void testProcessedProxyAuthV2Modify() throws Exception
{
ProxiedAuthV2Control authV2Control =
- new ProxiedAuthV2Control(new ASN1OctetString("dn:"));
+ new ProxiedAuthV2Control(new ASN1OctetString(
+ "dn:cn=Directory Manager,cn=Root DNs,cn=config"));
List<Control> controls = new ArrayList<Control>();
controls.add(authV2Control);
InvocationCounterPlugin.resetAllCounters();
diff --git a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/types/PrivilegeTestCase.java b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/types/PrivilegeTestCase.java
index 1f6f3f5..2168245 100644
--- a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/types/PrivilegeTestCase.java
+++ b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/types/PrivilegeTestCase.java
@@ -201,9 +201,18 @@
"uid: pwreset.target",
"userPassword: password");
-// FIXME -- It will likely be necessary to also have access control rules in
-// place to allow operations as necessary once that functionality has
-// integrated into the server.
+ TestCaseUtils.applyModifications(
+ "dn: o=test",
+ "changetype: modify",
+ "add: aci",
+ "aci: (version 3.0; acl \"Proxy Root\"; allow (proxy) " +
+ "userdn=\"ldap:///cn=Proxy Root,cn=Root DNs,cn=config\";)",
+ "aci: (version 3.0; acl \"Unprivileged Root\"; allow (proxy) " +
+ "userdn=\"ldap:///cn=Unprivileged Root,cn=Root DNs,cn=config\";)",
+ "aci: (version 3.0; acl \"Privileged User\"; allow (proxy) " +
+ "userdn=\"ldap:///cn=Privileged User,o=test\";)",
+ "aci: (targetattr=\"*\")(version 3.0; acl \"PWReset Target\"; " +
+ "allow (all) userdn=\"ldap:///cn=PWReset Target,o=test\";)");
// Build the array of connections we will use to perform the tests.
@@ -1365,6 +1374,7 @@
// Try to add the entry. If this fails with the proxy control, then add it
// with a root connection so we can do other things with it.
+ DN authDN = conn.getAuthenticationInfo().getAuthenticationDN();
AddOperation addOperation =
new AddOperation(conn, conn.nextOperationID(), conn.nextMessageID(),
controls, e.getDN(), e.getObjectClasses(),
@@ -1373,12 +1383,14 @@
if (hasProxyPrivilege)
{
- assertEquals(addOperation.getResultCode(), ResultCode.SUCCESS);
+ assertEquals(addOperation.getResultCode(), ResultCode.SUCCESS,
+ "Unexpected add failure for user " + authDN);
}
else
{
assertEquals(addOperation.getResultCode(),
- ResultCode.AUTHORIZATION_DENIED);
+ ResultCode.AUTHORIZATION_DENIED,
+ "Unexpected add success for user " + authDN);
TestCaseUtils.addEntry(e);
}
@@ -1395,12 +1407,14 @@
if (hasProxyPrivilege)
{
- assertEquals(modifyOperation.getResultCode(), ResultCode.SUCCESS);
+ assertEquals(modifyOperation.getResultCode(), ResultCode.SUCCESS,
+ "Unexpected mod failure for user " + authDN);
}
else
{
assertEquals(modifyOperation.getResultCode(),
- ResultCode.AUTHORIZATION_DENIED);
+ ResultCode.AUTHORIZATION_DENIED,
+ "Unexpected mod success for user " + authDN);
}
@@ -1414,13 +1428,15 @@
DN newEntryDN;
if (hasProxyPrivilege)
{
- assertEquals(modifyDNOperation.getResultCode(), ResultCode.SUCCESS);
+ assertEquals(modifyDNOperation.getResultCode(), ResultCode.SUCCESS,
+ "Unexpected moddn failure for user " + authDN);
newEntryDN = modifyDNOperation.getUpdatedEntry().getDN();
}
else
{
assertEquals(modifyDNOperation.getResultCode(),
- ResultCode.AUTHORIZATION_DENIED);
+ ResultCode.AUTHORIZATION_DENIED,
+ "Unexpected moddn success for user " + authDN);
newEntryDN = e.getDN();
}
@@ -1434,12 +1450,14 @@
if (hasProxyPrivilege)
{
- assertEquals(deleteOperation.getResultCode(), ResultCode.SUCCESS);
+ assertEquals(deleteOperation.getResultCode(), ResultCode.SUCCESS,
+ "Unexpected delete failure for user " + authDN);
}
else
{
assertEquals(deleteOperation.getResultCode(),
- ResultCode.AUTHORIZATION_DENIED);
+ ResultCode.AUTHORIZATION_DENIED,
+ "Unexpected delete success for user " + authDN);
InternalClientConnection rootConnection =
InternalClientConnection.getRootConnection();
--
Gitblit v1.10.0