search

linux-nvme / nvme-cli

NVMe management command line interface.
https://nvmexpress.org
GNU General Public License v2.0
1.49k stars 660 forks source link

Allow connect to use configured TLS keys #2532

Closed igaw closed 4 weeks ago

igaw commented 1 month ago

Extend the --tls_key and --keyring argument to accept also configured keys.

Depends on https://github.com/linux-nvme/libnvme/pull/894

hreinecke commented 1 month ago

Weelll ... there was a reason why I did use the key IDs to reference the keys. If you specify the key on the commandline it will be present in the commandline args for that process for everyone to see (just do a 'cat /proc//cmdline and you can read the entire commandline argument details), with not even basic security restrictions. So it'll be really easy for even unprivileged users to get to arbitrary keys. If you want to go that route then please use a filename as commandline argument. But not the key itself.

hreinecke commented 1 month ago

Which incidentally was the reasons why I want to rework DH-CHAP key handling to use the key store.

igaw commented 1 month ago

Yep, I thought the same. Thus I am not sure if we really want this and I got here due the work on https://github.com/linux-nvme/libnvme/issues/890 where @martin-gpy ask for the JSON output generated by the 'connect' command is showing the 'configured key'. The key in the keystore is obviously not the 'configured key' thus we can't produce a 'correct' JSON configuration file.

I think we should keep the discussion in place, thus let's have it here https://github.com/linux-nvme/libnvme/issues/890

igaw commented 1 month ago

And there is also nvme gen-tls-key and nvme check-tls-key which accepts/operate with the configured key.

hreinecke commented 1 month ago

We could insert the key description in the json output. With that we can lookup the matching key upon reload. That would avoid us to have to specify the actual key data anywhere, and would neatly tie in with existing mechanisms (ie we would either query the key store for a key ID or a key description).

igaw commented 1 month ago

During some more testing of various use cases, I found some more issues.

The last patch adds a new feature which allows to export a generated key to a keyfile. This should help the use case to maintain a /etc/nvme/tls-keys files. This patch is still WIP, needs some more love.