Re: Password identifiers, protocol aging and SCRAM protocol

On Wed, Nov 16, 2016 at 1:53 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)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