Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docs: redacting sensitive information in log records #43

Open
GeertJohan opened this issue Jan 7, 2015 · 13 comments
Open

Docs: redacting sensitive information in log records #43

GeertJohan opened this issue Jan 7, 2015 · 13 comments

Comments

@GeertJohan
Copy link

One thing I miss in log15 is a good way to redact/censor information from the logs.

From https://godoc.org/github.com/op/go-logging#Redactor

type Redactor interface {
    Redacted() interface{}
}

"Redactor is an interface for types that may contain sensitive information (like passwords), which shouldn't be printed to the log. The idea was found in relog as part of the vitness project. "

What are your thoughts about this?
I could implement and PR.

@inconshreveable
Copy link
Owner

It's pretty easy to implement this with your own handler. Do we think this is a common enough case that it needs to be included as a standard component?

@GeertJohan
Copy link
Author

I think it's good practice to motivate users to use censoring in logging. It might not be a common case for log15 users right now, but if I think about the purposes Go is used for, I can think of enough cases where censoring logs should happen.

Doing this is with a Handler a great idea, this also allows you to configure in the handler tree at which places to censor. But there are some questions to how this would work. For instance:

type foo struct {
  Number int
  Password *password
}

type password string

func (p *password) Redact() interface{} {
  return "-------"
}

// log using the struct: 
pass := password("secret")
myFoo := &foo{42, &pass}
logger.Info("oops", log15.Ctx{"foo123": myFoo})

The RedactionHandler should not change the value of myFoo.Password because myFoo might still be used after the logging call.
So you'd need to have the RedactionHandler to modify the complete value for "foo123" to a log15.Ctx and add all fields the foo struct has. So the new log ctx would look like:

log15.Ctx{
  "foo123": log15.Ctx{
    "Number": 42,
    "Password": "------",
  },
}

I believe this makes the whole redaction thing a bit harder to let it be implemented by users manually, so it would be a great addition to this package!

@GeertJohan
Copy link
Author

Any update on this?

@ChrisHines
Copy link
Collaborator

I've certainly encountered the need to redact passwords from log files. I have put the burden on the application code so far, but a more general approach does have some appeal.

Having said that, I'm not sure I understand how the Redactor interface helps handle nested fields when logging entire structures as a single value. Could you explain it further, or even better, prototype an implementation?

@GeertJohan
Copy link
Author

Now that I'm thinking about this again, there's not really a good point at which the value can be changed. Because the password value can be referenced through a pointer which is still accessible and being used in user code outside of the log package.
So the only way to safely do this would be to implement censoring in all formatters, right? For most formatters that would work, but the JSON formatter wil be an issue because the password type can be used in structs that are being marshalled to JSON, and the json package won't use the Redacted() methods..

I'll give this some more thought and see if I can come up with a better solution..

@inconshreveable
Copy link
Owner

@GeertJohan I'm not sure I understand why this is a problem. The redactor interface returns the new readacted object to log instead, so dealing with pointers and copying or whatever is necessary to do the redacting would be the application's responsibility.

@ChrisHines Seems like the options are: use reflect to recursively walk all fields of an object or only enforce the redactor guarantee for the context value itself.

I'm still unconvinced this is a common enough use case that it belongs in the library. Perhaps it belongs in ext.

A separate conversation should probably go into whether the ext package even makes sense and should continue to exist in v3.

@GeertJohan
Copy link
Author

Redacting mostly makes sense for more complicated structures with fields that should be redacted. Manually redacting log15.Ctx values is quite easy for the application to do, simply don't add password to the log15.Ctx you're logging.

For structs that are being logged, using reflect is the best way to find all the fields implementing Redactor. But reflect is quite expensive.

Futhermore: I think I would like log15 to not modify the values I hand it in the context. But JSON doesn't have any knowledge about the Redactor, so that means log15 must reflect and copy all values in the context and replace some values with their redacted form, before handig it to the json package. This is also very expensive.

In the end I think it's better to drop this issue and have redacting done by the application itself. We could include an example, although it's quite trivial.

type Account struct {
    // some fields
    Password string
}
func (a *Account) Redact() *Account {
    b := *a
    b.Password = "redacted"
    return &b
}

logger.Info("we have some account details", log15.Ctx{"account": a.Redact()})

@ChrisHines
Copy link
Collaborator

Agreed. Should we close this or change it to a documentation issue?

@GeertJohan
Copy link
Author

I guess we could make this a documentation issue yes. Maybe we could also write a short informational piece about security and privacy in logging in general, and how to do it with log15.
What is your current approach? You said "I have put the burden on the application code so far", can you give an example?

@inconshreveable
Copy link
Owner

I wonder if the best documentation for this would be to start a "examples" or "patterns" section of the README (or separate file) that shows how you might want to implement different kinds of Handlers in your own code.

@ChrisHines
Copy link
Collaborator

@GeertJohan My use case has been related to database connection strings. Here is an excerpt from a private package we have.

// Config contains the information needed to connect to a Microsoft SQL Server
// database.
type Config struct {
    Host           string `desc:"Database server"`
    Name           string `desc:"Database name"`
    User           string `desc:"Database user"`
    Pass           string `desc:"Database user password"`
    MaxConnections int    `toml:"max-connections" flag:"max-connections" desc:"Maximum database connections"`
}

// ConnString returns the connection string implied by the configuration. If
// fuzz is true, the password field will be obscured, making it safe to show
// in log files.
func (c Config) ConnString(fuzz bool) string {
    if fuzz {
        return connString(c.Host, c.Name, c.User, "#####")
    }
    return connString(c.Host, c.Name, c.User, c.Pass)
}

@GeertJohan
Copy link
Author

@inconshreveable Yes, that sounds like a good idea

@ChrisHines Thanks. I guess this method and "copy value and modify fields" could be in the examples.

@GeertJohan GeertJohan changed the title Adopt redactor/censor functionality Docs: redacting sensitive information in log records Apr 2, 2015
@davisford
Copy link

@inconshreveable the Redact() func is great, but sometimes you have this kind of case:

type Auth struct {
   Username string 
   Password string 
}

type Payload struct {
    Auth 
    Foo string 
    Bar int 
}

...


log.Error("something happened", "payload", payload)

Auth is embedded, and I can't Redact it. I can code around it but this is a pain. Would be nice to have some way to tag it or work around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants