splunk / splunk-tableau-wdc

Splunk Tableau Web Data Connector (WDC) Example
Apache License 2.0
20 stars 13 forks source link

Credentials are not handled securely #12

Closed hyphus closed 5 years ago

hyphus commented 5 years ago

The query parameter contains (effectively) plain text credentials. As most Splunk instances within an Enterprise use Active Directory authentication, this is a pretty big security risk.

The query parameter will show up in logs and can trivially be decoded. Additionally people may share this URL not realizing their credentials are included.

At a minimum there should be a big warning about credentials being exposed in plain text but ideally this would use a more secure authentication method. The WDC SDK has support for multiple authentication methods.

http://tableau.github.io/webdataconnector/docs/wdc_authentication

mayurah commented 5 years ago

@bcheezum This is by design, there's least you can do to have more security, i.e. hashing of password or something. Also, note that password is not cleartext, but encoded/obscured (not encrypted).

Link that you've shared states BasicAuth (which is Base64), and OAuth (would not work for all situation).

This is the the case with ODBC driver as well, if you could share your use case, maybe someone can provide better way to use this.

Some hygiene:

Note that this project is run by contributors and there may not be anything that one can do, unless say someone introduces a third-party Vault to manage secret. Let me know if you have any better suggestion, someone from community can chime in and work on it.

hyphus commented 5 years ago

In this case the fact that the credentials are encoded is actually worse as it's not clear to the end user that they are exposing their credentials. At a minimum there should be a warning about this, especially since the default username is "admin".

BasicAuth (especially over TLS) is significantly more secure than passing credentials as part of the query string. It won't show up in logs and you can't accidentally share them with someone.

mayurah commented 5 years ago

@bcheezum I will let someone from community pick on this as it would change the way solution is deployed for Web Servers, and may require some server side language to handle/parse BasicAuth requests.

Note that If someone can read GET request logs, possibly they might be able to read BasicAuth header as well, just saying! 🛡

Meanwhile I have added a note that credentials are encoded, and not encrypted in commit 02380e9.

Again, this is community supported project, feel free to make a PR!

mayurah commented 5 years ago

Token based auth via #16 shall fix this.

Marking as closed.