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

Censor write only variables logging and added a new callback to sanitize any logging that would be passed to the existing message_callback #911

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

WilcodenBesten
Copy link
Contributor

Describe your changes

  1. Added optional callback to sanitize all logging before being passed to the existing message_callback. Can be used to remove secrets from log lines.
  2. Added DeviceModel::get_mutability so the mutability of a component variable can be checked before the value change is logged. This prevents write-only varaiables to be logged

Checklist before requesting a review

  • I have performed a self-review of my code
  • [n/a] I have made corresponding changes to the documentation
  • [n/a] If OCPP 2.0.1: I have updated the OCPP 2.0.1 status document
  • I read the contribution documentation and made sure that my changes meet its requirements

…riable can be checked before the value change is logged. This prevents write-only varaiables to be logged
…o the existing message_callback. Can be used to remove secrets from log lines
@WilcodenBesten
Copy link
Contributor Author

The log line here: https://github.com/EVerest/libocpp/blob/main/lib/ocpp/common/websocket/websocket_libwebsockets.cpp#L996 can still leak secrets as it's not filtered. You could argue that it's debug so that's not needed, but better safe then sorry imo.

Would love to here you're opinion on that.

@@ -274,29 +281,37 @@ MessageLogging::~MessageLogging() {
}

void MessageLogging::charge_point(const std::string& message_type, const std::string& json_str) {
std::string copy_json_str = json_str;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if we can find a way to do this so it does not have to copy when there is no callback. These strings can be quite large (10s of KB) so we should not copy if we don't need to.

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

Successfully merging this pull request may close these issues.

2 participants