You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2021/06/30 20:18:28 UTC
[GitHub] [trafficserver] zwoop opened a new pull request #8021: rate_limit: Add a global hook to rate limit concurrent connections based on SNI
zwoop opened a new pull request #8021:
URL: https://github.com/apache/trafficserver/pull/8021
There's still work to do here (see the README) such as proper, reloadable YAML configurations for the global hook. The remap version is likely also resource intensive when you have very large remap files.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] zwoop commented on pull request #8021: rate_limit: Add a global hook to rate limit concurrent connections based on SNI
Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #8021:
URL: https://github.com/apache/trafficserver/pull/8021#issuecomment-892214203
Cherry-picked to v9.1.x branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] shinrich commented on a change in pull request #8021: rate_limit: Add a global hook to rate limit concurrent connections based on SNI
Posted by GitBox <gi...@apache.org>.
shinrich commented on a change in pull request #8021:
URL: https://github.com/apache/trafficserver/pull/8021#discussion_r662354297
##########
File path: plugins/experimental/rate_limit/sni_limiter.cc
##########
@@ -0,0 +1,131 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "tscore/ink_config.h"
+
+// Needs special OpenSSL APIs as a global plugin for early CLIENT_HELLO inspection
+#if TS_USE_HELLO_CB
+
+#include <unistd.h>
+#include <getopt.h>
+#include <cstdlib>
+
+#include "sni_selector.h"
+#include "sni_limiter.h"
+
+// This holds the VC user arg index for the SNI limiters.
+int gVCIdx = -1;
+
+///////////////////////////////////////////////////////////////////////////////
+// These continuations are "helpers" to the SNI limiter object. Putting them
+// outside the class implementation is just cleaner.
+//
+int
+sni_limit_cont(TSCont contp, TSEvent event, void *edata)
+{
+ TSVConn vc = static_cast<TSVConn>(edata);
+ SniSelector *selector = static_cast<SniSelector *>(TSContDataGet(contp));
+ TSReleaseAssert(selector);
+
+ switch (event) {
+ case TS_EVENT_SSL_CLIENT_HELLO: {
+ TSSslConnection ssl_conn = TSVConnSslConnectionGet(vc);
+ SSL *ssl = reinterpret_cast<SSL *>(ssl_conn);
+ std::string_view sni_name = getSNI(ssl);
+
+ if (sni_name.size() > 0) { // This should likely always succeed, but without it we can't do anything
+ SniRateLimiter *limiter = selector->find(sni_name);
+
+ TSDebug(PLUGIN_NAME, "CLIENT_HELLO on %.*s", static_cast<int>(sni_name.length()), sni_name.data());
+ if (limiter && !limiter->reserve()) {
+ if (!limiter->max_queue || limiter->full()) {
+ // We are running at limit, and the queue has reached max capacity, give back an error and be done.
+ TSVConnReenableEx(vc, TS_EVENT_ERROR);
+ TSDebug(PLUGIN_NAME, "Rejecting connection, we're at capacity and queue is full");
+ TSUserArgSet(vc, gVCIdx, nullptr);
+
+ return TS_ERROR;
+ } else {
+ TSUserArgSet(vc, gVCIdx, reinterpret_cast<void *>(limiter));
+ limiter->push(vc, contp);
+ TSDebug(PLUGIN_NAME, "Queueing the VC, we are at capacity");
+ }
+ } else {
+ // Not at limit on the handshake, we can re-enable
+ TSUserArgSet(vc, gVCIdx, reinterpret_cast<void *>(limiter));
+ TSVConnReenable(vc);
+ }
+ }
+ break;
+ }
+
+ case TS_EVENT_VCONN_CLOSE: {
+ SniRateLimiter *limiter = static_cast<SniRateLimiter *>(TSUserArgGet(vc, gVCIdx));
+
+ if (limiter) {
+ limiter->release();
+ }
Review comment:
Are you guaranteed that the vc is removed from the queue at this point? Can the other side give up and cause the VCONN_CLOSE event to occur?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] zwoop commented on pull request #8021: rate_limit: Add a global hook to rate limit concurrent connections based on SNI
Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #8021:
URL: https://github.com/apache/trafficserver/pull/8021#issuecomment-871928693
[approve ci autest]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] zwoop merged pull request #8021: rate_limit: Add a global hook to rate limit concurrent connections based on SNI
Posted by GitBox <gi...@apache.org>.
zwoop merged pull request #8021:
URL: https://github.com/apache/trafficserver/pull/8021
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] zwoop removed a comment on pull request #8021: rate_limit: Add a global hook to rate limit concurrent connections based on SNI
Posted by GitBox <gi...@apache.org>.
zwoop removed a comment on pull request #8021:
URL: https://github.com/apache/trafficserver/pull/8021#issuecomment-872506020
[approve ci clang-analyzer]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] zwoop removed a comment on pull request #8021: rate_limit: Add a global hook to rate limit concurrent connections based on SNI
Posted by GitBox <gi...@apache.org>.
zwoop removed a comment on pull request #8021:
URL: https://github.com/apache/trafficserver/pull/8021#issuecomment-871928693
[approve ci autest]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] zwoop commented on pull request #8021: rate_limit: Add a global hook to rate limit concurrent connections based on SNI
Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #8021:
URL: https://github.com/apache/trafficserver/pull/8021#issuecomment-872506020
[approve ci clang-analyzer]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] shinrich commented on a change in pull request #8021: rate_limit: Add a global hook to rate limit concurrent connections based on SNI
Posted by GitBox <gi...@apache.org>.
shinrich commented on a change in pull request #8021:
URL: https://github.com/apache/trafficserver/pull/8021#discussion_r662354297
##########
File path: plugins/experimental/rate_limit/sni_limiter.cc
##########
@@ -0,0 +1,131 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "tscore/ink_config.h"
+
+// Needs special OpenSSL APIs as a global plugin for early CLIENT_HELLO inspection
+#if TS_USE_HELLO_CB
+
+#include <unistd.h>
+#include <getopt.h>
+#include <cstdlib>
+
+#include "sni_selector.h"
+#include "sni_limiter.h"
+
+// This holds the VC user arg index for the SNI limiters.
+int gVCIdx = -1;
+
+///////////////////////////////////////////////////////////////////////////////
+// These continuations are "helpers" to the SNI limiter object. Putting them
+// outside the class implementation is just cleaner.
+//
+int
+sni_limit_cont(TSCont contp, TSEvent event, void *edata)
+{
+ TSVConn vc = static_cast<TSVConn>(edata);
+ SniSelector *selector = static_cast<SniSelector *>(TSContDataGet(contp));
+ TSReleaseAssert(selector);
+
+ switch (event) {
+ case TS_EVENT_SSL_CLIENT_HELLO: {
+ TSSslConnection ssl_conn = TSVConnSslConnectionGet(vc);
+ SSL *ssl = reinterpret_cast<SSL *>(ssl_conn);
+ std::string_view sni_name = getSNI(ssl);
+
+ if (sni_name.size() > 0) { // This should likely always succeed, but without it we can't do anything
+ SniRateLimiter *limiter = selector->find(sni_name);
+
+ TSDebug(PLUGIN_NAME, "CLIENT_HELLO on %.*s", static_cast<int>(sni_name.length()), sni_name.data());
+ if (limiter && !limiter->reserve()) {
+ if (!limiter->max_queue || limiter->full()) {
+ // We are running at limit, and the queue has reached max capacity, give back an error and be done.
+ TSVConnReenableEx(vc, TS_EVENT_ERROR);
+ TSDebug(PLUGIN_NAME, "Rejecting connection, we're at capacity and queue is full");
+ TSUserArgSet(vc, gVCIdx, nullptr);
+
+ return TS_ERROR;
+ } else {
+ TSUserArgSet(vc, gVCIdx, reinterpret_cast<void *>(limiter));
+ limiter->push(vc, contp);
+ TSDebug(PLUGIN_NAME, "Queueing the VC, we are at capacity");
+ }
+ } else {
+ // Not at limit on the handshake, we can re-enable
+ TSUserArgSet(vc, gVCIdx, reinterpret_cast<void *>(limiter));
+ TSVConnReenable(vc);
+ }
+ }
+ break;
+ }
+
+ case TS_EVENT_VCONN_CLOSE: {
+ SniRateLimiter *limiter = static_cast<SniRateLimiter *>(TSUserArgGet(vc, gVCIdx));
+
+ if (limiter) {
+ limiter->release();
+ }
Review comment:
Are you guaranteed that the vc is removed from the queue at this point?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org