Merge pull request from GHSA-4fjv-pmhg-3rfg

Cve 2020 26244

src/oic/oic/__init__.py

@@ -44,6 +44,7 @@
 from oic.oauth2.message import ErrorResponse
 from oic.oauth2.message import Message
 from oic.oauth2.message import MessageFactory
+from oic.oauth2.message import WrongSigningAlgorithm
 from oic.oauth2.util import get_or_post
 from oic.oic.message import SCOPE2CLAIMS
 from oic.oic.message import AccessTokenResponse
@@ -1432,7 +1433,13 @@ def sign_enc_algs(self, typ):
         return resp
 
     def _verify_id_token(
-        self, id_token, nonce="", acr_values=None, auth_time=0, max_age=0
+        self,
+        id_token,
+        nonce="",
+        acr_values=None,
+        auth_time=0,
+        max_age=0,
+        response_type="",
     ):
         """
         Verify IdToken.
@@ -1465,6 +1472,11 @@ def _verify_id_token(
         if _now > id_token["exp"]:
             raise OtherError("Passed best before date")
 
+        if response_type != ["code"] and id_token.jws_header["alg"] == "none":
+            raise WrongSigningAlgorithm(
+                "none is not allowed outside Authorization Flow."
+            )
+
         if (
             self.id_token_max_age
             and _now > int(id_token["iat"]) + self.id_token_max_age
@@ -1491,7 +1503,7 @@ def verify_id_token(self, id_token, authn_req):
         except KeyError:
             pass
 
-        for param in ["acr_values", "max_age"]:
+        for param in ["acr_values", "max_age", "response_type"]:
             try:
                 kwa[param] = authn_req[param]
             except KeyError:

src/oic/oic/consumer.py

@@ -3,6 +3,8 @@
 import warnings
 from typing import Dict
 from typing import Optional
+from typing import Tuple
+from typing import Union
 
 from oic import rndstr
 from oic.exception import AuthzError
@@ -22,6 +24,7 @@
 from oic.oic.message import BackChannelLogoutRequest
 from oic.oic.message import Claims
 from oic.oic.message import ClaimsRequest
+from oic.oic.message import IdToken
 from oic.utils import http_util
 from oic.utils.sanitize import sanitize
 from oic.utils.sdb import DictSessionBackend
@@ -340,6 +343,7 @@ def begin(self, scope="", response_type="", use_nonce=False, path="", **kwargs):
         if self.debug:
             _log_info("Redirecting to: %s" % location)
 
+        self.authz_req[areq["state"]] = areq
         return sid, location
 
     def _parse_authz(self, query="", **kwargs):
@@ -364,7 +368,16 @@ def _parse_authz(self, query="", **kwargs):
         self.redirect_uris = [self.sdb[_state]["redirect_uris"]]
         return aresp, _state
 
-    def parse_authz(self, query="", **kwargs):
+    def parse_authz(
+        self, query="", **kwargs
+    ) -> Union[
+        http_util.BadRequest,
+        Tuple[
+            Optional[AuthorizationResponse],
+            Optional[AccessTokenResponse],
+            Optional[IdToken],
+        ],
+    ]:
         """
         Parse authorization response from server.
 
@@ -375,17 +388,20 @@ def parse_authz(self, query="", **kwargs):
         ["id_token"]
         ["id_token", "token"]
         ["token"]
-
-        :return: A AccessTokenResponse instance
         """
         _log_info = logger.info
         logger.debug("- authorization -")
 
+        # FIXME: This shouldn't be here... We should rather raise a sepcific Client error
+        # That would simplify the return value of this function
+        # and drop bunch of assertions from tests added in this commit.
         if not query:
             return http_util.BadRequest("Missing query")
 
         _log_info("response: %s" % sanitize(query))
 
+        if "algs" not in kwargs:
+            kwargs["algs"] = self.sign_enc_algs("id_token")
         if "code" in self.consumer_config["response_type"]:
             aresp, _state = self._parse_authz(query, **kwargs)
 
@@ -410,9 +426,10 @@ def parse_authz(self, query="", **kwargs):
                 except KeyError:
                     pass
 
-            return aresp, atr, idt
         elif "token" in self.consumer_config["response_type"]:  # implicit flow
             _log_info("Expect Access Token Response")
+            aresp = None
+            _state = None
             atr = self.parse_response(
                 AccessTokenResponse,
                 info=query,
@@ -423,8 +440,8 @@ def parse_authz(self, query="", **kwargs):
             if isinstance(atr, ErrorResponse):
                 raise TokenError(atr.get("error"), atr)
 
-            idt = None
-            return None, atr, idt
+            idt = atr.get("id_token")
+
         else:  # only id_token
             aresp, _state = self._parse_authz(query, **kwargs)
 
@@ -437,8 +454,13 @@ def parse_authz(self, query="", **kwargs):
                     session_update(self.sso_db, _state, "smid", idt["sid"])
                 except KeyError:
                     pass
+            # Null the aresp as only id_token should be returned
+            aresp = atr = None
 
-            return None, None, idt
+        # Verify the IdToken if it was present
+        if idt is not None:
+            self.verify_id_token(idt, self.authz_req.get(_state or atr["state"]))
+        return aresp, atr, idt
 
     def complete(self, state):
         """

src/oic/oic/message.py

@@ -313,17 +313,19 @@ def verify_id_token(instance, check_hash=False, **kwargs):
 
     if check_hash:
         _alg = idt.jws_header["alg"]
-        # What if _alg == 'none'
-
-        hfunc = "HS" + _alg[-3:]
+        if _alg != "none":
+            hfunc = "HS" + _alg[-3:]
+        else:
+            # This is allowed only for `code` and it needs to be checked by a Client
+            hfunc = None
 
-        if "access_token" in instance:
+        if "access_token" in instance and hfunc is not None:
             if "at_hash" not in idt:
                 raise MissingRequiredAttribute("Missing at_hash property", idt)
             if idt["at_hash"] != jws.left_hash(instance["access_token"], hfunc):
                 raise AtHashError("Failed to verify access_token hash", idt)
 
-        if "code" in instance:
+        if "code" in instance and hfunc is not None:
             if "c_hash" not in idt:
                 raise MissingRequiredAttribute("Missing c_hash property", idt)
             if idt["c_hash"] != jws.left_hash(instance["code"], hfunc):
@@ -780,6 +782,11 @@ def verify(self, **kwargs):
         else:
             if (_iat + _storage_time) < (_now - _skew):
                 raise IATError("Issued too long ago")
+            if _now < (_iat - _skew):
+                raise IATError("Issued in the future")
+
+        if _exp < _iat:
+            raise EXPError("Invalid expiration time")
 
         return True