search

vladmihalcea / hypersistence-utils

The Hypersistence Utils library (previously known as Hibernate Types) gives you Spring and Hibernate utilities that can help you get the most out of your data access layer.
Apache License 2.0
2.37k stars 366 forks source link

ObjectMapperWrapper logging sensitive data on deserialisation errors #601

Open jlous opened 1 year ago

jlous commented 1 year ago

If ObjectMapperWrapper encounters a deserialisation error, it unconditionally dumps the entire json into the message of an IllegalArgumentException, which, if unexpected, will often make its way into the logs.

This is extremely ill-advised: Json in the production database will often contain private or sensitive data that should not be accessible to everyone with access to the techincal logs.

Jackson does the same thing by default (although truncating the content a little), but it can at least be easily configured not to (StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION) But this is of little help when the library using it logs it again by itself.

Logback-level pattern-based masking is not suitable for redacting large and arbitrary jsob clobs.

Please provide a config to omit json-content from these errors, to limit data-leakage via logs. Or maybe even better: drop it entirely, and leave content-logging it to the root-cause message from jackson, which is more informative about what is wrong anyway, even with content-quoting turned off.

vladmihalcea commented 1 year ago

It's not the ObjectMapperWrapper printing those failures into the application log. That's your own app doing that.

The ObjectMapperWrapper only throws the Exception, so your application should decide what is the best way to handle that.

jlous commented 1 year ago

When using a framework like Spring Boot, or any large system, you never have a complete overview of which exceptions might appear and from where. Logging exceptions is a normal and sensible thing to do, and this is one of the reasons sensitive data in exception messages is generally avoided, preferably as a sensible default but at least as a configuration. Filtering error messages from every source in a sensible way would require explicit knowledge about how each of them is constructs their messages in order to build a safe system.

Maybe the error wrapping could be made pluggable like the objectmapper itself

tor. 30. mar. 2023, 18:34 skrev Vlad Mihalcea @.***>:

It's not the ObjectMapperWrapper printing those failures into the application log. That's your own app doing that.

The ObjectMapperWrapper only throws the Exception, so your application should decide what is the best way to handle that.

— Reply to this email directly, view it on GitHub https://github.com/vladmihalcea/hypersistence-utils/issues/601#issuecomment-1490599070, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB2OUB4YNR2UXEKGSRIHIDW6WYZDANCNFSM6AAAAAAWNQ4KCQ . You are receiving this because you authored the thread.Message ID: @.***>

vladmihalcea commented 1 year ago

@jlous This is what Spring does when catching a JsonProcessingException.

So, here's what you'll have to do:

  1. Fork the project
  2. Change the ObjectMapperWrapper to use the strategy used by Spring
  3. Validate whether it's better for this use case
  4. Send me a Pull Request to review it