Обсуждение: scram-sha-256 authentication broken in FIPS mode

On Wed, Sep 05, 2018 at 03:29:31AM +0000, Alessandro Gherardi wrote:
> It looks like scram-sha-256 doesn't work when postgres is linked
> against FIPS-enabled OpenSSL and FIPS mode is turned on.
>
> Specifically, all login attempts fail with an OpenSSL error saying
> something along the lines of "Low level API call to digest SHA256
> forbidden in fips mode".

The error comes from libc, right?  Postgres can of course be configured
to work with FIPS without patching it, it just needs to be enabled
system-wide, which is what RedHat does, and what you are doing I guess?

> I think this issue could be solved by refactoring the code in
> sha2_openssl.c to use the OpenSSL EVP interface
> (see https://wiki.openssl.org/index.php/EVP_Message_Digests ).
> Any thoughts? Is this a known issue?

This report is the first of this kind since Postgres 10, which is where
the SHA2 interface for OpenSSL has been introduced.  So likely we'd need
to look into that more deeply..  This has the strong smell of a bug.  If
your system is new enough, you should have sha256() & co as system
functions, so you would see the failure as well?  The regression tests
would have likely complained.
--
Michael

On Wed, Sep 05, 2018 at 01:19:39PM +0000, Alessandro Gherardi wrote:
> Hi Michael,I'm actually running postgres on Windows.

First you may want to avoid top-posting.  This is not the style of the
community lists and this breaks the logic of a thread.

> I added code to fe-secure-openssl.c and be-secure-openssl.c that reads
> the Windows "standard" FIPS registry entry, and if FIPS is enabled
> calls FIPS_mode_set(1). This is to mimic to behavior of the .NET
> framework.

That's rather uncharted territory, as you are patching both the backend
*and* the client.  If we could prove that sha2-openssl.c is actually
unreliable even if FIPS is enabled system-wide with either SCRAM
authentication or any of the other hashing functions, then I would be
ready to accept a patch.  Now, as far as I can see and heard from other
folks for at least Linux, if FIPS is enabled at the OS level, then
Postgres would use it automatically and SCRAM is able to work.  I have
yet to hear that this part is broken.  As far as I know from companies
within the community which worked on STIG requirements, the thing
works.

> Below is the code I added to fe-secure-openssl.c, the code in
> be-secure-openssl.c is similar:
> Thoughts? I can try to fix the scram-sha-256 issue by using EVP and
> send you a merge request for the patch and the code below if you think
> my approach is correct.

That's a bit unreadable I am afraid :)
You may want to attach a patch after producing it with for example "git
format-patch -1".
--
Michael

On Mon, Sep 10, 2018 at 02:52:00PM +0000, Alessandro Gherardi wrote:
> I changed the implementation of the other SHA digests to use EVP
> also.  I verified that, with these changes, scram-sha-256 works when
> FIPS is enabled.

Hm...  I have spent a couple of hours setting up a CentOS 7 VM with FIPS
enabled to track down if there is actually a problem.  Here is what I
have done to enable it:
1) yum install dracut-fips
dracut -v -f
2) Update boot loader, where it is necessary to update
GRUB_CMDLINE_LINUX by adding to it "fips=1 boot=UUID=$PARTITION_UUID"
into /etc/default/grub.  PARTITION_UUID can be found with "blkid=/boot"
(depends on the partition layer by the way).
3) Disable prelinking (requires installation of package prelink) by
adding PRELINKING=no to /etc/sysconfig/prelink, then remove existing
prelinks with "prelink -u -a".

After a reboot, it is possible to see /proc/sys/crypto/fips_enabled set
to 1.  Once I did that, unfortunately I have not been able to spot
deficiencies when calling the low-level SHA APIs from OpenSSL, where
both SCRAM and all the in-core SSL functions are proving to work
correctly.  Calling directly FIPS_mode() within Postgres backends also
prove that FIPS is effectively enabled.  Anyway, on top of the remark
Alessandro has done above, this line from the OpenSSL docs has caught my
eyes:
https://www.openssl.org/docs/man1.1.0/crypto/SHA512_Init.html
"Applications should use the higher level functions EVP_DigestInit
etc. instead of calling the hash functions directly."

This is present in OpenSSL docs for some time:
commit: 4facdbb5fa9d791fc72dc78b9c3512ea1384df33
author: Ulf Möller <ulf@openssl.org>
date: Sun, 6 Feb 2000 23:26:31 +0000

Hence, intrinsically, we are in contradiction with the upstream docs.  I
have worked on the problem with the patch, which works down to OpenSSL
0.9.8, and should fix your issue.  This is based on what you sent
previously, except that I was not able to apply what was sent, so I
reworked the whole.  Alessandro, does this fix your problems?  I would
like to apply that down to v10 where SCRAM has been introduced.
--
Michael

On Tue, Sep 11, 2018 at 12:02:50PM +0900, Michael Paquier wrote:
> Hence, intrinsically, we are in contradiction with the upstream docs.  I
> have worked on the problem with the patch, which works down to OpenSSL
> 0.9.8, and should fix your issue.  This is based on what you sent
> previously, except that I was not able to apply what was sent, so I
> reworked the whole.  Alessandro, does this fix your problems?  I would
> like to apply that down to v10 where SCRAM has been introduced.

With the actual patch attached things are better.  So here it is.
--
Michael

On 11/09/2018 05:02, Michael Paquier wrote:
> Hence, intrinsically, we are in contradiction with the upstream docs.  I
> have worked on the problem with the patch, which works down to OpenSSL
> 0.9.8, and should fix your issue.  This is based on what you sent
> previously, except that I was not able to apply what was sent, so I
> reworked the whole.  Alessandro, does this fix your problems?  I would
> like to apply that down to v10 where SCRAM has been introduced.

I recommend letting this bake in the master branch for a while.  There
are a lot weirdly patched and alternative OpenSSL versions out there
that defy any documentation.

Of course, we should also see if this actually fixes the reported problem.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Tue, Sep 11, 2018 at 04:32:27PM +0200, Peter Eisentraut wrote:
> I recommend letting this bake in the master branch for a while.  There
> are a lot weirdly patched and alternative OpenSSL versions out there
> that defy any documentation.

Good point.  Such things have bitten in the past.  Okay, then let's do
something about sha2_openssl.c only on HEAD for now then, which I am
fine to finish wrapping.

> Of course, we should also see if this actually fixes the reported problem.

It seems to me that addressing FIPS concerns on Windows and getting our
hashing functions plugged with OpenSSL correctly are two separate
issues.  The second one also says that we are in the grey based on
OpenSSL docs, which worryies me.  And EVP_DigestInit is used in pgcrypto
for ages, where I don't recall seeing reports about that.
--
Michael

On Wed, Sep 12, 2018 at 07:24:24AM +0900, Michael Paquier wrote:
> Good point.  Such things have bitten in the past.  Okay, then let's do
> something about sha2_openssl.c only on HEAD for now then, which I am
> fine to finish wrapping.

I was looking at trying to commit this patch, however more needs to be
done in terms of error handling, as the proposed patch would happily
crash if EVP_MD_CTX cannot be allocated (understand OOM) in
EVP_DigestInit_ex if I read the OpenSSL code correctly (see
crypto/evp/digest.c).  Our lives would be facilitated if it was possible
to use directly EVP_MD_CTX and EVP_MD_CTX_init so as no allocation is
done but that's not doable as of 1.0.2.
--
Michael

On Mon, Sep 17, 2018 at 02:55:55PM +0000, Alessandro Gherardi wrote:
> Therefore, I believe the best option, at least for now, is calling
> FIPS_mode_set(1) in the application.

I am not so sure about that.  As you rightly mention, CentOS and RedHat
patch OpenSSL to allow FIPS to work.  Per my research, Ubuntu can also
enable FIPS but that's not the case of Debian, which is very popular (I
may be wrong about the last one but I use it daily).

One question I have is how are you actually able to use FIPS on Windows
with OpenSSL?  Is that from one of the tarballs available in
openssl.org, which are more than 1 year old?  Pure upstream code does
not give this option, and CentOS/RHEL use a customly-made patch, based
on which Postgres does not complain when calling the low-level hashing
functions, and we rely now on FIPS being enabled system-wide.  And that
actually works.  It seems to me that you are yourself using a custom
patch for OpenSSL, and that's actually a different flavor than the Linux
version as in your case the low-level hashing functions complain if
called directly in FIPS mode.

At the end, I think that we ought to wait and see if upstream OpenSSL
comes up with support for FIPS and how it integrates with it, on both
Linux *and* Windows, and then consider if Postgres needs to do more.
There is little point in merging now a patch for something which may or
may not be supported by OpenSSL now.  My bet, as things stand, is that
we could finish with something similar to what happens on Linux with a
system-wide switch that Postgres knows nothing about.  Perhaps that will
not be the case, but let's think about that once we know for sure.
--
Michael