[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 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);
            },
-- 
2.1.4