[pve-devel] [PATCH manager 1/1] renew pve-ssl.pem when it nearly expires

[Dominik Csapak]

>>>   
>>> +eval {
>>> +    # get CA and check issuer
>>> +    my $capath = "/etc/pve/pve-root-ca.pem";
>>> +    my $cainfo = PVE::Certificate::get_certificate_info($capath);
>>> +    if ($cainfo->{subject} !~ m|/CN=Proxmox Virtual Environment/.*/O=PVE Cluster Manager CA|) {
>>> +	die "Root CA is not issued by Proxmox VE";
>>> +    }
>>
>> Hmm, a bit scaremongering for people having their own CA here, not?
>> This get's in the log _daily_. It could be best to first check expiry,
>> so that we do not long 2years (or whatever the users had set as expiry
>> for their custom cert for nothing)..
>>
>> Then, maybe a syslog with info/notice level and a hint that the CA won't
>> get touched as it's not a PVE issued one would be better.
>> If it's about to expire this could also made a warning.
> 
> we could downgrade this (although putting your own CA there is something we
> haven't supported for quite some time). switching the order of checks
> around makes sense for sure:
> 
> if (expires_soon) {
>    check_ca
>    check_cert_issued_by_ca
>    check_cert_signed_by_ca
>    renew
> }
> 

i would argue that both checks are orthogonal to each other

my proposal would be:

expiry/renewal check (for certificates):
if (expires_soon)
  check cert issued by ca
  check cert signed by ca
  renew (without regard of the issuer of the ca, if it fails, it fails)

and another path where we check (and only log, not die):
ca expiry (e.g. 6 months before)
is the ca issued by proxmox ve (everything else is not really supported)
is the cert issued by the ca

if something is wrongly configured, it makes sense to report
it, as this can be the reason for e.g. failing spice connections

>>> +
>>> +    # get cert and check issuer and chain
>>> +    my $certpath = PVE::CertHelpers::default_cert_path_prefix($nodename).".pem";
>>> +    my $certinfo = PVE::Certificate::get_certificate_info($certpath);
>>> +    if ($certinfo->{issuer} ne $cainfo->{subject}) {
>>> +	die "SSL Certificate is not issued by Proxmox VE root CA";
>>
>> same here
>>
>>> +    }
>>> +
>>> +    # check if signed by our ca
>>
>> what is this comment above?
> 
> the chain from issuing CA certificate to signed leaf certificate is two-fold:
> - leaf certificate encodes (hash of) issuer's subject (this allows fast
>    path-resolution without crypto)
> - leaf certificate is signed with CA key (to ensure the path we found is
>    actually authenticated)
> 
> this now checks the latter, the check above checks the former.

i'll make the comments more expressive

>>
>>> +    # TODO
>>> +    # replace by low level interface in ssleay if version 1.86 is available
>>> +    PVE::Tools::run_command(['/usr/bin/openssl', 'verify', '-CAfile', $capath, $certpath]);
>>> +
>>> +    # check if expiry is < 2W
>>> +    if (PVE::Certificate::check_expiry($certpath, time() + 14*24*60*60)) {
>>> +	# create new certificate
>>> +	my $ip = PVE::Cluster::remote_node_ip($nodename);
>>> +	PVE::Cluster::gen_pve_ssl_cert(1, $nodename, $ip);
>>> +	print "Restarting pveproxy\n";
>>> +	PVE::Tools::run_command(['systemctl', 'reload-or-restart', 'pveproxy']);
>>> +    }
>>> +};
>>> +syslog ('err', "Checking/Renewing SSL certificate failed: $@") if $@;
>>> +
>>>   sub cleanup_tasks {
>>>   
>>>       my $taskdir = "/var/log/pve/tasks";
>>>