From a5fec950a048eae8cdb8a935e142290b2b93588c Mon Sep 17 00:00:00 2001 From: Giuseppe De Marco Date: Sun, 31 Dec 2023 11:10:41 +0100 Subject: [PATCH 1/3] feat: client assertion payload check --- spid_cie_oidc/provider/views/__init__.py | 12 +++++++++--- spid_cie_oidc/relying_party/views/__init__.py | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/spid_cie_oidc/provider/views/__init__.py b/spid_cie_oidc/provider/views/__init__.py index 3e884b50..8ca9832d 100644 --- a/spid_cie_oidc/provider/views/__init__.py +++ b/spid_cie_oidc/provider/views/__init__.py @@ -22,6 +22,7 @@ ValidationException ) from spid_cie_oidc.provider.models import OidcSession +from spid_cie_oidc.provider.schemas.client_assertion import ClientAssertion from spid_cie_oidc.provider.settings import ( OIDCFED_ATTRNAME_I18N, @@ -198,7 +199,12 @@ def check_client_assertion(self, client_id: str, client_assertion: str) -> bool: _op = self.get_issuer() _op_eid = _op.sub _op_eid_authz_endpoint = [_op.metadata['openid_provider']['authorization_endpoint']] - + + try: + ClientAssertion(**payload) + except Exception as e: + raise Exception(f"Client Assertion: json schema validation error: {e}") + if isinstance(_aud, str): _aud = [_aud] _allowed_auds = _aud + _op_eid_authz_endpoint @@ -208,7 +214,7 @@ def check_client_assertion(self, client_id: str, client_assertion: str) -> bool: f"Client assertion failed: {_sub} != {client_id}" ) # TODO Specialize exceptions - raise Exception() + raise Exception("Client Assertion: sub != client_id") if _op_eid: _allowed_auds.append(_op_eid) @@ -219,7 +225,7 @@ def check_client_assertion(self, client_id: str, client_assertion: str) -> bool: f"{self.request.build_absolute_uri()} not in {_allowed_auds}" ) # TODO Specialize exceptions - raise Exception() + raise Exception("Client Assertion: fake audience") tc = TrustChain.objects.get(sub=client_id, is_active=True) jwk = self.find_jwk(head, tc.metadata['openid_relying_party']['jwks']['keys']) diff --git a/spid_cie_oidc/relying_party/views/__init__.py b/spid_cie_oidc/relying_party/views/__init__.py index 572d78d3..5be98762 100644 --- a/spid_cie_oidc/relying_party/views/__init__.py +++ b/spid_cie_oidc/relying_party/views/__init__.py @@ -144,7 +144,7 @@ def get_token_request(self, auth_token, request, token_type): "aud": [audience], "iat": iat_now(), "exp": exp_from_now(), - "jti": str(uuid.uuid4()), + "jti": str(uuid.uuid4()) }, jwk_dict=rp_conf.jwks_core[0], ) From 7309f0e274b19dc9eca9e57b704f302b9bb939ad Mon Sep 17 00:00:00 2001 From: Giuseppe De Marco Date: Sun, 31 Dec 2023 11:16:43 +0100 Subject: [PATCH 2/3] feat: client assertion payload check - gitignore and new version --- .gitignore | 1 + spid_cie_oidc/__init__.py | 2 +- .../provider/schemas/client_assertion.py | 29 +++++++++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 spid_cie_oidc/provider/schemas/client_assertion.py diff --git a/.gitignore b/.gitignore index 9065e06d..70e8ff75 100644 --- a/.gitignore +++ b/.gitignore @@ -143,3 +143,4 @@ examples/wallet_trust_anchor/wallet_trust_anchor/settingslocal.py examples-docker/ docker-compose-externalrp.yml +*.old diff --git a/spid_cie_oidc/__init__.py b/spid_cie_oidc/__init__.py index bc86c944..67bc602a 100644 --- a/spid_cie_oidc/__init__.py +++ b/spid_cie_oidc/__init__.py @@ -1 +1 @@ -__version__ = "1.2.2" +__version__ = "1.3.0" diff --git a/spid_cie_oidc/provider/schemas/client_assertion.py b/spid_cie_oidc/provider/schemas/client_assertion.py new file mode 100644 index 00000000..b90731c5 --- /dev/null +++ b/spid_cie_oidc/provider/schemas/client_assertion.py @@ -0,0 +1,29 @@ +from spid_cie_oidc.entity.utils import iat_now + +from pydantic import BaseModel, AnyHttpUrl, constr, validator +from typing import Literal, Optional, List + + +class ClientAssertion(BaseModel): + iss: AnyHttpUrl + sub: AnyHttpUrl + iat: int + exp: int + jti: Optional[str] + aud: str | List[AnyHttpUrl] + + @validator("sub") + def iss_and_sub_must_match(cls, sub, values): + if values['iss'] != sub: + raise ValueError( + 'Client Assertion: iss and sub must have the same value' + ) + return sub + + @validator("exp") + def not_expired(cls, exp, values): + if not (values['iat'] < iat_now() < exp): + raise ValueError( + 'Client Assertion: exp must be greater than iat and less than the current time' + ) + return exp From dc49bd7e2f525f29e698e58c626137cf7a598b9f Mon Sep 17 00:00:00 2001 From: Giuseppe De Marco Date: Sun, 31 Dec 2023 11:44:16 +0100 Subject: [PATCH 3/3] feat: client assertion payload check - fix timings --- spid_cie_oidc/provider/schemas/client_assertion.py | 7 +++++-- spid_cie_oidc/provider/views/token_endpoint.py | 9 ++------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/spid_cie_oidc/provider/schemas/client_assertion.py b/spid_cie_oidc/provider/schemas/client_assertion.py index b90731c5..5b8242f9 100644 --- a/spid_cie_oidc/provider/schemas/client_assertion.py +++ b/spid_cie_oidc/provider/schemas/client_assertion.py @@ -22,8 +22,11 @@ def iss_and_sub_must_match(cls, sub, values): @validator("exp") def not_expired(cls, exp, values): - if not (values['iat'] < iat_now() < exp): + _now = iat_now() + if not (values['iat'] <= _now < exp): raise ValueError( - 'Client Assertion: exp must be greater than iat and less than the current time' + 'Client Assertion: exp must be greater than ' + 'iat and less than the current time.' + f'{values["iat"]} <= {_now} < {exp}' ) return exp diff --git a/spid_cie_oidc/provider/views/token_endpoint.py b/spid_cie_oidc/provider/views/token_endpoint.py index cc098d2f..f97b576e 100644 --- a/spid_cie_oidc/provider/views/token_endpoint.py +++ b/spid_cie_oidc/provider/views/token_endpoint.py @@ -105,9 +105,7 @@ def is_token_renewable(self, session) -> bool: ).first() id_token = unpad_jwt_payload(issuedToken.id_token) - consent_expiration = id_token['iat'] + OIDCFED_PROVIDER_MAX_CONSENT_TIMEFRAME - delta = consent_expiration - iat_now() if delta > 0: return True @@ -138,7 +136,7 @@ def grant_refresh_token(self, request, *args, **kwargs): refresh_token=request.POST['refresh_token'], revoked=False ).first() - + if not issued_token: return JsonResponse( { @@ -148,7 +146,6 @@ def grant_refresh_token(self, request, *args, **kwargs): }, status=400 ) - session = issued_token.session if not self.is_token_renewable(session): # pragma: no cover return JsonResponse( @@ -171,7 +168,7 @@ def grant_refresh_token(self, request, *args, **kwargs): id_token=iss_token_data['id_token'], refresh_token=iss_token_data['refresh_token'], token_type="Bearer", # nosec B106 - expires_in=expires_in, + expires_in=expires_in ) return JsonResponse(data) @@ -192,10 +189,8 @@ def post(self, request, *args, **kwargs): }, status=400 ) - self.commons = self.get_jwt_common_data() self.issuer = self.get_issuer() - # check client_assertion and client ownership try: self.check_client_assertion(