Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cookies survive wamp.session.kill_by_authid with or without reason "wamp.close.logout" #2051

Open
antonymott opened this issue Oct 1, 2022 · 8 comments

Comments

@antonymott
Copy link
Contributor

antonymott commented Oct 1, 2022

crossbar.io v22.6.1

Calls to wamp.session.kill_by_authid with any reason are handled by router/service.py so do not appear to cause the router to set the cookie to "not authenticated", unlike when the client explicitly closes the connection with reason "wamp.close.logout" which does work (router/session.pyLn1225).

After admin issues "wamp.session.kill_by_authid", the session is detached, but on the client, connection.open() automatically reauthenticates the user as the cookie in the browser persists as valid. Wampcra is bypassed when using cookie authentication.

To repeat:
authmethods: ["cookie", "anonymous", "wampcra"], then change to another user/session (i.e. admin or monitor) and call "wamp.session.kill_by_authid"

I started to try to fix this and do a pull request in router/service.py, something like (pseudocode):

#if details.reason == "wamp.close.logout", set cookie to "not authenticated"

set self._transport.factory._cookiestore.delAuth(cbtid)

# kick all transport protos (eg WampWebSocketServerProtocol) for the same auth cookie

for proto in self._transport.factory._cookiestore.getProtos(cbtid): if proto != self._transport: proto.sendClose() cnt_kicked += 1

but 'wamp.session.kill_by_..." calls are in a different module (around line 444 of router/service.py) so it was not obvious to me how to properly acquire scope of transports or cookiestore.

This security/control feature was implemented 4 years ago after this discussion: wamp-proto/wamp-proto#314. So I wonder if it might be user-error - no bug at all - and that I'm the one incorrectly using "wamp.session.kill_by_[session/authid]".

On the other hand, I searched for "cookie" in the discussion and no hits...so maybe cookie-authenticated users slipped through 😬

Here are server logs specific to the issue...note the authid does have its session detached by "wamp.session.kill_by_authid"...then immediately is able to reauthenticate using the same authid:

2022-10-01T00:16:09-0400 [Router 202987] <crossbar.router.router.Router.detach> router session detached from realm "realm1" (session=7750981284023503, detached_session_ids=1, authid="XQXG-7R9K-GHTT-4LML-T7AJ-57GV", authrole="public", authmethod="anonymous", authprovider="dynamic")
2022-10-01T00:16:11-0400 [Router 202987] <crossbar.router.protocol.WampWebSocketServerProtocol.onConnect>: parsed tracking/authentication cookie cbtid "rgI7hRwLQd87oovxcvR2BquX" from HTTP request headers
2022-10-01T00:16:11-0400 [Router 202987] <crossbar.router.protocol.WampWebSocketServerProtocol.onConnect>: tracking/authentication cookie cbtid "rgI7hRwLQd87oovxcvR2BquX" already set and stored
2022-10-01T00:16:11-0400 [Router 202987] <crossbar.router.protocol.WampWebSocketServerProtocol.onConnect> authenticated client via cookie cbtid=rgI7hRwLQd87oovxcvR2BquX as authid="XQXG-7R9K-GHTT-4LML-T7AJ-57GV", authrole="public", authmethod="anonymous", authprovider="cookie", authrealm="realm1"

@oberstet
Copy link
Contributor

oberstet commented Oct 1, 2022

@antonymott
Copy link
Contributor Author

The fix #2025 and testing verified client-initiated logout works, but admin-initiated logout ("wamp.close.logout") of an authid appears broken. Was anyone able to replicate? "wamp.close.logout" triggers onleave, but does not appear to interact with the cookiestore so users automatically log in again with their still-valid cookie.

@oberstet
Copy link
Contributor

but admin-initiated logout ("wamp.close.logout") of an authid appears broken

If you kill another session from your admin session using the WAMP meta API procedure wamp.session.kill and provide reason=="logout", the router side code that implements this meta API procedure

https://github.com/crossbario/crossbar/blob/master/crossbar/router/service.py#L433

will do the same as when the client itself had left its session with reason=="logout"

https://github.com/crossbario/crossbar-examples/blob/cfe7d866b2872eebdce3de5d311c6894c2868809/authentication/cookie/client.py#L117


further, once the router-side session object of a session that is to be left (ended / removed from router), this will run

# if asked to explicitly close the session

which will then call cs.delAuth(cbtid) to delete the cookie on that session from the cookiestore. it will also (potentially) kill all other sessions with the same cookie.


when a session is logged out, either from the session itself, or via the meta API, in both cases, this will hence log

self.log.info(

do you see that log message, but only for "client-initiated logout", and not for "admin-initiated logout"?

@antonymott
Copy link
Contributor Author

antonymott commented Oct 12, 2022

do you see that log message, but only for "client-initiated logout", and not for "admin-initiated logout"?

I see it only for client-initiated logout.

In particular, the logs are different depending on if I self logout as a client or using the WAMP meta API procedure(s). If I logout as client: connection.logout('wamp.close.logout') works and I get these logs (I added log messages in cookiestore.py to help me see what's going on, in particular if cs.delAuth(cbtid) is called):

2022-10-12T14:18:53-0400 [Router 97919] <crossbar.router.router.Router.detach> router session detached from realm "realm1" (session=8422617609344277, detached_session_ids=1, authid="79Q9-ESWN-P4U7-TJYV-CXY7-KTKW", authrole="public", authmethod="anonymous", authprovider="cookie")
2022-10-12T14:18:53-0400 [Router 97919] 🍪 CookieDB 🌱 _delAuth: 12pWq+j38dBkB+lan2cwO92y
2022-10-12T14:18:53-0400 [Router 97919] <crossbar.router.cookiestore.CookieStoreDatabaseBacked.delAuth> cookie with cbtid="12pWq+j38dBkB+lan2cwO92y" did exist and was deleted
2022-10-12T14:18:53-0400 [Router 97919] <crossbar.router.session.RouterSession.onLeave> wamp.close.logout completed with message
🎠 🎠 🎠 bye-bye 🎠 🎠 🎠 for session 8422617609344277 (cookie authentication deleted: "12pWq+j38dBkB+lan2cwO92y", pro-actively kicked (other) sessions: none)

I tried two of the WAMP meta API procedures: wamp.session.kill and wamp.session.kill_by_authid. And for each I used two reasons, the reason you asked me to use: reason=='logout' as well as reason=='wamp.close.logout' as the docs indicate to use all three words separated by dots and that does work for client self-logout. The logs are below, but I'll summarize: router session does detach...but not cs.delAuth(cbtid). Then the logs show the client attempts to login again using the same cookie which the router parses and authenticates. Missing but needed before the client attempts to login again are the bright red <crossbar.router.session.RouterSession.onLeave> wamp.close.logout and <crossbar.router.cookiestore.CookieStoreDatabaseBacked.delAuth> cookie with cbtid="N9xAGYKra7D48Y92WuHTiVPe" did exist and was deleted.

Logs from trying the meta API procedures:

2022-10-12T14:28:40-0400 [Router 99434] <crossbar.router.router.Router.detach> router session detached from realm "realm1" (session=635614755763746, detached_session_ids=1, authid="YVRQ-S3GT-XHN9-QPGR-9CSL-HNPF", authrole="public", authmethod="anonymous", authprovider="cookie")
2022-10-12T14:28:42-0400 [Router 99434] <crossbar.router.protocol.WampWebSocketServerProtocol.onConnect>: parsed tracking/authentication cookie cbtid "N9xAGYKra7D48Y92WuHTiVPe" from HTTP request headers
2022-10-12T14:28:42-0400 [Router 99434] <crossbar.router.protocol.WampWebSocketServerProtocol.onConnect>: tracking/authentication cookie cbtid "N9xAGYKra7D48Y92WuHTiVPe" already set and stored
2022-10-12T14:28:42-0400 [Router 99434] <crossbar.router.cookiestore.CookieStoreDatabaseBacked.getAuth> cookie auth info for "N9xAGYKra7D48Y92WuHTiVPe" retrieved: ('YVRQ-S3GT-XHN9-QPGR-9CSL-HNPF', 'public', 'anonymous', 'realm1', {'host': '127.0.0.1:8080', 'x_cb_node': 'xenon-99421', 'x_cb_worker': 'worker001', 'x_cb_peer': 'tcp4:127.0.0.1:43140', 'x_cb_pid': 99434, 'saltedPwd': None})
2022-10-12T14:28:42-0400 [Router 99434] <crossbar.router.protocol.WampWebSocketServerProtocol.onConnect> authenticated client via cookie cbtid=N9xAGYKra7D48Y92WuHTiVPe as authid="YVRQ-S3GT-XHN9-QPGR-9CSL-HNPF", authrole="public", authmethod="anonymous", authprovider="cookie", authrealm="realm1"

I connect using monitor.html as that makes it very easy to test, I first make sure a client is cookie-authenticated, then repeatedly to try to kill by session, by authid, pasting in each of the reasons into the form fields. As long as the cookie's lifetime was not expired, the client re-authenticates successfully with a new session every time.

At your end, do you find that when you kill by session or authid using the WAMP meta API procedure that it works, and you do see the cookie being deleted?

@oberstet
Copy link
Contributor

oberstet commented Oct 12, 2022

I see it only for client-initiated logout.

ok. that is unexpected. as in: it should log in both cases, and I don't understand why it wouldn't. IOW: this needs more investigation.

we do have automated tests for authentication, including cookie, and including "logout", and also retesting with cookie deleted (expecting auth to fail then to succeed the test) - but only from the session itself, not via WAMP meta API

https://github.com/crossbario/crossbar-examples/blob/master/authentication/test_cookie.sh
https://github.com/crossbario/crossbar-examples/blob/cfe7d866b2872eebdce3de5d311c6894c2868809/authentication/cookie/client.py#L111

At your end, do you find that when you kill by session or authid using the WAMP meta API procedure that it works, and you do see the cookie being deleted?

I'm not using cookies myself, I only created above test as I was working on auth related features for a customer project. and this test is missing a test via meta API .. which is more complicated (the test) to automate as 2 sessions are needed then .. anyways, yeah, sorry, don't know why it doesn't work ..

@antonymott
Copy link
Contributor Author

antonymott commented Oct 12, 2022

For the time being, I patched my use case with a most dreadful kluge, but it may make you smile. I can now log anyone (white-hat clients) out using WAMP meta API: I replaced the default argument of message='some custom message that nobody uses and could be removed' with a flattened json object that flags some monkey-patch js in the browser to trigger client-assisted logout...which does the job. I may be the only CB user who ever used it, but thanks for leaving the "message" parameter in, it helped! 😊

@antonymott
Copy link
Contributor Author

antonymott commented Oct 13, 2022

Update ! Good morning! I made a fix...tested and now WAMP meta API "kill by..." works so we can boot anyone off by session or authid and their cookie(s) deleted.

I didn't do a pull request yet because when you look over my fix, you may want to solve it a better way than I did (mine is not as DRY as it could be as there is similar code in onLeave), perhaps even in a different file as you know the innards so well. However, if you think it looks robust enough I'll happily do a pull request if that would help.

I followed the logic that WAMP meta API calls activate session.leave which is in class RouterSession(BaseSession) in service.py, but the main problem is that session.leave doesn't trigger "onLeave". I traced the "leave" call which does successfully fire leave in session.py (where the def is). Looking closer at "leave" I see it serves as a kind of Goodbye or aftereffect/cleanup. So probably the order that the existing “leave” needs to be fired should be after the important session.onLeave which actually does the job of deleting (“de-authenticating”) the cookie, closing transport protos and other stuff.

To the "leave" def (line 756 of session.py), I basically added the "if reason == " part and modified a "self.log.info(..." so it would work with the "leave" arguments which are type string, not type autobahn.wamp.types CloseDetails like those of the "onLeave" def:

   def leave(self, reason=None, message=None):
        """
        Implements :func:`autobahn.wamp.interfaces.ISession.leave`
        """
        session_id = self._session_id or self._previous_session_id

        cookie_deleted = None
        cnt_kicked = 0

        if reason == 'wamp.close.logout':
            # if cookie was set on transport
            if self._transport and hasattr(self._transport, '_cbtid') and self._transport._cbtid and self._transport.factory._cookiestore:
                cbtid = self._transport._cbtid
                cs = self._transport.factory._cookiestore
                print(f'🎠 {__name__} 🌱   🐝 🐞 \n\n\n\n 🎠   🎠   🎠  this seems to work great!  🎠   🎠   🎠 \n\n\n\n 🐝 🐞  ')
                # set cookie to "not authenticated"
                # cs.setAuth(cbtid, None, None, None, None, None)
                cs.delAuth(cbtid)
                cookie_deleted = cbtid
                # kick all transport protos (eg WampWebSocketServerProtocol) for the same auth cookie
                for proto in cs.getProtos(cbtid):
                    # but don't kick ourselves
                    if proto != self._transport:
                        proto.sendClose()
                        cnt_kicked += 1

            self.log.info(
                '{func} {action} completed with message {message} for session {session_id} (cookie authentication deleted: '
                '"{cookie_deleted}", pro-actively kicked (other) sessions: {cnt_kicked})',
                action=hlval('wamp.close.logout', color='red'),
                session_id=hlid(session_id),
                cookie_deleted=hlval(cookie_deleted, color='red') if cookie_deleted else 'none',
                cnt_kicked=hlval(cnt_kicked, color='red') if cnt_kicked else 'none',
                message=hlval(message, color='blue') if message else 'no message',
                func=hltype(self.onLeave))


        if not self._goodbye_sent:
            if reason:
                msg = wamp.message.Goodbye(reason, message)
            else:
                msg = wamp.message.Goodbye(message=message)

            self._transport.send(msg)
            self._goodbye_sent = True
        else:
            raise SessionNotReady("Already requested to close the session")

@oberstet
Copy link
Contributor

thanks for posting what you did! fwiw, we need an automated test of cookie auth testing cookie delete via WAMP meta API. this test should fail first, and then we can add a change to crossbar to fix it ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants