From aa9af0705ca02383bb411a78b484fc13be84aaa7 Mon Sep 17 00:00:00 2001 From: Firstyear Date: Sun, 2 Oct 2022 11:28:58 +1000 Subject: [PATCH] Fix issues with radius (#1084) --- kanidm_book/src/integrations/radius.md | 38 +++---- kanidm_rlm_python/Dockerfile | 1 + kanidmd/idm/src/actors/v1_read.rs | 2 +- kanidmd/idm/src/be/idl_arc_sqlite.rs | 80 ++++++------- kanidmd/idm/src/be/idl_sqlite.rs | 2 +- pykanidm/kanidm/radius/__init__.py | 136 +++++++++-------------- pykanidm/kanidm/radius/utils.py | 34 ++++++ pykanidm/kanidm/types.py | 2 +- pykanidm/tests/test_radius_check_vlan.py | 8 +- pykanidm/tests/test_radius_config.py | 8 +- pykanidm/tests/test_radius_token.py | 2 +- pykanidm/tests/test_types.py | 4 +- 12 files changed, 164 insertions(+), 153 deletions(-) create mode 100644 pykanidm/kanidm/radius/utils.py diff --git a/kanidm_book/src/integrations/radius.md b/kanidm_book/src/integrations/radius.md index ecb18ed48..65b12e85e 100644 --- a/kanidm_book/src/integrations/radius.md +++ b/kanidm_book/src/integrations/radius.md @@ -125,21 +125,22 @@ verify_hostnames = true # verify the hostname of the Kanidm server verify_ca = false # Strict CA verification ca = /data/ca.pem # Path to the kanidm ca -username = # Username of the RADIUS service account -password = # Generated secret for the service account + +auth_token = "ABC..." # Auth token for the service account + # See: kanidm service-account api-token generate # Default vlans for groups that don't specify one. -radius_default_vlan = 1 +radius_default_vlan = 1 # A list of Kanidm groups which must be a member # before they can authenticate via RADIUS. radius_required_groups = [ - "radius_access_allowed", + "radius_access_allowed@idm.example.com", ] # A mapping between Kanidm groups and VLANS radius_groups = [ - { name = "radius_access_allowed", vlan = 10 }, + { spn = "radius_access_allowed@idm.example.com", vlan = 10 }, ] # A mapping of clients and their authentication tokens @@ -150,11 +151,11 @@ radius_clients = [ # radius_cert_path = "/etc/raddb/certs/cert.pem" # the signing key for radius TLS -# radius_key_path = "/etc/raddb/certs/key.pem" +# radius_key_path = "/etc/raddb/certs/key.pem" # the diffie-hellman output -# radius_dh_path = "/etc/raddb/certs/dh.pem" +# radius_dh_path = "/etc/raddb/certs/dh.pem" # the CA certificate -# radius_ca_path = "/etc/raddb/certs/ca.pem" +# radius_ca_path = "/etc/raddb/certs/ca.pem" ``` @@ -164,21 +165,20 @@ radius_clients = [ ```toml url = "https://example.com" -username = "radius_service_account" -# The generated password from above -password = "cr4bzr0ol" +# The auth token for the service account +auth_token = "ABC..." # default vlan for groups that don't specify one. -radius_default_vlan = 99 +radius_default_vlan = 99 -# if the user is in one of these Kanidm groups, +# if the user is in one of these Kanidm groups, # then they're allowed to authenticate radius_required_groups = [ - "radius_access_allowed", + "radius_access_allowed@idm.example.com", ] radius_groups = [ - { name = "radius_access_allowed", vlan = 10 } + { spn = "radius_access_allowed@idm.example.com", vlan = 10 } ] radius_clients = [ @@ -196,11 +196,11 @@ radius_clients = [ ] ``` -Then re-create/run your docker instance and expose the ports by adding +Then re-create/run your docker instance and expose the ports by adding `-p 1812:1812 -p 1812:1812/udp` to the command. -If you have any issues, check the logs from the RADIUS output, as they tend -to indicate the cause of the problem. To increase the logging level you can +If you have any issues, check the logs from the RADIUS output, as they tend +to indicate the cause of the problem. To increase the logging level you can re-run your environment with debug enabled: ```shell @@ -215,7 +215,7 @@ docker run --name radiusd \ ``` Note: the RADIUS container *is* configured to provide -[Tunnel-Private-Group-ID](https://freeradius.org/rfc/rfc2868.html#Tunnel-Private-Group-ID), +[Tunnel-Private-Group-ID](https://freeradius.org/rfc/rfc2868.html#Tunnel-Private-Group-ID), so if you wish to use Wi-Fi-assigned VLANs on your infrastructure, you can assign these by groups in the configuration file as shown in the above examples. diff --git a/kanidm_rlm_python/Dockerfile b/kanidm_rlm_python/Dockerfile index f0776230f..4b7d2cb4e 100644 --- a/kanidm_rlm_python/Dockerfile +++ b/kanidm_rlm_python/Dockerfile @@ -50,6 +50,7 @@ RUN rm -rf /pkg/* USER radiusd ENV LD_PRELOAD=/usr/lib64/libpython3.so +ENV KANIDM_CONFIG_FILE="/data/kanidm" COPY kanidm_rlm_python/radius_entrypoint.py /radius_entrypoint.py CMD [ "/usr/bin/python3", "/radius_entrypoint.py" ] diff --git a/kanidmd/idm/src/actors/v1_read.rs b/kanidmd/idm/src/actors/v1_read.rs index 65db4166d..7ef2bd7e6 100644 --- a/kanidmd/idm/src/actors/v1_read.rs +++ b/kanidmd/idm/src/actors/v1_read.rs @@ -155,7 +155,7 @@ impl QueryServerReadV1 { #[instrument( level = "info", name = "online_backup", - skip(self, msg, outpath, versions) + skip_all, fields(uuid = ?msg.eventid) )] pub async fn handle_online_backup( diff --git a/kanidmd/idm/src/be/idl_arc_sqlite.rs b/kanidmd/idm/src/be/idl_arc_sqlite.rs index 1483c3683..7462e569d 100644 --- a/kanidmd/idm/src/be/idl_arc_sqlite.rs +++ b/kanidmd/idm/src/be/idl_arc_sqlite.rs @@ -162,45 +162,45 @@ macro_rules! get_idl { $itype:expr, $idx_key:expr ) => {{ - // SEE ALSO #259: Find a way to implement borrow for this properly. - // I don't think this is possible. When we make this dyn, the arc - // needs the dyn trait to be sized so that it *could* claim a clone - // for hit tracking reasons. That also means that we need From and - // some other traits that just seem incompatible. And in the end, - // we clone a few times in arc, and if we miss we need to insert anyway - // - // So the best path could be to replace IdlCacheKey with a compressed - // or smaller type. Perhaps even a small cache of the IdlCacheKeys that - // are allocated to reduce some allocs? Probably over thinking it at - // this point. - // - // First attempt to get from this cache. - let cache_key = IdlCacheKeyRef { - a: $attr, - i: $itype, - k: $idx_key, - }; - let cache_r = $self.idl_cache.get(&cache_key as &dyn IdlCacheKeyToRef); - // If hit, continue. - if let Some(ref data) = cache_r { - trace!( - cached_index = ?$itype, - attr = ?$attr, - idl = %data, - ); - return Ok(Some(data.as_ref().clone())); - } - // If miss, get from db *and* insert to the cache. - let db_r = $self.db.get_idl($attr, $itype, $idx_key)?; - if let Some(ref idl) = db_r { - let ncache_key = IdlCacheKey { - a: $attr.into(), - i: $itype.clone(), - k: $idx_key.into(), - }; - $self.idl_cache.insert(ncache_key, Box::new(idl.clone())) - } - Ok(db_r) + // SEE ALSO #259: Find a way to implement borrow for this properly. + // I don't think this is possible. When we make this dyn, the arc + // needs the dyn trait to be sized so that it *could* claim a clone + // for hit tracking reasons. That also means that we need From and + // some other traits that just seem incompatible. And in the end, + // we clone a few times in arc, and if we miss we need to insert anyway + // + // So the best path could be to replace IdlCacheKey with a compressed + // or smaller type. Perhaps even a small cache of the IdlCacheKeys that + // are allocated to reduce some allocs? Probably over thinking it at + // this point. + // + // First attempt to get from this cache. + let cache_key = IdlCacheKeyRef { + a: $attr, + i: $itype, + k: $idx_key, + }; + let cache_r = $self.idl_cache.get(&cache_key as &dyn IdlCacheKeyToRef); + // If hit, continue. + if let Some(ref data) = cache_r { + trace!( + cached_index = ?$itype, + attr = ?$attr, + idl = %data, + ); + return Ok(Some(data.as_ref().clone())); + } + // If miss, get from db *and* insert to the cache. + let db_r = $self.db.get_idl($attr, $itype, $idx_key)?; + if let Some(ref idl) = db_r { + let ncache_key = IdlCacheKey { + a: $attr.into(), + i: $itype.clone(), + k: $idx_key.into(), + }; + $self.idl_cache.insert(ncache_key, Box::new(idl.clone())) + } + Ok(db_r) }}; } @@ -366,6 +366,7 @@ impl<'a> IdlArcSqliteTransaction for IdlArcSqliteReadTransaction<'a> { exists_idx!(self, attr, itype) } + #[instrument(level = "trace", skip_all)] fn get_idl( &mut self, attr: &str, @@ -447,6 +448,7 @@ impl<'a> IdlArcSqliteTransaction for IdlArcSqliteWriteTransaction<'a> { exists_idx!(self, attr, itype) } + #[instrument(level = "trace", skip_all)] fn get_idl( &mut self, attr: &str, diff --git a/kanidmd/idm/src/be/idl_sqlite.rs b/kanidmd/idm/src/be/idl_sqlite.rs index ecceb9f17..0a3708471 100644 --- a/kanidmd/idm/src/be/idl_sqlite.rs +++ b/kanidmd/idm/src/be/idl_sqlite.rs @@ -210,6 +210,7 @@ pub trait IdlSqliteTransaction { self.exists_table(&tname) } + #[instrument(level = "trace", skip_all)] fn get_idl( &self, attr: &str, @@ -247,7 +248,6 @@ pub trait IdlSqliteTransaction { // have a corrupted index ..... None => IDLBitRange::new(), }; - trace!( miss_index = ?itype, attr = ?attr, diff --git a/pykanidm/kanidm/radius/__init__.py b/pykanidm/kanidm/radius/__init__.py index f1375c135..fc528d08c 100644 --- a/pykanidm/kanidm/radius/__init__.py +++ b/pykanidm/kanidm/radius/__init__.py @@ -1,5 +1,5 @@ -""" kanidm RADIUS module """ +""" kanidm RADIUS module """ import asyncio from functools import reduce import json @@ -9,68 +9,56 @@ from pathlib import Path import sys from typing import Any, Dict, Optional, Union -import aiohttp - -from kanidm import KanidmClient -from kanidm.types import AuthStepPasswordResponse, RadiusTokenGroup, RadiusTokenResponse -from kanidm.utils import load_config from kanidm.exceptions import NoMatchingEntries +from kanidm.types import AuthStepPasswordResponse, RadiusTokenResponse +from .. import KanidmClient from . import radiusd - -logging.basicConfig( - level=logging.DEBUG, - stream=sys.stderr, -) +from .utils import check_vlan # the list of places to try -config_paths = [ +CONFIG_PATHS = [ os.getenv("KANIDM_RLM_CONFIG", "/data/kanidm"), # container goodness "~/.config/kanidm", # for a user "/etc/kanidm/kanidm", # system-wide "../examples/kanidm", # test mode ] -CONFIG_PATH = None -for config_file_path in config_paths: - CONFIG_PATH = Path(config_file_path).expanduser().resolve() - if CONFIG_PATH.exists(): - break -if (CONFIG_PATH is None) or (not CONFIG_PATH.exists()): - logging.error( - "Failed to find configuration file, checked (%s), quitting!", config_paths +def instantiate(_: Any) -> Any: + """start up radiusd""" + logging.basicConfig( + level=logging.DEBUG, + stream=sys.stderr, ) - sys.exit(1) -config = load_config(str(CONFIG_PATH)) - -KANIDM_CLIENT = KanidmClient(config_file=CONFIG_PATH) -if KANIDM_CLIENT.config.auth_token is None: - logging.error("You need to specify auth_token in the configuration file!") - sys.exit(1) + logging.info("Starting up!") -def authenticate( - acct: str, - password: str, - kanidm_client: KanidmClient = KANIDM_CLIENT, -) -> Union[int, AuthStepPasswordResponse]: - """authenticate the RADIUS service account to Kanidm""" - logging.error("authenticate - %s:%s", acct, password) + config_path = None + for config_file_path in CONFIG_PATHS: + config_path = Path(config_file_path).expanduser().resolve() + if config_path.exists(): + break - try: - loop = asyncio.get_event_loop() - return loop.run_until_complete(kanidm_client.check_token_valid()) - except Exception as error_message: # pylint: disable=broad-except - logging.error("Failed to run kanidm.check_token_valid: %s", error_message) - return radiusd.RLM_MODULE_FAIL + if (config_path is None) or (not config_path.exists()): + logging.error( + "Failed to find configuration file, checked (%s), quitting!", CONFIG_PATHS + ) + sys.exit(1) + kanidm_client = KanidmClient(config_file=config_path) + if kanidm_client.config.auth_token is None: + logging.error("You need to specify auth_token in the configuration file!") + sys.exit(1) + os.environ["KANIDM_CONFIG_FILE"] = config_path.as_posix() + logging.info("Config file: %s", config_path.as_posix()) + return radiusd.RLM_MODULE_OK async def _get_radius_token( username: Optional[str] = None, - kanidm_client: KanidmClient = KANIDM_CLIENT, ) -> Optional[Dict[str, Any]]: """pulls the radius token for a client username""" + kanidm_client = KanidmClient(config_file=os.environ["KANIDM_CONFIG_FILE"]) if username is None: raise ValueError("Didn't get a username for _get_radius_token") # authenticate as the radius service account @@ -86,45 +74,13 @@ async def _get_radius_token( logging.debug(response.data) return response.data - -def check_vlan( - acc: int, - group: RadiusTokenGroup, - kanidm_client: Optional[KanidmClient] = None, -) -> int: - """checks if a vlan is in the config, - - acc is the default vlan - """ - logging.debug("acc=%s", acc) - if kanidm_client is None: - kanidm_client = KANIDM_CLIENT - # raise ValueError("Need to pass this a kanidm_client") - - for radius_group in kanidm_client.config.radius_groups: - group_name = group.spn.split("@")[0] - logging.debug( - "Checking '%s' radius_group against group %s", radius_group, group_name - ) - if radius_group.name == group_name: - return radius_group.vlan - logging.debug("returning default vlan: %s", acc) - return acc - - -def instantiate(_: Any) -> Any: - """start up radiusd""" - logging.info("Starting up!") - return radiusd.RLM_MODULE_OK - - # pylint: disable=too-many-locals def authorize( args: Any = Dict[Any, Any], - kanidm_client: KanidmClient = KANIDM_CLIENT, ) -> Any: """does the kanidm authorize step""" logging.info("kanidm python module called") + kanidm_client = KanidmClient(config_file=os.environ["KANIDM_CONFIG_FILE"]) # args comes in like this # ( # ('User-Name', ''), @@ -153,7 +109,7 @@ def authorize( tok = RadiusTokenResponse.parse_obj( loop.run_until_complete(_get_radius_token(username=user_id)) ) - logging.debug("radius_token: %s", tok) + logging.debug("radius information token: %s", tok) except NoMatchingEntries as error_message: logging.info( "kanidm RLM_MODULE_NOTFOUND after NoMatchingEntries for user_id %s: %s", @@ -164,8 +120,8 @@ def authorize( except Exception as error_message: # pylint: disable=broad-except logging.error("kanidm exception: %s, %s", type(error_message), error_message) if tok is None: - logging.info("kanidm RLM_MODULE_NOTFOUND due to no auth token") - return radiusd.RLM_MODULE_NOTFOUND + logging.info("kanidm RLM_MODULE_REJECT - unable to retrieve radius information token") + return radiusd.RLM_MODULE_REJECT # Get values out of the token name = tok.name @@ -174,14 +130,14 @@ def authorize( # Are they in the required group? req_sat = False + required_groups = kanidm_client.config.radius_required_groups for group in tok.groups: - group_name = group.spn.split("@")[0] - if group_name in kanidm_client.config.radius_required_groups: + if group.uuid in required_groups or group.spn in required_groups: req_sat = True - logging.info("User %s has a required group (%s)", name, group_name) + logging.info("User %s has a required group (%s)", name, group.spn) if req_sat is not True: logging.info("User %s doesn't have a group from the required list.", name) - return radiusd.RLM_MODULE_NOTFOUND + return radiusd.RLM_MODULE_REJECT # look up them in config for group vlan if possible. # TODO: work out the typing on this, WTF. @@ -206,3 +162,21 @@ def authorize( logging.info("OK! Returning details to radius for %s ...", name) return (radiusd.RLM_MODULE_OK, reply, config_object) + + + + +def authenticate( + acct: str, + password: str, +) -> Union[int, AuthStepPasswordResponse]: + """authenticate the RADIUS service account to Kanidm""" + kanidm_client = KanidmClient(config_file=os.environ["KANIDM_CONFIG_FILE"]) + logging.error("authenticate - %s:%s", acct, password) + + try: + loop = asyncio.get_event_loop() + return loop.run_until_complete(kanidm_client.check_token_valid()) + except Exception as error_message: # pylint: disable=broad-except + logging.error("Failed to run kanidm.check_token_valid: %s", error_message) + return radiusd.RLM_MODULE_FAIL diff --git a/pykanidm/kanidm/radius/utils.py b/pykanidm/kanidm/radius/utils.py new file mode 100644 index 000000000..2e6db587b --- /dev/null +++ b/pykanidm/kanidm/radius/utils.py @@ -0,0 +1,34 @@ +""" class utils """ + +from typing import Optional +import logging +import os + +from .. import KanidmClient +from ..types import RadiusTokenGroup + +def check_vlan( + acc: int, + group: RadiusTokenGroup, + kanidm_client: Optional[KanidmClient] = None, +) -> int: + """checks if a vlan is in the config, + + acc is the default vlan + """ + logging.debug("acc=%s", acc) + if kanidm_client is None: + if "KANIDM_CONFIG_FILE" in os.environ: + kanidm_client = KanidmClient(config_file=os.environ["KANIDM_CONFIG_FILE"]) + else: + raise ValueError("Need to pass this a kanidm_client") + + for radius_group in kanidm_client.config.radius_groups: + logging.debug( + "Checking vlan group '%s' against user group %s", radius_group.spn, group.spn + ) + if radius_group.spn == group.spn: + logging.info("returning new vlan: %s", radius_group.vlan) + return radius_group.vlan + logging.debug("returning already set vlan: %s", acc) + return acc diff --git a/pykanidm/kanidm/types.py b/pykanidm/kanidm/types.py index 8d00d2d2b..362709c24 100644 --- a/pykanidm/kanidm/types.py +++ b/pykanidm/kanidm/types.py @@ -93,7 +93,7 @@ class AuthStepPasswordResponse(BaseModel): class RadiusGroup(BaseModel): """group for kanidm radius""" - name: str + spn: str vlan: int @validator("vlan") diff --git a/pykanidm/tests/test_radius_check_vlan.py b/pykanidm/tests/test_radius_check_vlan.py index 62678e71e..cd696651e 100644 --- a/pykanidm/tests/test_radius_check_vlan.py +++ b/pykanidm/tests/test_radius_check_vlan.py @@ -7,7 +7,7 @@ import pytest from kanidm import KanidmClient from kanidm.types import KanidmClientConfig, RadiusTokenGroup -from kanidm.radius import check_vlan +from kanidm.radius.utils import check_vlan @pytest.mark.asyncio @@ -18,8 +18,8 @@ async def test_check_vlan(event_loop: Any) -> None: """ uri='https://kanidm.example.com' radius_groups = [ - { name = "crabz", "vlan" = 1234 }, - { name = "hello world", "vlan" = 12345 }, + { spn = "crabz@example.com", "vlan" = 1234 }, + { spn = "hello@world", "vlan" = 12345 }, ] """ ) @@ -34,7 +34,7 @@ async def test_check_vlan(event_loop: Any) -> None: assert ( check_vlan( acc=12345678, - group=RadiusTokenGroup(spn="crabz@domain.com", uuid="crabz"), + group=RadiusTokenGroup(spn="crabz@example.com", uuid="crabz"), kanidm_client=kanidm_client, ) == 1234 diff --git a/pykanidm/tests/test_radius_config.py b/pykanidm/tests/test_radius_config.py index c2e22e009..2a2497be2 100644 --- a/pykanidm/tests/test_radius_config.py +++ b/pykanidm/tests/test_radius_config.py @@ -10,7 +10,7 @@ from kanidm.types import KanidmClientConfig from kanidm.utils import load_config -EXAMPLE_CONFIG_FILE = "../../kanidm_rlm_python/examples/config" +EXAMPLE_CONFIG_FILE = "../examples/config" def test_load_config_file() -> None: @@ -30,7 +30,7 @@ def test_radius_groups() -> None: config_toml = """ radius_groups = [ - { name = "hello world", "vlan" = 1234 }, + { spn = "hello world", "vlan" = 1234 }, ] """ @@ -38,8 +38,8 @@ radius_groups = [ print(config_parsed) kanidm_config = KanidmClientConfig.parse_obj(config_parsed) for group in kanidm_config.radius_groups: - print(group.name) - assert group.name == "hello world" + print(group.spn) + assert group.spn == "hello world" def test_radius_clients() -> None: diff --git a/pykanidm/tests/test_radius_token.py b/pykanidm/tests/test_radius_token.py index 732f298e8..31c049fac 100644 --- a/pykanidm/tests/test_radius_token.py +++ b/pykanidm/tests/test_radius_token.py @@ -22,7 +22,7 @@ async def test_radius_call(client_configfile: KanidmClient) -> None: print("Doing auth_init using token") if client_configfile.config.auth_token is None: - raise ValueError("This path shouldn't be possible in the test!") + pytest.skip("You can't test auth if you don't have an auth_token in ~/.config/kanidm") result = await client_configfile.get_radius_token(RADIUS_TEST_USER) print(f"{result=}") diff --git a/pykanidm/tests/test_types.py b/pykanidm/tests/test_types.py index cc347b333..67eeabe83 100644 --- a/pykanidm/tests/test_types.py +++ b/pykanidm/tests/test_types.py @@ -33,13 +33,13 @@ def test_radiusgroup_vlan_zero() -> None: def test_radiusgroup_vlan_4096() -> None: """tests RadiusGroup's vlan validator""" - assert RadiusGroup(vlan=4096, name="crabzrool") + assert RadiusGroup(vlan=4096, spn="crabzrool@foo") def test_radiusgroup_vlan_no_name() -> None: """tests RadiusGroup's vlan validator""" with pytest.raises( - pydantic.error_wrappers.ValidationError, match="name\n.*field required" + pydantic.error_wrappers.ValidationError, match="spn\n.*field required" ): RadiusGroup( vlan=4096,