[pve-devel] [PATCH-SERIES common/manager] Fix SSL Certificate and Key Upload Issues

[Max Carrara]

... by checking whether the uploaded or existing private key matches
the uploaded SSL certificate (see bug #4552 [0]).

The subroutine that performs the check is added to `pve-common` as it
fits to the other subroutines regarding `Net::SSLeay` there. Some
minor formatting and whitespace fixes are added along the way.

The check is then performed in `pve-manager` whenever a new pair of
custom cert and key is going to be used.

During testing, it turned out that users aren't able to upload a
certificate without a key, as the web UI sends an empty string as the
private key's value if the key's field is left blank. This is also
fixed in a separate patch.


Testing
-------

Two self-signed SSL certificates as well as their keys were generated
for testing purposes:
  * openssl req -x509 -newkey rsa:4096 -keyout foo.key \
    -out foo.pem -sha256 -days 365 -nodes
  * openssl req -x509 -newkey rsa:4096 -keyout bar.key \
    -out bar.pem -sha256 -days 365 -nodes

Then, the following tests were performed via the UI:
  1. Upload `foo.pem` without key
    => fails, as no custom key exists
  2. Upload `foo.key` and `foo.pem`
    => succeeds; the custom pair is now in use
    (browser warning, viewing cert in UI)
  3. Upload `bar.pem` without key
    => fails, as `foo.key` doesn't match `bar.pem`
  4. Upload `bar.key` and `bar.pem`
    => succeeds; the existing pair is replaced with the uploaded pair
    (browser warning, viewing cert in UI)
  5. Upload `foo.pem` without key
    => fails, as `bar.key` doesn't match `foo.pem`
  6. Delete custom certificate, reload page, upload `foo.pem` again
    => fails, as `foo.key` does not exist anymore

However, what was not tested was setting up ACME via a custom domain. 
Since ACME certs are also updated via `PVE::CertHelpers::set_cert_files`,
the check performed is the same. So, if an HTTP or DNS challenge
returns an invalid key/cert pair (for whatever reason), attempting to
use that pair should therefore also fail. 
Is there anyway this can be tested locally (if necessary)?


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


pve-common:

Max Carrara (2):
  certificate: add subroutine that checks if cert and key match
  certificate: fix formatting and whitespace

 src/PVE/Certificate.pm | 49 ++++++++++++++++++++++++++++++++----------
 1 file changed, 38 insertions(+), 11 deletions(-)


pve-manager:

Max Carrara (2):
  fix #4552: certhelpers: check if custom cert and key match on change
  ui: cert upload: fix private key field sending empty string

 PVE/CertHelpers.pm                | 65 +++++++++++++++++++++++++++----
 www/manager6/node/Certificates.js |  7 ++++
 2 files changed, 64 insertions(+), 8 deletions(-)

-- 
2.30.2