Skip to content

Comments

[AMQ-9515] Redact JMSText in the HTTP params#1251

Closed
kenliao94 wants to merge 1 commit intoapache:mainfrom
kenliao94:redact_message_body
Closed

[AMQ-9515] Redact JMSText in the HTTP params#1251
kenliao94 wants to merge 1 commit intoapache:mainfrom
kenliao94:redact_message_body

Conversation

@kenliao94
Copy link
Contributor

As pointed out in https://issues.apache.org/jira/browse/AMQ-9515 message body is sensitive data and sending a text message using the web console shouldn't log the message content in the audit log.

@jbonofre jbonofre self-requested a review July 11, 2024 05:13
@kenliao94 kenliao94 force-pushed the redact_message_body branch from afcfb89 to 3d97ac5 Compare July 11, 2024 06:54
@mattrpav
Copy link
Contributor

I'm mostly -1 on this change.

I think this should be handled by configuring a filter at the logging framework level.

Some users may want body redacted, others certain header or properties fields.. or even just specific fields within the body.

Logging frameworks support configurable filters for this, I think this can be handled as a FAQ in ActiveMQ about how to add a logging filter.

@mattrpav mattrpav self-requested a review July 11, 2024 14:46
@kenliao94
Copy link
Contributor Author

I'm mostly -1 on this change.

I think this should be handled by configuring a filter at the logging framework level.

Some users may want body redacted, others certain header or properties fields.. or even just specific fields within the body.

Logging frameworks support configurable filters for this, I think this can be handled as a FAQ in ActiveMQ about how to add a logging filter.

Hi Matt, thanks for the feedback.

That's a good idea. Let me look into it today. Instead of making this change, we will ship a default configuration for audit log to mask that information and add a FAQ in ActiveMQ to instruct users on how to add their own or override the default. Does it sound good to you?

@mattrpav
Copy link
Contributor

we will ship a default configuration for audit log ..

If by 'we' you mean your organization, good to go =)

If by 'we' you mean Apache, then I don't see us changing the default Apache distribution config for this.

@kenliao94
Copy link
Contributor Author

we will ship a default configuration for audit log ..

If by 'we' you mean your organization, good to go =)

If by 'we' you mean Apache, then I don't see us changing the default Apache distribution config for this.

I can see your concern. By "we" I meant developers in this community :) When developers deploy ActiveMQ for their customers, their customers may want sensitive data to be redacted. For instance, in AuditLogEntry.java fields that are annotated by @Sensitive (such as password) are redacted in the audit.log. So in my opinion, it is probably better to have it default redacted (possibly other fields in the HTTP parameters as well) to avoid potential compliance issues. I will focus on documenting the instructions to configure the log filter nevertheless.

@kenliao94 kenliao94 closed this Aug 16, 2024
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