Commits

Anonymous 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.

Comments (0)

Files changed (1)

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)