On 9/26/16 2:02 AM, Michael Paquier wrote:
> On Mon, Sep 26, 2016 at 2:15 AM, David Steele <david@pgmasters.net> wrote:
>
> Thanks for the review and the comments!
>
>> I notice that the copyright from pgcrypto/sha1.c was carried over but
>> not the copyright from pgcrypto/sha2.c. I'm no expert on how this
>> works, but I believe the copyright from sha2.c must be copied over.
>
> Right, those copyright bits are missing:
> - * AUTHOR: Aaron D. Gifford <me@aarongifford.com>
> [...]
> - * Copyright (c) 2000-2001, Aaron D. Gifford
> The license block being the same, it seems to me that there is no need
> to copy it over. The copyright should be enough.
Looks fine to me.
>> Also, are there any plans to expose these functions directly to the user
>> without loading pgcrypto? Now that the functionality is in core it
>> seems that would be useful. In addition, it would make this patch stand
>> on its own rather than just being a building block.
>
> There have been discussions about avoiding enabling those functions by
> default in the distribution. We'd rather not do that...
OK.
>> * [PATCH 2/8] Move encoding routines to src/common/
>>
>> I wonder if it is confusing to have two of encode.h/encode.c. Perhaps
>> they should be renamed to make them distinct?
>
> Yes it may be a good idea to rename that, like encode_utils.[c|h] for
> the new files.
I like that better.
>> Couldn't md5_crypt_verify() be made more general and take the hash type?
>> For instance, password_crypt_verify() with the last param as the new
>> password type enum.
>
> This would mean incorporating the whole SASL message exchange into
> this routine because the password string is part of the scram
> initialization context, and it seems to me that it is better to just
> do once a lookup at the entry in pg_authid. So we'd finish with a more
> confusing code I am afraid. At least that's the conclusion I came up
> with when doing that.. md5_crypt_verify does only the work on a
> received password.
Ah, yes, I see now. I missed that when I reviewed patch 6.
--
-David
david@pgmasters.net
