pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Sergey Shinderuk <s.shinderuk@postgrespro.ru>
To: Michael Paquier <michael@paquier.xyz>
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Improve error handling of HMAC computations and SCRAM
Date: Thu, 13 Jan 2022 02:01:24 +0300
Message-ID: <8cbb56dd-63d6-a581-7a65-25a97ac4be03@postgrespro.ru> (raw )
In-Reply-To: <Yd68S6Xxmg5d2qRy@paquier.xyz >
References: <Yd0N9tSAIIkFd+qi@paquier.xyz >
<066c0624-fbb7-a025-afd6-d54683a96947@postgrespro.ru >
<Yd04X0JHdfLgKVC+@paquier.xyz >
<71b9a5d2-a3bf-83bc-a243-93dcf0bcfb3b@postgrespro.ru >
<Yd5RYX89Qy6Okv8u@paquier.xyz >
<Yd68S6Xxmg5d2qRy@paquier.xyz >
On 12.01.2022 14:32, Michael Paquier wrote:
> On Wed, Jan 12, 2022 at 12:56:17PM +0900, Michael Paquier wrote:
>> Attached is a rebased patch for the HMAC portions, with a couple of
>> fixes I noticed while going through this stuff again (mostly around
>> SASLprep and pg_fe_scram_build_secret), and a fix for a conflict
>> coming from 9cb5518. psql's \password is wrong to assume that the
>> only error that can happen for scran-sha-256 is an OOM, but we'll get
>> there.
>
> With an attachment, that's even better. (Thanks, Daniel.)
Gave it a thorough read. Looks good, except for errstr not set in a
couple of places (see the diff attached).
Didn't test it.
--
Sergey Shinderuk https://postgrespro.com/
diff --git a/src/common/hmac.c b/src/common/hmac.c
index 592f9b20a38..a27778e86b3 100644
--- a/src/common/hmac.c
+++ b/src/common/hmac.c
@@ -46,9 +46,7 @@ typedef enum pg_hmac_errno
PG_HMAC_ERROR_INTERNAL
} pg_hmac_errno;
-/*
- * Internal structure for pg_hmac_ctx->data with this implementation.
- */
+/* Internal pg_hmac_ctx structure */
struct pg_hmac_ctx
{
pg_cryptohash_ctx *hash;
diff --git a/src/common/hmac_openssl.c b/src/common/hmac_openssl.c
index c352f9db9e9..44f36d51dcb 100644
--- a/src/common/hmac_openssl.c
+++ b/src/common/hmac_openssl.c
@@ -60,9 +60,7 @@ typedef enum pg_hmac_errno
PG_HMAC_ERROR_OPENSSL
} pg_hmac_errno;
-/*
- * Internal structure for pg_hmac_ctx->data with this implementation.
- */
+/* Internal pg_hmac_ctx structure */
struct pg_hmac_ctx
{
HMAC_CTX *hmacctx;
diff --git a/src/common/scram-common.c b/src/common/scram-common.c
index 5f90397c66d..8896b1e73e4 100644
--- a/src/common/scram-common.c
+++ b/src/common/scram-common.c
@@ -44,7 +44,10 @@ scram_SaltedPassword(const char *password,
pg_hmac_ctx *hmac_ctx = pg_hmac_create(PG_SHA256);
if (hmac_ctx == NULL)
+ {
+ *errstr = pg_hmac_error(NULL); /* returns OOM */
return -1;
+ }
/*
* Iterate hash calculation of HMAC entry using given salt. This is
@@ -126,7 +129,10 @@ scram_ClientKey(const uint8 *salted_password, uint8 *result,
pg_hmac_ctx *ctx = pg_hmac_create(PG_SHA256);
if (ctx == NULL)
+ {
+ *errstr = pg_hmac_error(NULL); /* returns OOM */
return -1;
+ }
if (pg_hmac_init(ctx, salted_password, SCRAM_KEY_LEN) < 0 ||
pg_hmac_update(ctx, (uint8 *) "Client Key", strlen("Client Key")) < 0 ||
Attachments:
[text/plain] errstr-not-set.txt (1.6K, ../8cbb56dd-63d6-a581-7a65-25a97ac4be03@postgrespro.ru/2-errstr-not-set.txt)
download | inline diff:
diff --git a/src/common/hmac.c b/src/common/hmac.c
index 592f9b20a38..a27778e86b3 100644
--- a/src/common/hmac.c
+++ b/src/common/hmac.c
@@ -46,9 +46,7 @@ typedef enum pg_hmac_errno
PG_HMAC_ERROR_INTERNAL
} pg_hmac_errno;
-/*
- * Internal structure for pg_hmac_ctx->data with this implementation.
- */
+/* Internal pg_hmac_ctx structure */
struct pg_hmac_ctx
{
pg_cryptohash_ctx *hash;
diff --git a/src/common/hmac_openssl.c b/src/common/hmac_openssl.c
index c352f9db9e9..44f36d51dcb 100644
--- a/src/common/hmac_openssl.c
+++ b/src/common/hmac_openssl.c
@@ -60,9 +60,7 @@ typedef enum pg_hmac_errno
PG_HMAC_ERROR_OPENSSL
} pg_hmac_errno;
-/*
- * Internal structure for pg_hmac_ctx->data with this implementation.
- */
+/* Internal pg_hmac_ctx structure */
struct pg_hmac_ctx
{
HMAC_CTX *hmacctx;
diff --git a/src/common/scram-common.c b/src/common/scram-common.c
index 5f90397c66d..8896b1e73e4 100644
--- a/src/common/scram-common.c
+++ b/src/common/scram-common.c
@@ -44,7 +44,10 @@ scram_SaltedPassword(const char *password,
pg_hmac_ctx *hmac_ctx = pg_hmac_create(PG_SHA256);
if (hmac_ctx == NULL)
+ {
+ *errstr = pg_hmac_error(NULL); /* returns OOM */
return -1;
+ }
/*
* Iterate hash calculation of HMAC entry using given salt. This is
@@ -126,7 +129,10 @@ scram_ClientKey(const uint8 *salted_password, uint8 *result,
pg_hmac_ctx *ctx = pg_hmac_create(PG_SHA256);
if (ctx == NULL)
+ {
+ *errstr = pg_hmac_error(NULL); /* returns OOM */
return -1;
+ }
if (pg_hmac_init(ctx, salted_password, SCRAM_KEY_LEN) < 0 ||
pg_hmac_update(ctx, (uint8 *) "Client Key", strlen("Client Key")) < 0 ||
view thread (9+ messages) latest in thread
Message-ID: <8cbb56dd-63d6-a581-7a65-25a97ac4be03@postgrespro.ru>
Permalink: ../8cbb56dd-63d6-a581-7a65-25a97ac4be03@postgrespro.ru/
Also on: postgresql.org/message-id/8cbb56dd-63d6-a581-7a65-25a97ac4be03@postgrespro.ru
copy link · copy postgr.es
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: s.shinderuk@postgrespro.ru, michael@paquier.xyz, pgsql-hackers@lists.postgresql.org
Subject: Re: Improve error handling of HMAC computations and SCRAM
In-Reply-To: <8cbb56dd-63d6-a581-7a65-25a97ac4be03@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox