You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficserver.apache.org by danobi <gi...@git.apache.org> on 2017/03/10 19:29:17 UTC
[GitHub] trafficserver pull request #1567: Diags scrubbing mechanism
GitHub user danobi opened a pull request:
https://github.com/apache/trafficserver/pull/1567
Diags scrubbing mechanism
This should be the squashed commit message:
Diagnostic log scrubbing is intended an extra security measure to prevent sensitive strings from leaking to diagnostic logs. This is provided via a config option `proxy.config.diags.scrubs`.
For each line that is to be written to diagnostic logs, a regular expression(s) is applied to the line. If there are any matches, the matched substring is replaced with the replacement string.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/danobi/trafficserver diags_scrubbing
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/1567.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1567
----
commit 4d4b20bcc212837d4e8a37a13c39a60dfbf701b6
Author: Daniel Xu <dx...@dxuuu.xyz>
Date: 2017-03-09T22:56:53Z
Implement back end part of diags scrubbing
commit 37442e99d5796df0843c07d985ad3cd75966af12
Author: Daniel Xu <dx...@dxuuu.xyz>
Date: 2017-03-10T15:24:51Z
Use memcpy instead of loops
commit 9f251e4fb784a885a35073266a5f27c820220654
Author: Daniel Xu <dx...@dxuuu.xyz>
Date: 2017-03-10T15:27:02Z
Formatting
commit 8957e1429168fda79bd40fb5ca2e9f19096358da
Author: Daniel Xu <dx...@dxuuu.xyz>
Date: 2017-03-10T15:31:27Z
Prevent memory leak if pattern doesn't match
commit 58008e7b598eb6eb2af998fd5fe1766c32343f9f
Author: Daniel Xu <dx...@dxuuu.xyz>
Date: 2017-03-10T15:38:48Z
Prevent uninitialized free
commit 98b3c60f4c159fa0f82ca70ee8ce8b779f48efec
Author: Daniel Xu <dx...@dxuuu.xyz>
Date: 2017-03-10T15:41:28Z
Remove unneeded reference
commit 6db2a3e381303deaf6af99ede11a4edbfdc0b6c8
Author: Daniel Xu <dx...@dxuuu.xyz>
Date: 2017-03-10T18:59:11Z
Implement front end config for scrubbing
commit 26cd2b4cd24ee41895a16c8a4ab185d5849eabac
Author: Daniel Xu <dx...@dxuuu.xyz>
Date: 2017-03-10T19:24:44Z
Add documentation
commit 575d3d1f84a323efe441e18c0fcc31e3da456bc0
Author: Daniel Xu <dx...@dxuuu.xyz>
Date: 2017-03-10T19:24:56Z
Fix null check
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1567: [RFC] Diags scrubbing mechanism
Posted by danobi <gi...@git.apache.org>.
Github user danobi commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1567#discussion_r106495318
--- Diff: lib/ts/Scrubber.cc ---
@@ -0,0 +1,201 @@
+/** @file
+
+ Implementation for Scrubber.h
+
+ @section license License
+
+ 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 "Scrubber.h"
+
+Scrubber::Scrubber(const char *config)
+{
+ bool state_expecting_regex = true;
+ const ts::StringView delimiters("->;,", ts::StringView::literal);
+ ts::StringView regex;
+ ts::StringView replacement;
+ ts::StringView text(config);
+
+ this->config = ats_strdup(config);
+
+ // Loop over config string while extracting tokens
+ while (text) {
+ ts::StringView token = (text.ltrim(&isspace)).extractPrefix(&isspace);
+ if (token) {
+ // If we hit a delimiter, we change states and skip the token
+ if (!ts::StringView(token).ltrim(delimiters)) {
+ state_expecting_regex = !state_expecting_regex;
+ continue;
+ }
+
+ // Store our current token
+ if (state_expecting_regex) {
+ regex = token;
+ } else {
+ replacement = token;
+ }
+
+ // If we have a pair of (regex, replacement) tokens, we can go ahead and store those values into Diags
+ if (regex && replacement) {
+ scrub_add(regex, replacement);
+ regex.clear();
+ replacement.clear();
+ }
+ }
+ }
+
+ // This means that there was some kind of config or parse error.
+ // This doesn't catch *all* errors, and is just a heuristic so we can't actually print any meaningful message.
+ if ((regex && !replacement) || (!regex && replacement)) {
+ // Error("Error in scrubbing config parsing. Not enabling scrubbing.");
+ }
+}
+
+Scrubber::~Scrubber()
+{
+ if (config) {
+ ats_free(config);
+ }
+
+ for (auto s : scrubs) {
+ delete s;
+ }
+}
+
+bool
+Scrubber::scrub_add(const ts::StringView pattern, const ts::StringView replacement)
+{
+ pcre *re;
+ const char *error;
+ int erroroffset;
+ Scrub *s;
+
+ // Temp storage on stack is probably OK. It's unlikely someone will have long enough strings to blow the stack
+ char _pattern[pattern.size() + 1];
+ sprintf(_pattern, "%.*s", static_cast<int>(pattern.size()), pattern.ptr());
+
+ // compile the regular expression
+ re = pcre_compile(_pattern, 0, &error, &erroroffset, NULL);
+ if (!re) {
+ // Error("Unable to compile PCRE scrubbing pattern");
+ // Error("Scrubbing pattern failed at offset %d: %s.", erroroffset, error);
+ return false;
+ }
+
+ // add the scrub pattern to our list
+ s = new Scrub;
+ s->pattern = pattern;
+ s->replacement = replacement;
+ s->compiled_re = re;
+ scrubs.push_back(s);
+
+ return true;
+}
+
+void
+Scrubber::scrub_buffer(char *buffer) const
+{
+ // apply every Scrub in the vector, in order
+ for (auto s : scrubs) {
+ scrub_buffer(buffer, s);
+ }
+}
+
+void
+Scrubber::scrub_buffer(char *buffer, Scrub *scrub) const
+{
+ char *buffer_ptr;
+ int num_matched;
+ int match_len;
+ int replacement_len;
+ int buffer_len = strlen(buffer);
+
+ // execute regex
+ num_matched = pcre_exec(scrub->compiled_re, nullptr, buffer, buffer_len, 0, 0, scrub->ovector, scrub->OVECCOUNT);
+ if (num_matched < 0) {
+ switch (num_matched) {
+ case PCRE_ERROR_NOMATCH:
+ return;
+ default:
+ // Error("PCRE matching error %d\n", num_matched);
+ break;
+ }
+ }
+
+ /*
+ * When scrubbing the buffer in place, there are 2 scenarios we need to consider:
+ *
+ * 1) The replacement text length is shorter or equal to the text we want to scrub away
+ * 2) The replacement text is longer than the text we want to scrub away
+ *
+ * In case 1, we simply "slide" everything left a bit. Our final buffer should
+ * look like this (where XXXX is the replacment text):
+ *
+ * new_end orig_end
+ * V V
+ * -----------------------------------------
+ * |ORIGINAL TEXT|XXXX|ORIGINAL TEXT| |
+ * -----------------------------------------
+ *
+ * In case 2, since the final buffer would be longer than the original allocated buffer,
+ * we need to truncate everything that would have run over the original end of the buffer.
+ * The final buffer should look like this:
+ *
+ * new_end
+ * orig_end
+ * V
+ * -----------------------------------------
+ * |ORIGINAL TEXT|XXXXXXXXXXXXXXXXXXX|ORIGI|NAL TEXT
+ * -----------------------------------------
+ *
+ */
+
+ buffer_ptr = buffer;
+ match_len = scrub->ovector[1] - scrub->ovector[0];
+ replacement_len = scrub->replacement.size();
+
+ if (replacement_len <= match_len) { // case 1
+ // overwrite the matched text with the replacement text
+ buffer_ptr += scrub->ovector[0];
+ memcpy(buffer_ptr, scrub->replacement.ptr(), replacement_len);
+ buffer_ptr += replacement_len;
+
+ // slide everything after the matched text left to fill the gap including the '\0'
+ memmove(buffer_ptr, buffer + scrub->ovector[1], buffer_len - scrub->ovector[1] + 1);
+
+ // the last char should ALWAYS be a '\0' otherwise we did our math wrong
+ ink_assert(buffer[strlen(buffer)] == '\0');
--- End diff --
I must have been asleep at the wheel here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
clang format *successful*! https://ci.trafficserver.apache.org/job/clang-format-github/34/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
clang-analyzer build *failed*! https://ci.trafficserver.apache.org/job/clang-analyzer-github/293/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the issue:
https://github.com/apache/trafficserver/pull/1567
Spoke with Dan, we'll get the allocations cleanup before Leif has a stroke.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1567: [RFC] Diags scrubbing mechanism
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1567#discussion_r106482070
--- Diff: proxy/logging/LogObject.h ---
@@ -299,6 +299,10 @@ class LogObject : public RefCountObj
unsigned m_buffer_manager_idx;
LogBufferManager *m_buffer_manager;
+ // scrubbing data -- helps us scrub buffers of sensitive data
+ bool scrub_enabled = false;
--- End diff --
Are both of these needed? Does not `scubber` suffice by being null or not null?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [RFC] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
clang-analyzer build *successful*! https://ci.trafficserver.apache.org/job/clang-analyzer-github/320/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [RFC] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
clang format *successful*! https://ci.trafficserver.apache.org/job/clang-format-github/77/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
Linux build *successful*! https://ci.trafficserver.apache.org/job/linux-github/1624/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [RFC] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
clang format *successful*! https://ci.trafficserver.apache.org/job/clang-format-github/62/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
clang-analyzer build *failed*! https://ci.trafficserver.apache.org/job/clang-analyzer-github/307/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1567: [RFC] Diags scrubbing mechanism
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1567#discussion_r106505497
--- Diff: lib/ts/Diags.h ---
@@ -226,6 +242,10 @@ class Diags
mutable ink_mutex tag_table_lock; // prevents reconfig/read races
DFA *activated_tags[2]; // 1 table for debug, 1 for action
+ // Scrubbing variables -- replaces predefined regexp matches with the replacement string
+ bool scrub_enabled = false;
+ Scrubber *scrubber = nullptr;
--- End diff --
My long range plan is KIWF on `Vec.h` so we'll live with this for now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
clang format *failed*! https://ci.trafficserver.apache.org/job/clang-format-github/48/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [RFC] Diags scrubbing mechanism
Posted by danobi <gi...@git.apache.org>.
Github user danobi commented on the issue:
https://github.com/apache/trafficserver/pull/1567
@SolidWallOfCode This patch is done on my end and ready for review. If I find some time tonight I'll stare at the code and see if I can catch any bugs. It doesn't have any AFAICT.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [RFC] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
AU check *failed*! https://ci.trafficserver.apache.org/job/autest-github/59/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1567: [RFC] Diags scrubbing mechanism
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1567#discussion_r106505205
--- Diff: proxy/logging/LogObject.h ---
@@ -299,6 +299,10 @@ class LogObject : public RefCountObj
unsigned m_buffer_manager_idx;
LogBufferManager *m_buffer_manager;
+ // scrubbing data -- helps us scrub buffers of sensitive data
+ bool scrub_enabled = false;
--- End diff --
Hmmm, I'd need to think about if that make sense.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
RAT check *failed*! https://ci.trafficserver.apache.org/job/RAT-github/63/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1567: [RFC] Diags scrubbing mechanism
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1567#discussion_r106444530
--- Diff: lib/ts/Diags.cc ---
@@ -926,3 +917,29 @@ Diags::rebind_stderr(int new_fd)
}
return false;
}
+
+void
+Diags::vprintline(FILE *fp, char *buffer, va_list ap) const
+{
+ int nbytes = strlen(buffer);
+
+ // check if we need to scrub any patterns
+ if (unlikely(scrub_enabled)) {
+ char _buf[MAX_LOG_LINE_SIZE];
+ int r = vsnprintf(_buf, MAX_LOG_LINE_SIZE, buffer, ap);
+ if (r > MAX_LOG_LINE_SIZE) {
+ sprintf(_buf, "[WARN] Diags log line too long, dropping log\n");
+ } else if (r < 0) {
+ sprintf(_buf, "[WARN] Diags log line failed to encode with vsnprintf\n");
+ }
+ scrubber->scrub_buffer(_buf);
+ fprintf(fp, "%s", _buf);
--- End diff --
Actually, you have `r`, so `memcpy` could be used.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
RAT check *failed*! https://ci.trafficserver.apache.org/job/RAT-github/50/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
Intel CC build *successful*! https://ci.trafficserver.apache.org/job/icc-github/176/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
AU check *failed*! https://ci.trafficserver.apache.org/job/autest-github/45/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
Linux build *failed*! https://ci.trafficserver.apache.org/job/linux-github/1637/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1567: [WIP] Diags scrubbing mechanism
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1567#discussion_r105936302
--- Diff: lib/ts/Scrubber.cc ---
@@ -0,0 +1,138 @@
+#include "Scrubber.h"
+#include "MemView.h"
+
+Scrubber::Scrubber(const char *config)
+{
+ bool state_expecting_regex = true;
+ char *regex = nullptr;
+ char *replacement = nullptr;
+ const ts::StringView delimiters("->;,", ts::StringView::literal);
+ ts::StringView text(config);
+
+ this->config = ats_strdup(config);
--- End diff --
Another option would be for Scrubber to take ownership of the memory and avoid the extra alloc/copy. Beyond that, since the entire string is already persistent, you can use `StringView` instances for the pieces and not allocate for them at all. E.g., `regex` can be a `StringView` that points in to `config`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
clang format *successful*! https://ci.trafficserver.apache.org/job/clang-format-github/36/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
Intel CC build *successful*! https://ci.trafficserver.apache.org/job/icc-github/175/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1567: [RFC] Diags scrubbing mechanism
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1567#discussion_r106446114
--- Diff: lib/ts/Scrubber.cc ---
@@ -0,0 +1,201 @@
+/** @file
+
+ Implementation for Scrubber.h
+
+ @section license License
+
+ 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 "Scrubber.h"
+
+Scrubber::Scrubber(const char *config)
+{
+ bool state_expecting_regex = true;
+ const ts::StringView delimiters("->;,", ts::StringView::literal);
+ ts::StringView regex;
+ ts::StringView replacement;
+ ts::StringView text(config);
--- End diff --
This is problematic, because the `StringView` will be pointing in to external memory. This needs to point to the local memory in `this->config`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1567: [RFC] Diags scrubbing mechanism
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1567#discussion_r106445847
--- Diff: lib/ts/Scrubber.cc ---
@@ -0,0 +1,201 @@
+/** @file
+
+ Implementation for Scrubber.h
+
+ @section license License
+
+ 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 "Scrubber.h"
+
+Scrubber::Scrubber(const char *config)
+{
+ bool state_expecting_regex = true;
+ const ts::StringView delimiters("->;,", ts::StringView::literal);
--- End diff --
Make this `static` also, so that it's constructed at process start.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [RFC] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
Linux build *successful*! https://ci.trafficserver.apache.org/job/linux-github/1651/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1567: [RFC] Diags scrubbing mechanism
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1567#discussion_r106446324
--- Diff: lib/ts/Scrubber.cc ---
@@ -0,0 +1,201 @@
+/** @file
+
+ Implementation for Scrubber.h
+
+ @section license License
+
+ 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 "Scrubber.h"
+
+Scrubber::Scrubber(const char *config)
+{
+ bool state_expecting_regex = true;
+ const ts::StringView delimiters("->;,", ts::StringView::literal);
+ ts::StringView regex;
+ ts::StringView replacement;
+ ts::StringView text(config);
+
+ this->config = ats_strdup(config);
+
+ // Loop over config string while extracting tokens
+ while (text) {
+ ts::StringView token = (text.ltrim(&isspace)).extractPrefix(&isspace);
+ if (token) {
+ // If we hit a delimiter, we change states and skip the token
+ if (!ts::StringView(token).ltrim(delimiters)) {
+ state_expecting_regex = !state_expecting_regex;
+ continue;
+ }
+
+ // Store our current token
+ if (state_expecting_regex) {
+ regex = token;
+ } else {
+ replacement = token;
+ }
+
+ // If we have a pair of (regex, replacement) tokens, we can go ahead and store those values into Diags
+ if (regex && replacement) {
+ scrub_add(regex, replacement);
+ regex.clear();
+ replacement.clear();
+ }
+ }
+ }
+
+ // This means that there was some kind of config or parse error.
+ // This doesn't catch *all* errors, and is just a heuristic so we can't actually print any meaningful message.
+ if ((regex && !replacement) || (!regex && replacement)) {
--- End diff --
Why check this, if nothing is done?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
Intel CC build *successful*! https://ci.trafficserver.apache.org/job/icc-github/160/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1567: [RFC] Diags scrubbing mechanism
Posted by danobi <gi...@git.apache.org>.
Github user danobi commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1567#discussion_r106498117
--- Diff: proxy/logging/LogObject.h ---
@@ -299,6 +299,10 @@ class LogObject : public RefCountObj
unsigned m_buffer_manager_idx;
LogBufferManager *m_buffer_manager;
+ // scrubbing data -- helps us scrub buffers of sensitive data
+ bool scrub_enabled = false;
--- End diff --
Maybe. Depends on if we want to let users toggle scrubbing at runtime.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
AU check *failed*! https://ci.trafficserver.apache.org/job/autest-github/34/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1567: [RFC] Diags scrubbing mechanism
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1567#discussion_r106481662
--- Diff: lib/ts/Scrubber.h ---
@@ -0,0 +1,89 @@
+/** @file
+
+ The Scrubber class helps replace patterns of text inside a buffer with
+ replacement text.
+
+ @section license License
+
+ 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.
+ */
+
+#pragma once
+#include <vector>
+#include "pcre.h"
+
+#include "ts/ink_assert.h"
+#include "ts/ink_memory.h"
+#include "ts/MemView.h"
+
+struct Scrub {
+ ~Scrub()
+ {
+ if (compiled_re) {
+ pcre_free(const_cast<pcre *>(compiled_re));
+ }
+ }
+ static const int OVECCOUNT = 30;
+ ts::StringView pattern;
+ ts::StringView replacement;
+ const pcre *compiled_re;
+ int ovector[OVECCOUNT];
+};
+
+/*
+ * Class that helps scrub specific strings from buffers
+ */
+class Scrubber
+{
+public:
+ /*
+ * Parses config & constructs Scrubber
+ */
+ Scrubber(const char *config);
+ Scrubber(Scrubber &other) = delete;
+ ~Scrubber();
+
+ /*
+ * Add another expression to scrub for
+ *
+ * @returns whether or not the addition was successful
+ */
+ bool scrub_add(const ts::StringView pattern, const ts::StringView replacement);
+
+ /*
+ * Scrubs the buffer in place with all the Scrub objects stored in this class.
+ */
+ void scrub_buffer(char *buffer) const;
+
+ /*
+ * Config getter. Caller should NOT free
+ */
+ char *
+ get_config()
+ {
+ return config;
+ };
+
+private:
+ /*
+ * Scrubs the buffer in place with the passed in Scrub object
+ */
+ void scrub_buffer(char *buffer, Scrub *scrub) const;
+
+ char *config = nullptr;
--- End diff --
I recommend using `ats_scoped_str` here, to avoid having to explicitly clean up.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
Linux build *successful*! https://ci.trafficserver.apache.org/job/linux-github/1639/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [RFC] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
Linux build *successful*! https://ci.trafficserver.apache.org/job/linux-github/1665/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1567: [RFC] Diags scrubbing mechanism
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1567#discussion_r106445625
--- Diff: lib/ts/Diags.h ---
@@ -226,6 +242,10 @@ class Diags
mutable ink_mutex tag_table_lock; // prevents reconfig/read races
DFA *activated_tags[2]; // 1 table for debug, 1 for action
+ // Scrubbing variables -- replaces predefined regexp matches with the replacement string
+ bool scrub_enabled = false;
+ Scrubber *scrubber = nullptr;
--- End diff --
This is a good place to use `std::unique_ptr`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1567: [RFC] Diags scrubbing mechanism
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1567#discussion_r106444345
--- Diff: lib/ts/Diags.cc ---
@@ -926,3 +917,29 @@ Diags::rebind_stderr(int new_fd)
}
return false;
}
+
+void
+Diags::vprintline(FILE *fp, char *buffer, va_list ap) const
+{
+ int nbytes = strlen(buffer);
+
+ // check if we need to scrub any patterns
+ if (unlikely(scrub_enabled)) {
+ char _buf[MAX_LOG_LINE_SIZE];
+ int r = vsnprintf(_buf, MAX_LOG_LINE_SIZE, buffer, ap);
+ if (r > MAX_LOG_LINE_SIZE) {
+ sprintf(_buf, "[WARN] Diags log line too long, dropping log\n");
+ } else if (r < 0) {
+ sprintf(_buf, "[WARN] Diags log line failed to encode with vsnprintf\n");
+ }
+ scrubber->scrub_buffer(_buf);
+ fprintf(fp, "%s", _buf);
--- End diff --
Should probably just `strlcpy` here, although the `fprintf` is likely almost as fast.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
RAT check *successful*! https://ci.trafficserver.apache.org/job/RAT-github/47/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
Linux build *failed*! https://ci.trafficserver.apache.org/job/linux-github/1626/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [RFC] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
FreeBSD11 build *successful*! https://ci.trafficserver.apache.org/job/freebsd-github/1772/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
FreeBSD11 build *failed*! https://ci.trafficserver.apache.org/job/freebsd-github/1732/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
AU check *failed*! https://ci.trafficserver.apache.org/job/autest-github/47/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [RFC] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
FreeBSD11 build *successful*! https://ci.trafficserver.apache.org/job/freebsd-github/1757/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
clang format *successful*! https://ci.trafficserver.apache.org/job/clang-format-github/35/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1567: [RFC] Diags scrubbing mechanism
Posted by danobi <gi...@git.apache.org>.
Github user danobi commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1567#discussion_r106496765
--- Diff: lib/ts/Scrubber.h ---
@@ -0,0 +1,89 @@
+/** @file
+
+ The Scrubber class helps replace patterns of text inside a buffer with
+ replacement text.
+
+ @section license License
+
+ 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.
+ */
+
+#pragma once
+#include <vector>
+#include "pcre.h"
+
+#include "ts/ink_assert.h"
+#include "ts/ink_memory.h"
+#include "ts/MemView.h"
+
+struct Scrub {
+ ~Scrub()
+ {
+ if (compiled_re) {
+ pcre_free(const_cast<pcre *>(compiled_re));
+ }
+ }
+ static const int OVECCOUNT = 30;
--- End diff --
It was used in the PCRE example documentation. Really this could be any value >= 3.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
Intel CC build *successful*! https://ci.trafficserver.apache.org/job/icc-github/159/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [RFC] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
RAT check *successful*! https://ci.trafficserver.apache.org/job/RAT-github/75/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1567: [RFC] Diags scrubbing mechanism
Posted by danobi <gi...@git.apache.org>.
Github user danobi commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1567#discussion_r106495464
--- Diff: lib/ts/Diags.h ---
@@ -226,6 +242,10 @@ class Diags
mutable ink_mutex tag_table_lock; // prevents reconfig/read races
DFA *activated_tags[2]; // 1 table for debug, 1 for action
+ // Scrubbing variables -- replaces predefined regexp matches with the replacement string
+ bool scrub_enabled = false;
+ Scrubber *scrubber = nullptr;
--- End diff --
As discussed, due to namespace issues in `Vec.h`, using `unique_ptr` is more trouble than it's worth.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
RAT check *failed*! https://ci.trafficserver.apache.org/job/RAT-github/61/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the issue:
https://github.com/apache/trafficserver/pull/1567
Lots of allocation to be removed. This should be done without any allocation.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1567: [RFC] Diags scrubbing mechanism
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1567#discussion_r106480902
--- Diff: lib/ts/Scrubber.cc ---
@@ -0,0 +1,201 @@
+/** @file
+
+ Implementation for Scrubber.h
+
+ @section license License
+
+ 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 "Scrubber.h"
+
+Scrubber::Scrubber(const char *config)
+{
+ bool state_expecting_regex = true;
+ const ts::StringView delimiters("->;,", ts::StringView::literal);
+ ts::StringView regex;
+ ts::StringView replacement;
+ ts::StringView text(config);
+
+ this->config = ats_strdup(config);
+
+ // Loop over config string while extracting tokens
+ while (text) {
+ ts::StringView token = (text.ltrim(&isspace)).extractPrefix(&isspace);
+ if (token) {
+ // If we hit a delimiter, we change states and skip the token
+ if (!ts::StringView(token).ltrim(delimiters)) {
+ state_expecting_regex = !state_expecting_regex;
+ continue;
+ }
+
+ // Store our current token
+ if (state_expecting_regex) {
+ regex = token;
+ } else {
+ replacement = token;
+ }
+
+ // If we have a pair of (regex, replacement) tokens, we can go ahead and store those values into Diags
+ if (regex && replacement) {
+ scrub_add(regex, replacement);
+ regex.clear();
+ replacement.clear();
+ }
+ }
+ }
+
+ // This means that there was some kind of config or parse error.
+ // This doesn't catch *all* errors, and is just a heuristic so we can't actually print any meaningful message.
+ if ((regex && !replacement) || (!regex && replacement)) {
+ // Error("Error in scrubbing config parsing. Not enabling scrubbing.");
+ }
+}
+
+Scrubber::~Scrubber()
+{
+ if (config) {
+ ats_free(config);
+ }
+
+ for (auto s : scrubs) {
+ delete s;
+ }
+}
+
+bool
+Scrubber::scrub_add(const ts::StringView pattern, const ts::StringView replacement)
+{
+ pcre *re;
+ const char *error;
+ int erroroffset;
+ Scrub *s;
+
+ // Temp storage on stack is probably OK. It's unlikely someone will have long enough strings to blow the stack
+ char _pattern[pattern.size() + 1];
+ sprintf(_pattern, "%.*s", static_cast<int>(pattern.size()), pattern.ptr());
+
+ // compile the regular expression
+ re = pcre_compile(_pattern, 0, &error, &erroroffset, NULL);
+ if (!re) {
+ // Error("Unable to compile PCRE scrubbing pattern");
+ // Error("Scrubbing pattern failed at offset %d: %s.", erroroffset, error);
+ return false;
+ }
+
+ // add the scrub pattern to our list
+ s = new Scrub;
+ s->pattern = pattern;
+ s->replacement = replacement;
+ s->compiled_re = re;
+ scrubs.push_back(s);
+
+ return true;
+}
+
+void
+Scrubber::scrub_buffer(char *buffer) const
+{
+ // apply every Scrub in the vector, in order
+ for (auto s : scrubs) {
+ scrub_buffer(buffer, s);
+ }
+}
+
+void
+Scrubber::scrub_buffer(char *buffer, Scrub *scrub) const
+{
+ char *buffer_ptr;
+ int num_matched;
+ int match_len;
+ int replacement_len;
+ int buffer_len = strlen(buffer);
+
+ // execute regex
+ num_matched = pcre_exec(scrub->compiled_re, nullptr, buffer, buffer_len, 0, 0, scrub->ovector, scrub->OVECCOUNT);
+ if (num_matched < 0) {
+ switch (num_matched) {
+ case PCRE_ERROR_NOMATCH:
+ return;
+ default:
+ // Error("PCRE matching error %d\n", num_matched);
+ break;
+ }
+ }
+
+ /*
+ * When scrubbing the buffer in place, there are 2 scenarios we need to consider:
+ *
+ * 1) The replacement text length is shorter or equal to the text we want to scrub away
+ * 2) The replacement text is longer than the text we want to scrub away
+ *
+ * In case 1, we simply "slide" everything left a bit. Our final buffer should
+ * look like this (where XXXX is the replacment text):
+ *
+ * new_end orig_end
+ * V V
+ * -----------------------------------------
+ * |ORIGINAL TEXT|XXXX|ORIGINAL TEXT| |
+ * -----------------------------------------
+ *
+ * In case 2, since the final buffer would be longer than the original allocated buffer,
+ * we need to truncate everything that would have run over the original end of the buffer.
+ * The final buffer should look like this:
+ *
+ * new_end
+ * orig_end
+ * V
+ * -----------------------------------------
+ * |ORIGINAL TEXT|XXXXXXXXXXXXXXXXXXX|ORIGI|NAL TEXT
+ * -----------------------------------------
+ *
+ */
+
+ buffer_ptr = buffer;
+ match_len = scrub->ovector[1] - scrub->ovector[0];
+ replacement_len = scrub->replacement.size();
+
+ if (replacement_len <= match_len) { // case 1
+ // overwrite the matched text with the replacement text
+ buffer_ptr += scrub->ovector[0];
+ memcpy(buffer_ptr, scrub->replacement.ptr(), replacement_len);
+ buffer_ptr += replacement_len;
+
+ // slide everything after the matched text left to fill the gap including the '\0'
+ memmove(buffer_ptr, buffer + scrub->ovector[1], buffer_len - scrub->ovector[1] + 1);
+
+ // the last char should ALWAYS be a '\0' otherwise we did our math wrong
+ ink_assert(buffer[strlen(buffer)] == '\0');
+ } else { // case 2
+ // first slide all the text after the matched text right and truncate as necessary
+ int n_slide = buffer_len - scrub->ovector[0] - replacement_len;
+ if (n_slide < 0) {
+ // replacement string is too long, we need to shorten it
+ replacement_len -= -n_slide;
--- End diff --
`-= -` instead of `+=`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
FreeBSD11 build *failed*! https://ci.trafficserver.apache.org/job/freebsd-github/1731/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
FreeBSD11 build *failed*! https://ci.trafficserver.apache.org/job/freebsd-github/1744/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
FreeBSD11 build *failed*! https://ci.trafficserver.apache.org/job/freebsd-github/1745/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
AU check *failed*! https://ci.trafficserver.apache.org/job/autest-github/33/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1567: Diags scrubbing mechanism
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1567#discussion_r105472943
--- Diff: lib/ts/Diags.h ---
@@ -92,6 +94,26 @@ struct DiagsConfigState {
DiagsModeOutput outputs[DiagsLevel_Count]; // where each level prints
};
+struct Scrub {
+ ~Scrub()
+ {
+ if (pattern) {
--- End diff --
`ats_scoped_str` might be useful here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
Intel CC build *failed*! https://ci.trafficserver.apache.org/job/icc-github/174/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
RAT check *successful*! https://ci.trafficserver.apache.org/job/RAT-github/49/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [RFC] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
Intel CC build *successful*! https://ci.trafficserver.apache.org/job/icc-github/203/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [RFC] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
AU check *failed*! https://ci.trafficserver.apache.org/job/autest-github/74/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
Intel CC build *failed*! https://ci.trafficserver.apache.org/job/icc-github/173/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [RFC] Diags scrubbing mechanism
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on the issue:
https://github.com/apache/trafficserver/pull/1567
Looking better.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1567: [RFC] Diags scrubbing mechanism
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1567#discussion_r106456327
--- Diff: lib/ts/Scrubber.cc ---
@@ -0,0 +1,201 @@
+/** @file
+
+ Implementation for Scrubber.h
+
+ @section license License
+
+ 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 "Scrubber.h"
+
+Scrubber::Scrubber(const char *config)
+{
+ bool state_expecting_regex = true;
+ const ts::StringView delimiters("->;,", ts::StringView::literal);
+ ts::StringView regex;
+ ts::StringView replacement;
+ ts::StringView text(config);
+
+ this->config = ats_strdup(config);
+
+ // Loop over config string while extracting tokens
+ while (text) {
+ ts::StringView token = (text.ltrim(&isspace)).extractPrefix(&isspace);
+ if (token) {
+ // If we hit a delimiter, we change states and skip the token
+ if (!ts::StringView(token).ltrim(delimiters)) {
+ state_expecting_regex = !state_expecting_regex;
+ continue;
+ }
+
+ // Store our current token
+ if (state_expecting_regex) {
+ regex = token;
+ } else {
+ replacement = token;
+ }
+
+ // If we have a pair of (regex, replacement) tokens, we can go ahead and store those values into Diags
+ if (regex && replacement) {
+ scrub_add(regex, replacement);
+ regex.clear();
+ replacement.clear();
+ }
+ }
+ }
+
+ // This means that there was some kind of config or parse error.
+ // This doesn't catch *all* errors, and is just a heuristic so we can't actually print any meaningful message.
+ if ((regex && !replacement) || (!regex && replacement)) {
+ // Error("Error in scrubbing config parsing. Not enabling scrubbing.");
+ }
+}
+
+Scrubber::~Scrubber()
+{
+ if (config) {
+ ats_free(config);
+ }
+
+ for (auto s : scrubs) {
+ delete s;
+ }
+}
+
+bool
+Scrubber::scrub_add(const ts::StringView pattern, const ts::StringView replacement)
+{
+ pcre *re;
+ const char *error;
+ int erroroffset;
+ Scrub *s;
+
+ // Temp storage on stack is probably OK. It's unlikely someone will have long enough strings to blow the stack
+ char _pattern[pattern.size() + 1];
+ sprintf(_pattern, "%.*s", static_cast<int>(pattern.size()), pattern.ptr());
+
+ // compile the regular expression
+ re = pcre_compile(_pattern, 0, &error, &erroroffset, NULL);
+ if (!re) {
+ // Error("Unable to compile PCRE scrubbing pattern");
+ // Error("Scrubbing pattern failed at offset %d: %s.", erroroffset, error);
+ return false;
+ }
+
+ // add the scrub pattern to our list
+ s = new Scrub;
+ s->pattern = pattern;
+ s->replacement = replacement;
+ s->compiled_re = re;
+ scrubs.push_back(s);
+
+ return true;
+}
+
+void
+Scrubber::scrub_buffer(char *buffer) const
+{
+ // apply every Scrub in the vector, in order
+ for (auto s : scrubs) {
+ scrub_buffer(buffer, s);
+ }
+}
+
+void
+Scrubber::scrub_buffer(char *buffer, Scrub *scrub) const
+{
+ char *buffer_ptr;
+ int num_matched;
+ int match_len;
+ int replacement_len;
+ int buffer_len = strlen(buffer);
+
+ // execute regex
+ num_matched = pcre_exec(scrub->compiled_re, nullptr, buffer, buffer_len, 0, 0, scrub->ovector, scrub->OVECCOUNT);
+ if (num_matched < 0) {
+ switch (num_matched) {
+ case PCRE_ERROR_NOMATCH:
+ return;
+ default:
+ // Error("PCRE matching error %d\n", num_matched);
+ break;
+ }
+ }
+
+ /*
+ * When scrubbing the buffer in place, there are 2 scenarios we need to consider:
+ *
+ * 1) The replacement text length is shorter or equal to the text we want to scrub away
+ * 2) The replacement text is longer than the text we want to scrub away
+ *
+ * In case 1, we simply "slide" everything left a bit. Our final buffer should
+ * look like this (where XXXX is the replacment text):
+ *
+ * new_end orig_end
+ * V V
+ * -----------------------------------------
+ * |ORIGINAL TEXT|XXXX|ORIGINAL TEXT| |
+ * -----------------------------------------
+ *
+ * In case 2, since the final buffer would be longer than the original allocated buffer,
+ * we need to truncate everything that would have run over the original end of the buffer.
+ * The final buffer should look like this:
+ *
+ * new_end
+ * orig_end
+ * V
+ * -----------------------------------------
+ * |ORIGINAL TEXT|XXXXXXXXXXXXXXXXXXX|ORIGI|NAL TEXT
+ * -----------------------------------------
+ *
+ */
+
+ buffer_ptr = buffer;
+ match_len = scrub->ovector[1] - scrub->ovector[0];
+ replacement_len = scrub->replacement.size();
+
+ if (replacement_len <= match_len) { // case 1
+ // overwrite the matched text with the replacement text
+ buffer_ptr += scrub->ovector[0];
+ memcpy(buffer_ptr, scrub->replacement.ptr(), replacement_len);
+ buffer_ptr += replacement_len;
+
+ // slide everything after the matched text left to fill the gap including the '\0'
+ memmove(buffer_ptr, buffer + scrub->ovector[1], buffer_len - scrub->ovector[1] + 1);
+
+ // the last char should ALWAYS be a '\0' otherwise we did our math wrong
+ ink_assert(buffer[strlen(buffer)] == '\0');
+ } else { // case 2
+ // first slide all the text after the matched text right and truncate as necessary
+ int n_slide = buffer_len - scrub->ovector[0] - replacement_len;
--- End diff --
I think this should be `match_len`, not `replacement_len`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
FreeBSD11 build *failed*! https://ci.trafficserver.apache.org/job/freebsd-github/1730/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
AU check *failed*! https://ci.trafficserver.apache.org/job/autest-github/46/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
FreeBSD11 build *failed*! https://ci.trafficserver.apache.org/job/freebsd-github/1729/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
Linux build *failed*! https://ci.trafficserver.apache.org/job/linux-github/1638/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
FreeBSD11 build *failed*! https://ci.trafficserver.apache.org/job/freebsd-github/1743/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1567: [RFC] Diags scrubbing mechanism
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1567#discussion_r106447714
--- Diff: lib/ts/Scrubber.cc ---
@@ -0,0 +1,201 @@
+/** @file
+
+ Implementation for Scrubber.h
+
+ @section license License
+
+ 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 "Scrubber.h"
+
+Scrubber::Scrubber(const char *config)
+{
+ bool state_expecting_regex = true;
+ const ts::StringView delimiters("->;,", ts::StringView::literal);
+ ts::StringView regex;
+ ts::StringView replacement;
+ ts::StringView text(config);
+
+ this->config = ats_strdup(config);
+
+ // Loop over config string while extracting tokens
+ while (text) {
+ ts::StringView token = (text.ltrim(&isspace)).extractPrefix(&isspace);
+ if (token) {
+ // If we hit a delimiter, we change states and skip the token
+ if (!ts::StringView(token).ltrim(delimiters)) {
+ state_expecting_regex = !state_expecting_regex;
+ continue;
+ }
+
+ // Store our current token
+ if (state_expecting_regex) {
+ regex = token;
+ } else {
+ replacement = token;
+ }
+
+ // If we have a pair of (regex, replacement) tokens, we can go ahead and store those values into Diags
+ if (regex && replacement) {
+ scrub_add(regex, replacement);
+ regex.clear();
+ replacement.clear();
+ }
+ }
+ }
+
+ // This means that there was some kind of config or parse error.
+ // This doesn't catch *all* errors, and is just a heuristic so we can't actually print any meaningful message.
+ if ((regex && !replacement) || (!regex && replacement)) {
+ // Error("Error in scrubbing config parsing. Not enabling scrubbing.");
+ }
+}
+
+Scrubber::~Scrubber()
+{
+ if (config) {
+ ats_free(config);
+ }
+
+ for (auto s : scrubs) {
+ delete s;
+ }
+}
+
+bool
+Scrubber::scrub_add(const ts::StringView pattern, const ts::StringView replacement)
+{
+ pcre *re;
+ const char *error;
+ int erroroffset;
+ Scrub *s;
+
+ // Temp storage on stack is probably OK. It's unlikely someone will have long enough strings to blow the stack
+ char _pattern[pattern.size() + 1];
+ sprintf(_pattern, "%.*s", static_cast<int>(pattern.size()), pattern.ptr());
+
+ // compile the regular expression
+ re = pcre_compile(_pattern, 0, &error, &erroroffset, NULL);
+ if (!re) {
+ // Error("Unable to compile PCRE scrubbing pattern");
+ // Error("Scrubbing pattern failed at offset %d: %s.", erroroffset, error);
+ return false;
+ }
+
+ // add the scrub pattern to our list
+ s = new Scrub;
+ s->pattern = pattern;
+ s->replacement = replacement;
+ s->compiled_re = re;
+ scrubs.push_back(s);
+
+ return true;
+}
+
+void
+Scrubber::scrub_buffer(char *buffer) const
+{
+ // apply every Scrub in the vector, in order
+ for (auto s : scrubs) {
+ scrub_buffer(buffer, s);
+ }
+}
+
+void
+Scrubber::scrub_buffer(char *buffer, Scrub *scrub) const
+{
+ char *buffer_ptr;
+ int num_matched;
+ int match_len;
+ int replacement_len;
+ int buffer_len = strlen(buffer);
+
+ // execute regex
+ num_matched = pcre_exec(scrub->compiled_re, nullptr, buffer, buffer_len, 0, 0, scrub->ovector, scrub->OVECCOUNT);
+ if (num_matched < 0) {
+ switch (num_matched) {
+ case PCRE_ERROR_NOMATCH:
+ return;
+ default:
+ // Error("PCRE matching error %d\n", num_matched);
+ break;
+ }
+ }
+
+ /*
+ * When scrubbing the buffer in place, there are 2 scenarios we need to consider:
+ *
+ * 1) The replacement text length is shorter or equal to the text we want to scrub away
+ * 2) The replacement text is longer than the text we want to scrub away
+ *
+ * In case 1, we simply "slide" everything left a bit. Our final buffer should
+ * look like this (where XXXX is the replacment text):
+ *
+ * new_end orig_end
+ * V V
+ * -----------------------------------------
+ * |ORIGINAL TEXT|XXXX|ORIGINAL TEXT| |
+ * -----------------------------------------
+ *
+ * In case 2, since the final buffer would be longer than the original allocated buffer,
+ * we need to truncate everything that would have run over the original end of the buffer.
+ * The final buffer should look like this:
+ *
+ * new_end
+ * orig_end
+ * V
+ * -----------------------------------------
+ * |ORIGINAL TEXT|XXXXXXXXXXXXXXXXXXX|ORIGI|NAL TEXT
+ * -----------------------------------------
+ *
+ */
+
+ buffer_ptr = buffer;
+ match_len = scrub->ovector[1] - scrub->ovector[0];
+ replacement_len = scrub->replacement.size();
+
+ if (replacement_len <= match_len) { // case 1
+ // overwrite the matched text with the replacement text
+ buffer_ptr += scrub->ovector[0];
+ memcpy(buffer_ptr, scrub->replacement.ptr(), replacement_len);
+ buffer_ptr += replacement_len;
+
+ // slide everything after the matched text left to fill the gap including the '\0'
+ memmove(buffer_ptr, buffer + scrub->ovector[1], buffer_len - scrub->ovector[1] + 1);
+
+ // the last char should ALWAYS be a '\0' otherwise we did our math wrong
+ ink_assert(buffer[strlen(buffer)] == '\0');
--- End diff --
This is expensive and pointless - by definition, `strlen` will find a null so this will always be true, unless it segfaults. The index should be computed from the other lengths, not searched for, if you want to verify your math.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
RAT check *failed*! https://ci.trafficserver.apache.org/job/RAT-github/62/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
clang-analyzer build *failed*! https://ci.trafficserver.apache.org/job/clang-analyzer-github/291/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
clang-analyzer build *successful*! https://ci.trafficserver.apache.org/job/clang-analyzer-github/306/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
Linux build *successful*! https://ci.trafficserver.apache.org/job/linux-github/1625/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
clang format *successful*! https://ci.trafficserver.apache.org/job/clang-format-github/50/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1567: Diags scrubbing mechanism
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1567#discussion_r105471918
--- Diff: doc/admin-guide/files/records.config.en.rst ---
@@ -2975,6 +2975,38 @@ Diagnostic Logging Configuration
Specifies at what size to roll the diagnostics log at.
+.. ts:cv:: CONFIG proxy.config.diags.scrubs STRING NULL
+
+ Diagnostic log scrubbing is intended an extra security measure to prevent sensitive strings from leaking to
+ diagnistic logs. For example, if you wanted to scrub the regular expression ``string[a-z]`` from diagnostic
+ logs and replace it with ``XXXXXX``, use as the config string ``string[a-z] -> XXXXXX``. Note that the spaces
+ between the ``->`` are required.
+
+ If you want to scrub multiple regular expressions, use ``;`` to separate the pairs. For example:
+ ``string[a-z] -> XXXXXX ; doge$ -> doggy``. Once again, the spaces before and after the ``;`` are required.
+
+.. note::
+
+ Diagnostic log scrubbing supports PCRE format regular expressions.
+
+.. note::
+
+ Multiple captures are not supported. (ie. ``s/foo/bar/g`` is not suppored while ``s/foo/bar/`` is). Furthermore,
--- End diff --
"Multiple replacements"? That is, can I have capture groups? More than one capture group? Also, "suppored".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
AU check *failed*! https://ci.trafficserver.apache.org/job/autest-github/35/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
clang-analyzer build *failed*! https://ci.trafficserver.apache.org/job/clang-analyzer-github/292/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [RFC] Diags scrubbing mechanism
Posted by danobi <gi...@git.apache.org>.
Github user danobi commented on the issue:
https://github.com/apache/trafficserver/pull/1567
Personally, I don't think this feature has much use in open source. If nothing else, it probably encourages poor security practices. Even with this patch, someone could still do a `fprintf(stdout, "%s\n", sensitive_data")`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [RFC] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
clang-analyzer build *successful*! https://ci.trafficserver.apache.org/job/clang-analyzer-github/335/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request #1567: [RFC] Diags scrubbing mechanism
Posted by SolidWallOfCode <gi...@git.apache.org>.
Github user SolidWallOfCode commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/1567#discussion_r106481402
--- Diff: lib/ts/Scrubber.h ---
@@ -0,0 +1,89 @@
+/** @file
+
+ The Scrubber class helps replace patterns of text inside a buffer with
+ replacement text.
+
+ @section license License
+
+ 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.
+ */
+
+#pragma once
+#include <vector>
+#include "pcre.h"
+
+#include "ts/ink_assert.h"
+#include "ts/ink_memory.h"
+#include "ts/MemView.h"
+
+struct Scrub {
+ ~Scrub()
+ {
+ if (compiled_re) {
+ pcre_free(const_cast<pcre *>(compiled_re));
+ }
+ }
+ static const int OVECCOUNT = 30;
--- End diff --
Where does `30` come from? POOMA?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
clang-analyzer build *successful*! https://ci.trafficserver.apache.org/job/clang-analyzer-github/305/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
clang format *successful*! https://ci.trafficserver.apache.org/job/clang-format-github/37/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
clang format *failed*! https://ci.trafficserver.apache.org/job/clang-format-github/49/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [RFC] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
Intel CC build *successful*! https://ci.trafficserver.apache.org/job/icc-github/188/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by danobi <gi...@git.apache.org>.
Github user danobi commented on the issue:
https://github.com/apache/trafficserver/pull/1567
The last commit gets error.log scrubbed too. I'll fix the allocation stuff on Tuesday. Nobody tell Leif.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [WIP] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
clang-analyzer build *failed*! https://ci.trafficserver.apache.org/job/clang-analyzer-github/308/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: [RFC] Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
RAT check *successful*! https://ci.trafficserver.apache.org/job/RAT-github/90/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by bryancall <gi...@git.apache.org>.
Github user bryancall commented on the issue:
https://github.com/apache/trafficserver/pull/1567
Please swash the commits.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver issue #1567: Diags scrubbing mechanism
Posted by atsci <gi...@git.apache.org>.
Github user atsci commented on the issue:
https://github.com/apache/trafficserver/pull/1567
RAT check *successful*! https://ci.trafficserver.apache.org/job/RAT-github/48/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---