search

Azure / azure-storage-blob-go

Microsoft Azure Blob Storage Library for Go
MIT License
157 stars 102 forks source link

Logging of timeout contains non redacted secret. #133

Open martin-ljunggren opened 5 years ago

martin-ljunggren commented 5 years ago

Which version of the SDK was used?

0.6.0

Which platform are you using? (ex: Windows, Linux, Debian)

Linux

What problem was encountered?

When a request using StageBlock times out, the sig parameter in the url is supposed to be redacted. This is done, but the error from the http client is also appended to the log message, and it contains the non redacted sig parameter. I haven't been able to find a way to disable this log message but I guess it is supposed to be forced. Am I doing something wrong or is it a bug?

How can we reproduce the problem in the simplest way?

Have you found a mitigation/solution?

martin-ljunggren commented 5 years ago

Bumping this issue. Is there anything I can add to make it easier to investigate?

JohnRusk commented 5 years ago

Thanks Martin. Yeah, its a bug. We can suggest who interim solutions right now, but we are still thinking about the best long-term fix.

Interim solutions: Both of these require updateing to the latest azure-pipeline-go (which I think you'll get if you just update to the latest azure-storage-blob-go'. The solutions are:

  1. Call the new SetForceLogEnabled method, passing in false. That turns the forced logging off totally. Or
  2. Call the new SetLogSanitizer method. That lets you supply an interface of something which is capable of redacting all logged text. That's what we are currently doing in AzCopy v10.

The reason why the AzCopy code is not baked into the blob SDK is that there can be secrets from non-azure platforms, with the new PutBlockFromURL support. (e.g. a AWS presigned URL as the copy source). And, we don't know exactly which non-azure platforms users of the blob SDK may use. So we are considering whether to do something along those lines anyway (in spite of that information gap), or to change the way errors are logged, or both... Would welcome any suggestions you have.

martin-ljunggren commented 5 years ago

Thanks! I will try the sanitizer, it sounds like a good option. Will it apply to all logs or only forced logs?

JohnRusk commented 5 years ago

The pipeline package only does forced logging, so pipeline.SetLogSanitizer only applies to forced logging. If you want to use it also for any other logging that you do, then you need to hook it in yourself at some appropriate place in your applications logging infrastructure. E.g. in AzCopy we do so in a routine that all logging passes through, so we know we won't miss anything.