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 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/13672/5/be/src/rpc/cookie-util.cc File be/src/rpc/cookie-util.cc: http://gerrit.cloudera.org:8080/#/c/13672/5/be/src/rpc/cookie-util.cc@104 PS5, Line 104: > I don't really have a better suggestion, it would probably be overcomplicat Done http://gerrit.cloudera.org:8080/#/c/13672/6/be/src/transport/THttpServer.cpp File be/src/transport/THttpServer.cpp: http://gerrit.cloudera.org:8080/#/c/13672/6/be/src/transport/THttpServer.cpp@181 PS6, Line 181: if (metrics_enabled_) http_metrics_->total_cookie_auth_success_->Increment(1); > I thought about whether it would be a good idea to limit the number of cook Good point. A well behaved client should only ever be returning one cookie to us, but we probably still want to be at least a little flexible for not-quite-well-behaved clients. We should probably only bother attempting to check the signature for a single cookie - we don't need to accommodate clients that are so poorly behaved as to provide us with multiple cookies matching our cookie name, since that's clearly way out of spec. We probably also eventually will want to have some sort of rate limiting of requests or other restrictions on a per connection or per client host basis or similar to minimize possible ddos, either unintentional or malicious. That's a problem both for checking cookies and for ldap/kerberos auth attempts. Of course, the key motivation of all of this http client work is to allow for proxying of connections through something like Apache Knox, and in that case we can hopefully rely on the proxy to take the brunt of any misbehaved clients. -- 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: 7 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: Tim Armstrong <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Fri, 23 Aug 2019 22:52:30 +0000 Gerrit-HasComments: Yes
