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

Channel state contains un-obfuscated user data #4842

Closed
lukebakken opened this issue May 18, 2022 · 6 comments
Closed

Channel state contains un-obfuscated user data #4842

lukebakken opened this issue May 18, 2022 · 6 comments
Milestone

Comments

@lukebakken
Copy link
Collaborator

Related to #3803

See https://groups.google.com/g/rabbitmq-users/c/Toq7BRq2Npk

The mailing list user reports a case where an LDAP password is logged as part of mfargs to rabbit_channel:start_link.

It appears that the impl field of the #auth_user record could be obfuscated to prevent scenarios like this.

In addition, the user information is stored in the channel state which could be logged at some point -

https://github.com/rabbitmq/rabbitmq-server/blob/master/deps/rabbit/src/rabbit_channel.erl#L531

@SimonUnge
Copy link
Member

Hi. I have started looking into this issue.

I hope a simple closure(fun wrapper) might suffice?

@michaelklishin
Copy link
Member

Maybe. We use a separate library in several other places such as Federation links and Shovels.

@SimonUnge
Copy link
Member

Ah, ok yes that might be even better then! Thanks @michaelklishin

@SimonUnge
Copy link
Member

Since 'impl' seem to be of type any(), and the rabbit_authz_backend requirement is only that it is returned, my assumption is that I update all modules that implement the modules (and uses the impl value), and obfuscate/de-obfuscate the field.

Since it can be of type any(), it is up to the module to decide how it wants the field obfuscated, but for simplicity I could
A) use the creddentials_obfuscation lib on the term, regardless of its type (i.e credentials_obfuscation:encrypt(term_to_binary(Imp)) and wise versa, or
B) wrap the value in a fun.

Are those options viable?

@michaelklishin
Copy link
Member

They are both viable but we only have real world experience with option A. Which has its own issues: the library has to be started before first client connects and it has to be seeded correctly during cluster operations, in particular addition of new nodes.

@SimonUnge
Copy link
Member

I will start with using a wrapper then, as there will be no issues in terms of apps started in the right order etc. But can easily switch if there is a feeling it is not safe enough?

Wrapper is usually what I've gone with in the past when we want to avoid secrets in stack traces. Also, recommended by EEF Security WG - https://erlef.github.io/security-wg/secure_coding_and_deployment_hardening/sensitive_data.html

SimonUnge added a commit to SimonUnge/rabbitmq-server that referenced this issue Nov 9, 2022
SimonUnge added a commit to SimonUnge/rabbitmq-server that referenced this issue Nov 9, 2022
@michaelklishin michaelklishin added this to the 3.11.4 milestone Nov 10, 2022
SimonUnge added a commit to SimonUnge/rabbitmq-server that referenced this issue Nov 10, 2022
michaelklishin added a commit that referenced this issue Nov 13, 2022
#4842: obfuscate a sensitive field in authenticated user state
mergify bot pushed a commit that referenced this issue Nov 13, 2022
(cherry picked from commit 09d84e6)
mergify bot pushed a commit that referenced this issue Nov 13, 2022
(cherry picked from commit c267f4d)
michaelklishin added a commit that referenced this issue Nov 13, 2022
#4842: obfuscate a sensitive field in authenticated user state (backport #6397)
mergify bot pushed a commit that referenced this issue Nov 13, 2022
(cherry picked from commit 09d84e6)
(cherry picked from commit 724b9b2)
mergify bot pushed a commit that referenced this issue Nov 13, 2022
(cherry picked from commit c267f4d)
(cherry picked from commit 06131e2)
michaelklishin added a commit that referenced this issue Nov 13, 2022
#4842: obfuscate a sensitive field in authenticated user state (backport #6397) (backport #6415)
mergify bot pushed a commit that referenced this issue Nov 13, 2022
(cherry picked from commit 09d84e6)
(cherry picked from commit 724b9b2)
(cherry picked from commit 249fdef)
mergify bot pushed a commit that referenced this issue Nov 13, 2022
(cherry picked from commit c267f4d)
(cherry picked from commit 06131e2)
(cherry picked from commit 179c884)
michaelklishin added a commit that referenced this issue Nov 13, 2022
#4842: obfuscate a sensitive field in authenticated user state (backport #6397) (backport #6415) (backport #6416)
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

No branches or pull requests

3 participants