Add sanitize_field_names spec #334
Conversation
💔 Build Failed
Expand to view the summary
Build stats
Steps errorsExpand to view the steps failures
Log outputExpand to view the last 100 lines of log output
|
|
This will be a breaking change in python (adding globbing, which is implicit in the current implementation), so we'll have to do a major version. But I think it will be worth it to standardize across agents. |
|
Another option that doesn't require a new major version right now would be to do the change in two steps: Align the default with the other agents but retain the auto-glob behavior by automatically adding wildcards to the beginning and end of each pattern (unless there already is a wildcard). Examples: That behavior can be removed in the next major version to fully align. |
|
Oh, that's interesting, I didn't know about these checks. I think it definitely makes sense to leave them in but they seem to be orthogonal to this config option anyway. Could you create a follow-up spec PR to make sure we do that check across agents? |
|
We could also consider removing the check. I'm skeptical that it provides any real world benefit? Compared to checking every value ever and how few may be sending credit card numbers in this way, under another key than those already checked? That's all just speculation, of course. |
|
I'd be fine with removing the credit card checks, their value seems dubious to me. |
|
So upgrade path is
|
|
With This would apply to Python and Ruby. Other agents don't add the additional wildcards. |
|
Why don't we sanitize the query string? Sure, the querystring shouldn't be used for sensitive data, but the interweb isn't really known for following best practices... I'm asking because in the Python agent we do currently sanitize the query string. Removing that would be a breaking change. |
Both in the .NET and in the Java doc we quote owasp.org: I can't give a very specific answer on the One quote I remember is (not tagging the person :D ) :
Not sure how big the overhead would be to parse it... I'd guess it's not that bad. I personally think that not sanitizing query string is fine. |
|
While checking within the agent is definitely expensive with apparent low value for now, there is definitely a huge cost implied by any leak of sensitive data (credit card numbers, SSN, ...), and often implied deleting and/or re-indexing. However, we could apply those checks/regexes at apm-server level, or even with an ingest pipeline to provide the same benefit to the user, while data would still be technically captured by agents, it won't be stored . Security-minded people won't do stupid things, thus they won't use query string. However, we can't assume that everyone is one of them. If we could prevent users from shooting in their own foot with a reasonable cost (thus probably not within the agents), that would still be valuable. |
There was a problem hiding this comment.
RUM agent will not be introducing a new config option as there is no use-case ATM and the agent does not capture HTTP headers (set-cookie), post body, form encoded data. The only place we do sanitisation is on the URL where we already redact most of the parameters by default.
We will align with the agents on this config once we see any new use cases.
|
Declared good to merge by @graphaelli over Slack 😊 |
|
@elastic/apm-agent-devs Please update the table above with links to issues if applicable (some already have this behaviour.) |
|
If your agent already works this way (or the setting is n/a) please state so in the table. |
Supersedes #319
Note that the implementation of this spec also includes adding the option to Kibana.