From 46c4f782da62b58ba8cb67d7a522f355c327eb79 Mon Sep 17 00:00:00 2001 From: Jarno Elonen Date: Mon, 30 Jan 2023 14:21:29 +0200 Subject: [PATCH] Optimize header cache RAM layout, add tests for cache hit/miss --- README.md | 12 ++++++++---- run-tests.sh | 39 ++++++++++++++++++++++----------------- src/main.rs | 21 +++++++++++---------- test/Dockerfile | 1 + test/nginx-site.conf | 9 +++++++-- 5 files changed, 49 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index 89daf17..f160b16 100644 --- a/README.md +++ b/README.md @@ -53,10 +53,11 @@ The `ldap_return_attribs`, if not empty, specifies a comma-separated list of LDA attributes to return to Nginx in HTTP headers. The header names are prefixed with `X-Ldap-Authz-Res-`, so for example `displayName` attribute is returned in `X-Ldap-Authz-Res-displayName` header. Use `ldap_return_attribs = *` to return all -attributes (mainly useful for debugging). +attributes (mainly useful for debugging). Attributes with multiple values are +concatenated with `;` separator. -If LDAP query returns multiple results, the first one is used. To see all results, -use `--debug` option to write them to log. +If LDAP query returns multiple objects, the first one is used. To see the rest, +use `--debug` option to log them. ## Cache @@ -70,6 +71,9 @@ A cache entry takes probably about 1kB of RAM, unless you requested all LDAP att Technically, each config section gets its own cache, so you can have different cache sizes and retention times for different sections. +HTTP response headers contain `X-Ldap-Cached` header that is set to `1` if the response +was served from cache, and `0` if it was a fresh query. + ## Building The server is written in Rust and can be built with `cargo build --release`. @@ -214,7 +218,7 @@ sed -i 's@ldap_server_url *=.*@ldap_server_url = ldap://127.0.0.1:3890@' example cargo run -- example.ini --debug & # Test request directly against ldap_authz_proxy -curl http://127.0.0.1:10567/admins -H "X-Ldap-Authz-Username:alice" +curl http://127.0.0.1:10567/admins -H "X-Ldap-Authz-Username:alice" -I # Cleanup kill %1 # Or do: fg + ctrl-c diff --git a/run-tests.sh b/run-tests.sh index 41b4c3c..42ee194 100755 --- a/run-tests.sh +++ b/run-tests.sh @@ -8,10 +8,7 @@ echo "-------------------------------------------------------------------------- cd test DOCKER_BUILDKIT=1 docker compose build docker compose up -d - -# Make sure we stop the containers when we exit, regardless of how -trap 'echo "---"; docker compose down' EXIT - +trap 'echo "---"; docker compose down' EXIT # Make sure we stop the containers when we exit, regardless of how echo "---------------------------------------------" echo "Waiting for services to start up..." @@ -47,10 +44,13 @@ echo "---------------------------------------------" function request() { FOLDER="$1" CREDS="$2" - RES=$(curl -s http://127.0.0.1:8090/$FOLDER/ -u "$CREDS" -I) + # Don't remove the "/index.html", otherwise Nginx makes an internal redirect, and warms up LDAP cache prematurely + RES=$(curl -s http://127.0.0.1:8090/$FOLDER/index.html -u "$CREDS" -I) + # cat <<< """$RES""" >&2 HTTP_CODE=$(grep HTTP <<< """$RES""" | awk '{print $2}' | tr -d '\r\n') - DISPLAY_NAME=$(grep 'X-Display-Name' <<< """$RES""" | sed 's/^.*: //' | tr -d '\r\n') || true - echo "${HTTP_CODE}${DISPLAY_NAME}" + DISPLAY_NAME=$(grep -i 'X-Display-Name' <<< """$RES""" | sed 's/^.*: //' | tr -d '\r\n') || true + LDAP_CACHED=$(grep -i 'X-LDAP-Cached' <<< """$RES""" | sed 's/^.*: //' | tr -d '\r\n') || true + echo "${HTTP_CODE}${DISPLAY_NAME} c${LDAP_CACHED}" } function test() { @@ -59,24 +59,29 @@ function test() { EXPECTED="$3" ACTUAL="$(request $FOLDER $CREDS)" if [ "$ACTUAL" != "$EXPECTED" ]; then - echo "Test FAILED - expected $EXPECTED, got $ACTUAL for '$FOLDER' with '$CREDS'" + echo "Test FAILED - expected '$EXPECTED', got '$ACTUAL' for '$FOLDER' with '$CREDS'" else echo "Test OK for '$FOLDER' with '$CREDS' ($ACTUAL == $EXPECTED)" fi } function do_tests() { - test "user-page" "alice:alice" "200Alice Alison" - test "admin-page" "alice:alice" "200" - test "user-page" "alice:BADPASSWORD" "401" - test "admin-page" "alice:BADPASSWORD" "401" + test "user-page" "alice:alice" "200Alice Alison c0" + test "admin-page" "alice:alice" "200 c0" + test "user-page" "alice:BADPASSWORD" "401 c" + test "admin-page" "alice:BADPASSWORD" "401 c" - test "user-page" "bob:bob" "200Bob Bobrikov" - test "admin-page" "bob:bob" "403" - test "user-page" "bob:BADPASSWORD" "401" - test "admin-page" "bob:BADPASSWORD" "401" + test "user-page" "bob:bob" "200Bob Bobrikov c0" + test "admin-page" "bob:bob" "403 c" + test "user-page" "bob:BADPASSWORD" "401 c" + test "admin-page" "bob:BADPASSWORD" "401 c" - test "bad-page" "alice:alice" "404" + test "bad-page" "alice:alice" "404 c" + + echo "(Repeat and check that query came from cache)" + test "user-page" "alice:alice" "200Alice Alison c1" + test "admin-page" "alice:alice" "200 c1" + test "user-page" "bob:bob" "200Bob Bobrikov c1" } # Run the tests and summarize diff --git a/src/main.rs b/src/main.rs index 8da4b12..e01e4dd 100644 --- a/src/main.rs +++ b/src/main.rs @@ -41,7 +41,7 @@ Options: "#; type Sha256Hash = sha2::digest::generic_array::GenericArray; -type LdapSearchRes = Option>; // HashMap or None if not found +type LdapSearchRes = Option>; // HashMap or None if not found type LdapCache = LruCache; struct ReqContext { @@ -59,7 +59,7 @@ struct LdapAnswer { /// Caches the result for the next time. /// /// TODO: Pool connections. Currently a new LDAP connection is created for every request unless cache is hit. -async fn ldap_check(conf: &ConfigSection, username: &str, cache: &Arc>) -> ldap3::result::Result { +async fn ldap_query(conf: &ConfigSection, username: &str, cache: &Arc>) -> ldap3::result::Result { let query = conf.ldap_query.replace("%USERNAME%", &ldap3::ldap_escape(username)); tracing::debug!("LDAP string: {}", query); @@ -102,7 +102,7 @@ async fn ldap_check(conf: &ConfigSection, username: &str, cache: &Arc 0 { @@ -110,8 +110,8 @@ async fn ldap_check(conf: &ConfigSection, username: &str, cache: &Arc>().join(", "); - attribs.insert(key, vals_comb); + let vals_comb = vals.iter().map(|v| v.as_str()).collect::>().join(";"); + attribs.push((key, vals_comb)); } } } @@ -168,17 +168,16 @@ async fn http_handler(req: Request, ctx: Arc) -> Result { span.in_scope(|| { tracing::error!("LDAP error: {:?}", e); }); return Response::builder().status(StatusCode::BAD_GATEWAY).body(Body::from("LDAP error")) }, Ok(la) => { let span = span.record("cached", &la.cached); - return if let Some(ldap_res) = la.ldap_res { + if let Some(ldap_res) = la.ldap_res { span.in_scope(|| { tracing::info!("User authorized Ok"); }); let mut resp = Response::new(Body::from("200 OK - LDAP result found")); - // Store LDAP result attributes to response HTTP headers for (key, val) in ldap_res.iter() { let hname = match HeaderName::from_str(format!("X-LDAP-RES-{}", key).as_str()) { @@ -198,10 +197,12 @@ async fn http_handler(req: Request, ctx: Arc) -> Result