peekjef72 / veeam_exporter

Prometheus exporter for Veeam Entreprise Manager
GNU General Public License v3.0
7 stars 2 forks source link

Secure Credentials? #11

Open Levente-B opened 1 year ago

Levente-B commented 1 year ago

Greetings Good Sir :)

first of all thank you for this great tool! I was wondering if you've considered implementing some form of encryption solution instead of storing the credentials in a plain text config?

Or is such an option already available?

Cheers, Levente

peekjef72 commented 1 year ago

Hi,

Thanks!

Well, currently there is no plan to implement such a thing. I've no idea what a nice solution should be. If you have don't hesitate to share with me.

I've already thought to one solution, but I don't know if it is stupid or not:

I thing this solution has a minimal security: password is not in clear text, but since the traffic between prometheus and exporter is in clear text too, the key can be intercepted very easily. it is something like http "basic-authentication"... so encrypting is quite useless! Maybe in conjonction with a certificate for the http server of the exporter...

Levente-B commented 1 year ago

Hello :)

I didn't formulate the question clearly enough, sorry for that. Communication between the Exporter and Prometheus hosts is not really the issue here as they are trusted 99% of the time (there are cases in a corporate environment, when the two are in different hands, but that's not that common and there are usually other safeguards in place). Of course securing protocols mitigate any obvious vulnerability. There are also tools for securing the sensitive info on the monitoring side.

What I was referring to is the fact that the config uses the PW in clear text to access the Enterprise Manager API and there is no cypher mechanism implemented in the code itself. When I tried the dry run, I noticed that the exporter also prints the credentials out when returning the metrics too. And the user needs portal admin rights to access agent data too (other metrics run fine with read-only). This means if either the Exporter or the Monitoring side gets compromised, a bad actor could trigger restore jobs en-masse with the credentials.

It's not an issue in a home setup of course, but in the scope of enterprise environment would mean catastrophe.

A sensible solution would be putting an encrypted key in the config instead of the raw PW and implement a function for the tool where one can define a keystore with which it can then decrypt and connect to the Enterprise Manager.

I really like this tool (it's 100% in-line with the functionality we need) so I very much intend to use it on our productive system.

I will try to come up with a change which I would gladly share with you! I'm not that adept at py though (started learning not long ago), so it'll probably take some time.

peekjef72 commented 1 year ago

Hi,

I noticed that the exporter also prints the credentials out when returning the metrics too.

you are right, but only in debug logging.

A sensible solution would be putting an encrypted key in the config instead of the raw PW and implement a function for the tool where one can define a keystore with which it can then decrypt and connect to the Enterprise Manager.

Well, it seems to me that the same thing but with one step more complex... if password to connect to the keystore is in config, reader of the file can connect to keystore and retrieve the cnx elements too. But... ;)

Greettings

peekjef72 commented 1 year ago

Hi,

I had a busy day ! :-)

I've build a new branch : crypted_password for future 1.1.2 Read the README (bottom of the page) for configuration. (below an extract of new README)


Password encryption

There is no miracle solution since no user interaction is possible, including the launching time of the exporter. This exporter now includes a basic method to crypt/decrypt password based a symetric 128 bits key (16 bytes) shared between prometheus and the exporter.

How it works:

The exporter config file now doesn't expose the password in clear text, and it can authenticate to the Veeam Server when it requires by decrypting the password with the authkey send by Prometheus.

The authkey is not kept by the exporter: it is received, used if login is required, then dropt. Same for the decrypted password: it is used to build the basic authorisation header then zeroed (but may remained in memory because of python immutable string...)

Most of the time, the exporter will use the authentication token received after a successfull login unless you set keep_session to false for the target. So decryption has a litle impact on performance.


Maybe can you have a look and tell me if it should do the job. Don't hesitate if something seems unclear. Depending on your feedback I would publish a new release.

Levente-B commented 1 year ago

Hello :) apologies for the late response, things are complicated here atm. It looks great, I'll update and try fiddling with it. I'll probably be able to give a feedback by the end of the week if that's okay.

Levente-B commented 1 year ago

Hi Jean-Francois,

so I tested the server side functions (without Prometheus), did some debugging with a colleague of mine and here are the findings.

1. for a dryrun on the Veeam Exporter host, the method in core_exporter.py doesn't get the authkey.

With the following changes in core_exporter.py, it works:

line 80: def login(self, auth_key): line 85: if self.ns_session_login(auth_key) == self.SUCCESS: line 97: def ns_session_login(self, auth_key): line 102: login = self.api.login(auth_key) line 141: if self.login(local_vars['authkey']):

_I am not sure if this change would affect the process of the authkey coming from Prometheus though....

2. I noticed that the encrypt_passwd script returns a different base64 string every run, with same encryption key. Is that intended? (dry run was successful with the key)

3. in the config: instead of

    auth:
  # should be basic or encrypted
  type: encrypted

should be

    authmode:
  # should be basic or encrypted
  type: encrypted

With these changes, I was able to successfully connect to get the metrics from Enterprise Manager.

Also, an little additional note is that for Python script usage, the commands change: python3 ./veeam_exporter_cmd & python3 ./veeam_exporter_cmd -n -v -b /[veeam_exporter_folder] --config_file /[config folder]/config.yml etc.

Perhaps it would be good to distinguish in documentation to avoid confusion.

I hope this helps, please have a look and let me know if there is anything else I should pay attention to. Next step will be too hook Prometheus up. I will update accordingly. :)

Wish you a great evening!

Cheers, Levente

peekjef72 commented 1 year ago

Hi Levente,

Thanks for all you comments and tests. for 1): argh... it is not the valid files that I've published... sorry. I'm not connected to internet (and github) at office for security reasons, so I have to juggle between several working directory trees. I will double check next time. but effectively the core_exporter.py in the repos is not the file I've on the "test" host.. Sorry for that.

for 2) that is perfectly normal: a part of the cipher are random bytes. it is the "normal" behavior and due to the aes implementation. for 3) you are right: I will update the README. the conf/config.yml has the valid param but the doc is wrong. I will add the part to start from cmd line.

I will update the branch this week-end. Many thanks.

JFPIK-

peekjef72 commented 1 year ago

branch has been updated. If you can have a look and reports me if you notice something wrong. Else I will publish a new release next week.

JFPIK-

Levente-B commented 1 year ago

Hi Jean-Francois :)

is there any additional difference in code between the previous and this one aside from the ones mentioned above (in core exporter and the config)? If there is not, I don't think reinstall would be necessary, as it works confirmed (we still need to test the Prometheus side, but that's out of my hands unfortunately).

BR, Levente

peekjef72 commented 1 year ago

Hi, No evolution at all, that you have already fixed.

Levente-B commented 1 year ago

Hi Jean-Francois,

apologies for the late feedback, it took a while to make the Prometheus side up and running as it belongs to a separate team. We managed to do it and works great!

For documentational record, one additional thing we had to do was make a daemon for the exporter. The steps:

[Unit] Description=Veeam Exporter Service After=multi-user.target

[Service] Type=simple Restart=always ExecStart=/usr/bin/python3 /[installation_directory]/veeam_exporter-crypted_passwd/veeam_exporter_cmd -b /[installation_directory]/veeam_exporter-crypted_passwd/veeam_exporter/ --config_file /[installation_directory]/veeam_exporter-crypted_passwd/veeam_exporter/conf/config.yml

[Install] WantedBy=multi-user.target

Check if daemon is running and communicates:

This being necessary might have been due to our monitoring configuration being company specific. Home setups might not need it (I don't know).

Hope this helps! Again, thank you for this great tool and taking the time to cater to my request about implementing the encryption and all! Wish you a great week forward!

BR, Levente

peekjef72 commented 1 year ago

Hi,

That sure, you need a "unit" service to manage the epxorter. currently we manage (at my company) this part in an global ansible exporter installation role, that deploy the exporter and add the service. That why i didn't include the "service part" in sources, but you are right that not everybody knows how to set it... Great to know that it works fine. I hope to release a fully new version of the exporter ported to GO language. I will notice you when it is ready to test; the goal is to have a binary that is considerably more resistant to evolution of os, python, python's modules ...

Levente-B commented 1 year ago

Hi again Jean-Francois,

so this week something weird happened. Connection between Veeam- and Exporter hosts is gone. Dry run prompts key and runs properly after input. Live run however does not ask for key, just returns "veeam_exporter[1587]: level=ERROR - Login Session Failed on Veeam Host: Incorrect AES key length (60 bytes)"

Any idea what could have caused it? Nothing was changed, I didn't even touch the hosts before the error. Auth key is currently 128bit.

BR, Levente

peekjef72 commented 1 year ago

Hi again Jean-Francois,

so this week something weird happened. Connection between Veeam- and Exporter hosts is gone. Dry run prompts key and runs properly after input. Live run however does not ask for key, just returns "veeam_exporter[1587]: level=ERROR - Login Session Failed on Veeam Host: Incorrect AES key length (60 bytes)"

Any idea what could have caused it? Nothing was changed, I didn't even touch the hosts before the error. Auth key is currently 128bit.

BR, Levente

Hi, sorry for the problem. Do you have any chance to restart the exporter with debug level to see precisely the request sent by prometheus ? normally the auth_key sent in the query can be used to decrypt with tool "passwd_crypt" with -d option. maybe can you check if it is ok or not.

The code is quite simple: login function in veeam_api.py (line 243): it tries to decrypt with auth_key received from prometheus and password read from file but previously base64decoded (just after reading the config file); the decrypt function itself is in the same file (l. 85): it builds a cipher then try to decrypt the ciphertext. the exception raise at line 101: meaning that it is the module cipher.verify that do exception; in general because decrypted plaintext is invalid.

JFPIK-

Levente-B commented 1 year ago

Hi :) so we managed to confirm the issue doesn't stem from the code! The discrepancy was caused by Prometheus constantly querying the service with a wrong auth key. After disabling the port, no errors during normal run. Good to know nothing needs to be debugged, but goes to show how having separate departments responsible for different aspects of a topic can be a hassle... Wish you a great weekend!

Levente

wally007 commented 9 months ago

@peekjef72 , why don't you simply support environment variables in the exporter ?

Then its up to the deployment mechanism to deliver secrets to the container (ansilbe, podman, kubernetes ) This way, you could store your container in the container registry without storing credentials (encrypted or not)

peekjef72 commented 8 months ago

@peekjef72 , why don't you simply support environment variables in the exporter ?

Then its up to the deployment mechanism to deliver secrets to the container (ansilbe, podman, kubernetes ) This way, you could store your container in the container registry without storing credentials (encrypted or not)

Hi wally007,

There are no fundamental reasons at all! But, the exporter configuration model uses files to fully express the targets including user/password, so there were no needs to add some extra params extracted from env for user /passwd.

This python version of the exporter is now not maintained and is replaced by a GO version accessible to httpapi_exporter. The new configuration's model allows to defined authentication model (user/passwd/attrs) identified by name; this model maybe should use user/passwd values set by env. Feel free to create an "new feature" issue into that project.

wally007 commented 8 months ago

Would be great to make a note that his repository is depreciated or simply archive the repo to read-only. Thanks for the effort and sharing.