On Fri, Jan 23, 2026 at 07:53:00PM +0100, Stoiko Ivanov wrote:
> Thanks for tackling this and looking into certificate generation!
> 
> two suggestions/comments inline:
> 
> On Thu, 22 Jan 2026 11:55:16 +0100
> Arthur Bied-Charreton <[email protected]> wrote:
> 
> > Add the keyUsage[1] extension to the PVE root CA to comply with RFC
> > 5280. Python started to enforce this as of 3.13 by defaulting to using the 
> > VERIFY_X509_STRICT flag, which breaks clients like Ansible.
> > 
> > The authorityKeyIdentifier[2] and subjectKeyIdentifier[3] extensions are
> > not strictly required for fixing this issue, however RFC 5280 mandates
> > them for conforming CAs, so adding them makes sense as well.
> > 
> > [1] https://www.rfc-editor.org/rfc/rfc5280#section-4.2.1.3
> > [2] https://www.rfc-editor.org/rfc/rfc5280#section-4.2.1.1
> > [3] https://www.rfc-editor.org/rfc/rfc5280#section-4.2.1.2
> > 
> > Signed-off-by: Arthur Bied-Charreton <[email protected]>
> > ---
> > This fix is not required for PBS and PDM, since they only use self-signed
> > certificates.
> mentioning this here provides helpful information - thanks!
> (PMG also does not create a CA - so should not be affected either (without
> checking explicitly))
> 
> > 
> > You can run the script below to test the changes, should fail with
> > "CA cert does not include key usage extension" before applying the patch,
> > and succeed afterwards. 
> > 
> > ```
> > #!/usr/bin/env python3
> > 
> > import socket
> > import ssl
> > import sys
> > 
> > try:
> >     context = ssl.create_default_context(cafile="/etc/pve/pve-root-ca.pem")
> >     context.check_hostname = True
> >     context.verify_mode = ssl.CERT_REQUIRED
> > 
> >     with socket.create_connection(("localhost", 8006), timeout=10) as sock:
> >         with context.wrap_socket(sock, server_hostname="localhost") as 
> > ssock:
> >             print(f"success")
> > 
> > except ssl.SSLCertVerificationError as e:
> >     print(e)
> > ```
> thanks for the short script - helped a lot in seeing what's needed, and
> verifying the results.
> I'd argue that this could even be part of the commit message (more likely
> to be found again in the future. (best-case would if there's something
> shorter to achieve the same result).
> 
> >     
> >  src/PVE/Cluster/Setup.pm | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> > 
> > diff --git a/src/PVE/Cluster/Setup.pm b/src/PVE/Cluster/Setup.pm
> > index 75d3507..d95a278 100644
> > --- a/src/PVE/Cluster/Setup.pm
> > +++ b/src/PVE/Cluster/Setup.pm
> > @@ -426,6 +426,25 @@ sub gen_pveca_cert {
> >      my $uuid_str;
> >      UUID::unparse($uuid, $uuid_str);
> >  
> > +    my $sslconf = <<__EOD;
> > +[req]
> > +distinguished_name = req_distinguished_name
> > +x509_extensions = v3_ca
> > +
> > +[ req_distinguished_name ]
> > +
> > +[ v3_ca ]
> > +basicConstraints = critical,CA:TRUE
> > +keyUsage = critical,keyCertSign,cRLSign
> > +authorityKeyIdentifier = keyid:always,issuer
> > +subjectKeyIdentifier = hash
> 
> newer versions of openssl finally have a way of adding extensions without
> the need to edit the openssl config, or providing overrides in temporary
> files (or via bash `<(printf...` redirects.
> (noticed this a while ago when again searching how to add a SAN to a
> self-signed certificate.
> 
> Based on /etc/ssl/openssl.cnf on a PVE9 node replacing the config file, by
> `-addext 'keyUsage=critical,keyCertSign,cRLSign'` on the openssl
> command-line should be enough (and yields `success` with your script from
> above on a test-machine)
> 
> 
> > +__EOD
> > +
> > +    my $cfgfn = "/tmp/pvesslconf-$$.tmp";
> I would consider generating temporary files in /run, or any other
> directory that is not world-writable. I think there's a number of issues
> which were caused by potential symlink-races in /tmp.
> 
> alternatively/additionally - opening with O_CREAT | O_EXCL (see `man 2
> open`) might help with the potential for races.
> pve-common/src/PVE/File.pm has `file_set_contents` which looks like it
> does the right thing.
> 
> As you said off-list that this is based on other similar code in
> pve-cluster - switching those sites too might be a nice additional
> improvement - though not strictly tied to this patch.
> 
> A quick look through PVE::Cluster::Setup makes me think that
> probably most of the temporary config-file additions could also be
> replaced by one or two `-addext` command-line parameters and a fitting
> `-subj` (this can contain `organizationName` (as O=) and
> `organizationalUnitName` (as OU=) I think)
> 
> 
> > +    my $fh = IO::File->new($cfgfn, "w");
> > +    print $fh $sslconf;
> > +    close($fh);
> > +
> >      eval {
> >          # wrap openssl with faketime to prevent bug #904
> >          run_silent_cmd([
> > @@ -439,6 +458,8 @@ sub gen_pveca_cert {
> >              '-new',
> >              '-x509',
> >              '-nodes',
> > +            '-config',
> > +            $cfgfn,
> >              '-key',
> >              $pveca_key_fn,
> >              '-out',
> > @@ -448,6 +469,8 @@ sub gen_pveca_cert {
> >          ]);
> >      };
> >  
> > +    unlink $cfgfn;
> > +
> >      die "generating pve root certificate failed:\n$@" if $@;
> >  
> >      return 1;
> 
Hey Stoiko, thanks a lot for the review and all the feedback! I
addressed your suggestions in v2:
https://lore.proxmox.com/pve-devel/[email protected]/T/#t


_______________________________________________
pve-devel mailing list
[email protected]
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to