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 2022/10/11 18:28:55 UTC
[GitHub] [trafficserver] ezelkow1 opened a new pull request, #9134: Adding regex capability based on pristine host name
ezelkow1 opened a new pull request, #9134:
URL: https://github.com/apache/trafficserver/pull/9134
Adds a new `regex_host` field to the maxmind plugin and also renames `regex` to `regex_path`. This allows doing regex based allow/deny based on the pristine hostname that the request came in on which can be useful for a variety of scenarios
--
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] ezelkow1 commented on a diff in pull request #9134: Adding regex capability based on pristine host name
Posted by GitBox <gi...@apache.org>.
ezelkow1 commented on code in PR #9134:
URL: https://github.com/apache/trafficserver/pull/9134#discussion_r992808048
##########
plugins/experimental/maxmind_acl/mmdb.cc:
##########
@@ -694,19 +740,39 @@ Acl::eval_country(MMDB_entry_data_s *entry_data, const char *path, int path_len)
ret = true;
}
+ // test for path regex
if (nullptr != path && 0 != path_len) {
- if (!allow_regex[output].empty()) {
- for (auto &i : allow_regex[output]) {
+ if (!allow_regex_path[output].empty()) {
+ for (auto &i : allow_regex_path[output]) {
if (PCRE_ERROR_NOMATCH != pcre_exec(i._rex, i._extra, path, path_len, 0, PCRE_NOTEMPTY, nullptr, 0)) {
- TSDebug(PLUGIN_NAME, "Got a regex allow hit on regex: %s, country: %s", i._regex_s.c_str(), output);
+ TSDebug(PLUGIN_NAME, "Got a regex allow hit on regex path: %s, country: %s", i._regex_s.c_str(), output);
ret = true;
}
}
}
- if (!deny_regex[output].empty()) {
- for (auto &i : deny_regex[output]) {
+ if (!deny_regex_path[output].empty()) {
+ for (auto &i : deny_regex_path[output]) {
if (PCRE_ERROR_NOMATCH != pcre_exec(i._rex, i._extra, path, path_len, 0, PCRE_NOTEMPTY, nullptr, 0)) {
- TSDebug(PLUGIN_NAME, "Got a regex deny hit on regex: %s, country: %s", i._regex_s.c_str(), output);
+ TSDebug(PLUGIN_NAME, "Got a regex deny hit on regex path: %s, country: %s", i._regex_s.c_str(), output);
+ ret = false;
Review Comment:
true, thanks, yea the host section for the deny should probably check if it is already set to false and if it is leave it as false. Though the plugin can run in sort of a 'reverse' mode, where if you only set allow fields then by default it will deny everything else that isnt explicitly allowed.
So let me think about this, see if there is a way to logic a way out based on if its running in deny-everything mode plus there has been an allow or the default mode and you've already gotten a denial based on path (this was always the most brain hurting part of this plugin)
--
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] ezelkow1 closed pull request #9134: Adding regex capability based on pristine host name
Posted by GitBox <gi...@apache.org>.
ezelkow1 closed pull request #9134: Adding regex capability based on pristine host name
URL: https://github.com/apache/trafficserver/pull/9134
--
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] ezelkow1 commented on pull request #9134: Adding regex capability based on pristine host name
Posted by GitBox <gi...@apache.org>.
ezelkow1 commented on PR #9134:
URL: https://github.com/apache/trafficserver/pull/9134#issuecomment-1276551021
Closing, changing the regex to just be the full URL which should hopefully be a much smaller change
--
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] cmcfarlen commented on a diff in pull request #9134: Adding regex capability based on pristine host name
Posted by GitBox <gi...@apache.org>.
cmcfarlen commented on code in PR #9134:
URL: https://github.com/apache/trafficserver/pull/9134#discussion_r992787760
##########
plugins/experimental/maxmind_acl/mmdb.cc:
##########
@@ -694,19 +740,39 @@ Acl::eval_country(MMDB_entry_data_s *entry_data, const char *path, int path_len)
ret = true;
}
+ // test for path regex
if (nullptr != path && 0 != path_len) {
- if (!allow_regex[output].empty()) {
- for (auto &i : allow_regex[output]) {
+ if (!allow_regex_path[output].empty()) {
+ for (auto &i : allow_regex_path[output]) {
if (PCRE_ERROR_NOMATCH != pcre_exec(i._rex, i._extra, path, path_len, 0, PCRE_NOTEMPTY, nullptr, 0)) {
- TSDebug(PLUGIN_NAME, "Got a regex allow hit on regex: %s, country: %s", i._regex_s.c_str(), output);
+ TSDebug(PLUGIN_NAME, "Got a regex allow hit on regex path: %s, country: %s", i._regex_s.c_str(), output);
ret = true;
}
}
}
- if (!deny_regex[output].empty()) {
- for (auto &i : deny_regex[output]) {
+ if (!deny_regex_path[output].empty()) {
+ for (auto &i : deny_regex_path[output]) {
if (PCRE_ERROR_NOMATCH != pcre_exec(i._rex, i._extra, path, path_len, 0, PCRE_NOTEMPTY, nullptr, 0)) {
- TSDebug(PLUGIN_NAME, "Got a regex deny hit on regex: %s, country: %s", i._regex_s.c_str(), output);
+ TSDebug(PLUGIN_NAME, "Got a regex deny hit on regex path: %s, country: %s", i._regex_s.c_str(), output);
+ ret = false;
Review Comment:
If this is denied due to regex_path, should this skip the allow_regex_host check below? There needs to be some precedence here since there are now two ways to both allow and deny. Whatever is decided should be documented as well.
--
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] ezelkow1 commented on a diff in pull request #9134: Adding regex capability based on pristine host name
Posted by GitBox <gi...@apache.org>.
ezelkow1 commented on code in PR #9134:
URL: https://github.com/apache/trafficserver/pull/9134#discussion_r992808048
##########
plugins/experimental/maxmind_acl/mmdb.cc:
##########
@@ -694,19 +740,39 @@ Acl::eval_country(MMDB_entry_data_s *entry_data, const char *path, int path_len)
ret = true;
}
+ // test for path regex
if (nullptr != path && 0 != path_len) {
- if (!allow_regex[output].empty()) {
- for (auto &i : allow_regex[output]) {
+ if (!allow_regex_path[output].empty()) {
+ for (auto &i : allow_regex_path[output]) {
if (PCRE_ERROR_NOMATCH != pcre_exec(i._rex, i._extra, path, path_len, 0, PCRE_NOTEMPTY, nullptr, 0)) {
- TSDebug(PLUGIN_NAME, "Got a regex allow hit on regex: %s, country: %s", i._regex_s.c_str(), output);
+ TSDebug(PLUGIN_NAME, "Got a regex allow hit on regex path: %s, country: %s", i._regex_s.c_str(), output);
ret = true;
}
}
}
- if (!deny_regex[output].empty()) {
- for (auto &i : deny_regex[output]) {
+ if (!deny_regex_path[output].empty()) {
+ for (auto &i : deny_regex_path[output]) {
if (PCRE_ERROR_NOMATCH != pcre_exec(i._rex, i._extra, path, path_len, 0, PCRE_NOTEMPTY, nullptr, 0)) {
- TSDebug(PLUGIN_NAME, "Got a regex deny hit on regex: %s, country: %s", i._regex_s.c_str(), output);
+ TSDebug(PLUGIN_NAME, "Got a regex deny hit on regex path: %s, country: %s", i._regex_s.c_str(), output);
+ ret = false;
Review Comment:
true, thanks, yea the host section for the deny should probably check if it is already set to false and if it is leave it as false. Though the plugin can run in sort of a 'reverse' mode, where if you only set allow fields then by default it will deny everything else that isnt explicitly allowed, since it doesnt make sense to only set allow fields unless you want to explicitly deny everything else.
So let me think about this, see if there is a way to logic a way out based on if its running in deny-everything mode plus there has been an allow or the default mode and you've already gotten a denial based on path (this was always the most brain hurting part of this plugin). I think just from looking at it though doing a check on if it's already denied via path might be good enough
--
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] ezelkow1 commented on a diff in pull request #9134: Adding regex capability based on pristine host name
Posted by GitBox <gi...@apache.org>.
ezelkow1 commented on code in PR #9134:
URL: https://github.com/apache/trafficserver/pull/9134#discussion_r992808048
##########
plugins/experimental/maxmind_acl/mmdb.cc:
##########
@@ -694,19 +740,39 @@ Acl::eval_country(MMDB_entry_data_s *entry_data, const char *path, int path_len)
ret = true;
}
+ // test for path regex
if (nullptr != path && 0 != path_len) {
- if (!allow_regex[output].empty()) {
- for (auto &i : allow_regex[output]) {
+ if (!allow_regex_path[output].empty()) {
+ for (auto &i : allow_regex_path[output]) {
if (PCRE_ERROR_NOMATCH != pcre_exec(i._rex, i._extra, path, path_len, 0, PCRE_NOTEMPTY, nullptr, 0)) {
- TSDebug(PLUGIN_NAME, "Got a regex allow hit on regex: %s, country: %s", i._regex_s.c_str(), output);
+ TSDebug(PLUGIN_NAME, "Got a regex allow hit on regex path: %s, country: %s", i._regex_s.c_str(), output);
ret = true;
}
}
}
- if (!deny_regex[output].empty()) {
- for (auto &i : deny_regex[output]) {
+ if (!deny_regex_path[output].empty()) {
+ for (auto &i : deny_regex_path[output]) {
if (PCRE_ERROR_NOMATCH != pcre_exec(i._rex, i._extra, path, path_len, 0, PCRE_NOTEMPTY, nullptr, 0)) {
- TSDebug(PLUGIN_NAME, "Got a regex deny hit on regex: %s, country: %s", i._regex_s.c_str(), output);
+ TSDebug(PLUGIN_NAME, "Got a regex deny hit on regex path: %s, country: %s", i._regex_s.c_str(), output);
+ ret = false;
Review Comment:
true, thanks, yea the host section for the deny should probably check if it is already set to false and if it is leave it as false. Though the plugin can run in sort of a 'reverse' mode, where if you only set allow fields then by default it will deny everything else that isnt explicitly allowed, since it doesnt make sense to only set allow fields unless you want to explicitly deny everything else.
So let me think about this, see if there is a way to logic a way out based on if its running in deny-everything mode plus there has been an allow or the default mode and you've already gotten a denial based on path (this was always the most brain hurting part of this plugin)
--
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