[pve-devel] Something missing in http://pve.proxmox.com/wiki/HTTPS_Certificate_Configuration_(Version_4.x_and_newer) ?

[Fabian Grünbichler]

On Tue, Nov 22, 2016 at 02:48:03PM +0100, Stefan Priebe - Profihost AG wrote:
> Am 22.11.2016 um 14:38 schrieb Fabian Grünbichler:
> > On Tue, Nov 22, 2016 at 01:57:47PM +0100, Fabian Grünbichler wrote:
> >> On Tue, Nov 22, 2016 at 01:09:17PM +0100, Stefan Priebe - Profihost AG wrote:
> >>> Hi,
> >>>
> >>> Am 22.11.2016 um 12:26 schrieb Fabian Grünbichler:
> >>>> On Tue, Nov 22, 2016 at 12:11:22PM +0100, Stefan Priebe - Profihost AG wrote:
> >>>>> Am 22.11.2016 um 11:56 schrieb Dietmar Maurer:
> >>>>>> I think this commit should solve the issue:
> >>>>>>
> >>>>>> https://git.proxmox.com/?p=pve-manager.git;a=commitdiff;h=333dd203d5e07d9d3e20d3674a2e3ff2fc89fa8c
> >>>>>>
> >>>>>>> Please can you test with latest version from git?
> >>>>>
> >>>>> Already running that version ;-) But thank you for pointing me to this
> >>>>> commit. If i revert that one it's working fine again.
> >>>>>
> >>>>> The issue in my case was that the verify in HTTPServer.pm verify_cb was
> >>>>> failing.
> >>>>>
> >>>>> The documentation says:
> >>>>> "fullchain.pem (your certificate and all intermediate certificates,
> >>>>> excluding the root certificate, in PEM format)"
> >>>>>
> >>>>> With the full chain it's not working. I then removed the whole chain and
> >>>>> only putted my final crt into that one and now it's working fine. With
> >>>>> the full chain $depth was 2 in my case.
> >>>>
> >>>> I will do some more testing later on - is it possible that you put the
> >>>> certificates into the file in the "wrong" order (leaf first, then
> >>>> intermediate would be correct)?
> >>>>
> >>>> The pinning is supposed to only verify the last certificate in the chain
> >>>> (as returned by the server), so whether you have a chain of depth 2 or 3
> >>>> (self-signed root + leaf or root + ca + leaf) does not matter at all.
> >>>> But when reading from a .pem file with multiple certificates OpenSSL
> >>>> reads the first one in the file, so it's possible that in your case we
> >>>> attempt to compare the leaf certificate's fingerprint (from the
> >>>> connection / server) to the CA certificate's fingerprint (from the .pem
> >>>> file), which obviously does not work.
> >>>>
> >>>> If you want, you could send me the certificate file off-list (only the
> >>>> certificate please! unless those are from a test node with self-signed
> >>>> certificates that you don't care about) and I will try to recreate your
> >>>> setup for further tests...
> >>>
> >>> As this is a real life certificate which is not self signed but signed
> >>> from the ca i'm not comfortable with that.
> >>
> >> understandable. but maybe you could confirm the order? you can just copy
> >> the file and split at the end/start marker, with "openssl x509 -in
> >> PEMFILE -noout -subject" you should get the subject of each resulting
> >> PEMFILE
> >>
> >>>
> >>> But may be you're already right and the order in the file is important.
> >>>
> >>> Wouldn't it be easier for users to just put the sigle .crt into this
> >>> location and put the ca chain into pve-root-ca.pem ?
> >>
> >> that does not work (unfortunately) - AnyEvent cannot load the chain / CA
> >> certificate from a separate file.. since the root certificate is not
> >> supposed to be part of the chain sent by the server (it needs to be
> >> trusted separately anyway), this is not a problem for the self-signed
> >> chain with depth 2. but for the "regular" chain of depth 3 you need to
> >> put the leaf/server certificate and the intermediate(s) into
> >> pveproxy-ssl.pem (and the root into your OS/browser, if it is not
> >> already there ;)).
> >>
> >> I'll test some more, and as a last workaround we could do the split of
> >> pveproxy-ssl.pem in the verification code (and simply only take the one
> >> that is not a CA certificate - this is easy to check because the CA
> >> property is a certificate extension, but of course we have additional
> >> overhead and complexity for parsing and splitting..).
> > 
> > seems like the error was a flipped default return value in one of the
> > last refactorings before sending the patch - could you try the following
> > patch?
> > 
> > a wrong order in pveproxy-ssl.pem would already completely break the
> > whole pveproxy, so this cannot be the issue ;)
> >
> > 
> > -- >8 --
> > 
> > Subject: [PATCH manager] fix SSL verify callback for certificate chains
> > 
> > ignoring parts of the chain means saying they are verified,
> > because the verify callback results are chained together
> > starting with the highest depth.
> > ---
> >  PVE/HTTPServer.pm | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/PVE/HTTPServer.pm b/PVE/HTTPServer.pm
> > index db1faae..3460050 100755
> > --- a/PVE/HTTPServer.pm
> > +++ b/PVE/HTTPServer.pm
> > @@ -700,7 +700,7 @@ sub proxy_request {
> >             verify_cb => sub {
> >                 my (undef, undef, undef, $depth, undef, undef, $cert) = @_;
> >                 # we don't care about intermediate or root certificates
> > -               return 0 if $depth != 0;
> > +               return 1 if $depth != 0;
> >                 # check server certificate against cache of pinned FPs
> >                 return check_cert_fingerprint($cert);
> >             },
> > 
> 
> Yes that works. But is this correct? Does it not skip the whole verify
> if depth is != 0?
> 

No. The result of the verification is an AND of verify_cb for all the
depths, and we only care about depth 0 (i.e., the leaf certificate). An
earlier version of this patch where I tested with chains returned
$preverify (OpenSSL's verification result before the callback is called)
for the non-zero depths, but this broke when I included the self-signed
pve-ssl.pem in the pinning checks as well. I changed the return from
$preverify to 0 instead of 1 and apparently did not test with chains
again afterwards.

You can check the correct behaviour by restarting pveproxy on node A (to
empty the cache, without having any connections open that might trigger
recaching), then replacing the certificate file on node B without
restarting the pveproxy there. Now connect to node B over node A - the
running pveproxy on node B will have the old certificate, but the
pveproxy on node A will fill its cache with the new certificate from
/etc/pve/nodes/B, and the verification will fail (just like it would for
an actual MITM).

When testing, beware that HTTP keep-alive between the nodes, the
certificate fingerprint cache and restarted pveproxy workers can
interfere with expected results (but the above should work if you follow
the steps as described).