Enable debug_redact for Java Logging

[anilburakbilsel]

What language does this apply to?

Java and Proto version -> libprotoc 23.4

Describe the problem you are trying to solve.

There are many ways that protocol buffers might be stringified into logs and then printed into stack traces or service/application logs, etc. The built-in behavior stringifies the entire protocol buffer (protobuf) recursively, including all field data.

So for example, assuming that Person is a protobuf generated class (so it extends com.google.protobuf.GeneratedMessageV3) and there is an instance of it "person" then System.out.print(person); will print all the fields and values.

I can see a two conversations going on around redacting sensitive information from protocol buffers when they are stringified. Those discussions can be found at:

• https://groups.google.com/g/protobuf/c/uENdMwYSgmo?pli=1
• https://github.com/protocolbuffers/protobuf/issues/1160

It seems like debug_redact has been added into descriptor.proto and generate the code, I can see that the generated Java files can have access to it. However, when I try to stringfy the object, the values for debug_redactenabled fields are still being printed into the logs. I believe that happens because there is no check in toString() method for this field in Java.

Describe the solution you'd like

It seems like currently toString() method uses private printFieldValue() method while stringifyinh values - and it does not check whether the descriptor is being used or not - or whether debug_redact set to true or not. Maybe we can add a check at the beginning of that method, so that if the debug_redact is set to true then the value will not be printed. The following might be added before line 546 (in this method), where the switch statement is:

......
    private void printFieldValue(
        final FieldDescriptor field, final Object value, final TextGenerator generator)
        throws IOException {
      if (field.getOptions().hasDebugRedact() && field.getOptions().getDebugRedact()) {
        break;  // do not print anything
     }
      switch (field.getType()) {
        case INT32:
        case SINT32:
        case SFIXED32:
......
}

There are other ways to achieve this goal as well but this is a straigtforward solution I believe.

Dear @acozzette, I kindly wanted to ping you as well since you have some background information and idea about this issues. And it seems like you also recently made some changes into TextFormat.java class.

Additional context

• Seems like a possible way is already present in Go: https://github.com/bufbuild/prototransform/blame/main/filter.go

https://github.com/protocolbuffers/protobuf/blob/71a9ae22326d4a9b9fc6c4c87265c2967d4497ab/java/core/src/main/java/com/google/protobuf/TextFormat.java#L543-L607

[zhangskz]

I believe the internal proposal around debug_redact intended for toString() in Java to replace redacted fields with a placeholder value.

Let me follow up with the original proposal author to see where this is at.

[anilburakbilsel]

Hello @zhangskz , thank you. Yes, a placeholder value can be sufficient as well - I think the main purpose here is just to hide the sensitive field values when the object is converted to a string (or printed out) so that nothing sensitive will be printed within logs. In that case, as a simple solution, maybe the following can be added:

......
    private void printFieldValue(
        final FieldDescriptor field, final Object value, final TextGenerator generator)
        throws IOException {
      if (field.getOptions().hasDebugRedact() && field.getOptions().getDebugRedact()) {
        generator.print("REDACTED_VALUE");;  // print a placeholder value
     }
      switch (field.getType()) {
        case INT32:
        case SINT32:
        case SFIXED32:
......
}

Having such a change in Java will definitely make the use of debug_redact more accurate and possible.

(With that being said, it seems like in Go, the current approach is not to print out anything).

[zhangskz]

So we are definitely still planning on making this happen in Java and are tracking this proposal and implementation internally. We're still focused on C++ currently, after which we will tackle this similarly in Java.

[zhangskz]

This will likely be in ~early 2024.

[zellyn]

I assume the C++ version will take care of Ruby, etc. Are y'all planning on making the same change in Go?

(Redaction of fields marked with the squareup.redacted option is basically the main remaining reason for our internal protobuf forks, and I would love to unfork and just use upstream…)

[github-actions[bot]]

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago.

[Jakub-CZ]

Bump

[anilburakbilsel]

Hello @zhangskz, wanted to kindly bump this issue to remind. Thank you!

[gcastillo-bakkt]

Bump

[adam2k]

@zhangskz bump. 🙏

[DariusParvin]

@zhangskz bump 🙏

[zhangskz]

Followed up with internal contributor to see if this is still being worked on for early 2024.

[zhangskz]

Looks like we're still expecting this in end of Q1 or early Q2. This should likely end up in the 27.x or 28.x accordingly.

[anilburakbilsel]

Greetings team. Do we have an update on this issue? Thank you/

[github-actions[bot]]

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago. This issue will be closed and archived after 14 additional days without activity.

[anilburakbilsel]

Hello! Just wanted to kindly ping about this issue.

[DariusParvin]

@zhangskz bump 🙏