search

marcingminski / sqlwatch

SQL Server Performance Monitor
https://docs.sqlwatch.io
Other
435 stars 171 forks source link

encryption of central repository user #428

Open chrgraefe opened 2 years ago

chrgraefe commented 2 years ago

Is your feature request related to a problem? Please describe. We are using a RDS database instance for storing our data. Another application server should upload the collected data from the monitored instances. I tried to encrypt the password of the user which is responsible for die central repository database

Describe the solution you'd like Please document a solution to get this user password correctly encrypted.

marcingminski commented 2 years ago

I am not sure I understand your question. Are you talking about encrypting user credentials that are stored in the metatable for the remote instances? What have you tried so far? This is already documented: https://docs.sqlwatch.io/central-repository/data-import/SqlWatchImport/#credential-encryption

chrgraefe commented 2 years ago

Hi, I'm talking about the encryption of the user password of the central repository user stored in the app.config file of the importer

marcingminski commented 2 years ago

Can you help me understand the benefit? You would still need to keep the encryption key within the same config file as the encrypted password. Even if you used a machine key, anyone who has access to the application server will be able to decrypt the password. At the end of the day, the application will have to decrypt the password before it can talk to the central repository anyway.

The connection string can be encrypted via a web server so if someone hacks into the web server, they're not getting the connection strings to the database, but we're not running a webserver, just a console application within the internal network, not exposed to the internet.

Perhaps you should just limit access to the app.config file to administrators.

Having said that, this functionality can be added in the future versions if it is a justified requrement.

chrgraefe commented 2 years ago

Hi Marc,

this my configuration (extract) My missing point is, how to encrypt the key "CentralRepositorySqlSecret".

<appSettings>

<!-- Central repository connection details -->
<add key="CentralRepositorySqlInstance" value="amazon.sql-rds.server.com" />
<add key="CentralRepositorySqlDatabase" value="SqlWatchRepository" />

<!-- It is recommended to use Windows Authentication.
Even withouth an AD, you can create a new Windows Account and
SQL proxy, grant it access to the Central Repository database
and Run SQL Agent CMD step as that proxy -->
<add key="CentralRepositorySqlUser" value="SqlWatchRepository_User" />
<!-- Sql Password Encrypted with either Machine or User key-->
<add key="CentralRepositorySqlSecret" value="MySecretPassword" />

</appSettings>
marcingminski commented 2 years ago

Apologies, this must be my error as this comment should have been removed. I was looking at encrypting passwords (as it's the right to do) but then came to the above conclusion that it would provide very minimal benefit as the encryption key would be in the config file together with the encrypted string.

However, it's easily doable so happy to implement it if you really need it? If it's urgent for you, I can probably do a quick release that would encrypt central repo password for you.

chrgraefe commented 2 years ago

Hi I don't need the encryption, but decryption is applied for the CentralRepositorySqlSecret in the source code. https://github.com/marcingminski/sqlwatch/blob/7a57ce4f617772a97f28391f8d31182b15cd7171/SqlWatch.Monitor/Project.SqlWatchImport/SqlWatchInstance.cs#L43

marcingminski commented 2 years ago

This is for the remote instances. I do encrypt passwords for the remote instances and it makes perfect sense as they are stored in a table, and the encryption key in the config file so two different places. There is also a much higher risk as remote instances are production servers often storing all sorts of sensitive data. If anyone would get access to the SQLWATCH database they would not be able to get passwords out of it.

chrgraefe commented 2 years ago

Yes, I'm totally fine with it, but the same logic for en-/decryption is used for die central repository. Look here: https://github.com/marcingminski/sqlwatch/blob/7a57ce4f617772a97f28391f8d31182b15cd7171/SqlWatch.Monitor/Project.SqlWatchImport/Program.cs#L91

marcingminski commented 2 years ago

That's just getting plain text from the Config file. SqlSecret is just a name for the SqlPassword, it does not imply that the string is encrypted. (perhaps that's misleading?)

The only place where the secret decryption happens is in:

https://github.com/marcingminski/sqlwatch/blob/7a57ce4f617772a97f28391f8d31182b15cd7171/SqlWatch.Monitor/Project.SqlWatchImport/SqlWatchInstance.cs#L43

which is where we build a connection string to the remote instances. Tools.Decypt is used for decryption

chrgraefe commented 2 years ago

Yes, this right, but for the SqlWatchRepository the code is using the same logic of class "SqlWatchInstance" for repository and remote instances.

https://github.com/marcingminski/sqlwatch/blob/7a57ce4f617772a97f28391f8d31182b15cd7171/SqlWatch.Monitor/Project.SqlWatchImport/Program.cs#L86