Re: Password identifiers, protocol aging and SCRAM protocol

On Wed, Nov 16, 2016 at 1:53 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
>> Yeah, I don't see a point to that.
>
> OK, by doing so here is what I have. The patch generated by
> format-patch, as well as diffs generated by git diff -M are reduced
> and the patch gets half in size. They could be reduced more by adding
> at the top of sha2.c a couple of defined to map the old SHAXXX_YYY
> variables with their PG_ equivalents, but that does not seem worth it
> to me, and diffs are listed line by line.

All right, this version is much easier to review.  I am a bit puzzled,
though.  It looks like src/common will include sha2.o if built without
OpenSSL and sha2_openssl.o if built with OpenSSL.  So far, so good.
One would think, then, that pgcrypto would not need to worry about
these functions any more because libpgcommon_srv.a is linked into the
server, so any references to those symbols would presumably just work.
However, that's not what you did.  On Windows, you added a dependency
on libpgcommon which I think is unnecessary because that stuff is
already linked into the server.  On non-Windows systems, however, you
have instead taught pgcrypto to copy the source file it needs from
src/common and recompile it.  I don't understand why you need to do
any of that, or why it should be different on Windows vs. non-Windows.
So I think that the changes for the pgcrypto Makefile could just look
like this:

diff --git a/contrib/pgcrypto/Makefile b/contrib/pgcrypto/Makefile
index 805db76..ddb0183 100644
--- a/contrib/pgcrypto/Makefile
+++ b/contrib/pgcrypto/Makefile
@@ -1,6 +1,6 @@
 # contrib/pgcrypto/Makefile

-INT_SRCS = md5.c sha1.c sha2.c internal.c internal-sha2.c blf.c rijndael.c \
+INT_SRCS = md5.c sha1.c internal.c internal-sha2.c blf.c rijndael.c \
         fortuna.c random.c pgp-mpi-internal.c imath.c
 INT_TESTS = sha2

And for Mkvcbuild.pm I think you could just do this:

diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index de764dd..1993764 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -114,6 +114,15 @@ sub mkvcbuild
       md5.c pg_lzcompress.c pgfnames.c psprintf.c relpath.c rmtree.c
       string.c username.c wait_error.c);

+    if ($solution->{options}->{openssl})
+    {
+        push(@pgcommonallfiles, 'sha2_openssl.c');
+    }
+    else
+    {
+        push(@pgcommonallfiles, 'sha2.c');
+    }
+
     our @pgcommonfrontendfiles = (
         @pgcommonallfiles, qw(fe_memutils.c file_utils.c
           restricted_token.c));
@@ -422,7 +431,7 @@ sub mkvcbuild
     {
         $pgcrypto->AddFiles(
             'contrib/pgcrypto',   'md5.c',
-            'sha1.c',             'sha2.c',
+            'sha1.c',
             'internal.c',         'internal-sha2.c',
             'blf.c',              'rijndael.c',
             'fortuna.c',          'random.c',

Is there some reason that won't work?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company