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

Password Leak through System/Inputs #5408

Closed
mbirch-ca opened this Issue Dec 14, 2018 · 4 comments

Comments

Projects
None yet
6 participants
@mbirch-ca
Copy link

mbirch-ca commented Dec 14, 2018

When loading the System Inputs, the returned data includes passwords in plain text making them visible to any user with access to the system.

Expected Behavior

It should not be possible for any user (reader or admin) to see any password.

Current Behavior

When loading the web interface, the browser network tools can see passwords in plain text. While the browser displays '********', the JSON includes the password

The "inputs" screen is a default permission for a Reader making these passwords visible to any user.

Possible Solution

A similar issue was reported and resolved relating to LDAP credentials being visible in #3763

To protect passwords, it probably makes more sense not to the send any passwords when viewing or modifying settings. Keeping this stored on the server would ensure its secure and only update when an admin makes a change.

Steps to Reproduce (for bugs)

  1. From your web browser, enable the network monitor
  2. Open the System > Inputs page
  3. From the browser network monitor, search for "inputs"
  4. Display the response and look in the JSON object for the passwords

Context

Information leakage of passwords would be seen as a security concern depending upon what it can access. (Many standards require protection of log data.)

Depending upon the type of input, the security exposure can be increased if a message bus (like Kafka) is used and contains other data. Given that the screen also provides information about the TLS certificate store, there is additional information exposure if the certificate store could be retrieved. (Which is easier based on having the location and password.)

Your Environment

  • Graylog Version: 2.4.3
  • Operating System: Windows 7/10
  • Browser version: Firefox (latest), Chrome (latest)

@dennisoelkers dennisoelkers added this to the 3.0.0 milestone Dec 14, 2018

@dennisoelkers

This comment has been minimized.

Copy link
Member

dennisoelkers commented Dec 14, 2018

Thanks for reporting this, @mbirch-ca! We will address this asap.

@jalogisch jalogisch added the triaged label Dec 17, 2018

@mpfz0r mpfz0r self-assigned this Dec 18, 2018

dennisoelkers added a commit that referenced this issue Dec 20, 2018

Mask password fields of inputs returned by the REST API.
Before this change, input details returned by the REST API would contain
all configuration fields without any modification. This implies that
password fields are also contained using their original value, showing
configured password for inputs in clear text.

This change now iterates over configuration fields checking for the
presence of password fields and replace their content with `<password
set>` instead of the original value if they are not empty.

Fixes #5408.

@mpfz0r mpfz0r assigned dennisoelkers and unassigned mpfz0r Dec 20, 2018

dennisoelkers added a commit that referenced this issue Dec 21, 2018

Mask password fields of inputs returned by the REST API.
Before this change, input details returned by the REST API would contain
all configuration fields without any modification. This implies that
password fields are also contained using their original value, showing
configured password for inputs in clear text.

This change now iterates over configuration fields checking for the
presence of password fields and replace their content with `<password
set>` instead of the original value if they are not empty.

Fixes #5408.

@bernd bernd added L M and removed L labels Jan 3, 2019

dennisoelkers added a commit that referenced this issue Jan 7, 2019

Mask password fields of inputs returned by the REST API.
Before this change, input details returned by the REST API would contain
all configuration fields without any modification. This implies that
password fields are also contained using their original value, showing
configured password for inputs in clear text.

This change now iterates over configuration fields checking for the
presence of password fields and replace their content with `<password
set>` instead of the original value if they are not empty.

Fixes #5408.

dennisoelkers added a commit that referenced this issue Jan 7, 2019

Mask password fields of inputs returned by the REST API.
Before this change, input details returned by the REST API would contain
all configuration fields without any modification. This implies that
password fields are also contained using their original value, showing
configured password for inputs in clear text.

This change now iterates over configuration fields checking for the
presence of password fields and replace their content with `<password
set>` instead of the original value if they are not empty.

Fixes #5408.

@mpfz0r mpfz0r closed this in a562a33 Jan 8, 2019

@mpfz0r mpfz0r referenced this issue Jan 8, 2019

Closed

Mask password fields of inputs #5464

4 of 9 tasks complete

mpfz0r added a commit that referenced this issue Jan 8, 2019

Mask password fields of inputs returned by the REST API. (#5432)
* Mask password fields of inputs returned by the REST API.

Before this change, input details returned by the REST API would contain
all configuration fields without any modification. This implies that
password fields are also contained using their original value, showing
configured password for inputs in clear text.

This change now iterates over configuration fields checking for the
presence of password fields and replace their content with `<password
set>` instead of the original value if they are not empty.

Fixes #5408.

* Adding test for actual resource method, including license headers.

* Adding test for complete input list retrievel.

* Adding guard clause for null parameters.

* Using locales for toLowerCase.

* Handling null values in map.

* Do not mask passwords in input config for users with edit permission.

If a user contains the required permission to edit an input, passwords
in the input's config are not masked. This is prevented so the input
edit dialog still functions in the same way as before.

* Adding/adapting tests.

(Cherry-Pick from a562a33)

kmerz added a commit that referenced this issue Jan 8, 2019

Mask password fields of inputs 3.0 (#5467)
* Mask password fields of inputs returned by the REST API. (#5432)

* Mask password fields of inputs returned by the REST API.

Before this change, input details returned by the REST API would contain
all configuration fields without any modification. This implies that
password fields are also contained using their original value, showing
configured password for inputs in clear text.

This change now iterates over configuration fields checking for the
presence of password fields and replace their content with `<password
set>` instead of the original value if they are not empty.

Fixes #5408.

* Adding test for actual resource method, including license headers.

* Adding test for complete input list retrievel.

* Adding guard clause for null parameters.

* Using locales for toLowerCase.

* Handling null values in map.

* Do not mask passwords in input config for users with edit permission.

If a user contains the required permission to edit an input, passwords
in the input's config are not masked. This is prevented so the input
edit dialog still functions in the same way as before.

* Adding/adapting tests.

(Cherry-Pick from a562a33)

* Fixing import.
@mbirch-ca

This comment has been minimized.

Copy link
Author

mbirch-ca commented Jan 14, 2019

@kmerz , do you know if this will be resolved in the next release? (Maybe in the 2.5 branch if 3.0 is not going to be released?)

Was looking for a method to mitigate this risk but found I was blocked given the potential impact of issue #5420 as the default permission of Reader provides access to System/Inputs.

@edmundoa

This comment has been minimized.

Copy link
Member

edmundoa commented Jan 23, 2019

@mbirch-ca We plan to release 2.5.2 including a fix for this issue in the next days, so please stay tuned!

@edmundoa

This comment has been minimized.

Copy link
Member

edmundoa commented Jan 23, 2019

Refs #5550

bernd added a commit that referenced this issue Feb 28, 2019

Mask password fields of inputs returned by the REST API. (#5432)
* Mask password fields of inputs returned by the REST API.

Before this change, input details returned by the REST API would contain
all configuration fields without any modification. This implies that
password fields are also contained using their original value, showing
configured password for inputs in clear text.

This change now iterates over configuration fields checking for the
presence of password fields and replace their content with `<password
set>` instead of the original value if they are not empty.

Fixes #5408.

* Adding test for actual resource method, including license headers.

* Adding test for complete input list retrievel.

* Adding guard clause for null parameters.

* Using locales for toLowerCase.

* Handling null values in map.

* Do not mask passwords in input config for users with edit permission.

If a user contains the required permission to edit an input, passwords
in the input's config are not masked. This is prevented so the input
edit dialog still functions in the same way as before.

* Adding/adapting tests.

(cherry picked from commit a562a33)

bernd added a commit that referenced this issue Feb 28, 2019

Mask password fields of inputs returned by the REST API. (#5432)
* Mask password fields of inputs returned by the REST API.

Before this change, input details returned by the REST API would contain
all configuration fields without any modification. This implies that
password fields are also contained using their original value, showing
configured password for inputs in clear text.

This change now iterates over configuration fields checking for the
presence of password fields and replace their content with `<password
set>` instead of the original value if they are not empty.

Fixes #5408.

* Adding test for actual resource method, including license headers.

* Adding test for complete input list retrievel.

* Adding guard clause for null parameters.

* Using locales for toLowerCase.

* Handling null values in map.

* Do not mask passwords in input config for users with edit permission.

If a user contains the required permission to edit an input, passwords
in the input's config are not masked. This is prevented so the input
edit dialog still functions in the same way as before.

* Adding/adapting tests.

(cherry picked from commit a562a33)

edmundoa added a commit that referenced this issue Feb 28, 2019

Mask password fields of inputs returned by the REST API. (#5432) (#5734)
* Mask password fields of inputs returned by the REST API. (#5432)

* Mask password fields of inputs returned by the REST API.

Before this change, input details returned by the REST API would contain
all configuration fields without any modification. This implies that
password fields are also contained using their original value, showing
configured password for inputs in clear text.

This change now iterates over configuration fields checking for the
presence of password fields and replace their content with `<password
set>` instead of the original value if they are not empty.

Fixes #5408.

* Adding test for actual resource method, including license headers.

* Adding test for complete input list retrievel.

* Adding guard clause for null parameters.

* Using locales for toLowerCase.

* Handling null values in map.

* Do not mask passwords in input config for users with edit permission.

If a user contains the required permission to edit an input, passwords
in the input's config are not masked. This is prevented so the input
edit dialog still functions in the same way as before.

* Adding/adapting tests.

(cherry picked from commit a562a33)

* Fixing import. (#5465)


(cherry picked from commit a9a1df0)

edmundoa added a commit that referenced this issue Feb 28, 2019

Mask password fields of inputs returned by the REST API. (#5432) (#5733)
* Mask password fields of inputs returned by the REST API. (#5432)

* Mask password fields of inputs returned by the REST API.

Before this change, input details returned by the REST API would contain
all configuration fields without any modification. This implies that
password fields are also contained using their original value, showing
configured password for inputs in clear text.

This change now iterates over configuration fields checking for the
presence of password fields and replace their content with `<password
set>` instead of the original value if they are not empty.

Fixes #5408.

* Adding test for actual resource method, including license headers.

* Adding test for complete input list retrievel.

* Adding guard clause for null parameters.

* Using locales for toLowerCase.

* Handling null values in map.

* Do not mask passwords in input config for users with edit permission.

If a user contains the required permission to edit an input, passwords
in the input's config are not masked. This is prevented so the input
edit dialog still functions in the same way as before.

* Adding/adapting tests.

(cherry picked from commit a562a33)

* Fixing import. (#5465)


(cherry picked from commit a9a1df0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.