Commits

ja...@nottheoilrig.com  committed d6874cb

Key associations on OP endpoint URL *and* assoc_handle, to prevent a malicious user from causing us to establish an association with an OP that it controls, then forging assertions from an OP that it doesn't control.

  • Participants
  • Parent commits 1300f50

Comments (0)

Files changed (1)

File openid2rp/testapp.py

     ('Launchpad', 'https://login.launchpad.net/favicon.ico', 'https://login.launchpad.net/')
     )
              
-sessions = []
+# Mapping from OP Endpoint URL and association handle to association response
+sessions = {}
+
 class Handler(BaseHTTPServer.BaseHTTPRequestHandler):
 
     def write(self, payload, type):
                     session = associate(services, url)
                 except ValueError, e:
                     return self.error(str(e))
-                sessions.append(session)
+                sessions[url, session['assoc_handle']] = session
                 self.send_response(307) # temporary redirect - do not cache
                 self.send_header("Location", request_authentication
                                  (services, url, session['assoc_handle'],
                     session = associate(services, url)
                 except ValueError, e:
                     return self.error(str(e))
-                sessions.append(session)
+                sessions[url, session['assoc_handle']] = session
                 self.send_response(307)
                 self.send_header("Location", request_authentication
                                  (services, url, session['assoc_handle'],
                     return self.rp_discovery()
                 if query['openid.mode'][0] == 'cancel':
                     return self.write('Login failed', 'text/plain')
+
+                # If no Claimed Identifier is present in the response, the
+                # assertion is not about an identifier
+                try:
+                    claimed_id, = query['openid.claimed_id']
+                except KeyError:
+                    return self.error('Assertion is not about an identifier')
+
+                # If the Claimed Identifier in the assertion is a URL and
+                # contains a fragment, the fragment part and the fragment
+                # delimiter character "#" MUST NOT be used for the purposes of
+                # verifying the discovered information
+                try:
+                    no_fragment = claimed_id[:claimed_id.index('#')]
+                except ValueError:
+                    no_fragment = claimed_id
+
+                _, op_endpoint, _ = discover(no_fragment)
                 handle = query['openid.assoc_handle'][0]
-                for session in sessions:
-                    if session['assoc_handle'] == handle:
-                        break
-                else:
-                    session = None
-                if not session:
+                try:
+                    session = sessions[op_endpoint, handle]
+                except KeyError:
                     return self.error('Not authenticated (no session)')
                 try:
                     signed = authenticate(session, querystring)