troubleshoot
troubleshoot copied to clipboard
replicatedhq
Make redaction of IP addresses opt-in
Describe the rationale for the suggested feature.
Many times during troubleshooting issues with networking, particularly Weave issues, we ask folks to retrieve logs and other info that include IP addresses. While this information is redacted from all Troubleshoot bundles, we do not see the IP addresses and so have to ask outside of Troubleshoot for the information if it is necessary to have to understand the problem.
Describe the feature
- Remove the IP address redaction from the Troubleshoot default redactors in the Troubleshoot code itself
- Update the docs to remove the statement that it's redacted by default
- Folks that want IP address redaction need to go and add that into their custom redactors (as documented)
Describe alternatives you've considered
- Indexing redactors - so that e.g. address
192.168.1.1gets replaced with something specific to that address, e.g.a.b.c.d. This would relieve many of the issues we have with redaction right now, however if an end user needs to go check a firewall rule they are going to need the addresses, not the index. - CLI switch to disable some redactors including IP addresses. This would mean anyone using the CLI has the option to not redact IP addresses, which keeps the default behavior, but does require folks to potentially collect a bundle twice (once after learning that IP info is required).
Additional context
Environments with particularly restrictive security requirements do require IP addresses to be redacted. This request changes the default to not redact, rather than redact.
An alternative request is here https://github.com/replicatedhq/troubleshoot/issues/449
I have worked in environments with this restriction before, however they aren't limited to IP addresses. Generally this restriction is derived from the need to protect the network architecture. That's why mac addresses are included for example (it's not just a rule to restrict IP).
However, that same requirement in my experience also includes host names and ports. For kubernetes I would think service names would also be included. Considering pod names and namespaces are part of DNS entries this gets pretty useless fairly quickly.
We should be careful not to make a bad choice for users that can use the tool in the name of protecting users that can't. The only way I've been able to work in environments with this level of restriction before was restricting who has access to log files entirely. It would be fantastic if we could serve the highly restricted users, I think we should be working off of more than theoretical use cases if it negatively impacts the vast majority of users.
That's to say I think the default should be changed to opt in. The impact to most users is very high it should be applied only when needed.
Such a change needs appropriate documentation to notify users, but that's a reasonable thing to do.
I also agree that the default behavior should be opt in.
If a project that imports Troubleshoot has a requirement of redacting all IP addresses, with this change they should still have the flexibility to do so, but this project making that decision globally seems strange to me. Although likely to be the most common usage, should we make the assumption that every use case of support bundle collection will result in exchanging the bundle with a non-trusted party?
We've not come to a conclusion here. I know of one fork that is a fork in part because they need to remove the IP address redaction.
How about a slightly different proposal:
- we first add a CLI switch that will disable the IP address redactor
- later down the track, we add the ability in redactor specs to disable certain, named, default redactors. That way, if someone programs in disabling IP address redactors in their spec, they don't need to use the CLI to get that functionality.
@marccampbell since this doesn't change the default behavior do you see that as a more acceptable solution for now? If we need the IPs for a particular use case, we can ask folks to just run from the CLI with the appropriate arg, and keep some kind of watch on how often we need to do that.
My concern with that proposal is two fold.
- We're telling people if they have networking issues they automatically have to go through a feedback loop for us to tell them to do something different. Right off the bat a whole category of issues can never hit the goal of never having to ask the user for more information.
- We've setup a UX which involves "Define what you want" and two special case "Define what you don't want" redactors. Striving for a 1 size fits all redactor I believe is an impossibility and I'd rather we remove any defaults and provide what we think are good templates for people to build from so that the inevitable incompatibilities between a broad range of users can be addressed where it belongs, with the end users who doesn't need to satisfy every possible redaction combination.
I don't want to sound like changing a default isn't a big deal. I recognize it is. I'm generally in favor of projects putting a high value on their users experience. In this case, I feel like the vast majority of users benefit from a new and better default and I don't want to see a strict interpretation of a general rule to make the experience worse for most users.
I can't provide data publicly on why I think the vast majority of users would prefer this. But anyone internal to Replicated can simply review our support cases and consider the quantity of cases that post an IP address anywhere in the conversation. It happens regularly, I can think of only a few times where people can't provide data and those users wouldn't provide the data regardless of any available redaction even when we offer it as a solution.
I wanted to see if I could find something to try and explain some of the above. This comes with a ton of caveats but hopefully it's more helpful than harmful.
A review of cases where end users posted an IP address in GitHub found 71 different issues in 2022 (so far obviously). This doesn't cover uploaded files, phone calls, screen shares, etc. It also doesn't include anything else networking like mac address, hostnames, ports. So overall it's not the perfect statistic but hopefully it helps provide some explanation that's more than me saying "it's in data you can't see".
@marccampbell and @AmberAlston wanted to check in with y'all on this proposal. We've heard from the D2IQ team at our last community meeting and I've posted some data, what data I could get, above about quantity of users to help better inform the discussions as previously requested.
Given we would need, as suggested, to coordinate a clear announcement about how a user could opt back in do either of you still have concerns or want further discussion on changing this default? To be clear, the documentation already states how you enable this, it just didn't match the code which is why you don't see a documentation change.
ADR is merged, but we haven't actually made the change yet. #734 addresses this.