diff --git a/apisix/plugins/cas-auth.lua b/apisix/plugins/cas-auth.lua index d949636f5c75..d6462904052d 100644 --- a/apisix/plugins/cas-auth.lua +++ b/apisix/plugins/cas-auth.lua @@ -16,12 +16,15 @@ ---- local core = require("apisix.core") local http = require("resty.http") +local hmac = require("resty.openssl.hmac") +local bit = require("bit") local ngx = ngx local ngx_re_match = ngx.re.match +local ngx_encode_base64 = ngx.encode_base64 +local ngx_decode_base64 = ngx.decode_base64 local CAS_REQUEST_URI = "CAS_REQUEST_URI" local COOKIE_NAME = "CAS_SESSION" -local COOKIE_PARAMS = "; Path=/; HttpOnly" local SESSION_LIFETIME = 3600 local STORE_NAME = "cas_sessions" @@ -35,9 +38,19 @@ local schema = { idp_uri = {type = "string"}, cas_callback_uri = {type = "string"}, logout_uri = {type = "string"}, + cookie = { + type = "object", + properties = { + secret = {type = "string", minLength = 32}, + secure = {type = "boolean", default = true}, + samesite = {type = "string", enum = {"Lax", "None"}, default = "Lax"}, + }, + required = {"secret"}, + }, }, + encrypt_fields = {"cookie.secret"}, required = { - "idp_uri", "cas_callback_uri", "logout_uri" + "idp_uri", "cas_callback_uri", "logout_uri", "cookie" } } @@ -45,9 +58,18 @@ local _M = { version = 0.1, priority = 2597, name = plugin_name, - schema = schema + schema = schema, } +local function cookie_attrs(conf) + local attrs = "; Path=/; HttpOnly" + if conf.cookie.secure then + attrs = attrs .. "; Secure" + end + attrs = attrs .. "; SameSite=" .. conf.cookie.samesite + return attrs +end + function _M.check_schema(conf) local check = {"idp_uri"} core.utils.check_https(check, conf, plugin_name) @@ -63,41 +85,101 @@ local function get_session_id(ctx) return ctx.var["cookie_" .. COOKIE_NAME] end -local function set_our_cookie(name, val) - core.response.add_header("Set-Cookie", name .. "=" .. val .. COOKIE_PARAMS) +local function set_our_cookie(conf, name, val) + core.response.add_header("Set-Cookie", name .. "=" .. val .. cookie_attrs(conf)) +end + +local function compute_hmac(secret, val) + local h, err = hmac.new(secret, "sha256") + if not h then return nil, err end + local ok, err2 = h:update(val) + if not ok then return nil, err2 end + return h:final() +end + +local function eq_const_time(a, b) + if #a ~= #b then return false end + local diff = 0 + for i = 1, #a do + diff = bit.bor(diff, bit.bxor(a:byte(i), b:byte(i))) + end + return diff == 0 +end + +local function sign_value(secret, val) + local sig, err = compute_hmac(secret, val) + if not sig then + core.log.error("cas-auth: hmac sign failed: ", err) + return nil + end + return ngx_encode_base64(val, true) .. "." .. ngx_encode_base64(sig, true) +end + +local function verify_value(secret, signed) + if not signed then return nil end + local dot = signed:find(".", 1, true) + if not dot then return nil end + local val = ngx_decode_base64(signed:sub(1, dot - 1)) + local sig = ngx_decode_base64(signed:sub(dot + 1)) + if not val or not sig then return nil end + local expected, err = compute_hmac(secret, val) + if not expected then + core.log.error("cas-auth: hmac verify failed: ", err) + return nil + end + if not eq_const_time(sig, expected) then return nil end + return val +end + +local function is_safe_redirect(uri) + if not uri or uri == "" then return false end + if uri:sub(1, 1) ~= "/" then return false end + if uri:sub(1, 2) == "//" then return false end + if uri:find("\\", 1, true) then return false end + if uri:find("[\r\n]") then return false end + return true end +-- Exposed for unit tests; not part of the plugin's public API. +_M._test_helpers = { + sign_value = sign_value, + verify_value = verify_value, + is_safe_redirect = is_safe_redirect, +} + local function first_access(conf, ctx) local login_uri = conf.idp_uri .. "/login?" .. ngx.encode_args({ service = uri_without_ticket(conf, ctx) }) - core.log.info("first access: ", login_uri, - ", cookie: ", ctx.var.http_cookie, ", request_uri: ", ctx.var.request_uri) - set_our_cookie(CAS_REQUEST_URI, ctx.var.request_uri) + core.log.info("cas-auth: redirecting unauthenticated request to IdP") + local signed = sign_value(conf.cookie.secret, ctx.var.request_uri) + if signed then + set_our_cookie(conf, CAS_REQUEST_URI, signed) + end core.response.set_header("Location", login_uri) return ngx.HTTP_MOVED_TEMPORARILY end local function with_session_id(conf, ctx, session_id) -- does the cookie exist in our store? - local user = store:get(session_id); - core.log.info("ticket=", session_id, ", user=", user) + local user = store:get(session_id) if user == nil then - set_our_cookie(COOKIE_NAME, "deleted; Max-Age=0") + set_our_cookie(conf, COOKIE_NAME, "deleted; Max-Age=0") return first_access(conf, ctx) else -- refresh the TTL store:set(session_id, user, SESSION_LIFETIME) + core.log.info("cas-auth: session refreshed for user=", user) end end -local function set_store_and_cookie(session_id, user) +local function set_store_and_cookie(conf, session_id, user) -- place cookie into cookie store local success, err, forcible = store:add(session_id, user, SESSION_LIFETIME) if success then if forcible then core.log.info("CAS cookie store is out of memory") end - set_our_cookie(COOKIE_NAME, session_id) + set_our_cookie(conf, COOKIE_NAME, session_id) else if err == "no memory" then core.log.emerg("CAS cookie store is out of memory") @@ -119,12 +201,12 @@ local function validate(conf, ctx, ticket) if res and res.status == ngx.HTTP_OK and res.body ~= nil then if core.string.find(res.body, "") then - local m = ngx_re_match(res.body, "(.*?)", "jo"); + local m = ngx_re_match(res.body, "(.*?)", "jo") if m then return m[1] end else - core.log.info("CAS serviceValidate failed: ", res.body) + core.log.info("CAS serviceValidate did not return authenticationSuccess") end else core.log.error("validate ticket failed: status=", (res and res.status), @@ -135,11 +217,15 @@ end local function validate_with_cas(conf, ctx, ticket) local user = validate(conf, ctx, ticket) - if user and set_store_and_cookie(ticket, user) then - local request_uri = ctx.var["cookie_" .. CAS_REQUEST_URI] - set_our_cookie(CAS_REQUEST_URI, "deleted; Max-Age=0") - core.log.info("ticket: ", ticket, - ", cookie: ", ctx.var.http_cookie, ", request_uri: ", request_uri, ", user=", user) + if user and set_store_and_cookie(conf, ticket, user) then + local request_uri = verify_value(conf.cookie.secret, + ctx.var["cookie_" .. CAS_REQUEST_URI]) + set_our_cookie(conf, CAS_REQUEST_URI, "deleted; Max-Age=0") + if not is_safe_redirect(request_uri) then + core.log.warn("cas-auth: rejected unsafe redirect target, falling back to /") + request_uri = "/" + end + core.log.info("cas-auth: validation succeeded for user=", user) core.response.set_header("Location", request_uri) return ngx.HTTP_MOVED_TEMPORARILY else @@ -153,9 +239,9 @@ local function logout(conf, ctx) return ngx.HTTP_UNAUTHORIZED end - core.log.info("logout: ticket=", session_id, ", cookie=", ctx.var.http_cookie) + core.log.info("cas-auth: logout invoked") store:delete(session_id) - set_our_cookie(COOKIE_NAME, "deleted; Max-Age=0") + set_our_cookie(conf, COOKIE_NAME, "deleted; Max-Age=0") core.response.set_header("Location", conf.idp_uri .. "/logout") return ngx.HTTP_MOVED_TEMPORARILY @@ -176,12 +262,12 @@ function _M.access(conf, ctx) return ngx.HTTP_BAD_REQUEST, {message = "invalid logout request from IdP, no ticket"} end - core.log.info("Back-channel logout (SLO) from IdP: LogoutRequest: ", data) + core.log.info("cas-auth: SLO request received from IdP") local session_id = ticket - local user = store:get(session_id); + local user = store:get(session_id) if user then store:delete(session_id) - core.log.info("SLO: user=", user, ", tocket=", ticket) + core.log.info("cas-auth: SLO session deleted for user=", user) end else local session_id = get_session_id(ctx) diff --git a/docs/en/latest/plugins/cas-auth.md b/docs/en/latest/plugins/cas-auth.md index 37d23b068a30..0456b7fc3cb5 100644 --- a/docs/en/latest/plugins/cas-auth.md +++ b/docs/en/latest/plugins/cas-auth.md @@ -35,11 +35,15 @@ to do authentication, from the SP (service provider) perspective. ## Attributes -| Name | Type | Required | Description | -| ----------- | ----------- | ----------- | ----------- | -| `idp_uri` | string | True | URI of IdP. | -| `cas_callback_uri` | string | True | redirect uri used to callback the SP from IdP after login or logout. | -| `logout_uri` | string | True | logout uri to trigger logout. | +| Name | Type | Required | Default | Description | +| ----------- | ----------- | ----------- | ----------- | ----------- | +| `idp_uri` | string | True | | URI of IdP. | +| `cas_callback_uri` | string | True | | redirect uri used to callback the SP from IdP after login or logout. | +| `logout_uri` | string | True | | logout uri to trigger logout. | +| `cookie` | object | True | | configuration for the cookies the plugin issues during the CAS login flow. | +| `cookie.secret` | string | True | | secret (32+ characters) used to sign the request URI cookie. The same value must be configured on every APISIX node. Generate with e.g. `openssl rand -base64 48`. | +| `cookie.secure` | boolean | False | `true` | whether to set the `Secure` attribute on the issued cookies. Set to `false` only for deployments where the protected route is not served over HTTPS (e.g. internal-only or development environments). | +| `cookie.samesite` | string | False | `"Lax"` | value for the `SameSite` cookie attribute. Allowed values are `"Lax"` and `"None"`; `"Strict"` is intentionally not supported because it breaks the IdP→SP redirect when the IdP is on a different site. | ## Enable Plugin @@ -64,7 +68,10 @@ curl http://127.0.0.1:9180/apisix/admin/routes/cas1 -H "X-API-KEY: $admin_key" - "cas-auth": { "idp_uri": "http://127.0.0.1:8080/realms/test/protocol/cas", "cas_callback_uri": "/anything/cas_callback", - "logout_uri": "/anything/logout" + "logout_uri": "/anything/logout", + "cookie": { + "secret": "please-replace-with-a-32+-char-random-secret" + } } }, "upstream": { diff --git a/t/lib/keycloak_cas.lua b/t/lib/keycloak_cas.lua index 7e578014ce8f..a24be01c336d 100644 --- a/t/lib/keycloak_cas.lua +++ b/t/lib/keycloak_cas.lua @@ -22,6 +22,10 @@ local default_opts = { idp_uri = "http://127.0.0.1:8080/realms/test/protocol/cas", cas_callback_uri = "/cas_callback", logout_uri = "/logout", + cookie = { + secret = "0123456789abcdef0123456789abcdef", + secure = false, + }, } function _M.get_default_opts() diff --git a/t/plugin/cas-auth.t b/t/plugin/cas-auth.t index 4a2bfe7e874b..9da02e5fc5ce 100644 --- a/t/plugin/cas-auth.t +++ b/t/plugin/cas-auth.t @@ -221,3 +221,141 @@ passed assert(res.status == 200) } } + + + +=== TEST 5: schema rejects missing cookie.secret +--- config + location /t { + content_by_lua_block { + local plugin = require("apisix.plugins.cas-auth") + local ok, err = plugin.check_schema({ + idp_uri = "http://127.0.0.1:8080", + cas_callback_uri = "/cas_callback", + logout_uri = "/logout", + cookie = {}, + }) + ngx.say(ok and "passed" or err) + } + } +--- response_body_like +.*property "secret" is required.* + + + +=== TEST 6: schema rejects cookie.secret shorter than 32 chars +--- config + location /t { + content_by_lua_block { + local plugin = require("apisix.plugins.cas-auth") + local ok, err = plugin.check_schema({ + idp_uri = "http://127.0.0.1:8080", + cas_callback_uri = "/cas_callback", + logout_uri = "/logout", + cookie = { secret = "tooshort" }, + }) + ngx.say(ok and "passed" or err) + } + } +--- response_body_like +.*string too short.* + + + +=== TEST 7: schema rejects cookie.samesite=Strict +--- config + location /t { + content_by_lua_block { + local plugin = require("apisix.plugins.cas-auth") + local ok, err = plugin.check_schema({ + idp_uri = "http://127.0.0.1:8080", + cas_callback_uri = "/cas_callback", + logout_uri = "/logout", + cookie = { + secret = "0123456789abcdef0123456789abcdef", + samesite = "Strict", + }, + }) + ngx.say(ok and "passed" or err) + } + } +--- response_body_like +.*samesite.* + + + +=== TEST 8: is_safe_redirect rejects external and protocol-relative URLs +--- config + location /t { + content_by_lua_block { + local plugin = require("apisix.plugins.cas-auth") + local h = plugin._test_helpers + local cases = { + {"/foo", true}, + {"/foo?bar=baz", true}, + {"https://evil.com/x", false}, + {"//evil.com/x", false}, + {"\\\\evil.com", false}, + {"/foo\r\nLocation: x", false}, + {"", false}, + {nil, false}, + } + for _, c in ipairs(cases) do + local got = h.is_safe_redirect(c[1]) + if got ~= c[2] then + ngx.say("FAIL ", tostring(c[1]), " expected ", tostring(c[2]), + " got ", tostring(got)) + return + end + end + ngx.say("passed") + } + } +--- response_body +passed + + + +=== TEST 9: sign and verify roundtrip + tamper detection +--- config + location /t { + content_by_lua_block { + local plugin = require("apisix.plugins.cas-auth") + local h = plugin._test_helpers + local secret = "0123456789abcdef0123456789abcdef" + local signed = h.sign_value(secret, "/foo?bar=baz") + assert(signed, "sign_value returned nil") + + local got = h.verify_value(secret, signed) + if got ~= "/foo?bar=baz" then + ngx.say("FAIL roundtrip got=", tostring(got)) + return + end + + -- flip last char of the signature segment + local tampered = signed:sub(1, -2) .. + (signed:sub(-1) == "A" and "B" or "A") + if h.verify_value(secret, tampered) ~= nil then + ngx.say("FAIL tampered signature accepted") + return + end + + -- a different secret must not validate + if h.verify_value("XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX", signed) ~= nil then + ngx.say("FAIL wrong secret accepted") + return + end + + -- nil and malformed inputs + if h.verify_value(secret, nil) ~= nil + or h.verify_value(secret, "no-dot-here") ~= nil + or h.verify_value(secret, "abc.def") ~= nil then + ngx.say("FAIL malformed cookie accepted") + return + end + + ngx.say("passed") + } + } +--- response_body +passed diff --git a/t/plugin/security-warning.t b/t/plugin/security-warning.t index f2d2e3ea6375..0dca62e7bbd4 100644 --- a/t/plugin/security-warning.t +++ b/t/plugin/security-warning.t @@ -164,7 +164,10 @@ Using authz-keycloak access_denied_redirect_uri with no TLS is a security risk local ok, err = plugin.check_schema({ idp_uri = "http://a.com", cas_callback_uri = "/a/b", - logout_uri = "/c/d" + logout_uri = "/c/d", + cookie = { + secret = "0123456789abcdef0123456789abcdef", + }, }) if not ok then @@ -189,7 +192,10 @@ risk local ok, err = plugin.check_schema({ idp_uri = "https://a.com", cas_callback_uri = "/a/b", - logout_uri = "/c/d" + logout_uri = "/c/d", + cookie = { + secret = "0123456789abcdef0123456789abcdef", + }, }) if not ok then ngx.say(err)