Thanks for the review!

Em qua., 8 de jan. de 2025 às 07:49, Peter Eisentraut
<pe...@eisentraut.org> escreveu:
>
> This patch is surprisingly compact and straightforward for providing
> such complex functionality.
>
> I have one major code comment that needs addressing:
>
> In src/interfaces/libpq/fe-auth-scram.c, there is:
>
> +       memcpy(ClientKey, state->conn->scram_client_key_binary,
> SCRAM_MAX_KEY_LEN);
>
> Here you are assuming that scram_client_key_binary has a fixed length,
> but the allocation is
>
> +       len = pg_b64_dec_len(strlen(conn->scram_client_key));
> +       conn->scram_client_key_len = len;
> +       conn->scram_client_key_binary = malloc(len);
>
> And scram_client_key is passed by the client.  There needs to be some
> verification that what is passed in is of the right length.
>
> At the moment, we only support one variant of SCRAM, so all the keys
> etc. are of a fixed length.  But we should make sure that this wouldn't
> break in confusing ways in the future if that is no longer the case.
>
Fixed

> postgres_fdw has this error message:
>
>      ereport(ERROR,
>              (errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
>               errmsg("password or GSSAPI delegated credentials required"),
>               errdetail("Non-superusers must delegate GSSAPI credentials
> or provide a password in the user mapping.")));
>
> Maybe the option of having SCRAM pass-through should be mentioned here?
> It seems sort of analogous to the delegate GSSAPI credentials case.
>
Yeah, I also think that makes sense.

I've made all changes on the attached v2.

-- 
Matheus Alcantara

Attachment: v2-0001-postgres_fdw-SCRAM-authentication-pass-through.patch
Description: Binary data

Reply via email to