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



_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to