replication-manager icon indicating copy to clipboard operation
replication-manager copied to clipboard

Database credentials are leaked in API and web interface

Open PeterJanRoes opened this issue 2 months ago • 16 comments

I am currently studying the API and web interface of Replication Manager to see how I can use it to discover all the clusters that are monitored by Replication Manager, and to discover the current topology and state per cluster. I want to automatically add items to our monitoring system Zabbix for the discovered clusters and their topologies.

While looking into the API and web interface I learned that the web interface uses the API, which is great, but I also noticed that in all API endpoints that I tried, the database credentials of the root user and/or the replication user are present in the responses. I think this is a major security concern and I currently do not see why this would ever be needed. For me this would be a reason to not use the web interface or the API. I know that the API can be protected with user credentials and can run on HTTPS, but I really think the credentials should never leave the Replication Manager server.

Maybe there is a setting controlling this behavior? If not, do you agree the credentials should never be exposed like this?

PeterJanRoes avatar May 04 '24 11:05 PeterJanRoes

Hi, thank you for your information. We will put it into our enhance list. Hope we can cater this for the next release!

caffeinated92 avatar May 06 '24 00:05 caffeinated92

Great. For me the endpoint /api/{clusterName}/topology/servers (and /api/{clusterName}/topology/master) seem to be the most useful for monitoring. They both include a "dsn": "root:..." field in the response which includes the credentials for the root user. I think this field should be removed from the response.

Also, I would like to be able to use a (simple) endpoint that lists all configured clusters so I can use the topology endpoints for those. Currently I was looking into the /api/clusters endpoint. It returns fields like dbServersCredential, replicationCredential, shardproxyCredential, apiCredentials and more. It seems that this endpoint returns way too much sensitive information (also besides the credentials) for simple monitoring purposes.

Please note that I briefly inspected the attached pull request. It seems to only address the apiCredentials field and not the rest of the credentials. I am not sure whether it is still work in progress, so I just wanted to mention it just in case.

PeterJanRoes avatar May 06 '24 07:05 PeterJanRoes

Peter excellent catch this for sure must be remove from every endpoint

svaroqui avatar May 06 '24 07:05 svaroqui

Already removed more credentials from json response

ahfa92 avatar May 06 '24 08:05 ahfa92

Well, for other sensitive data, we already planned to trim unessential values. But we need to check whether the values are used or not.

ahfa92 avatar May 06 '24 08:05 ahfa92

Guys please we may break many features here, please try to encrypt all your passwords in config file with the keygen et password parameter report again if any clear credential leaking in such case

svaroqui avatar May 06 '24 08:05 svaroqui

The DSN may be an issue , a possible solution would be not to send direct memory structure but more the serialization on disk that we already take good care of not printing the decoded value

svaroqui avatar May 06 '24 08:05 svaroqui

See the code in config.go

func (conf *Config) DecryptSecretsFromConfig() {
	conf.Secrets = map[string]Secret{
		"api-credentials":                       {"", ""},
		"api-credentials-external":              {"", ""},
		"db-servers-credential":                 {"", ""},
		"monitoring-write-heartbeat-credential": {"", ""},
		"onpremise-ssh-credential":              {"", ""},
		"replication-credential":                {"", ""},
		"shardproxy-credential":                 {"", ""},
		"haproxy-password":                      {"", ""},
		"maxscale-pass":                         {"", ""},
		"myproxy-password":                      {"", ""},
		"proxysql-password":                     {"", ""},
		"proxyjanitor-password":                 {"", ""},
		"vault-secret-id":                       {"", ""},
		"opensvc-p12-secret":                    {"", ""},
		"backup-restic-aws-access-secret":       {"", ""},
		"backup-streaming-aws-access-secret":    {"", ""},
		"backup-restic-password":                {"", ""},
		"arbitration-external-secret":           {"", ""},
		"alert-pushover-user-token":             {"", ""},
		"alert-pushover-app-token":              {"", ""},
		"git-acces-token":                       {"", ""},
		"mail-smtp-password":                    {"", ""},
		"cloud18-gitlab-password":               {"", ""},
		"vault-token":                           {"", ""},
		"api-oauth-client-secret":               {"", ""}}

svaroqui avatar May 06 '24 08:05 svaroqui

reverted and will add more in documentation

caffeinated92 avatar May 06 '24 08:05 caffeinated92

for secured credentials, you have to use api-credentials-secure-config = true

caffeinated92 avatar May 06 '24 09:05 caffeinated92

Okay, thank you for your help up till now. I have to say that I can appreciate the possibility to be able to encrypt credentials but for me it would complicate matters quite a lot. I run Replication Manager in a container on Kubernetes and the database credentials are stored in Kubernetes secrets. Once they enter the container I do not care anymore whether they are encrypted or not (because the container is protected in an other way), and it would be harder to convert the Kubernetes credentials to encrypted passwords that Replication Manager uses on container startup. It just adds another unneeded layer I would say. However, it is not impossible, so I will try to add support for encryption to my container.

But overall I think it still is not a good idea to share credentials through the API, encrypted or not. I like the idea of having cleaner objects that are serialized in the API, that do not have any unneeded, possibly sensitive information.

PeterJanRoes avatar May 06 '24 11:05 PeterJanRoes

I converted the container to use encrypted credentials now. At least for the root and replication credentials the password is not in plain text anymore. I do not know if the credentials I do not use are handled correctly as well.

The only thing left is the dsn field. This still shows the plain text password and not the encrypted variant.

PeterJanRoes avatar May 06 '24 17:05 PeterJanRoes

Yes we are for sure going to fixe this in next release , it is hard to modify the network payload to be different from the disk payload, we are using the json disk serialization for saving api config change and inject parameters from our cloud solution. and the password rotation can use it for disk based json and store new password in case of rotation.

svaroqui avatar May 06 '24 18:05 svaroqui

DSN already obfuscated from JSON in 840b6564183167a8d5ea32dfd5ca749cffcaa208 and proxysql user in 3b66b3c3b20644cc97d0190cf940716bce066a2b

caffeinated92 avatar May 07 '24 07:05 caffeinated92

Hello Peter, we are currently lacking secure provisioning on kubernetes using init container, would you like to contribute in this ? Today the init container get his config from http and does not use any secret the way it is done with opensvc

svaroqui avatar May 07 '24 08:05 svaroqui