From b0fca6aa160475bbcc0298db903f709825aaaa46 Mon Sep 17 00:00:00 2001 From: Hugo Hakim Damer Date: Fri, 15 Oct 2021 12:06:37 +0200 Subject: [PATCH 1/2] RD: Add basic support for security policies This allows users of the resource directory implementation to define policies policies, i.e. limit access to specific endpoint names and sectors. The configuration happens using a separate JSON file that needs to be provided with the `--security-policy` command line option. The file format is currently described in the rd.py module description, a CDDL description (like in credentials.cddl) is not included yet. --- aiocoap/cli/rd.py | 352 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 343 insertions(+), 9 deletions(-) diff --git a/aiocoap/cli/rd.py b/aiocoap/cli/rd.py index 4c9e5187..7b3296ff 100644 --- a/aiocoap/cli/rd.py +++ b/aiocoap/cli/rd.py @@ -11,17 +11,127 @@ Known Caveats: - * It is very permissive. Not only is no security implemented. - * This may and will make exotic choices about discoverable paths whereever it can (see StandaloneResourceDirectory documentation) * Split-horizon is not implemented correctly - * Unless enforced by security (ie. not so far), endpoint and sector names + * Unless enforced by security, endpoint and sector names (ep, d) are not checked for their lengths or other validity. * Simple registrations don't cache .well-known/core contents + + * Security Policies are currently limited to a per-endpoint level. Some + use cases might require permissions to be set on a per-resource level, + i.e. limiting which relation types or resource types a specific endpoint + is allowed to register (see draft-ietf-core-resource-directory-28, + section 7.2) + +Security Policies +~~~~~~~~~~~~~~~~~ + +The RD has support for security policies to limit access to specific endpoints +and endpoint names to pre-specified credentials (e.g. DTLS keys). + +Security policies are defined in a JSON file and enabled using the +`--security-policy [SECURITY_POLICY]` command line option. + +Each policy entry maps a credential reference/claim (as specified in the +credentials file, see credentials.(py|cddl)) to the permissions that clients +with these credentials should have when accessing this endpoint. + +Currently, three permission bits are defined: + + * `read` : Allows accessing the links registered for this endpoint. This + also includes requests to the resource/endpoint lookup interface, + i.e. if this permission is not set, links from this endpoint are + not shown in resource/endpoint lookups. + + * `write` : Allows updating, modifying or deleting a registration. + + * `create`: Allows creating a registration for this endpoint and sector + name combination. Setting the `write` bit also sets this bit + implicitly, but not the other way around. + +Policies can be defined on a per-sector and a per-endpoint basis. If a policy +is defined for a specific endpoint, the policies of its sector are ignored. +The sector policies therefore apply to all endpoints that **do not** have their +own policies defined, including ones that have not been created yet. + +In addition to the permission bits, there is a global setting called +`registrar_has_permissions`, which is enabled by default and grants a +registrar full permissions for the registrations they themselves created, even +though they are not defined explicitly. + +By enabling this setting and giving registrars just the `create` permission bit +for a specific sector (and no `write` permission), it is possible to achieve a +"first-come-first-remembered" security policy, where registrars can register +arbritrary endpoint names, while other clients may not alter a registration +after another registrar has used that name. + +Policies for the default sector/no sector (if no sector is set) can be defined +using the special ":default:" sector name. +Policies that should apply to all clients even if they don't have any +credentials (default policies) can be set using the special "*" credential +reference/claim. + +If a client has multiple claims/multiple policies apply (e.g. the default +policy and another one), the permission bits are combined/ORed. + +An example of such a security policy JSON file is shown here: +``` +{ + "sectors": { + ":default:": { + "endpoints": { + "ep1": { + "policies": { + ":key_1": { + "read": true, + "write": true, + "create": true + } + } + } + }, + "policies": { + ":key_2": { + "write": true, + "create": true + }, + "*": { + "read": true, + "write": false, + "create": false + } + } + } + }, + "registrar_has_permissions": true +} +``` + +This example file will result in the following rules: + + * Registrations for other sectors than the default one (i.e. no sector) are + not allowed. + + * Registrants possessing the credentials for the ":key_2" claim (the + credentials need to be specified in the credentials JSON file) may create + endpoints with arbritrary names, except for the "ep1" endpoint name. + After these endpoints are registered, anyone with the ":key_2" claim may + modify these registrations, because they have the sector-wide `write` + permission. + + * Registrants possesing the credentials for the ":key_1" claim may not + create registrations with arbritrary endpoint names, but can create + registrations with the endpoint name "ep1". + + * The "ep1" registration is invisible to people that don't have credentials + for the ":key_1" claim, i.e. links registered there do not appear in any + lookups, while all other registrations are visible to all clients, even + those that don't have any credentials. + """ import string @@ -31,6 +141,9 @@ import argparse from urllib.parse import urljoin import itertools +from enum import IntFlag +from pathlib import Path +import json import aiocoap from aiocoap.resource import Site, Resource, ObservableResource, PathCapable, WKCResource, link_format_to_message @@ -81,12 +194,23 @@ def pop_single_arg(query, name): raise error.BadRequest("Multiple values for %r" % name) return query.pop(name)[0] +def get_single_arg(query, name): + """Out of query which is the output of query_split, pick the single value + at the key name, raise a suitable BadRequest on error, or return None if + nothing is there. The value remains in the query dictionary.""" + + if name not in query: + return None + if len(query[name]) > 1: + raise error.BadRequest("Multiple values for %r" % name) + return query.get(name)[0] + class CommonRD: # "Key" here always means an (ep, d) tuple. entity_prefix = ("reg",) - def __init__(self, proxy_domain=None): + def __init__(self, proxy_domain=None, security_policy_data=None): super().__init__() self._by_key = {} # key -> Registration @@ -97,6 +221,138 @@ def __init__(self, proxy_domain=None): self.proxy_domain = proxy_domain self.proxy_active = {} # uri_host -> Remote + self.policy = self.SecurityPolicy._policy_from_structureddata(security_policy_data) + + class SecurityPolicy: + class Permissions(IntFlag): + CREATE = 4 + READ = 2 + WRITE = 1 | CREATE + NONE = 0 + ALL = CREATE | READ | WRITE + + class EndpointPolicy: + def __init__(self, credential_policies): + self.policies = credential_policies + + class SectorPolicy: + def __init__(self, credential_policies, endpoints=None): + self.endpoints = dict() if endpoints is None else endpoints + self.policies = credential_policies + + @classmethod + def _policy_from_structureddata(cls, data=None): + + if data is None: + return cls(False) + + def _read_policy_entries(policy_data): + permissions = cls.Permissions.NONE + + if policy_data.get('read', False): + permissions = permissions | cls.Permissions.READ + + if policy_data.get('write', False): + permissions = permissions | cls.Permissions.WRITE + + if policy_data.get('create', False): + permissions = permissions | cls.Permissions.CREATE + + return permissions + + sectors = dict() + for sector_name, sector_data in data['sectors'].items(): + endpoints = dict() + for endpoint_name, endpoint_data in sector_data['endpoints'].items(): + ep_policies = {cred_ref if cred_ref != "*" else None: _read_policy_entries(policy_data) for cred_ref, policy_data in endpoint_data['policies'].items()} + endpoints[endpoint_name] = cls.EndpointPolicy(ep_policies) + + sec_policies = {cred_ref if cred_ref != "*" else None: _read_policy_entries(policy_data) for cred_ref, policy_data in sector_data['policies'].items()} + if sector_name == ":default:": + sector_name = None + sectors[sector_name] = cls.SectorPolicy(sec_policies, endpoints) + + return cls(True, sectors, True if 'registrar_has_permissions' not in data else data['registrar_has_permissions']) + + def __init__(self, enable, sectors=None, registrar_full_permissions=True): + self.enable = enable + self.sectors = dict() if sectors is None else sectors + self.registrar_full_permissions = registrar_full_permissions + + def infer_ep_name(self, claims, sector_name=None): + # Find endpoint name if none was provided (according to + # draft-ietf-core-resource-directory, section 5). + endpoint_name = None + if sector_name in self.sectors: + sector = self.sectors[sector_name] + + # Check if the client is allowed to make registrations for + # arbritrary names, if so, we can't infer one. + applicable_claims = set(claims).intersection(sector.policies.keys()) + for claim in applicable_claims: + if sector.policies[claim].permissions & self.Permissions.CREATE: + return None + + # Check all endpoints for which a policy exists whether they + # have an entry for this set of credentials. + for ep_name_iter, endpoint in sector.endpoints.items(): + applicable_claims = set(claims).intersection(endpoint.policies.keys()) + for claim in applicable_claims: + if endpoint.policies[claim].permissions & self.Permissions.CREATE: + # We can only infer the endpoint name if the + # registrant only has one possible endpoint name it + # can register. + # Otherwise, do not provide an inferred name + if endpoint_name is not None: + return None + # Infer endpoint name + endpoint_name = ep_name_iter + + return endpoint_name + + def get_permissions(self, claims, sector_name, endpoint_name, is_registrar=False): + # If security policies are disabled, allow everything. + if not self.enable: + return self.Permissions.ALL + + # If the registrar of a specific endpoint should have full + # permissions and the requester is the registrar, return full + # permissions. + if is_registrar and self.registrar_full_permissions: + return self.Permissions.ALL + + # No policy for the sector exists, so there should be no access. + if sector_name not in self.sectors: + return self.Permissions.NONE + + claims = set(claims) + # Default permissions that should always apply regardless of claims + # are stored in the policy dictionary with the key "None". + claims.add(None) + + sector = self.sectors[sector_name] + + # If no endpoint specific policy is found, default to the sector- + # wide policy. + policies = sector.policies + + # Check if a policy specific to the endpoint name exists, apply + # this one instead if this is the case. + if endpoint_name in sector.endpoints: + policies = sector.endpoints[endpoint_name].policies + + + # Take all claims for which relevant policies exist, and see which + # ones match with the ones the client has. + applicable_claims = claims.intersection(policies.keys()) + # Combine all permissions that someone with this set of claims + # should have. + permissions = self.Permissions.NONE + for claim in applicable_claims: + permissions = permissions | policies[claim] + + return permissions + class Registration: # FIXME: split this into soft and hard grace period (where the former # may be 0). the node stays discoverable for the soft grace period, but @@ -125,6 +381,8 @@ def __init__(self, static_registration_parameters, path, network_remote, delete_ self.proxy_host = proxy_host self._setproxyremote_cb = setproxyremote_cb + self.registrar = network_remote + self.update_params(network_remote, registration_parameters, is_initial=True) def update_params(self, network_remote, registration_parameters, is_initial=False): @@ -242,6 +500,14 @@ def get_based_links(self): result.append(Link(href, data)) return LinkFormat(result) + @property + def ep(self): + return self.registration_parameters['ep'][0] + + @property + def d(self): + return self.registration_parameters.get('d', [None])[0] + async def shutdown(self): pass @@ -371,7 +637,35 @@ def __init__(self, common_rd): if isinstance(self, ObservableResource): self.common_rd.register_change_callback(self.updated_state) -class DirectoryResource(ThingWithCommonRD, Resource): +class RegistrationCreationResourceBase(ThingWithCommonRD): + + def _prepare_creation(self, request): + registration_parameters = query_split(request) + + claims = set(request.remote.authenticated_claims) + + ep_name = get_single_arg(registration_parameters, 'ep') + sector_name = get_single_arg(registration_parameters, 'd') + + # Try to infer an endpoint name from the security policy if none was + # set. If this isn't possible, we will err later (either when checking + # permissions with an Unauthorized error or when calling + # initialize_endpoint with a BadRequest, giving more information about + # what could be wrong than just responding with BadRequest here) + if ep_name is None: + ep_name = self.common_rd.policy.infer_ep_name(claims, sector_name=sector_name) + if ep_name is not None: + registration_parameters['ep'].append(ep_name) + + permissions = self.common_rd.policy.get_permissions(claims, sector_name=sector_name, endpoint_name=ep_name) + + if not permissions & CommonRD.SecurityPolicy.Permissions.CREATE: + raise error.Unauthorized("Operation not allowed due to security policy") + + return registration_parameters + + +class DirectoryResource(RegistrationCreationResourceBase, ThingWithCommonRD, Resource): ct = link_format_to_message.supported_ct rt = "core.rd" @@ -381,7 +675,7 @@ class DirectoryResource(ThingWithCommonRD, Resource): async def render_post(self, request): links = link_format_from_message(request) - registration_parameters = query_split(request) + registration_parameters = self._prepare_creation(request) if self.registration_warning: # Conveniently placed so it could be changed to something setting @@ -403,14 +697,29 @@ class RegistrationResource(Resource): def __init__(self, registration): self.reg = registration + def _get_permissions(self, request): + is_registrar = request.remote == self.reg.registrar + return self.common_rd.policy.get_permissions(request.remote.authenticated_claims, + sector_name=self.reg.d, + endpoint_name=self.reg.ep, + is_registrar=is_registrar + ) + async def render_get(self, request): + if not self._get_permissions(request) & CommonRD.SecurityPolicy.Permissions.READ: + raise error.Unauthorized("Operation not allowed due to security policy") + return link_format_from_message(request, self.reg.links) def _update_params(self, msg): query = query_split(msg) self.reg.update_params(msg.remote, query) + async def render_post(self, request): + if not self._get_permissions(request) & CommonRD.SecurityPolicy.Permissions.WRITE: + raise error.Unauthorized("Operation not allowed due to security policy") + self._update_params(request) if request.opt.content_format is not None or request.payload: @@ -419,6 +728,9 @@ async def render_post(self, request): return aiocoap.Message(code=aiocoap.CHANGED) async def render_put(self, request): + if not self._get_permissions(request) & CommonRD.SecurityPolicy.Permissions.WRITE: + raise error.Unauthorized("Operation not allowed due to security policy") + # this is not mentioned in the current spec, but seems to make sense links = link_format_from_message(request) @@ -428,6 +740,9 @@ async def render_put(self, request): return aiocoap.Message(code=aiocoap.CHANGED) async def render_delete(self, request): + if not self._get_permissions(request) & CommonRD.SecurityPolicy.Permissions.WRITE: + raise error.Unauthorized("Operation not allowed due to security policy") + self.reg.delete() return aiocoap.Message(code=aiocoap.DELETED) @@ -470,6 +785,13 @@ async def render_get(self, request): candidates = self.common_rd.get_endpoints() + candidates = (c for c in candidates if + self.common_rd.policy.get_permissions( + request.remote.authenticated_claims, + sector_name=c.d, + endpoint_name=c.ep, + ) & CommonRD.SecurityPolicy.Permissions.READ) + for search_key, search_values in query.items(): if search_key in ('page', 'count'): continue # filtered last @@ -509,6 +831,13 @@ async def render_get(self, request): query = query_split(request) eps = self.common_rd.get_endpoints() + eps = (ep for ep in eps if + self.common_rd.policy.get_permissions( + request.remote.authenticated_claims, + sector_name=ep.d, + endpoint_name=ep.ep, + ) & CommonRD.SecurityPolicy.Permissions.READ) + candidates = ((e, c) for e in eps for c in e.get_based_links().links) for search_key, search_values in query.items(): @@ -550,7 +879,7 @@ async def render_get(self, request): return link_format_to_message(request, LinkFormat(candidates)) -class SimpleRegistration(ThingWithCommonRD, Resource): +class SimpleRegistration(RegistrationCreationResourceBase, ThingWithCommonRD, Resource): #: Issue a custom warning when registrations come in via this interface registration_warning = None @@ -559,7 +888,7 @@ def __init__(self, common_rd, context): self.context = context async def render_post(self, request): - query = query_split(request) + query = self._prepare_creation(request) if 'base' in query: raise error.BadRequest("base is not allowed in simple registrations") @@ -690,12 +1019,17 @@ async def start(self, args=None): parser.add_argument("--proxy-domain", help="Enable the RD proxy extension. Example: `.proxy.example.net` will produce base URIs like `coap://node1.proxy.example.net/`. The names must all resolve to an address the RD is bound to.", type=str) parser.add_argument("--lwm2m-compat", help="Compatibility mode for LwM2M clients that can not perform some discovery steps (moving the registration resource to `/rd`)", action='store_true', default=None) parser.add_argument("--no-lwm2m-compat", help="Disable all compativility with LwM2M clients that can not perform some discovery steps (not even accepting registrations at `/rd` with warnings)", action='store_false', dest='lwm2m_compat') + parser.add_argument('--security-policy', help="JSON file describing the security policy to use for the RD.", type=Path) options = parser.parse_args(args if args is not None else sys.argv[1:]) # Putting in an empty site to construct the site with a context self.context = await server_context_from_arguments(None, options) - self.site = StandaloneResourceDirectory(context=self.context, proxy_domain=options.proxy_domain, lwm2m_compat=options.lwm2m_compat) + security_policy_data = None + if options.security_policy is not None: + security_policy_data = json.load(options.security_policy.open("rb")) + + self.site = StandaloneResourceDirectory(context=self.context, proxy_domain=options.proxy_domain, lwm2m_compat=options.lwm2m_compat, security_policy_data=security_policy_data) self.context.serversite = self.site async def shutdown(self): From 68e44b58e3b83d1426928615f2a8a5f8dc33a76a Mon Sep 17 00:00:00 2001 From: Hugo Hakim Damer Date: Fri, 29 Oct 2021 01:45:46 +0200 Subject: [PATCH 2/2] RD: Add CDDL file for security policies, small changes to policy file format --- aiocoap/cli/rd.py | 41 ++++++++++++++++++++++----------- aiocoap/cli/securitypolicy.cddl | 36 +++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 13 deletions(-) create mode 100644 aiocoap/cli/securitypolicy.cddl diff --git a/aiocoap/cli/rd.py b/aiocoap/cli/rd.py index 7b3296ff..af31f8bc 100644 --- a/aiocoap/cli/rd.py +++ b/aiocoap/cli/rd.py @@ -72,7 +72,7 @@ Policies for the default sector/no sector (if no sector is set) can be defined using the special ":default:" sector name. Policies that should apply to all clients even if they don't have any -credentials (default policies) can be set using the special "*" credential +credentials (default policies) can be set using the special ":default:" credential reference/claim. If a client has multiple claims/multiple policies apply (e.g. the default @@ -94,12 +94,12 @@ } } }, - "policies": { + "default_policies": { ":key_2": { "write": true, "create": true }, - "*": { + ":default:": { "read": true, "write": false, "create": false @@ -161,6 +161,20 @@ IMMUTABLE_PARAMETERS = ('ep', 'd', 'proxy') +# Sector key in the security policy configuration whose value should be used +# for the "default" sector (i.e. if no sector is set). +# Might need to be changed if there is a situation where ":default:" is used +# as an actual sector name. +DEFAULT_SECTOR_KEY = ':default:' + +# Claim/credential reference key in the policy map whose value should be +# treated as the "default" permissions for users that don't have any claims. +# Might need to be changed if there is a situation where ":default:" is used +# as an actual claim name (even though this can also be fixed by just using +# a different name, since the claim names are only used internally and +# arbritrarily set in the credentials file). +DEFAULT_CLAIM_KEY = ':default:' + def query_split(msg): """Split a message's query up into (key, [*value]) pairs from a ?key=value&key2=value2 style Uri-Query options. @@ -263,16 +277,16 @@ def _read_policy_entries(policy_data): sectors = dict() for sector_name, sector_data in data['sectors'].items(): endpoints = dict() - for endpoint_name, endpoint_data in sector_data['endpoints'].items(): - ep_policies = {cred_ref if cred_ref != "*" else None: _read_policy_entries(policy_data) for cred_ref, policy_data in endpoint_data['policies'].items()} + for endpoint_name, endpoint_data in sector_data.get('endpoints', dict()).items(): + ep_policies = {cred_ref if cred_ref != DEFAULT_CLAIM_KEY else None: _read_policy_entries(policy_data) for cred_ref, policy_data in endpoint_data['policies'].items()} endpoints[endpoint_name] = cls.EndpointPolicy(ep_policies) - sec_policies = {cred_ref if cred_ref != "*" else None: _read_policy_entries(policy_data) for cred_ref, policy_data in sector_data['policies'].items()} - if sector_name == ":default:": + sec_policies = {cred_ref if cred_ref != DEFAULT_CLAIM_KEY else None: _read_policy_entries(policy_data) for cred_ref, policy_data in sector_data['default_policies'].items()} + if sector_name == DEFAULT_SECTOR_KEY: sector_name = None sectors[sector_name] = cls.SectorPolicy(sec_policies, endpoints) - return cls(True, sectors, True if 'registrar_has_permissions' not in data else data['registrar_has_permissions']) + return cls(True, sectors, data.get('registrar_has_permissions', True)) def __init__(self, enable, sectors=None, registrar_full_permissions=True): self.enable = enable @@ -687,18 +701,19 @@ async def render_post(self, request): return aiocoap.Message(code=aiocoap.CREATED, location_path=regresource.path) -class RegistrationResource(Resource): +class RegistrationResource(ThingWithCommonRD, Resource): """The resource object wrapping a registration is just a very thin and ephemeral object; all those methods could just as well be added to Registration with `s/self.reg/self/g`, making RegistrationResource(reg) = reg (or handleded in a single RegistrationDispatchSite), but this is kept here for better separation of model and interface.""" - def __init__(self, registration): + def __init__(self, common_rd, registration): + super().__init__(common_rd) self.reg = registration def _get_permissions(self, request): - is_registrar = request.remote == self.reg.registrar + is_registrar = type(request.remote) is type(self.reg.registrar) and request.remote == self.reg.registrar return self.common_rd.policy.get_permissions(request.remote.authenticated_claims, sector_name=self.reg.d, endpoint_name=self.reg.ep, @@ -709,7 +724,7 @@ async def render_get(self, request): if not self._get_permissions(request) & CommonRD.SecurityPolicy.Permissions.READ: raise error.Unauthorized("Operation not allowed due to security policy") - return link_format_from_message(request, self.reg.links) + return link_format_to_message(request, self.reg.links) def _update_params(self, msg): query = query_split(msg) @@ -754,7 +769,7 @@ async def render(self, request): except KeyError: raise error.NotFound - entity = RegistrationResource(entity) + entity = RegistrationResource(self.common_rd, entity) return await entity.render(request.copy(uri_path=())) diff --git a/aiocoap/cli/securitypolicy.cddl b/aiocoap/cli/securitypolicy.cddl new file mode 100644 index 00000000..051cd663 --- /dev/null +++ b/aiocoap/cli/securitypolicy.cddl @@ -0,0 +1,36 @@ +security-policy = { + "sectors": sector-map, + ? "registrar_has_permissions": bool, ; Will default to "true" if not defined +} + +sector-map = { + * tstr => sector-policy, +} + +sector-policy = { + ? "endpoints": endpoint-map, + ? "default_policies": policy-map +} + +endpoint-map = { + * tstr => endpoint-policy +} + +endpoint-policy = { + "policies": policy-map +} + +; These are the same as in credentials.cddl +claim = uripattern / credential-reference +uripattern = tstr .regexp "[^:]" +credential-reference = tstr .regexp ":.+" + +policy-map = { + * claim => policy +} + +policy = { + ? "write": bool, ; Will default to "false" if not defined + ? "read": bool, ; Will default to "false" if not defined + ? "create": bool, ; Will default to "false" if not defined +}