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

[Fabian Grünbichler]

On November 21, 2019 1:19 pm, Thomas Lamprecht wrote:
> On 10/28/19 11:39 AM, Dominik Csapak wrote:
>> but only if the ca is ours, and the cert is issued by our ca
>> (by checking the issuer and openssl verify)
>> 
>> this way we can reduce the lifetime of the certs without having
>> to worry that they ran out
>> 
>> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
>> ---
>>  PVE/CertHelpers.pm |  6 ++++++
>>  bin/pveupdate      | 33 +++++++++++++++++++++++++++++++++
>>  2 files changed, 39 insertions(+)
>> 
>> diff --git a/PVE/CertHelpers.pm b/PVE/CertHelpers.pm
>> index 52316aa0..7e088cb9 100644
>> --- a/PVE/CertHelpers.pm
>> +++ b/PVE/CertHelpers.pm
>> @@ -38,6 +38,12 @@ sub cert_path_prefix {
>>      return "/etc/pve/nodes/${node}/pveproxy-ssl";
>>  }
>>  
>> +sub default_cert_path_prefix {
>> +    my ($node) = @_;
>> +
>> +    return "/etc/pve/nodes/${node}/pve-ssl";
>> +}
>> +
>>  sub cert_lock {
>>      my ($timeout, $code, @param) = @_;
>>  
>> diff --git a/bin/pveupdate b/bin/pveupdate
>> index 5a42ce73..10b5c8f0 100755
>> --- a/bin/pveupdate
>> +++ b/bin/pveupdate
>> @@ -15,6 +15,7 @@ use PVE::Cluster;
>>  use PVE::APLInfo;
>>  use PVE::SafeSyslog;
>>  use PVE::RPCEnvironment;
>> +use PVE::Tools;
>>  use PVE::API2::Subscription;
>>  use PVE::API2::APT;
>>  use PVE::API2::ACME;
>> @@ -72,6 +73,38 @@ eval {
>>  };
>>  syslog ('err', "Renewing ACME certificate failed: $@") if $@;
>>  
>> +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
}

>> +
>> +    # 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.
> 
>> +    # 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";
>>