Project

General

Profile

Bug #35346

auth_oidc : l'attribut jwkset d'un fournisseur OIDC n'est pas toujours une instance jwcrypto.JWKSet

Added by Paul Marillonnet 13 days ago. Updated 13 days ago.

Status:
Solution validée
Priority:
Normal
Category:
-
Target version:
-
Start date:
09 Aug 2019
Due date:
% Done:

0%

Patch proposed:
No
Planning:
No

Description

Le bout de code fautif :

class OIDCProvider(models.Model):  
    # ...
    @property                                                                                       
    def jwkset(self):                                                                               
        # ...                            
        if self.idtoken_algo == self.ALGO_HMAC:                                                     
            return JWK(kty='oct', k=base64url_encode(self.client_secret.encode('utf-8')))           

0001-auth_oidc-make-OIDCProvider.jwkset-always-be-a-jwcry.patch View (1.04 KB) Paul Marillonnet, 09 Aug 2019 03:33 PM


Related issues

Related to Authentic 2 - Development #31862: authn OIDC : vérifier la signature de l'ID Token reçu En cours 29 Mar 2019

History

#1 Updated by Paul Marillonnet 13 days ago

  • Assignee set to Paul Marillonnet
  • Status changed from Nouveau to En cours

#2 Updated by Paul Marillonnet 13 days ago

#3 Updated by Frédéric Péters 13 days ago

  • Related to Development #31862: authn OIDC : vérifier la signature de l'ID Token reçu added

#4 Updated by Paul Marillonnet 13 days ago

  • Status changed from Solution proposée to En cours
  • Patch proposed changed from Yes to No

Ah oui mais non, je ne comprends pas pourquoi le code est en deux parties, en fonction du type d'algo. Et le patch rend ça encore moins propre. Je réessaie

#5 Updated by Paul Marillonnet 13 days ago

Discussions sur le salon :

en mode HMAC il faut ignorer le jwkset_json

Oui, j'y vois plus clair après relecture des spécs à ce sujet. Mais ce qui me gène vraiment c'est qu'on s'attend à ce que la propriété OIDCProvider.jwkset soit absolument raccord avec OIDCProvider.jwkset_json, càd une simple instanciation jwcrypto.jwk.JWKSet d'un dump de l'attribut OIDCProvider.jwkset_json.

Ainsi, si la signature de l'ID Token est basée sur HMAC, ce n'est pas simplement qu'il faut ignorer jwkset_json, c'est que la fonction appelante doit savoir qu'il ne faut pas faire appel à cette propriété OIDC.jwkset, mais plutôt simplement retrouver la clé à partir du client_secret.

#6 Updated by Paul Marillonnet 13 days ago

Le fait qu'un algo de signature HMAC n'empêche à mon avis pas d'avoir un JWKSet ne contenant que la clé correspondante m'amènerait à proposer quelque chose comme ça :

diff --git a/src/authentic2_auth_oidc/models.py b/src/authentic2_auth_oidc/models.py
index 74dd4d086..23149a6d1 100644
--- a/src/authentic2_auth_oidc/models.py
+++ b/src/authentic2_auth_oidc/models.py
@@ -156,11 +156,19 @@ class OIDCProvider(models.Model):
     def jwkset(self):
         from authentic2.crypto import base64url_encode

-        if self.idtoken_algo == self.ALGO_RSA:
-            if self.jwkset_json:
-                return JWKSet.from_json(json.dumps(self.jwkset_json))
-        if self.idtoken_algo == self.ALGO_HMAC:
-            return JWK(kty='oct', k=base64url_encode(self.client_secret.encode('utf-8')))
+        # RSA keys are declared at registration time hence are already present
+        # in the jwkset_json attribute.
+        if self.jwkset_json:
+            return JWKSet.from_json(json.dumps(self.jwkset_json))
+        elif self.idtoken_algo == self.ALGO_HMAC:
+            jwks = JWKSet()
+            key = JWK(kty='oct', k=base64url_encode(self.client_secret.encode('utf-8')))
+            jwks.add_key(key)
+            # Avoid symmetric JWK computation on next access, and adjust
+            # the content of the corresponding JSON model attribute:
+            self.jwkset_json = json.loads(jkws.export())
+            self.save()
+            return jwks
         return None

     def __str__(self):

#7 Updated by Paul Marillonnet 13 days ago

Hmm, ok une seconde relecture des spécs me remet les idées en place : pas de rapport le client_secret et la jwks_uri. Je pense qu'un patch est quand même nécessaire ici, parce qu'un attribut nommé jwkset devrait renvoyer JWKSet quoiqu'il arrive.

#8 Updated by Paul Marillonnet 13 days ago

Et donc plus simplement, répercuter dans le code cette absence de lien logique entre le jwks_uri d'un fournisseur OIDC et le client_secret délivré :

diff --git a/src/authentic2_auth_oidc/models.py b/src/authentic2_auth_oidc/models.py
index 74dd4d086..64bf1c441 100644
--- a/src/authentic2_auth_oidc/models.py
+++ b/src/authentic2_auth_oidc/models.py
@@ -154,13 +154,8 @@ class OIDCProvider(models.Model):

     @property
     def jwkset(self):
-        from authentic2.crypto import base64url_encode
-
-        if self.idtoken_algo == self.ALGO_RSA:
-            if self.jwkset_json:
-                return JWKSet.from_json(json.dumps(self.jwkset_json))
-        if self.idtoken_algo == self.ALGO_HMAC:
-            return JWK(kty='oct', k=base64url_encode(self.client_secret.encode('utf-8')))
+        if self.jwkset_json:
+            return JWKSet.from_json(json.dumps(self.jwkset_json))
         return None

     def __str__(self):

Avec bien sûr toutes les modifications que cela implique dans ce module et dans les tests.

#9 Updated by Benjamin Dauvergne 13 days ago

  • Status changed from En cours to Solution validée

Ok pour le dernier patch.

Also available in: Atom PDF