swift-log icon indicating copy to clipboard operation
swift-log copied to clipboard

Adding support for privacy level in string interpolation

Open Frizlab opened this issue 2 years ago • 12 comments

On macOS, the (Apple’s) Logger object uses a custom OSLogMessage object which allows it to have a custom string interpolation with support for format arguments privacy levels.

Would it be possible to add support for said string interpolation so we could finally have an efficient and fully feature-compatible OSLog Logger for swift-log?

Thanks!

Frizlab avatar Aug 05 '21 14:08 Frizlab

@frizlab – While not exactly what you're asking for, I have written something like this and mentioned it on another issue #208 to see if this is appropriate for this package. If not, it's still usable from my own package. Feel free to use.

shaps80 avatar Sep 22 '21 10:09 shaps80

I noticed some advice regarding privacy here. Would be great to have a section in the docs about this.

nashysolutions avatar Jan 04 '23 07:01 nashysolutions

That is a good idea @nashysolutions. would you or @Frizlab like to make a PR with proposed language? I suspect we may also want to consider a new metadata type for sensitive data that the log backend can reduct

tomerd avatar Jan 04 '23 18:01 tomerd

We've implemented a similar mechanism in our local code - is this something that might be generally useful?

extension Logger.MetadataValue {
  /// The different styles of privacy masks that can be applied to a metadata value.
  internal enum PrivacyMask {
    /// The value will be hashed and converted into an 8 character hex representation.
    case hash
    /// Replaces the value with `*****`.
    case redact
  }

  /// Replaces the value with `*****`.
  ///
  /// This is great for when the actual value is not of importance and should be masked when not debugging.
  ///
  /// For example:
  ///
  ///     let logger: Logger = ...
  ///     logger.info(
  ///       "replaced value with mask pattern",
  ///       metadata: [
  ///         "password": .mask(user.password)
  ///       ]
  ///     )
  public static func mask(_ value: Any) -> Logger.MetadataValue {
    return .stringConvertible(MaskedValue(value, mask: .redact))
  }

  /// The value will be hashed and converted into an 8 character hex representation.
  ///
  /// This is great for when identity of value between individual logs is important, but the actual value should still be masked.
  ///
  /// For example:
  ///
  ///     let logger: Logger = ...
  ///     logger.info(
  ///       "obfuscated value",
  ///       metadata: [
  ///         "ip_address": .hash(user.ipAddress)
  ///       ]
  ///     )
  public static func hash(_ value: Any) -> Logger.MetadataValue {
    return .stringConvertible(MaskedValue(value, mask: .hash))
  }
}

internal struct MaskedValue: CustomStringConvertible {
  internal let underlying: Any

  private let mask: Logger.MetadataValue.PrivacyMask

  internal var description: String {
    switch mask {
    case .hash: return self.hash("\(self.underlying)")
    case .redact: return "*****"
    }
  }

  fileprivate init(_ value: Any, mask: Logger.MetadataValue.PrivacyMask) {
    self.underlying = value
    self.mask = mask
  }

  private func hash(_ value: String) -> String {
    var hasher = Hasher()
    hasher.combine(value)
    return "\(String(format: "%x", hasher.finalize()))"
  }
}

Mordil avatar Jun 28 '23 17:06 Mordil

this seems interesting to me. @ktoso? @weissi?

tomerd avatar Jun 28 '23 19:06 tomerd

Here's an example of the LogHandler transforming the metadata


switch metadataValue {
case let .stringConvertible(maskedValue as MaskedValue) where self.logLevel <= .debug:
      return "\(maskedValue.underlying)"

default: return "\(metadataValue)"
}

Mordil avatar Jun 29 '23 00:06 Mordil

@Mordil @tomerd @Frizlab I actually open sourced exactly this around 2 years ago. It was discussed with the team here and on Swift Forums but was deemed inappropriate for this library.

So I pulled swift-log and then added the interpolations for similar convenience. It's been used in several projects since its release and I haven't had any reported issues.

It obviously doesn't have the same performance characteristics in production builds, since OSLog only executes its closures when logs are being pulled into Console.app or Xcode. It uses print which most other logging libraries do, so I don't see this as being an issue. Just full disclosure 👍

https://github.com/shaps80/Logging

shaps80 avatar Jul 03 '23 08:07 shaps80

@shaps80 Do you have links to the discussions where it was determined this was inappropriate for the library to offer directly?

Mordil avatar Jul 11 '23 18:07 Mordil

Yeah, https://forums.swift.org/t/swiftlog-stringinterpolation/52313/3 You actually were one of the last comments agreeing to this hahaha.

On this repo there was also: https://github.com/apple/swift-log/issues/208

shaps80 avatar Jul 12 '23 15:07 shaps80

@tomerd For apps I think adding privacy works just fine. For libraries it's IMHO not possible to sensibly annotate. Take a URL, some apps have sensitive URLs, other apps don't but the HTTP client library couldn't know how it's used.

My personal opinion is: If you need privacy, then

  • stick everything that's not constant into metadata (e.g. logger.info("user logged in", metadata: ["user-name": "\(userName)", "request-uuid": "\(request.uuid)"]))
  • in your LogHandler, redact every metadata value that isn't specifically allow listed

So the above message (logger.info("user logged in", metadata: ["user-name": "\(userName)", "request-uuid": "\(request.uuid)"])) should IMHO be logged as

user logged in [user-name: <redacted>, request-uuid: 1F4A3360-AEEC-48A4-92E4-60DA38823004]

because an IMHO sensible LogHandler configuration would (for a hypothetical SuperCoolPrivacyLogHandler) could look something like

SuperCoolPrivacyLogHandler(
    label: ...,
    configuration: SuperCoolPrivacyLogHandlerConfiguration(
        nonRedactedMetadataKeysList: ["request-uuid", "error", "timestamp", "count", "ahc-error", "ahc-connection-id", ...]
    )
)

The reason is that IMHO only the application can know what has PII and what doesn't. If the HTTP client is only used for requests that never contain PII, then it would be safe to add ahc-url or whatever into the nonRedactedMetadataKeysList but for many other apps that wouldn't be acceptable at all.


If it becomes too tedious to list each and every field, then I would suggest a convention for your application: For example if you say that every metadata key that starts with public: is never redacted. So logger.info("user logged in", metadata: ["user-name": "\(userName)", "public:user-id-hash": "\(sha256(userID))"] or something.


I'm guessing the tl;dr is:

  • swift-log is deliberately versatile and unopinionated. If you want to implement special privacy controls go nuts in your LogHandler
  • I don't think PII can be decided on the log message production side (i.e. the libraries that log). It has to be decided by a central point that is controlled by the application as only the application understands what is and what isn't PII in its domain (might even depend by country and what not)

weissi avatar Jul 12 '23 22:07 weissi

one thing to keep in mind is that the application is also a "producer" log events. its makes perfect sense to me that libraries do not force PII decision on the app, but its nice for the apps to be able to emit ones taking into account PII considerations. that said, I can see how this can be handled in a sophisticated enough handler / backend with nice configuration for this kind of settings

tomerd avatar Jul 13 '23 00:07 tomerd

one thing to keep in mind is that the application is also a "producer" log events. its makes perfect sense to me that libraries do not force PII decision on the app, but its nice for the apps to be able to emit ones taking into account PII considerations. that said, I can see how this can be handled in a sophisticated enough handler / backend with nice configuration for this kind of settings

Totally agree with everything. I guess I was trying to say doing something that adds privacy for the app only but ignores the libraries isn't any good. And yes, sophisticated LogHandlers are the way. I wish there was a properly configurable and featureful "logging middle-end" that would take care of these things once and for all.

weissi avatar Jul 13 '23 09:07 weissi