Re: Refactor MD5 implementations and switch to EVP for OpenSSL

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Refactor MD5 implementations and switch to EVP for OpenSSL
Date: 2020-12-07 13:15:58
Message-ID: 28005E26-0544-4912-9828-7AAA7CA31BB2@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 4 Dec 2020, at 08:05, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Tue, Nov 10, 2020 at 01:28:09PM +0900, Michael Paquier wrote:
>> The CF bot has been complaining on Windows and this issue is fixed in
>> the attached. A refresh of src/tools/msvc for pgcrypto was just
>> missing.
>
> Now that HEAD has the necessary infrastructure to be able to plug in
> easily new cryptohash routines, here is a rebased patch for MD5.

This is a pretty straightforward patch which given the cryptohash framework
added previously does what it says on the tin. +1 on rolling MD5 into what we
already have for SHA2 now. Applies clean and all tests pass.

One (two-part) comment on the patch though:

The re-arrangement of the code does lead to some attribution confusion however
IMO. pgcrypto/md5.c and src/common/md5.c are combined with the actual MD5
implementation from pgcrypto/md5.c and the PG glue code from src/common/md5.c.
That is in itself a good choice, but the file headers are intact and now claims
two implementations of which only one remains.

Further, bytesToHex was imported in commit 957613be18e6b7 with attribution to
"Sverre H. Huseby <sverrehu(at)online(dot)no>" without a PGDG defined copyright,
which was later added in 2ca65f716aee9ec441dda91d91b88dd7a00bffa1. This patch
moves bytesToHex from md5.c (where his attribution is) to md5_common.c with no
maintained attribution. Off the cuff it seems we should either attribute in a
comment, or leave the function and export it, but the usual "I'm not a lawyer"
disclaimer applies. Do you have any thoughts?

> The amount of code shaved is still nice:
> 13 files changed, 641 insertions(+), 775 deletions(-)

Always nice with a net minus patch with sustained functionality.

cheers ./daniel

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Смирнов Денис 2020-12-07 13:23:42 PoC Refactor AM analyse API
Previous Message Ashutosh Bapat 2020-12-07 12:27:22 Re: [PATCH] Keeps tracking the uniqueness with UniqueKey