diff --git a/example.ini b/example.ini index 232a61a..4d78940 100644 --- a/example.ini +++ b/example.ini @@ -4,14 +4,17 @@ username_http_header = "X-Ldap-Authz-Username" ; Example LDAP server configuration. This is for Active Directory, ; and makes a recursive membership query to given group. -;ldap_server_url = ldap://127.0.0.1:3890 ldap_server_url = ldap://dc1.example.test:389 ldap_conn_timeout = 10.0 ldap_bind_dn = "CN=service,CN=Users,DC=example,DC=test" ldap_bind_password = "password123" ldap_search_base = "DC=example,DC=test" ldap_query = "(&(objectCategory=Person)(sAMAccountName=%USERNAME%)(memberOf:1.2.840.113556.1.4.1941:=CN=%MY_CUSTOM_VAR%,CN=Users,DC=example,DC=test))" -ldap_attribs = "displayName, givenName, sn, mail" + +; Attributes to fetch from LDAP. +; (You can specify this multiple times, or use a comma-separated list:) +ldap_attribs = "givenName, sn, mail" +ldap_attribs = "displayName" ; Cache size (these are defaults) cache_time = 30 @@ -28,12 +31,13 @@ http_path = "/users$" ; Ldap query references variable MY_CUSTOM_VAR above. Set it for this query: query_vars = "MY_CUSTOM_VAR = ACL_Users" ; Fetch additional attributes from LDAP my performing additional queries -; if this one succeeds. See below for their definitions. -sub_queries = "is_beta_tester, is_bug_reporter, is_peer_support" - +; if this one succeeds. See below for section definitions. +sub_queries = "is_beta_tester, is_bug_reporter" +sub_queries = "is_peer_support" [admins] http_path = "/admins$" +http_path = "/admins$" query_vars = "MY_CUSTOM_VAR = ACL_Admins" ; Fictional example: instruct backend app to show debug info for admins set_attribs_on_success = "extraGroups = show_debug_info" @@ -53,6 +57,7 @@ set_attribs_on_success = "extraGroups = beta_tester" [is_bug_reporter] query_vars = "MY_CUSTOM_VAR = Role_Bug_Reporters" -set_attribs_on_success = "extraGroups = bug_reporter, extraGroups = show_debug_info" +set_attribs_on_success = "extraGroups = bug_reporter" +set_attribs_on_success = "extraGroups = show_debug_info" ; Circular references are pruned, so this nonsense won't crash - it's just useless: sub_queries = "is_bug_reporter, users" diff --git a/run-tests.sh b/run-tests.sh index 274fe57..46d5463 100755 --- a/run-tests.sh +++ b/run-tests.sh @@ -84,6 +84,9 @@ function do_tests() { test "user-page" "charlie:BADPASSWORD" "401 c eg:" test "bad-page" "alice:alice123" "404 c eg:" + + # Test username quoting with malicious characters, should give 401, not 500 + test "user-page" ")=&%)):password" "401 c eg:" echo "(Repeat and check that query came from cache)" test "user-page" "alice:alice123" "200Alice Alison c1 eg:beta_tester" diff --git a/src/config.rs b/src/config.rs index 0f71acf..1f8f286 100644 --- a/src/config.rs +++ b/src/config.rs @@ -13,7 +13,8 @@ use crate::SubQueryJoin; macro_rules! config_options { - ($($name:ident: $type:ty = $default:expr ; $help:expr),*) => { + ($($name:ident $([$multi:tt])? : $type:ty = $default:expr ; $help:expr),*) => { + #[derive(Debug, Clone)] pub(crate) struct ConfigSection { $( @@ -21,6 +22,16 @@ macro_rules! config_options { )* } + fn is_multiline(opt: &str) -> bool { + match opt { + $( + $( stringify!($name) => { assert!(stringify!($multi) == "MULTILINE"); true } )? + )* + _ => false, + } + } + + const CONFIG_OPTIONS: &[(&str, &str, Option<&str>)] = &[ $( (stringify!($name), $help, $default), @@ -31,26 +42,25 @@ macro_rules! config_options { Configuration file in in INI format: [default] - ; Default values for all sections (can be overridden in the sections) - key = value1 - key = value2 + ; Default values for all other sections + option1 = value1 + option2 = value2 ... [section1] - ; (same keys as in [default], if values differ) + ; (any options that differ from [default]) [section2] - ; (same keys as in [default], if values differ) + ; (any options that differ from [default]) ... -You can define defaults for other sections in the [default] section. -It's not used for anything else. - -Every section must have a unique name, and the name 'default' is reserved. +Every section must have a unique name. -Descriptions of the config keys: +Descriptions of possible config options follows. Options marked with '(+)' +are comma-separated lists - if specified multiple times, their values are +concatenated. "##; @@ -63,7 +73,12 @@ Descriptions of the config keys: } CONFIG_HELP_INTRO.to_string() + &CONFIG_OPTIONS.iter() .filter(|(key, _, _)| *key != "section") - .map(|(key, help, def)| format!(" {} {}\n\n {}\n\n\n", key, fmt_def(def), help.replace("\n", "\n ") )) + .map(|(key, help, def)| format!(" {}{} {}\n\n {}\n\n\n", + key, + if is_multiline(key) { " (+)" } else { "" }, + fmt_def(def), + help.replace("\n", "\n ") + )) .collect::() } @@ -97,9 +112,9 @@ config_options! { ldap_search_base: String = None; "LDAP base DN to search in (e.g. 'OU=users,DC=example,DC=com')", ldap_scope: ldap3::Scope = Some("subtree"); "LDAP search scope. Must be 'subtree', 'onelevel' or 'base')", ldap_query: String = None; "LDAP query to use. May contain '%USERNAME%', which will be quoted and replaced.\nExample: '(&(objectClass=person)(sAMAccountName=%USERNAME%))", - ldap_attribs: Vec = Some("CN"); "LDAP attributes to return (e.g. 'displayName, givenName, sn, mail'). Must not be empty.", + ldap_attribs [MULTILINE]: Vec = Some("CN"); "LDAP attributes to return (e.g. 'displayName, givenName, sn, mail'). Must not be empty.", - query_vars: HashMap = Some(""); concat!( + query_vars [MULTILINE]: HashMap = Some(""); concat!( "Extra variables to use in the query, in addition to %USERNAME%.\n", "You can use these to avoid repeating long query strings in different sections.\n", "\n", @@ -117,11 +132,11 @@ config_options! { attrib_delimiter: String = Some(";"); "Delimiter to use when concatenating multiple values of an attribute", deduplicate_attribs: bool = Some("true"); "Whether to deduplicate attribute values.\nExample: 'someAttr=foo,bar,foo,foo' becomes 'someAttr=foo,bar')", - set_attribs_on_success: Vec<(String, Vec)> = Some(""); concat!( + set_attribs_on_success [MULTILINE] : Vec<(String, Vec)> = Some(""); concat!( "Attributes to set manually if the main query succeeds.\n", "If empty, only the attributes returned by LDAP queries are set.\n", "Format: 'attribute=value1, attribute=value2, attribute2= ...'"), - sub_queries: Vec = Some(""); concat!( + sub_queries [MULTILINE]: Vec = Some(""); concat!( "Section names of optional sub-queries.'.\n", "\n", "Sub-queries can check for additional conditions and/or set additional attributes.\n", @@ -190,6 +205,13 @@ pub(crate) fn parse_config(config_file: &str) -> Result, Erro bail!("Unknown key(s) in section [{}]: {}", §ion_name, unknown_keys.join(", ")); } + // Only allow certain keys to appear multiple times + for (key, _) in sect_props.iter() { + if sect_props.get_all(key).count() > 1 && !is_multiline(key) { + bail!("Key '{}' (in section [{}]) is not a list, and therefore cannot appear mutiple times.", key, section_name); + } + } + if section_name == "default" { continue; } @@ -213,8 +235,13 @@ pub(crate) fn parse_config(config_file: &str) -> Result, Erro bail!("Config option(s) not set in section [{}]: {}", section_name, missing_keys.join(", ")); } - let get = |key: &str| sect_props.get(key) - .unwrap_or_else(|| panic!("BUG: missing key '{key}' after checking that it's not missing?!")) + // Helper function to get a value from the section. Combines multiple values with a comma. + let get = |key: &str| sect_props.get_all(key) + .map(|v| v.trim()) + .filter(|v| !v.is_empty()) + .collect::>() + .join(", ") + .trim() .to_string(); // Compile regex @@ -226,6 +253,8 @@ pub(crate) fn parse_config(config_file: &str) -> Result, Erro anyhow!("Invalid value for option '{key}' in section [{section_name}]: {}.\n -- {}", get(key), help_for_key(key)) }; + /// Parse a comma-separated list of assignments. + /// E.g. "key1=value1, key2=value2, key3=value3" fn split_assignments(s: &str) -> Result, Error> { let mut res = Vec::new(); for assignment in s.split(',') diff --git a/src/main.rs b/src/main.rs index 84e4996..302b393 100644 --- a/src/main.rs +++ b/src/main.rs @@ -170,7 +170,7 @@ async fn ldap_query( } }; }; - panic!("BUG: ldap_query() retry loop should never have reached this point"); + unreachable!(); }.await; let result_entries = match res {