From 6c63512467ead758dce49fc50c31dd462fa9097d Mon Sep 17 00:00:00 2001 From: Dylan Tack Date: Tue, 4 May 2021 23:31:24 -0700 Subject: [PATCH 1/3] Support rotation of RSA keys --- docs/oidc.rst | 26 +++++++++++++++++++++++++- docs/settings.rst | 19 +++++++++++++++++++ oauth2_provider/settings.py | 2 ++ oauth2_provider/views/oidc.py | 19 +++++++++++++++---- tests/presets.py | 1 + tests/settings.py | 18 ++++++++++++++++++ tests/test_oidc_views.py | 26 ++++++++++++++++++++++++++ 7 files changed, 106 insertions(+), 5 deletions(-) diff --git a/docs/oidc.rst b/docs/oidc.rst index 87fadce57..ba69e984f 100644 --- a/docs/oidc.rst +++ b/docs/oidc.rst @@ -100,6 +100,30 @@ change this class to derive from ``oauthlib.openid.Server`` instead of With ``RSA`` key-pairs, the public key can be generated from the private key, so there is no need to add a setting for the public key. + +Rotating the RSA private key +~~~~~~~~~~~~~~~~~~~~~~~~ +Extra keys can be published in the jwks_uri with the ``OIDC_RSA_PRIVATE_KEYS_INACTIVE`` +setting. For example::: + + OAUTH2_PROVIDER = { + "OIDC_RSA_PRIVATE_KEY": os.environ.get("OIDC_RSA_PRIVATE_KEY"), + "OIDC_RSA_PRIVATE_KEYS_INACTIVE": [ + os.environ.get("OIDC_RSA_PRIVATE_KEY_2"), + os.environ.get("OIDC_RSA_PRIVATE_KEY_3") + ] + # ... other settings + } + +To rotate, follow these steps: + +#. Generate a new key, and add it to the inactive set. Then deploy the app. +#. Swap the active and inactive keys, then re-deploy. +#. After some reasonable amount of time, remove the inactive key. At a minimum, + you should wait ``ID_TOKEN_EXPIRE_SECONDS`` to ensure the key isn't removed + before valid tokens expire. + + Using ``HS256`` keys ~~~~~~~~~~~~~~~~~~~~ @@ -297,7 +321,7 @@ query, and other details. JwksInfoView ~~~~~~~~~~~~ -Available at ``/o/.well-known/jwks.json``, this view provides details of the key used to sign +Available at ``/o/.well-known/jwks.json``, this view provides details of the keys used to sign the JWTs generated for ID tokens, so that clients are able to verify them. diff --git a/docs/settings.rst b/docs/settings.rst index de7bcf85c..f21d9d6cc 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -262,6 +262,25 @@ Default: ``""`` The RSA private key used to sign OIDC ID tokens. If not set, OIDC is disabled. +OIDC_RSA_PRIVATE_KEYS_INACTIVE +~~~~~~~~~~~~~~~~~~~~ +Default: ``[]`` + +An array of *inactive* RSA private keys. These keys are not used to sign tokens, +but are published in the jwks_uri location. + +This is useful for providing a smooth transition during key rotation. +``OIDC_RSA_PRIVATE_KEY`` can be replaced, and recently decommissioned keys +should be retained in this inactive list. + +OIDC_JWKS_MAX_AGE_SECONDS +~~~~~~~~~~~~~~~~~~~~~~ +Default: ``3600`` + +The max-age value for the Cache-Control header on jwks_uri. + +This enables the verifier to safely cache the JWK Set and not have to re-download +the document for every token. OIDC_USERINFO_ENDPOINT ~~~~~~~~~~~~~~~~~~~~~~ diff --git a/oauth2_provider/settings.py b/oauth2_provider/settings.py index b862fca7a..9a996b0c2 100644 --- a/oauth2_provider/settings.py +++ b/oauth2_provider/settings.py @@ -72,6 +72,8 @@ "OIDC_ISS_ENDPOINT": "", "OIDC_USERINFO_ENDPOINT": "", "OIDC_RSA_PRIVATE_KEY": "", + "OIDC_RSA_PRIVATE_KEYS_INACTIVE": [], + "OIDC_JWKS_MAX_AGE_SECONDS": 3600, "OIDC_RESPONSE_TYPES_SUPPORTED": [ "code", "token", diff --git a/oauth2_provider/views/oidc.py b/oauth2_provider/views/oidc.py index 00c8c3fa4..b4bb8869b 100644 --- a/oauth2_provider/views/oidc.py +++ b/oauth2_provider/views/oidc.py @@ -71,12 +71,23 @@ class JwksInfoView(OIDCOnlyMixin, View): def get(self, request, *args, **kwargs): keys = [] if oauth2_settings.OIDC_RSA_PRIVATE_KEY: - key = jwk.JWK.from_pem(oauth2_settings.OIDC_RSA_PRIVATE_KEY.encode("utf8")) - data = {"alg": "RS256", "use": "sig", "kid": key.thumbprint()} - data.update(json.loads(key.export_public())) - keys.append(data) + for pem in [ + oauth2_settings.OIDC_RSA_PRIVATE_KEY, + *oauth2_settings.OIDC_RSA_PRIVATE_KEYS_INACTIVE, + ]: + + key = jwk.JWK.from_pem(pem.encode("utf8")) + data = {"alg": "RS256", "use": "sig", "kid": key.thumbprint()} + data.update(json.loads(key.export_public())) + keys.append(data) response = JsonResponse({"keys": keys}) response["Access-Control-Allow-Origin"] = "*" + response["Cache-Control"] = ( + "Cache-Control: public, " + + f"max-age={oauth2_settings.OIDC_JWKS_MAX_AGE_SECONDS}, " + + f"stale-while-revalidate={oauth2_settings.OIDC_JWKS_MAX_AGE_SECONDS}, " + + f"stale-if-error={oauth2_settings.OIDC_JWKS_MAX_AGE_SECONDS}" + ) return response diff --git a/tests/presets.py b/tests/presets.py index 214f804ef..438da1e03 100644 --- a/tests/presets.py +++ b/tests/presets.py @@ -12,6 +12,7 @@ "OIDC_ISS_ENDPOINT": "http://localhost/o", "OIDC_USERINFO_ENDPOINT": "http://localhost/o/userinfo/", "OIDC_RSA_PRIVATE_KEY": settings.OIDC_RSA_PRIVATE_KEY, + "OIDC_RSA_PRIVATE_KEYS_INACTIVE": settings.OIDC_RSA_PRIVATE_KEYS_INACTIVE, "SCOPES": { "read": "Reading scope", "write": "Writing scope", diff --git a/tests/settings.py b/tests/settings.py index 1d295982e..bc7a55130 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -134,6 +134,24 @@ dTnvCVtA59ne4LEVie/PMH/odQWY0SxVm/76uBZv/1vY -----END RSA PRIVATE KEY-----""" +OIDC_RSA_PRIVATE_KEYS_INACTIVE = [ + """-----BEGIN RSA PRIVATE KEY----- +MIICXAIBAAKBgQDSpXNtxaD9+DKBnSWJNoV6h0PZuSKeGPyA8n0/as/O+oboiYj1 +gqQSTwPFxzt5Zy52fDmIQvzDH+2CihpGIeJh9SsUEFd8DXkP/Xk91f/mAbytBsnt +czFCtihFRxWbbBAMHh8i5HuxM+rH2nw5Hh/74GLE58zk5rtIRS1DyS+uUQIDAQAB +AoGAca57Ci4TQZ02XL8bp9610Le5hYIlzZ78fvbfY19YwYJxVoQLVzxnIb5k8dMh +JNbru2Q1hHVqhj/v5Xh0z46v5mTOeyQj8F1O6NCkzHtCfF029j8A9+pfNqyQhCa/ +nJqsNShFW+uhK67d7QfqtRRR6B30XsIHgND7QJuc14mDkdUCQQD3OpzLZugdTtuW +u+DdrdSjMBbW2p1+NFr8T20Rv+LoMvweZLSuMelAoog8fNxF6xQs7wLw+Tf5z56L +mptnur6TAkEA2h6WL3ippJ6/7H45suxP1dJI+Qal7V2KAMVGbv6Jal9rcKid0PpD +K1uPZwx2o/hkdobPY0HRIFaxpOtwC4FKCwJAYTmWodMFY0k4yA14wBT1c3uc77+n +ghM62NCvdvR8Wo56YcV+3KZaMYX5h7getAxfsdAI2xVXMxG4KvSROvjQqwJAaZ+W +KrbLr6QQXH1jg3lbz7ddDvphL2i0g1sEmIs6EADVDmEYyzHlhQF5l/U5Hn4SaDMw +Cmi81GQm8i3wvCGHsQJBAJC2VVcZ4VIehr3nAbI46w6cXGP6lpBbwT2FxSydRHqz +wfGZQ+qAAThGg3OInQNMqItypEEo3oZhKKvjD1N/iTw= +-----END RSA PRIVATE KEY-----""" +] + OAUTH2_PROVIDER_ACCESS_TOKEN_MODEL = "oauth2_provider.AccessToken" OAUTH2_PROVIDER_APPLICATION_MODEL = "oauth2_provider.Application" OAUTH2_PROVIDER_REFRESH_TOKEN_MODEL = "oauth2_provider.RefreshToken" diff --git a/tests/test_oidc_views.py b/tests/test_oidc_views.py index 5cbae5402..46040f86d 100644 --- a/tests/test_oidc_views.py +++ b/tests/test_oidc_views.py @@ -71,6 +71,7 @@ def test_get_connect_discovery_info_without_rsa_key(self): @pytest.mark.oauth2_settings(presets.OIDC_SETTINGS_RW) class TestJwksInfoView(TestCase): def test_get_jwks_info(self): + self.oauth2_settings.OIDC_RSA_PRIVATE_KEYS_INACTIVE = [] expected_response = { "keys": [ { @@ -93,6 +94,31 @@ def test_get_jwks_info_no_rsa_key(self): self.assertEqual(response.status_code, 200) assert response.json() == {"keys": []} + def test_get_jwks_info_multiple_rsa_keys(self): + expected_response = { + "keys": [ + { + "alg": "RS256", + "e": "AQAB", + "kid": "s4a1o8mFEd1tATAIH96caMlu4hOxzBUaI2QTqbYNBHs", + "kty": "RSA", + "n": "mwmIeYdjZkLgalTuhvvwjvnB5vVQc7G9DHgOm20Hw524bLVTk49IXJ2Scw42HOmowWWX-oMVT_ca3ZvVIeffVSN1-TxVy2zB65s0wDMwhiMoPv35z9IKHGMZgl9vlyso_2b7daVF_FQDdgIayUn8TQylBxEU1RFfW0QSYOBdAt8", # noqa + "use": "sig", + }, + { + "alg": "RS256", + "e": "AQAB", + "kid": "AJ_IkYJUFWqiKKE2FvPIESroTvownbaj0OzL939oIIE", + "kty": "RSA", + "n": "0qVzbcWg_fgygZ0liTaFeodD2bkinhj8gPJ9P2rPzvqG6ImI9YKkEk8Dxcc7eWcudnw5iEL8wx_tgooaRiHiYfUrFBBXfA15D_15PdX_5gG8rQbJ7XMxQrYoRUcVm2wQDB4fIuR7sTPqx9p8OR4f--BixOfM5Oa7SEUtQ8kvrlE", # noqa + "use": "sig", + }, + ] + } + response = self.client.get(reverse("oauth2_provider:jwks-info")) + self.assertEqual(response.status_code, 200) + assert response.json() == expected_response + @pytest.mark.django_db @pytest.mark.parametrize("method", ["get", "post"]) From 1ac49a192557c49f569a17936f71ea32295c4eb4 Mon Sep 17 00:00:00 2001 From: Dylan Tack Date: Thu, 6 May 2021 17:03:45 -0700 Subject: [PATCH 2/3] add author --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index 50589287a..88491adea 100644 --- a/AUTHORS +++ b/AUTHORS @@ -25,6 +25,7 @@ David Smith Diego Garcia Dulmandakh Sukhbaatar Dylan Giesler +Dylan Tack Emanuele Palazzetti Federico Dolce Frederico Vieira From a832e560ac9a4287ff0fe7588d8375fbd72b9b3a Mon Sep 17 00:00:00 2001 From: Dylan Tack Date: Thu, 6 May 2021 17:08:40 -0700 Subject: [PATCH 3/3] changelog for #950 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b2deb05d..c8e3fe50a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added * #712, #636, #808. Calls to `django.contrib.auth.authenticate()` now pass a `request` to provide compatibility with backends that need one. +* #950 Add support for RSA key rotation. ### Fixed * #524 Restrict usage of timezone aware expire dates to Django projects with USE_TZ set to True.