Re: Refactor SCRAM code to dynamically handle hash type and key length

Peter Eisentraut <peter.eisentraut@enterprisedb.com>

From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
To: Michael Paquier <michael@paquier.xyz>, Postgres hackers <pgsql-hackers@lists.postgresql.org>
Cc: "Jonathan S. Katz" <jkatz@postgresql.org>, Daniel Gustafsson <daniel@yesql.se>
Date: 2022-12-14T13:39:43Z
Lists: pgsql-hackers
On 14.12.22 03:38, Michael Paquier wrote:
> This patch passes check-world and the CI is green.  I have tested as
> well the patch with SCRAM verifiers coming from a server initially on
> HEAD, so it looks pretty solid seen from here, being careful of memory
> leaks in the frontend, mainly.

The changes from local arrays to dynamic allocation appear to introduce 
significant complexity.  I would reconsider that.  If we consider your 
reasoning

 > While investigating on what it would take to extend SCRAM to use new
 > hash methods (say like the RFC draft for SCRAM-SHA-512), I have been
 > quickly reminded of the limitations created by SCRAM_KEY_LEN, which is
 > the key length that we use in the HMAC and hash computations when
 > creating a SCRAM verifier or when doing a SASL exchange.

then the obvious fix there is to change the definition of SCRAM_KEY_LEN 
to PG_SHA512_DIGEST_LENGTH, which would be a much smaller and simpler 
change.  We don't have to support arbitrary key sizes, so a fixed-size 
array seems appropriate.




Commits

  1. Remove hardcoded dependency to cryptohash type in the internals of SCRAM