Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/13672 )
Change subject: IMPALA-8584: Add cookie support to the HTTP HS2 server ...................................................................... Patch Set 3: (8 comments) http://gerrit.cloudera.org:8080/#/c/13672/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13672/2//COMMIT_MSG@7 PS2, Line 7: IMPALA-8584: Add cookie support to the HTTP HS2 server > Some questions about the design here: It turns out that Hive doesn't store the cookies, it just uses a SHA256 HMAC, as you suggested. This approach has various advantages, so I switched to that. This solves your first concern - each client that authenticates without using a cookie with get its own cookie generated and sent back to it, and cookies aren't tied to sessions or connections. For your second question, I think that people generally use sticky sessions. The hs2 sessions themselves aren't shared via the statestore, so reconnecting to a new impalad requires calling OpenSession(). So, my patch doesn't share the key. Of course, if we think sharing the key is an important optimization it would be straightforward to add. http://gerrit.cloudera.org:8080/#/c/13672/2/be/src/rpc/authentication.cc File be/src/rpc/authentication.cc: http://gerrit.cloudera.org:8080/#/c/13672/2/be/src/rpc/authentication.cc@546 PS2, Line 546: , &SaslMutexU > Any idea what the source of this entropy is? Is it subject to entropy delet Done http://gerrit.cloudera.org:8080/#/c/13672/2/be/src/rpc/authentication.cc@562 PS2, Line 562: GENERAL_CALLBACKS[0].id = SASL_CB_LOG; > Do we want any expiration on the cookie? Or is the default (session cookie) Done http://gerrit.cloudera.org:8080/#/c/13672/2/be/src/rpc/cookie-mgr.h File be/src/rpc/cookie-mgr.h: http://gerrit.cloudera.org:8080/#/c/13672/2/be/src/rpc/cookie-mgr.h@56 PS2, Line 56: > nit: the cookie Done http://gerrit.cloudera.org:8080/#/c/13672/2/be/src/rpc/cookie-mgr.h@69 PS2, Line 69: > nit: spelling -> cookies Done http://gerrit.cloudera.org:8080/#/c/13672/2/be/src/rpc/cookie-mgr.cc File be/src/rpc/cookie-mgr.cc: http://gerrit.cloudera.org:8080/#/c/13672/2/be/src/rpc/cookie-mgr.cc@54 PS2, Line 54: > Should this be * 100 ? Done http://gerrit.cloudera.org:8080/#/c/13672/2/be/src/rpc/cookie-mgr.cc@61 PS2, Line 61: > hrm, this is odd. YOu can't just do .emplace(username, Cookie(cookie, UnixM Done http://gerrit.cloudera.org:8080/#/c/13672/2/be/src/transport/THttpServer.cpp File be/src/transport/THttpServer.cpp: http://gerrit.cloudera.org:8080/#/c/13672/2/be/src/transport/THttpServer.cpp@104 PS2, Line 104: } else if (THRIFT_strncasecmp(header, "Cookie", sz) == 0 && useCookies_) { > the Cookie header can pass multiple cookies, separated by ';'. It doesn't s Done -- To view, visit http://gerrit.cloudera.org:8080/13672 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I647c06f94ef91aa3b6413e91576c4ec506ed57f4 Gerrit-Change-Number: 13672 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Andrew Sherman <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Tue, 09 Jul 2019 01:22:33 +0000 Gerrit-HasComments: Yes
