From 5cdce74a9ac94e3a4b5e369b22ab6fcf9bbbb384 Mon Sep 17 00:00:00 2001
From: abobrov <abobrov@localhost>
Date: Mon, 18 May 2009 23:17:06 +0000
Subject: [PATCH] - patch [Issue 3984] & [Issue 3989] : Security issues with Assertion, Pre-Read, Post-Read Controls.
---
opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java | 29 ++++-
opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java | 12 ++
opends/src/server/org/opends/server/core/DefaultAccessControlHandler.java | 24 ++++
opends/src/server/org/opends/server/workflowelement/ndb/NDBSearchOperation.java | 12 ++
opends/src/server/org/opends/server/authorization/dseecompat/AciLDAPOperationContainer.java | 16 +++
opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyDNOperation.java | 31 ++++--
opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendCompareOperation.java | 13 ++
opends/src/server/org/opends/server/api/AccessControlHandler.java | 42 ++++++++
opends/src/server/org/opends/server/authorization/dseecompat/AciHandler.java | 45 +++++---
opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendDeleteOperation.java | 22 +++-
opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java | 27 ++++-
11 files changed, 218 insertions(+), 55 deletions(-)
diff --git a/opends/src/server/org/opends/server/api/AccessControlHandler.java b/opends/src/server/org/opends/server/api/AccessControlHandler.java
index ccb590a..6f5f534 100644
--- a/opends/src/server/org/opends/server/api/AccessControlHandler.java
+++ b/opends/src/server/org/opends/server/api/AccessControlHandler.java
@@ -319,6 +319,30 @@
/**
+ * Indicates whether the provided operation search filter is allowed
+ * based on the access control configuration. This method should not
+ * alter the provided operation in any way.
+ *
+ * @param operation
+ * The operation for which to make the determination.
+ * @param entry
+ * The entry for which to make the determination.
+ * @param filter
+ * The filter to check access on.
+ * @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(Operation operation, Entry entry,
+ SearchFilter filter) throws DirectoryException;
+
+
+
+ /**
* Indicates whether the provided search result entry may be sent to
* the client. Implementations <b>must not under any
* circumstances</b> modify the search entry in any way.
@@ -357,6 +381,24 @@
/**
+ * Filter the contents of the provided entry such that it no longer
+ * contains any attributes or values that the client is not
+ * permitted to access.
+ *
+ * @param operation
+ * The operation with which the provided entry is
+ * associated.
+ * @param entry
+ * The entry to be filtered.
+ * @return Returns the entry with filtered attributes and values
+ * removed.
+ */
+ public abstract SearchResultEntry filterEntry(
+ Operation operation, Entry entry);
+
+
+
+ /**
* Indicates whether the provided search result reference may be
* sent to the client based on the access control configuration.
*
diff --git a/opends/src/server/org/opends/server/authorization/dseecompat/AciHandler.java b/opends/src/server/org/opends/server/authorization/dseecompat/AciHandler.java
index 829d1e9..c88ec16 100644
--- a/opends/src/server/org/opends/server/authorization/dseecompat/AciHandler.java
+++ b/opends/src/server/org/opends/server/authorization/dseecompat/AciHandler.java
@@ -230,24 +230,8 @@
- /*
- * TODO Rename this method. Needs to be changed in SearchOperation. I
- * find the name of the filterEntry method to be misleading because it
- * works on a search operation but has nothing to do with the search
- * filter. Something like "removeDisallowedAttributes" would be
- * clearer.
- */
-
/**
- * Checks access on each attribute in an entry. It removes those
- * attributes that fail access check.
- *
- * @param operation
- * The search operation class containing information to check
- * access on.
- * @param entry
- * The entry containing the attributes.
- * @return The entry to return minus filtered attributes.
+ * {@inheritDoc}
*/
@Override
public SearchResultEntry filterEntry(SearchOperation operation,
@@ -284,6 +268,19 @@
/**
* {@inheritDoc}
*/
+ @Override
+ public SearchResultEntry filterEntry(Operation operation, Entry entry)
+ {
+ AciLDAPOperationContainer operationContainer =
+ new AciLDAPOperationContainer(operation, (ACI_READ), entry);
+ return accessAllowedAttrs(operationContainer);
+ }
+
+
+
+ /**
+ * {@inheritDoc}
+ */
@Override()
public void finalizeAccessControlHandler()
{
@@ -561,6 +558,20 @@
* {@inheritDoc}
*/
@Override
+ public boolean isAllowed(Operation operation, Entry entry,
+ SearchFilter filter) throws DirectoryException
+ {
+ AciLDAPOperationContainer operationContainer =
+ new AciLDAPOperationContainer(operation, (ACI_READ), entry);
+ return testFilter(operationContainer, filter);
+ }
+
+
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
public boolean mayProxy(Entry proxyUser, Entry proxiedUser,
Operation op)
{
diff --git a/opends/src/server/org/opends/server/authorization/dseecompat/AciLDAPOperationContainer.java b/opends/src/server/org/opends/server/authorization/dseecompat/AciLDAPOperationContainer.java
index 8354477..6e14b0b 100644
--- a/opends/src/server/org/opends/server/authorization/dseecompat/AciLDAPOperationContainer.java
+++ b/opends/src/server/org/opends/server/authorization/dseecompat/AciLDAPOperationContainer.java
@@ -51,6 +51,20 @@
private List<Modification> modifications;
/**
+ * Constructor interface for all currently supported LDAP operations.
+ * @param operation The compare operation to evaluate.
+ * @param rights The rights of a compare operation.
+ * @param entry The entry for evaluation.
+ */
+ public AciLDAPOperationContainer(Operation operation,
+ int rights, Entry entry)
+ {
+ super(operation, rights, entry);
+ this.searchEntry = new SearchResultEntry(entry);
+ }
+
+
+ /**
* Constructor interface for the compare operation.
* @param operation The compare operation to evaluate.
* @param rights The rights of a compare operation.
@@ -58,7 +72,7 @@
public AciLDAPOperationContainer(LocalBackendCompareOperation operation,
int rights)
{
- super(operation, rights, operation.getEntryToCompare());
+ super(operation, rights, operation.getEntryToCompare());
}
diff --git a/opends/src/server/org/opends/server/core/DefaultAccessControlHandler.java b/opends/src/server/org/opends/server/core/DefaultAccessControlHandler.java
index c634bd2..9b8d633 100644
--- a/opends/src/server/org/opends/server/core/DefaultAccessControlHandler.java
+++ b/opends/src/server/org/opends/server/core/DefaultAccessControlHandler.java
@@ -192,6 +192,18 @@
* {@inheritDoc}
*/
@Override
+ public boolean isAllowed(Operation operation, Entry entry,
+ SearchFilter filter) throws DirectoryException
+ {
+ return true;
+ }
+
+
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
public boolean maySend(SearchOperation searchOperation,
SearchResultEntry searchEntry)
{
@@ -216,6 +228,18 @@
* {@inheritDoc}
*/
@Override
+ public SearchResultEntry filterEntry(Operation operation, Entry entry)
+ {
+ // No implementation required.
+ return new SearchResultEntry(entry);
+ }
+
+
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
public boolean maySend(DN dn, SearchOperation searchOperation,
SearchResultReference searchReference)
{
diff --git a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java
index bb0ace1..118297f 100644
--- a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java
+++ b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendAddOperation.java
@@ -1464,14 +1464,26 @@
if (oid.equals(OID_LDAP_ASSERTION))
{
+ // RFC 4528 mandates support for Add operation basically
+ // suggesting an asertion on self. As daft as it may be
+ // we gonna have to support this for RFC compliance.
LDAPAssertionRequestControl assertControl =
getRequestControl(LDAPAssertionRequestControl.DECODER);
try
{
- // FIXME -- We need to determine whether the current user has
- // permission to make this determination.
SearchFilter filter = assertControl.getSearchFilter();
+
+ // Check if the current user has permission to make
+ // this determination.
+ if (!AccessControlConfigManager.getInstance().
+ getAccessControlHandler().isAllowed(this, entry, filter))
+ {
+ throw new DirectoryException(
+ ResultCode.INSUFFICIENT_ACCESS_RIGHTS,
+ ERR_CONTROL_INSUFFICIENT_ACCESS_RIGHTS.get(oid));
+ }
+
if (! filter.matchesEntry(entry))
{
throw new DirectoryException(ResultCode.ASSERTION_FAILED,
@@ -1620,12 +1632,13 @@
}
}
- // FIXME -- Check access controls on the entry to see if it should
- // be returned or if any attributes need to be stripped
- // out..
- SearchResultEntry searchEntry = new SearchResultEntry(addedEntry);
+ // Check access controls on the entry and strip out
+ // any not allowed attributes.
+ SearchResultEntry searchEntry =
+ AccessControlConfigManager.getInstance().
+ getAccessControlHandler().filterEntry(this, addedEntry);
LDAPPostReadResponseControl responseControl =
- new LDAPPostReadResponseControl(searchEntry);
+ new LDAPPostReadResponseControl(searchEntry);
addResponseControl(responseControl);
}
}
diff --git a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendCompareOperation.java b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendCompareOperation.java
index aab4329..5a67494 100644
--- a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendCompareOperation.java
+++ b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendCompareOperation.java
@@ -448,9 +448,18 @@
try
{
- // FIXME -- We need to determine whether the current user has
- // permission to make this determination.
SearchFilter filter = assertControl.getSearchFilter();
+
+ // Check if the current user has permission to make
+ // this determination.
+ if (!AccessControlConfigManager.getInstance().
+ getAccessControlHandler().isAllowed(this, entry, filter))
+ {
+ throw new DirectoryException(
+ ResultCode.INSUFFICIENT_ACCESS_RIGHTS,
+ ERR_CONTROL_INSUFFICIENT_ACCESS_RIGHTS.get(oid));
+ }
+
if (! filter.matchesEntry(entry))
{
throw new DirectoryException(ResultCode.ASSERTION_FAILED,
diff --git a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendDeleteOperation.java b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendDeleteOperation.java
index 6b60609..aea75df 100644
--- a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendDeleteOperation.java
+++ b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendDeleteOperation.java
@@ -532,9 +532,18 @@
try
{
- // FIXME -- We need to determine whether the current user has
- // permission to make this determination.
SearchFilter filter = assertControl.getSearchFilter();
+
+ // Check if the current user has permission to make
+ // this determination.
+ if (!AccessControlConfigManager.getInstance().
+ getAccessControlHandler().isAllowed(this, entry, filter))
+ {
+ throw new DirectoryException(
+ ResultCode.INSUFFICIENT_ACCESS_RIGHTS,
+ ERR_CONTROL_INSUFFICIENT_ACCESS_RIGHTS.get(oid));
+ }
+
if (! filter.matchesEntry(entry))
{
throw new DirectoryException(ResultCode.ASSERTION_FAILED,
@@ -680,10 +689,11 @@
}
}
- // FIXME -- Check access controls on the entry to see if it should
- // be returned or if any attributes need to be stripped
- // out..
- SearchResultEntry searchEntry = new SearchResultEntry(entryCopy);
+ // Check access controls on the entry and strip out
+ // any not allowed attributes.
+ SearchResultEntry searchEntry =
+ AccessControlConfigManager.getInstance().
+ getAccessControlHandler().filterEntry(this, entryCopy);
LDAPPreReadResponseControl responseControl =
new LDAPPreReadResponseControl(preReadRequest.isCritical(),
searchEntry);
diff --git a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyDNOperation.java b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyDNOperation.java
index 451d4f1..e53dcc2 100644
--- a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyDNOperation.java
+++ b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyDNOperation.java
@@ -756,9 +756,18 @@
try
{
- // FIXME -- We need to determine whether the current user has
- // permission to make this determination.
SearchFilter filter = assertControl.getSearchFilter();
+
+ // Check if the current user has permission to make
+ // this determination.
+ if (!AccessControlConfigManager.getInstance().
+ getAccessControlHandler().isAllowed(this, currentEntry, filter))
+ {
+ throw new DirectoryException(
+ ResultCode.INSUFFICIENT_ACCESS_RIGHTS,
+ ERR_CONTROL_INSUFFICIENT_ACCESS_RIGHTS.get(oid));
+ }
+
if (! filter.matchesEntry(currentEntry))
{
throw new DirectoryException(ResultCode.ASSERTION_FAILED,
@@ -1095,10 +1104,11 @@
}
}
- // FIXME -- Check access controls on the entry to see if it should
- // be returned or if any attributes need to be stripped
- // out..
- SearchResultEntry searchEntry = new SearchResultEntry(entry);
+ // Check access controls on the entry and strip out
+ // any not allowed attributes.
+ SearchResultEntry searchEntry =
+ AccessControlConfigManager.getInstance().
+ getAccessControlHandler().filterEntry(this, entry);
LDAPPreReadResponseControl responseControl =
new LDAPPreReadResponseControl(preReadRequest.isCritical(),
searchEntry);
@@ -1145,10 +1155,11 @@
}
}
- // FIXME -- Check access controls on the entry to see if it should
- // be returned or if any attributes need to be stripped
- // out..
- SearchResultEntry searchEntry = new SearchResultEntry(entry);
+ // Check access controls on the entry and strip out
+ // any not allowed attributes.
+ SearchResultEntry searchEntry =
+ AccessControlConfigManager.getInstance().
+ getAccessControlHandler().filterEntry(this, entry);
LDAPPostReadResponseControl responseControl =
new LDAPPostReadResponseControl(searchEntry);
diff --git a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java
index 0431fb0..e468240 100644
--- a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java
+++ b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java
@@ -782,9 +782,18 @@
try
{
- // FIXME -- We need to determine whether the current user has
- // permission to make this determination.
SearchFilter filter = assertControl.getSearchFilter();
+
+ // Check if the current user has permission to make
+ // this determination.
+ if (!AccessControlConfigManager.getInstance().
+ getAccessControlHandler().isAllowed(this, currentEntry, filter))
+ {
+ throw new DirectoryException(
+ ResultCode.INSUFFICIENT_ACCESS_RIGHTS,
+ ERR_CONTROL_INSUFFICIENT_ACCESS_RIGHTS.get(oid));
+ }
+
if (! filter.matchesEntry(currentEntry))
{
throw new DirectoryException(ResultCode.ASSERTION_FAILED,
@@ -2115,9 +2124,11 @@
}
}
- // FIXME -- Check access controls on the entry to see if it should be
- // returned or if any attributes need to be stripped out..
- SearchResultEntry searchEntry = new SearchResultEntry(entry);
+ // Check access controls on the entry and strip out
+ // any not allowed attributes.
+ SearchResultEntry searchEntry =
+ AccessControlConfigManager.getInstance().
+ getAccessControlHandler().filterEntry(this, entry);
LDAPPreReadResponseControl responseControl =
new LDAPPreReadResponseControl(preReadRequest.isCritical(),
searchEntry);
@@ -2163,9 +2174,11 @@
}
}
- // FIXME -- Check access controls on the entry to see if it should be
- // returned or if any attributes need to be stripped out..
- SearchResultEntry searchEntry = new SearchResultEntry(entry);
+ // Check access controls on the entry and strip out
+ // any not allowed attributes.
+ SearchResultEntry searchEntry =
+ AccessControlConfigManager.getInstance().
+ getAccessControlHandler().filterEntry(this, entry);
LDAPPostReadResponseControl responseControl =
new LDAPPostReadResponseControl(searchEntry);
diff --git a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java
index b80adaf..b80325c 100644
--- a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java
+++ b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendSearchOperation.java
@@ -361,8 +361,6 @@
try
{
- // FIXME -- We need to determine whether the current user has
- // permission to make this determination.
SearchFilter assertionFilter = assertControl.getSearchFilter();
Entry entry;
try
@@ -387,6 +385,16 @@
ERR_SEARCH_NO_SUCH_ENTRY_FOR_ASSERTION.get());
}
+ // Check if the current user has permission to make
+ // this determination.
+ if (!AccessControlConfigManager.getInstance().
+ getAccessControlHandler().isAllowed(this, entry, assertionFilter))
+ {
+ throw new DirectoryException(
+ ResultCode.INSUFFICIENT_ACCESS_RIGHTS,
+ ERR_CONTROL_INSUFFICIENT_ACCESS_RIGHTS.get(oid));
+ }
+
if (! assertionFilter.matchesEntry(entry))
{
throw new DirectoryException(ResultCode.ASSERTION_FAILED,
diff --git a/opends/src/server/org/opends/server/workflowelement/ndb/NDBSearchOperation.java b/opends/src/server/org/opends/server/workflowelement/ndb/NDBSearchOperation.java
index 690cbcc..fcc42d3 100644
--- a/opends/src/server/org/opends/server/workflowelement/ndb/NDBSearchOperation.java
+++ b/opends/src/server/org/opends/server/workflowelement/ndb/NDBSearchOperation.java
@@ -304,8 +304,6 @@
try
{
- // FIXME -- We need to determine whether the current user has
- // permission to make this determination.
SearchFilter assertionFilter = assertControl.getSearchFilter();
Entry entry;
try
@@ -330,6 +328,16 @@
ERR_SEARCH_NO_SUCH_ENTRY_FOR_ASSERTION.get());
}
+ // Check if the current user has permission to make
+ // this determination.
+ if (!AccessControlConfigManager.getInstance().
+ getAccessControlHandler().isAllowed(this, entry, assertionFilter))
+ {
+ throw new DirectoryException(
+ ResultCode.INSUFFICIENT_ACCESS_RIGHTS,
+ ERR_CONTROL_INSUFFICIENT_ACCESS_RIGHTS.get(oid));
+ }
+
if (! assertionFilter.matchesEntry(entry))
{
throw new DirectoryException(ResultCode.ASSERTION_FAILED,
--
Gitblit v1.10.0