[pve-devel] [PATCH v3 manager 1/3] fix #4552: certhelpers: check if custom cert and key match on change

[Thomas Lamprecht]

Am 22/03/2023 um 16:41 schrieb Max Carrara:
> On 3/14/23 16:08, Max Carrara wrote:
>> It is now checked whether the new custom SSL certificate actually
>> matches the provided or existing custom key.
>>
>> Also, the new custom certificate and key pair is now validated
>> *before* it is used or replaced with the existing pair. Safety copies
>> are still made; if a pair is currently in use, it is therefore left
>> untouched until the new one is valid.
>>
>> Signed-off-by: Max Carrara <m.carrara at proxmox.com>
>> ---
>>  NOTE: This patch requies a version bump+upload of pve-common.
>>
> 

shortly sumarizing what we talked off-list yesterday.
 

> So, it's now easier to lock oneself out of their own PVE instance, imo.

1) not really relevant, as we always recommend an additional out-of-band
channel, relevant for *every* update, which could, even if quite unlikely,
introduce a setup specific regression that renders the API inaccessible
too, maybe even without any user interaction required. There's a reason
that servers got OOB management stacks like IPMI/iKVM/...
2) getting a cert+key now and manually uploading it is quite uncommon
nowadays, especially for inexperienced users thanks to ACME and projects
like Let's Encrypt. Note also that we documented how to manually switch
the cert since years (maybe even over a decade) and I don't have any
reports of this error happening to users, definitively not frequently
and that even before automatic ACME integration existed.

> 
> Additionally, if the host with the invalid key/cert pair is in a
> cluster, it cannot be accessed via another host in the same cluster
> either - it's displayed as online, but *no* actions in the UI can be
> performed anymore.
> 
> I'm not sure what other implications a key/cert mismatch has, but
> since it requires the user to log in via SSH, manually delete the
> mismatching key/cert pair, and then running `pvecm updatecerts -f`.

yes setting up wrong certificates, independent if done via UI or via
API/CLI, which was always possible, or even the filesystem directly
(nothing you can check there), needs to be cleaned up - nothing new
there...

> 
> Therefore I feel like this is rather important to include in PVE 7.4,
> so if there are any open questions/issues with this patch, I'd gladly
> answer/fix/update/etc. anything if necessary.
> 

Manual certificate uploads are a bit of a niche use case especially since
we have full ACME implementation and switching certs out of any HTTP
related stack, be it PVE, a nginx/apache/... or whatever else there is,
has always needed some care - and an out-of-band channel besides the
thing one is currently changing (API access here). 

> 
> [0] https://bugzilla.proxmox.com/show_bug.cgi?id=4552