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). _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
