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

NPI Exposed in info log #3222

Closed
brentleeper opened this issue Mar 17, 2020 · 10 comments
Closed

NPI Exposed in info log #3222

brentleeper opened this issue Mar 17, 2020 · 10 comments
Labels
status: duplicate There is already an issue similar to this. The link to it should be present

Comments

@brentleeper
Copy link

brentleeper commented Mar 17, 2020

Affects Version(s): <Spring Integration version>
Current

In org.springframework.integration.handler.support.MessagingMethodInvokerHelper.fallbackToInvokeExpression(), the message is logged using LOGGER.info, this should be LOGGER.warn.

Also, this log message currently fully exposes the contents of the ParametersWrapper 'parameters' in the INFO logs and I believe this is undesired, as parameters is exposing its state too openly for this logging level and should be split from the above into LOGGER.debug

@artembilan
Copy link
Member

The WARN is a bit lower severity than INFO. That means when you enable INFO for logging you are going to see WARN, too. So, your concern about logging level is not clear.
Also we select a level according some logic: there is no reason to make a WARN here because we are going to do a fallback therefore your program is not going to fail.

The parameter masking is kinda not related to this issue and there are many places where we print messages with their payload and headers.
If you wish we can make this message under DEBUG, but again: is this really only a place where you worry about your data exposure?

@artembilan artembilan added the status: waiting-for-reporter Needs a feedback from the reporter label Mar 18, 2020
@brentleeper
Copy link
Author

Leaving the logging level at info for the message is agreeable. However, in my current use case, logging the payload and headers in the info log is exposing NPI in the log file. For now my work around is just to set the logger to off.

@brentleeper
Copy link
Author

NPI - Non Public Information

@artembilan
Copy link
Member

OK. Understandable.

So, we can treat this issue like:

Whenever we log a content of the message under INFO, consider to move such a possibly sensitive information under DEBUG level, leaving with an INFO just a generic note.

I don't say that we are going to do that because this is not a trivial task and we also throw exceptions in many places with the message caused an error. How to be over here then?

Wouldn't it be better to not have an INFO or DEBUG logging level for the framework and log reasonable messages using your own components ?

@brentleeper
Copy link
Author

I understand that this would be a large undertaking. But what you describe above is exactly what I am thinking and we do have logging in our own components.

For now the work around is acceptable, but if there is an issue related to the framework it may be more difficult trace.

@artembilan
Copy link
Member

Well, for that reason an issue is going to be logged under ERROR or WARN. In most case of course it is going to be an exception with the whole stack trace to track the reason or so.

@artembilan
Copy link
Member

@garyrussell ,

I would like to hear your opinion about this issue.

Thanks

@garyrussell
Copy link
Contributor

Maybe ask Spring Framework to add:

public interface MessageToStringFunction extends Function<Message<?>, String> {
}

and setToStringFunction(MessageToStringFunction function) to GenericMessage and our MessageBuilder can inject a user-supplied implementation into every message. That way the user can obfuscate/remove NPI.

??

@artembilan
Copy link
Member

@artembilan artembilan added status: waiting-for-triage The issue need to be evaluated and its future decided and removed status: waiting-for-reporter Needs a feedback from the reporter labels Mar 18, 2020
@brentleeper brentleeper changed the title INFO logging should be WARN NPI Exposed in info log Mar 19, 2020
@artembilan artembilan added status: on-hold-dependency Waiting for a fix to a another project and removed status: waiting-for-triage The issue need to be evaluated and its future decided labels Oct 6, 2022
@artembilan
Copy link
Member

Duplicate of #9416

@artembilan artembilan marked this as a duplicate of #9416 Nov 5, 2024
@artembilan artembilan closed this as not planned Won't fix, can't repro, duplicate, stale Nov 5, 2024
@artembilan artembilan added status: duplicate There is already an issue similar to this. The link to it should be present and removed status: on-hold-dependency Waiting for a fix to a another project labels Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate There is already an issue similar to this. The link to it should be present
Projects
None yet
Development

No branches or pull requests

3 participants