Add ACP checking to exists operations. (#2790)

This commit is contained in:
Firstyear 2024-05-24 13:28:01 +10:00 committed by GitHub
parent 3723abb25d
commit 1e1414b38b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 47 additions and 11 deletions

View file

@ -235,28 +235,28 @@ pub trait AccessControlsTransaction<'a> {
// }
}
// Contains all the way to eval acps to entries
#[instrument(level = "debug", name = "access::search_filter_entries", skip_all)]
fn search_filter_entries(
#[instrument(level = "debug", name = "access::filter_entries", skip_all)]
fn filter_entries(
&self,
se: &SearchEvent,
ident: &Identity,
filter_orig: &Filter<FilterValid>,
entries: Vec<Arc<EntrySealedCommitted>>,
) -> Result<Vec<Arc<EntrySealedCommitted>>, OperationError> {
// Prepare some shared resources.
// Get the set of attributes requested by this se filter. This is what we are
// going to access check.
let requested_attrs: BTreeSet<&str> = se.filter_orig.get_attr_set();
let requested_attrs: BTreeSet<&str> = filter_orig.get_attr_set();
// First get the set of acps that apply to this receiver
let related_acp = self.search_related_acp(&se.ident);
let related_acp = self.search_related_acp(ident);
// For each entry.
let entries_is_empty = entries.is_empty();
let allowed_entries: Vec<_> = entries
.into_iter()
.filter(|e| {
match apply_search_access(&se.ident, related_acp.as_slice(), e) {
match apply_search_access(ident, related_acp.as_slice(), e) {
SearchResult::Denied => false,
SearchResult::Grant => true,
SearchResult::Allow(allowed_attrs) => {
@ -285,6 +285,16 @@ pub trait AccessControlsTransaction<'a> {
Ok(allowed_entries)
}
// Contains all the way to eval acps to entries
#[inline(always)]
fn search_filter_entries(
&self,
se: &SearchEvent,
entries: Vec<Arc<EntrySealedCommitted>>,
) -> Result<Vec<Arc<EntrySealedCommitted>>, OperationError> {
self.filter_entries(&se.ident, &se.filter_orig, entries)
}
#[instrument(
level = "debug",
name = "access::search_filter_entry_attributes",

View file

@ -312,10 +312,36 @@ pub trait QueryServerTransaction<'a> {
let lims = ee.get_limits();
be_txn.exists(lims, &vfr).map_err(|e| {
admin_error!(?e, "backend failure");
OperationError::Backend
})
if ee.ident.is_internal() {
// We take a fast-path on internal because we can skip loading entries
// at all in this case.
be_txn.exists(lims, &vfr).map_err(|e| {
admin_error!(?e, "backend failure");
OperationError::Backend
})
} else {
// For external idents, we need to load the entries else we can't apply
// access controls to them.
let res = self.get_be_txn().search(lims, &vfr).map_err(|e| {
admin_error!(?e, "backend failure");
OperationError::Backend
})?;
// ⚠️ Compare / Exists is annoying security wise. It has the
// capability to easily leak information based on comparisons
// that have been made. In the external account case, we need
// to filter entries as a result.
// Apply ACP before we return the bool state.
let access = self.get_accesscontrols();
access
.filter_entries(&ee.ident, &ee.filter_orig, res)
.map_err(|e| {
admin_error!(?e, "Unable to access filter entries");
e
})
.map(|entries| !entries.is_empty())
}
}
fn name_to_uuid(&mut self, name: &str) -> Result<Uuid, OperationError> {