diff --git a/apisix/plugins/openid-connect.lua b/apisix/plugins/openid-connect.lua index eb0d3b526c58..38037a68d023 100644 --- a/apisix/plugins/openid-connect.lua +++ b/apisix/plugins/openid-connect.lua @@ -95,6 +95,16 @@ local schema = { description = "the key used for the encrypt and HMAC calculation", minLength = 16, }, + secret_fallbacks = { + type = "array", + items = { + type = "string", + minLength = 16, + }, + description = "List of alternative secrets used when doing key " + .. "rotation. Cookies sealed with a previous secret still " + .. "decrypt while new cookies are sealed with `secret`.", + }, cookie_name = { type = "string", description = "session cookie name", @@ -465,7 +475,8 @@ local schema = { } }, encrypt_fields = {"client_secret", "client_rsa_private_key", - "session.secret", "session.redis.password"}, + "session.secret", "session.secret_fallbacks", + "session.redis.password"}, required = {"client_id", "discovery"} } diff --git a/docs/en/latest/plugins/openid-connect.md b/docs/en/latest/plugins/openid-connect.md index e196d6922206..d2573348e326 100644 --- a/docs/en/latest/plugins/openid-connect.md +++ b/docs/en/latest/plugins/openid-connect.md @@ -68,6 +68,7 @@ The `openid-connect` Plugin supports the integration with [OpenID Connect (OIDC) | set_refresh_token_header | boolean | False | false | | If true and if the refresh token is available, set the value in the `X-Refresh-Token` request header. | | session | object | False | | | Session configuration used when `bearer_only` is `false` and the Plugin uses Authorization Code flow. | | session.secret | string | True | | 16 or more characters | Key used for session encryption and HMAC operation when `bearer_only` is `false`. | +| session.secret_fallbacks | array[string] | False | | each 16 or more characters | List of alternative secrets used for key rotation. Forwarded to lua-resty-session as `secret_fallbacks`. Cookies sealed with any of these secrets still decrypt, while new cookies are sealed with `session.secret`. See [Session key rotation](#session-key-rotation). | | session.cookie_name | string | False | | | Session cookie name. Forwarded to [lua-resty-session](https://github.com/bungle/lua-resty-session#configuration) 4.x as `cookie_name`. | | session.cookie_path | string | False | | | Cookie path scope. Forwarded to lua-resty-session as `cookie_path`. | | session.cookie_domain | string | False | | | Cookie domain scope. Forwarded to lua-resty-session as `cookie_domain`. | @@ -404,6 +405,44 @@ The following diagram illustrates the interaction between different entities whe When `set_userinfo_header` is `true` (the default), the Plugin sets user info data in the `X-Userinfo` request header, which the Upstream can use for further processing. +### Session key rotation + +When sessions are stored in the cookie (`session.storage` is `cookie`, the default), the cookie is sealed with a key derived from `session.secret`. Rotating `session.secret` on its own invalidates every existing cookie at once, forcing all users to re-authenticate. In a high-availability deployment where several APISIX instances are updated one by one, it also causes a window where an instance carrying the new secret cannot decrypt a cookie issued by an instance still using the old one. + +Use `session.secret_fallbacks` to rotate the secret without disrupting active sessions. Cookies sealed with any secret listed in `session.secret_fallbacks` still decrypt, while new cookies are sealed with `session.secret`. Rotate in three stages, completing the rollout of each stage across all instances before starting the next: + +1. Pre-seed: keep the current key as `session.secret` and add the new key to `session.secret_fallbacks`. Every instance can now decrypt cookies sealed with either key, but still seals new cookies with the current key. + + ```json + { + "session": { + "secret": "current-secret-at-least-16-characters", + "secret_fallbacks": ["new-secret-at-least-16-characters"] + } + } + ``` + +2. Flip: make the new key the `session.secret` and keep the previous key in `session.secret_fallbacks`. New cookies are sealed with the new key; cookies still sealed with the previous key continue to decrypt and are re-sealed with the new key on their next write. + + ```json + { + "session": { + "secret": "new-secret-at-least-16-characters", + "secret_fallbacks": ["current-secret-at-least-16-characters"] + } + } + ``` + +3. Retire: once no cookie can still be sealed with the previous key (after `session.absolute_timeout` has elapsed), remove it from `session.secret_fallbacks`. + + ```json + { + "session": { + "secret": "new-secret-at-least-16-characters" + } + } + ``` + ## Troubleshooting This section covers a few commonly seen issues when working with this Plugin to help you troubleshoot. diff --git a/t/plugin/openid-connect-session-rotation.t b/t/plugin/openid-connect-session-rotation.t new file mode 100644 index 000000000000..e8c0c4f2baf6 --- /dev/null +++ b/t/plugin/openid-connect-session-rotation.t @@ -0,0 +1,233 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +use t::APISIX 'no_plan'; + +repeat_each(1); +no_long_string(); +no_root_location(); +no_shuffle(); + +add_block_preprocessor(sub { + my ($block) = @_; + + if ((!defined $block->error_log) && (!defined $block->no_error_log)) { + $block->set_value("no_error_log", "[error]"); + } + + if (!defined $block->request) { + $block->set_value("request", "GET /t"); + } +}); + +run_tests(); + +__DATA__ + +=== TEST 1: set up two routes - one with session.secret_fallbacks, one without +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + + -- Route guarded by openid-connect whose CURRENT session.secret is the + -- "new" key, and whose PREVIOUS key is kept in session.secret_fallbacks. + -- This models the "flip" stage of a rotation: new cookies are sealed + -- with `secret`, old cookies still open via a fallback. + local code = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "plugins": { + "openid-connect": { + "client_id": "kbyuFDidLLm280LIwVFiazOqjO3ty8KH", + "client_secret": "60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa", + "discovery": "http://127.0.0.1:1980/.well-known/openid-configuration", + "redirect_uri": "http://127.0.0.1:1984/callback", + "ssl_verify": false, + "use_pkce": false, + "set_userinfo_header": true, + "renew_access_token_on_expiry": false, + "unauth_action": "deny", + "session": { + "secret": "new_session_secret_at_least_16", + "secret_fallbacks": ["old_session_secret_at_least_16"] + } + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/uri" + }]] + ) + if code >= 300 then + ngx.say("route 1 failed: ", code) + return + end + + -- Same, but WITHOUT secret_fallbacks: a cookie sealed with the old key + -- can no longer be decrypted. This is the negative control. + local code = t('/apisix/admin/routes/2', + ngx.HTTP_PUT, + [[{ + "plugins": { + "openid-connect": { + "client_id": "kbyuFDidLLm280LIwVFiazOqjO3ty8KH", + "client_secret": "60Op4HFM0I8ajz0WdiStAbziZ-VFQttXuxixHHs2R7r7-CW8GR79l-mmLqMhc-Sa", + "discovery": "http://127.0.0.1:1980/.well-known/openid-configuration", + "redirect_uri": "http://127.0.0.1:1984/callback", + "ssl_verify": false, + "use_pkce": false, + "set_userinfo_header": true, + "renew_access_token_on_expiry": false, + "unauth_action": "deny", + "session": { + "secret": "new_session_secret_at_least_16" + } + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/uri-no-fallback" + }]] + ) + if code >= 300 then + ngx.say("route 2 failed: ", code) + return + end + + ngx.say("passed") + } + } +--- response_body +passed + + + +=== TEST 2: a cookie sealed with the OLD key is accepted via secret_fallbacks +--- config + location /t { + content_by_lua_block { + local http = require("resty.http") + local session = require("resty.session") + local core = require("apisix.core") + + -- Forge the session openid-connect writes after a successful login, + -- sealed with the OLD key. With a non-expired access token and + -- renew_access_token_on_expiry disabled, no IdP is contacted; the + -- plugin takes the "already authenticated" path and must open the + -- cookie via session.secret_fallbacks. + local s = session.new({ secret = "old_session_secret_at_least_16" }) + s:set("authenticated", true) + s:set("last_authenticated", ngx.time()) + s:set("id_token", { sub = "a UID" }) + s:set("access_token", "fake-access-token") + s:set("access_token_expiration", ngx.time() + 3600) + s:set("user", core.json.decode( + '{"sub":"a UID","name":"Testuser One","roles":[]}')) + + local ok, err = s:save() + if not ok then + ngx.say("failed to save session: ", err) + return + end + + local cookie = ngx.header["Set-Cookie"] + if type(cookie) == "table" then + cookie = cookie[1] + end + ngx.header["Set-Cookie"] = nil + + local httpc = http.new() + local res, err = httpc:request_uri("http://127.0.0.1:1984/uri", { + headers = { Cookie = cookie }, + }) + if not res then + ngx.say("request failed: ", err) + return + end + + local has_userinfo = res.body:match("x%-userinfo: ") ~= nil + if res.status == 200 and has_userinfo then + ngx.say("fallback-accepted") + else + ngx.say("unexpected: status=", res.status, + " userinfo=", tostring(has_userinfo)) + end + } + } +--- response_body +fallback-accepted + + + +=== TEST 3: negative control - without secret_fallbacks the OLD cookie is rejected +--- config + location /t { + content_by_lua_block { + local http = require("resty.http") + local session = require("resty.session") + local core = require("apisix.core") + + -- Same OLD-key cookie, but the route has no secret_fallbacks, so the + -- plugin cannot decrypt it and (unauth_action = deny) returns 401. + -- This proves the fallback is what makes TEST 2 succeed. + local s = session.new({ secret = "old_session_secret_at_least_16" }) + s:set("authenticated", true) + s:set("last_authenticated", ngx.time()) + s:set("id_token", { sub = "a UID" }) + s:set("access_token", "fake-access-token") + s:set("access_token_expiration", ngx.time() + 3600) + s:set("user", core.json.decode( + '{"sub":"a UID","name":"Testuser One","roles":[]}')) + + local ok, err = s:save() + if not ok then + ngx.say("failed to save session: ", err) + return + end + + local cookie = ngx.header["Set-Cookie"] + if type(cookie) == "table" then + cookie = cookie[1] + end + ngx.header["Set-Cookie"] = nil + + local httpc = http.new() + local res, err = httpc:request_uri("http://127.0.0.1:1984/uri-no-fallback", { + headers = { Cookie = cookie }, + }) + if not res then + ngx.say("request failed: ", err) + return + end + + if res.status == 401 then + ngx.say("no-fallback-rejected") + else + ngx.say("unexpected: status=", res.status) + end + } + } +--- response_body +no-fallback-rejected diff --git a/t/plugin/openid-connect2.t b/t/plugin/openid-connect2.t index b7877ac33255..c54f6683178f 100644 --- a/t/plugin/openid-connect2.t +++ b/t/plugin/openid-connect2.t @@ -82,6 +82,34 @@ __DATA__ assert(ok and case.session and case.session.secret and case.session.secret == "test_secret_more_than_16", "user-set secret is incorrect") end, }, + { + name = "session.secret_fallbacks accepts an array of valid secrets", + data = {client_id = "a", client_secret = "b", discovery = "c", bearer_only = false, session = {secret = "new_secret_at_least_16", secret_fallbacks = {"old_secret_at_least_16", "older_secret_at_least_16"}}}, + cb = function(ok, err, case) + assert(ok and case.session.secret_fallbacks and case.session.secret_fallbacks[1] == "old_secret_at_least_16" and case.session.secret_fallbacks[2] == "older_secret_at_least_16", "session.secret_fallbacks should be accepted") + end, + }, + { + name = "session.secret_fallbacks rejects an element shorter than 16 characters", + data = {client_id = "a", client_secret = "b", discovery = "c", bearer_only = false, session = {secret = "new_secret_at_least_16", secret_fallbacks = {"short"}}}, + cb = function(ok, err, case) + assert(not ok and err == "property \"session\" validation failed: property \"secret_fallbacks\" validation failed: failed to validate item 1: string too short, expected at least 16, got 5", "too short fallback should fail validation, got: " .. tostring(err)) + end, + }, + { + name = "session.secret_fallbacks rejects a non-array value", + data = {client_id = "a", client_secret = "b", discovery = "c", bearer_only = false, session = {secret = "new_secret_at_least_16", secret_fallbacks = "old_secret_at_least_16"}}, + cb = function(ok, err, case) + assert(not ok and err == "property \"session\" validation failed: property \"secret_fallbacks\" validation failed: wrong type: expected array, got string", "non-array fallbacks should fail validation, got: " .. tostring(err)) + end, + }, + { + name = "session.secret without secret_fallbacks stays valid (backward compatible)", + data = {client_id = "a", client_secret = "b", discovery = "c", bearer_only = false, session = {secret = "new_secret_at_least_16"}}, + cb = function(ok, err, case) + assert(ok and case.session.secret == "new_secret_at_least_16" and case.session.secret_fallbacks == nil, "session without fallbacks should remain valid") + end, + }, } local plugin = require("apisix.plugins.openid-connect")