From 2efbc89a7236f5ed2a55dfa0b5b45e292d0e1181 Mon Sep 17 00:00:00 2001 From: gnuxie Date: Fri, 30 Sep 2022 13:32:43 +0100 Subject: [PATCH] initial access control rework --- src/models/AccessControlUnit.ts | 318 ++++++++++++++++++++++++++++++++ src/models/ListRule.ts | 46 +++++ src/models/PolicyList.ts | 31 +++- 3 files changed, 385 insertions(+), 10 deletions(-) create mode 100644 src/models/AccessControlUnit.ts diff --git a/src/models/AccessControlUnit.ts b/src/models/AccessControlUnit.ts new file mode 100644 index 0000000..46595b9 --- /dev/null +++ b/src/models/AccessControlUnit.ts @@ -0,0 +1,318 @@ +/* +Copyright 2019-2022 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import PolicyList, { ChangeType, ListRuleChange } from "./PolicyList"; +import { EntityType, ListRule, Recommendation, RULE_SERVER, RULE_USER } from "./ListRule"; +import { LogService, UserID } from "matrix-bot-sdk"; +import { ServerAcl } from "./ServerAcl"; + +/** + * The ListRuleCache is a cache for all the rules in a list for a specific entity type and recommendation. + * The cache can then be used to quickly test against all the rules for that specific entity/recommendation. + * E.g. The cache can be used for all the m.ban rules for users in a set of lists to conveniently test members of a room. + * While some effort has been made to optimize the testing of entities, the main purpose of this class is to stop + * ad-hoc destructuring of policy lists to test rules against entities. + * + * Note: This cache should not be used to unban or introspect about the state of `PolicyLists`, for this + * see `PolicyList.unban` and `PolicyList.rulesMatchingEntity`, as these will make sure to account + * for unnormalized entity types. + */ +class ListRuleCache { + /** + * Glob rules always have to be scanned against every entity. + */ + private readonly globRules: Map = new Map(); + /** + * This table allows us to skip matching an entity against every literal. + */ + private readonly literalRules: Map = new Map(); + private readonly listUpdateListener: ((list: PolicyList, changes: ListRuleChange[]) => void); + + constructor( + /** + * The entity type that this cache is for e.g. RULE_USER. + */ + public readonly entityType: EntityType, + /** + * The recommendation that this cache is for e.g. m.ban (RECOMMENDATION_BAN). + */ + public readonly recommendation: Recommendation, + ) { + this.listUpdateListener = (list: PolicyList, changes: ListRuleChange[]) => this.updateCache(changes); + } + + /** + * Test the entitiy for the first matching rule out of all the watched lists. + * @param entity e.g. an mxid for a user, the server name for a server. + * @returns A single `ListRule` matching the entity. + */ + public test(entity: string): ListRule|null { + const literalRule = this.literalRules.get(entity); + if (literalRule !== undefined) { + return literalRule[0]; + } + for (const rule of this.globRules.values()) { + if (rule[0].isMatch(entity)) { + return rule[0]; + } + } + return null; + } + + /** + * Watch a list and add all its rules (and future rules) to the cache. + * Will automatically update with the list. + * @param list A PolicyList. + */ + public watchList(list: PolicyList): void { + list.on('PolicyList.update', this.listUpdateListener); + const rules = list.rulesOfKind(this.entityType, this.recommendation); + rules.forEach(this.internRule, this); + } + + /** + * Unwatch a list and remove all of its rules from the cache. + * Will stop updating the cache from this list. + * @param list A PolicyList. + */ + public unwatchList(list: PolicyList): void { + list.removeListener('PolicyList.update', this.listUpdateListener); + const rules = list.rulesOfKind(this.entityType, this.recommendation); + rules.forEach(this.uninternRule, this); + } + + /** + * @returns True when there are no rules in the cache. + */ + public isEmpty(): boolean { + return this.globRules.size + this.literalRules.size === 0; + } + + /** + * Returns all the rules in the cache, without duplicates from different lists. + */ + public get allRules(): ListRule[] { + return [...this.literalRules.values(), ...this.globRules.values()].map(rules => rules[0]); + } + + /** + * Remove a rule from the cache as it is now invalid. e.g. it was removed from a policy list. + * @param rule The rule to remove. + */ + private uninternRule(rule: ListRule) { + /** + * Remove a rule from the map, there may be rules from different lists in the cache. + * We don't want to invalidate those. + * @param map A map of entities to rules. + */ + const removeRuleFromMap = (map: Map) => { + const entry = map.get(rule.entity); + if (entry !== undefined) { + const newEntry = entry.filter(internedRule => internedRule.sourceEvent.event_id !== rule.sourceEvent.event_id); + if (newEntry.length === 0) { + map.delete(rule.entity); + } else { + map.set(rule.entity, newEntry); + } + } + }; + if (rule.isGlob()) { + removeRuleFromMap(this.globRules); + } else { + removeRuleFromMap(this.literalRules); + } + } + + /** + * Add a rule to the cache e.g. it was added to a policy list. + * @param rule The rule to add. + */ + private internRule(rule: ListRule) { + /** + * Add a rule to the map, there might be duplicates of this rule in other lists. + * @param map A map of entities to rules. + */ + const addRuleToMap = (map: Map) => { + const entry = map.get(rule.entity); + if (entry !== undefined) { + entry.push(rule); + } else { + map.set(rule.entity, [rule]); + } + } + if (rule.isGlob()) { + addRuleToMap(this.globRules); + } else { + addRuleToMap(this.literalRules); + } + } + + /** + * Update the cache for a single `ListRuleChange`. + * @param change The change made to a rule that was present in the policy list. + */ + private updataCacheForChange(change: ListRuleChange): void { + if (change.rule.kind !== this.entityType || change.rule.recommendation !== this.recommendation) { + return; + } + switch (change.changeType) { + case ChangeType.Added: + case ChangeType.Modified: + this.internRule(change.rule); + break; + case ChangeType.Removed: + this.uninternRule(change.rule); + break; + default: + throw new TypeError(`Uknown ListRule change type: ${change.changeType}`); + } + } + + /** + * Update the cache for a change in a policy list. + * @param changes The changes that were made to list rules since the last update to this policy list. + */ + private updateCache(changes: ListRuleChange[]) { + changes.forEach(this.updataCacheForChange, this); + } +} + +export enum Access { + /// The entity was explicitly banned by a policy list. + Banned, + /// The entity did not match any allow rule. + NotAllowed, + /// The user was allowed implicitly (it matched an allow rule and did not match any banning rules). + Allowed, +} + +/** + * A description of the access an entity has. + * If the access is `Banned`, then a single rule that bans the entity will be included. + */ +export interface EntityAccess { + readonly outcome: Access, + readonly rule?: ListRule, +} + +/** + * This allows us to work the access an entity has to some thing based on a set of watched/unwatched lists. + */ +export default class AccessControlUnit { + private readonly userBans = new ListRuleCache(RULE_USER, Recommendation.Ban); + private readonly serverBans = new ListRuleCache(RULE_SERVER, Recommendation.Ban); + private readonly userAllows = new ListRuleCache(RULE_USER, Recommendation.Allow); + private readonly serverAllows = new ListRuleCache(RULE_SERVER, Recommendation.Allow); + private readonly caches = [this.userBans, this.serverBans, this.userAllows, this.serverAllows] + + constructor(policyLists: PolicyList[]) { + policyLists.forEach(this.watchList, this); + } + + public watchList(list: PolicyList) { + for (const cache of this.caches) { + cache.watchList(list); + } + } + + public unwatchList(list: PolicyList) { + for (const cache of this.caches) { + cache.watchList(list); + } + } + + /** + * Test whether the server is allowed by the ACL unit. + * @param domain The server name to test. + * @returns A description of the access that the server has. + */ + public testServer(domain: string): EntityAccess { + return this.testEntity(domain, this.serverAllows, this.serverBans); + } + + /** + * Test whether the user is allowed by the ACL unit. + * Does not test the domain of the user id. + * @param mxid The user id to test. + * @returns A description of the access that the user has. + */ + public testUserWithoutServer(mxid: string): EntityAccess { + return this.testEntity(mxid, this.userAllows, this.userBans); + } + + /** + * Test whether the user is allowed by the ACL unit. Does take the user's server into consideration. + * @param mxid The user id to test. + * @returns A description of the access that the user or their server has. + */ + public testUser(mxid: UserID): EntityAccess { + const userAccess = this.testUserWithoutServer(mxid.toString()); + if (userAccess.outcome === Access.Allowed) { + return this.testServer(mxid.domain); + } else { + return userAccess; + } + } + + private testEntity(entity: string, allowCache: ListRuleCache, bannedCache: ListRuleCache): EntityAccess { + // Check if the entity is explicitly allowed. + // We have to infer that a rule exists for '*' if the allowCache is empty, otherwise you brick the ACL. + const allowRule = allowCache.test(entity); + if (allowRule === null && !allowCache.isEmpty()) { + return { outcome: Access.NotAllowed } + } + // Now check if the entity is banned. + const banRule = bannedCache.test(entity); + if (banRule !== null) { + return { outcome: Access.Banned, rule: banRule }; + } + // If they got to this point, they're allowed!! + return { outcome: Access.Allowed }; + } + + /** + * Create a ServerAcl instance from the rules contained in this unit. + * @param serverName The name of the server that you are operating from, used to ensure you cannot brick yourself. + * @returns A new `ServerAcl` instance with deny and allow entries created from the rules in this unit. + */ + public createServerAcl(serverName: string) { + const acl = new ServerAcl(serverName).denyIpAddresses(); + const allowedServers = this.serverAllows.allRules; + // Allowed servers (allow). + if (allowedServers.length === 0) { + acl.allowServer('*'); + } else { + for (const rule of allowedServers) { + acl.allowServer(rule.entity); + } + if (this.testServer(serverName).outcome === Access.NotAllowed) { + acl.allowServer(serverName); + LogService.warn('AccessControlUnit', `The server ${serverName} we are operating from was not on the allowed when constructing the server ACL, so it will be injected it into the server acl. Please check the ACL lists.`) + } + } + // Banned servers (deny). + for (const rule of this.serverBans.allRules) { + if (rule.isMatch(serverName)) { + LogService.warn('AccessControlUnit', `The server ${serverName} we are operating from was found to be banned by ${rule.entity} by a rule from the event: ${rule.sourceEvent.event_id}, ` + + 'while constructing a server acl. Ignoring the rule. Please check the ACL lists.' + ); + } else { + acl.denyServer(rule.entity); + } + } + return acl; + } +} diff --git a/src/models/ListRule.ts b/src/models/ListRule.ts index 9ad0764..18988f5 100644 --- a/src/models/ListRule.ts +++ b/src/models/ListRule.ts @@ -55,6 +55,12 @@ export enum Recommendation { /// is considered absolutely absolutely perfect by whoever issued /// this ListRule. Opinion = "org.matrix.msc3845.opinion", + + /** + * This is a rule that recommends allowing a user to participate. + * Used for the construction of allow lists. + */ + Allow = "org.matrix.mjolnir.allow", } /** @@ -75,6 +81,11 @@ const RECOMMENDATION_OPINION_VARIANTS: string[] = [ Recommendation.Opinion ]; +const RECOMMENDATION_ALLOW_VARIANTS: string[] = [ + // Unstable + Recommendation.Allow +] + export const OPINION_MIN = -100; export const OPINION_MAX = +100; @@ -125,6 +136,13 @@ export abstract class ListRule { return this.glob.test(entity); } + /** + * @returns Whether the entity in he rule represents a Matrix glob (and not a literal). + */ + public isGlob(): boolean { + return /[*?]/.test(this.entity); + } + /** * Validate and parse an event into a ListRule. * @@ -173,6 +191,8 @@ export abstract class ListRule { return null; } return new ListRuleOpinion(event, entity, reason, kind, opinion); + } else if (RECOMMENDATION_ALLOW_VARIANTS.includes(recommendation)) { + return new ListRuleAllow(event, entity, reason, kind); } else { // As long as the `recommendation` is defined, we assume // that the rule is correct, just unknown. @@ -207,6 +227,32 @@ export class ListRuleBan extends ListRule { } } +/** + * A rule representing an "allow". + */ + export class ListRuleAllow extends ListRule { + constructor( + /** + * The event source for the rule. + */ + sourceEvent: MatrixStateEvent, + /** + * The entity covered by this rule, e.g. a glob user ID, a room ID, a server domain. + */ + entity: string, + /** + * A human-readable reason for this rule, for audit purposes. + */ + reason: string, + /** + * The type of entity for this rule, e.g. user, server domain, etc. + */ + kind: EntityType, + ) { + super(sourceEvent, entity, reason, kind, Recommendation.Allow) + } +} + /** * A rule representing an "opinion" */ diff --git a/src/models/PolicyList.ts b/src/models/PolicyList.ts index 8fc4113..ca3a8f0 100644 --- a/src/models/PolicyList.ts +++ b/src/models/PolicyList.ts @@ -86,6 +86,13 @@ class PolicyList extends EventEmitter { // Events that we have already informed the batcher about, that we haven't loaded from the room state yet. private batchedEvents = new Set(); + /** + * This is used to annotate state events we store with the rule they are associated with. + * If we refactor this, it is important to also refactor any listeners to 'PolicyList.update' + * which will assume `ListRule`s that are removed will be identital (Object.is) to when they were added. + */ + private static readonly EVENT_RULE_ANNOTATION_KEY = 'org.matrix.mjolnir.annotation.rule'; + /** * Construct a PolicyList, does not synchronize with the room. * @param roomId The id of the policy room, i.e. a room containing MSC2313 policies. @@ -134,19 +141,23 @@ class PolicyList extends EventEmitter { /** * Return all the active rules of a given kind. * @param kind e.g. RULE_SERVER (m.policy.rule.server). Rule types are always normalised when they are interned into the PolicyList. + * @param recommendation A specific recommendation to filter for e.g. `m.ban`. Please remember recommendation varients are normalized. * @returns The active ListRules for the ban list of that kind. + * FIXME: it would be nice if this was a generic to the ListRule type, (and get the recommendation type from it) but that seems hard to do, maybe impossible?. + * TFW you mess up type erasure so bad comeon. */ - private rulesOfKind(kind: string): ListRule[] { + public rulesOfKind(kind: string, recommendation?: Recommendation): ListRule[] { const rules: ListRule[] = [] const stateKeyMap = this.state.get(kind); if (stateKeyMap) { for (const event of stateKeyMap.values()) { - const rule = event?.unsigned?.rule; - // README! If you are refactoring this and/or introducing a mechanism to return the list of rules, - // please make sure that you *only* return rules with `m.ban` or create a different method - // (we don't want to accidentally ban entities). - if (rule && rule.kind === kind && rule.recommendation === Recommendation.Ban) { - rules.push(rule); + const rule = event[PolicyList.EVENT_RULE_ANNOTATION_KEY]; + if (rule && rule.kind === kind) { + if (recommendation === undefined) { + rules.push(rule); + } else if (rule.recommendation === recommendation) { + rules.push(rule); + } } } } @@ -353,10 +364,10 @@ class PolicyList extends EventEmitter { // If we haven't got any information about what the rule used to be, then it wasn't a valid rule to begin with // and so will not have been used. Removing a rule like this therefore results in no change. - if (changeType === ChangeType.Removed && previousState?.unsigned?.rule) { + if (changeType === ChangeType.Removed && previousState?.[PolicyList.EVENT_RULE_ANNOTATION_KEY]) { const sender = event.unsigned['redacted_because'] ? event.unsigned['redacted_because']['sender'] : event.sender; changes.push({ - changeType, event, sender, rule: previousState.unsigned.rule, + changeType, event, sender, rule: previousState[PolicyList.EVENT_RULE_ANNOTATION_KEY], ...previousState ? { previousState } : {} }); // Event has no content and cannot be parsed as a ListRule. @@ -368,7 +379,7 @@ class PolicyList extends EventEmitter { // Invalid/unknown rule, just skip it. continue; } - event.unsigned.rule = rule; + event[PolicyList.EVENT_RULE_ANNOTATION_KEY] = rule; if (changeType) { changes.push({ rule, changeType, event, sender: event.sender, ...previousState ? { previousState } : {} }); }