Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1n7brw-0007kc-7H for pgsql-hackers@arkaria.postgresql.org; Wed, 12 Jan 2022 11:32:44 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1n7brv-0007na-1n for pgsql-hackers@arkaria.postgresql.org; Wed, 12 Jan 2022 11:32:43 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1n7brt-0007nH-N7 for pgsql-hackers@lists.postgresql.org; Wed, 12 Jan 2022 11:32:42 +0000 Received: from out4-smtp.messagingengine.com ([66.111.4.28]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1n7brp-0007AM-If for pgsql-hackers@lists.postgresql.org; Wed, 12 Jan 2022 11:32:41 +0000 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id C29DF5C00F2; Wed, 12 Jan 2022 06:32:34 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Wed, 12 Jan 2022 06:32:34 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=paquier.xyz; h= date:from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=fm3; bh=poIEkgsHSJYgC9tgZRnoTzB87EX qXEJiTlJFFFU0oQI=; b=bxApdeL9q5tuHTynbUuOQnrRnUNXdL/QUH8E9kmcZ5m BzXezg7+mc+EFY9aV6Yz8b3dtSiWCRcZ1f55GTLNEmEZgbHINf4p6f13oce3LD/7 UP3atspPZoNlPITqQ2ByD5dblLF/bQzoUeoJm2SpRhYWa/5cIevno07QfiRnKBhN gm8Pgi2X+dtRtAvly88xiH5Q3MRmNXn0j5SRiUGvWdSsSn0dKNRhtTZs2nhp21+a 4rKF3ly8fZgreZTOg5pSfbnSLf70e6zOBCBkb9eBqNyiHem49QGRHTRlHtJnO4QL UOsiRXYYoKCHiyQSn3ZOJ6faKyBQqQwa8V0wYUFgPBA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; bh=poIEkg sHSJYgC9tgZRnoTzB87EXqXEJiTlJFFFU0oQI=; b=eBfwqN255nBZvgrOzPkb2A 8PoMPA8lxu+P0F1F9dGSBU6aDvYf7m/tBCx42LL+sOT1715h1U/pu2BHF4t9cWBp avsHDwoTi3N/nTxQPQ23YZ3aNE8jxGfQ+YT0dNgR9QqsUC+Lmg7QkLHi71JIQL9w Fhy34LlFTssNN87LDyJ3GeJZglkfXwcxR1LMiu/gdXVmGbyYX5yawyIv6gRPm6T6 HyLNgTKhGPRpPrvkkCJbj0vQhPi+r7oQC1N3xVBdlw6co7j3quGkPF/AcNGnlfUR kN3DVTaNiwFEXsAYETzmZZsrE0g3xJ+YJ0ub9t1AqvzTz/GdwqPYWrVN7UdJIQPg == X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddrtddugddtfecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenfg hrlhcuvffnffculdejtddmnecujfgurhepfffhvffukfhfgggtuggjsehgtderredttddv necuhfhrohhmpefoihgthhgrvghlucfrrghquhhivghruceomhhitghhrggvlhesphgrqh huihgvrhdrgiihiieqnecuggftrfgrthhtvghrnhepvdegudeuhfdtueeltedtveejheeh ieevueeigeelteegleejleeiueeiheegvefhnecuvehluhhsthgvrhfuihiivgeptdenuc frrghrrghmpehmrghilhhfrhhomhepmhhitghhrggvlhesphgrqhhuihgvrhdrgiihii X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 12 Jan 2022 06:32:32 -0500 (EST) Date: Wed, 12 Jan 2022 20:32:27 +0900 From: Michael Paquier To: Sergey Shinderuk Cc: Postgres hackers Subject: Re: Improve error handling of HMAC computations and SCRAM Message-ID: References: <066c0624-fbb7-a025-afd6-d54683a96947@postgrespro.ru> <71b9a5d2-a3bf-83bc-a243-93dcf0bcfb3b@postgrespro.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="TGqISen7ZWQtDCvg" Content-Disposition: inline In-Reply-To: List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --TGqISen7ZWQtDCvg Content-Type: multipart/mixed; boundary="nDAZZZ0rWjxYgQKL" Content-Disposition: inline --nDAZZZ0rWjxYgQKL Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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.) -- Michael --nDAZZZ0rWjxYgQKL Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v2-0001-Improve-HMAC-error-handling.patch" Content-Transfer-Encoding: quoted-printable =46rom a6bcfefa9a8dd98bdc6f0e105f7b55dc8739c49e Mon Sep 17 00:00:00 2001 =46rom: Michael Paquier Date: Wed, 12 Jan 2022 12:46:27 +0900 Subject: [PATCH v2] Improve HMAC error handling --- src/include/common/hmac.h | 1 + src/include/common/scram-common.h | 14 +++-- src/backend/libpq/auth-scram.c | 22 ++++--- src/common/hmac.c | 64 ++++++++++++++++++++ src/common/hmac_openssl.c | 90 ++++++++++++++++++++++++++++ src/common/scram-common.c | 47 +++++++++++---- src/interfaces/libpq/fe-auth-scram.c | 67 +++++++++++++++------ src/interfaces/libpq/fe-auth.c | 10 ++-- src/interfaces/libpq/fe-auth.h | 3 +- 9 files changed, 269 insertions(+), 49 deletions(-) diff --git a/src/include/common/hmac.h b/src/include/common/hmac.h index cf7aa17be4..c18783fe11 100644 --- a/src/include/common/hmac.h +++ b/src/include/common/hmac.h @@ -25,5 +25,6 @@ extern int pg_hmac_init(pg_hmac_ctx *ctx, const uint8 *ke= y, size_t len); extern int pg_hmac_update(pg_hmac_ctx *ctx, const uint8 *data, size_t len); extern int pg_hmac_final(pg_hmac_ctx *ctx, uint8 *dest, size_t len); extern void pg_hmac_free(pg_hmac_ctx *ctx); +extern const char *pg_hmac_error(pg_hmac_ctx *ctx); =20 #endif /* PG_HMAC_H */ diff --git a/src/include/common/scram-common.h b/src/include/common/scram-c= ommon.h index d53b4fa7f5..d1f840c11c 100644 --- a/src/include/common/scram-common.h +++ b/src/include/common/scram-common.h @@ -47,12 +47,16 @@ #define SCRAM_DEFAULT_ITERATIONS 4096 =20 extern int scram_SaltedPassword(const char *password, const char *salt, - int saltlen, int iterations, uint8 *result); -extern int scram_H(const uint8 *str, int len, uint8 *result); -extern int scram_ClientKey(const uint8 *salted_password, uint8 *result); -extern int scram_ServerKey(const uint8 *salted_password, uint8 *result); + int saltlen, int iterations, uint8 *result, + const char **errstr); +extern int scram_H(const uint8 *str, int len, uint8 *result, + const char **errstr); +extern int scram_ClientKey(const uint8 *salted_password, uint8 *result, + const char **errstr); +extern int scram_ServerKey(const uint8 *salted_password, uint8 *result, + const char **errstr); =20 extern char *scram_build_secret(const char *salt, int saltlen, int iterati= ons, - const char *password); + const char *password, const char **errstr); =20 #endif /* SCRAM_COMMON_H */ diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c index 7c9dee70ce..ee7f52218a 100644 --- a/src/backend/libpq/auth-scram.c +++ b/src/backend/libpq/auth-scram.c @@ -465,6 +465,7 @@ pg_be_scram_build_secret(const char *password) pg_saslprep_rc rc; char saltbuf[SCRAM_DEFAULT_SALT_LEN]; char *result; + const char *errstr =3D NULL; =20 /* * Normalize the password with SASLprep. If that doesn't work, because @@ -482,7 +483,8 @@ pg_be_scram_build_secret(const char *password) errmsg("could not generate random salt"))); =20 result =3D scram_build_secret(saltbuf, SCRAM_DEFAULT_SALT_LEN, - SCRAM_DEFAULT_ITERATIONS, password); + SCRAM_DEFAULT_ITERATIONS, password, + &errstr); =20 if (prep_password) pfree(prep_password); @@ -509,6 +511,7 @@ scram_verify_plain_password(const char *username, const= char *password, uint8 computed_key[SCRAM_KEY_LEN]; char *prep_password; pg_saslprep_rc rc; + const char *errstr =3D NULL; =20 if (!parse_scram_secret(secret, &iterations, &encoded_salt, stored_key, server_key)) @@ -539,10 +542,10 @@ scram_verify_plain_password(const char *username, con= st char *password, =20 /* Compute Server Key based on the user-supplied plaintext password */ if (scram_SaltedPassword(password, salt, saltlen, iterations, - salted_password) < 0 || - scram_ServerKey(salted_password, computed_key) < 0) + salted_password, &errstr) < 0 || + scram_ServerKey(salted_password, computed_key, &errstr) < 0) { - elog(ERROR, "could not compute server key"); + elog(ERROR, "could not compute server key: %s", errstr); } =20 if (prep_password) @@ -1113,6 +1116,7 @@ verify_client_proof(scram_state *state) uint8 client_StoredKey[SCRAM_KEY_LEN]; pg_hmac_ctx *ctx =3D pg_hmac_create(PG_SHA256); int i; + const char *errstr =3D NULL; =20 /* * Calculate ClientSignature. Note that we don't log directly a failure @@ -1133,7 +1137,8 @@ verify_client_proof(scram_state *state) strlen(state->client_final_message_without_proof)) < 0 || pg_hmac_final(ctx, ClientSignature, sizeof(ClientSignature)) < 0) { - elog(ERROR, "could not calculate client signature"); + elog(ERROR, "could not calculate client signature: %s", + pg_hmac_error(ctx)); } =20 pg_hmac_free(ctx); @@ -1143,8 +1148,8 @@ verify_client_proof(scram_state *state) ClientKey[i] =3D state->ClientProof[i] ^ ClientSignature[i]; =20 /* Hash it one more time, and compare with StoredKey */ - if (scram_H(ClientKey, SCRAM_KEY_LEN, client_StoredKey) < 0) - elog(ERROR, "could not hash stored key"); + if (scram_H(ClientKey, SCRAM_KEY_LEN, client_StoredKey, &errstr) < 0) + elog(ERROR, "could not hash stored key: %s", errstr); =20 if (memcmp(client_StoredKey, state->StoredKey, SCRAM_KEY_LEN) !=3D 0) return false; @@ -1389,7 +1394,8 @@ build_server_final_message(scram_state *state) strlen(state->client_final_message_without_proof)) < 0 || pg_hmac_final(ctx, ServerSignature, sizeof(ServerSignature)) < 0) { - elog(ERROR, "could not calculate server signature"); + elog(ERROR, "could not calculate server signature: %s", + pg_hmac_error(ctx)); } =20 pg_hmac_free(ctx); diff --git a/src/common/hmac.c b/src/common/hmac.c index 6e46dc28a1..592f9b20a3 100644 --- a/src/common/hmac.c +++ b/src/common/hmac.c @@ -38,6 +38,14 @@ #define FREE(ptr) free(ptr) #endif =20 +/* Set of error states */ +typedef enum pg_hmac_errno +{ + PG_HMAC_ERROR_NONE =3D 0, + PG_HMAC_ERROR_OOM, + PG_HMAC_ERROR_INTERNAL +} pg_hmac_errno; + /* * Internal structure for pg_hmac_ctx->data with this implementation. */ @@ -45,6 +53,8 @@ struct pg_hmac_ctx { pg_cryptohash_ctx *hash; pg_cryptohash_type type; + pg_hmac_errno error; + const char *errreason; int block_size; int digest_size; =20 @@ -75,6 +85,8 @@ pg_hmac_create(pg_cryptohash_type type) return NULL; memset(ctx, 0, sizeof(pg_hmac_ctx)); ctx->type =3D type; + ctx->error =3D PG_HMAC_ERROR_NONE; + ctx->errreason =3D NULL; =20 /* * Initialize the context data. This requires to know the digest and @@ -152,12 +164,16 @@ pg_hmac_init(pg_hmac_ctx *ctx, const uint8 *key, size= _t len) /* temporary buffer for one-time shrink */ shrinkbuf =3D ALLOC(digest_size); if (shrinkbuf =3D=3D NULL) + { + ctx->error =3D PG_HMAC_ERROR_OOM; return -1; + } memset(shrinkbuf, 0, digest_size); =20 hash_ctx =3D pg_cryptohash_create(ctx->type); if (hash_ctx =3D=3D NULL) { + ctx->error =3D PG_HMAC_ERROR_OOM; FREE(shrinkbuf); return -1; } @@ -166,6 +182,8 @@ pg_hmac_init(pg_hmac_ctx *ctx, const uint8 *key, size_t= len) pg_cryptohash_update(hash_ctx, key, len) < 0 || pg_cryptohash_final(hash_ctx, shrinkbuf, digest_size) < 0) { + ctx->error =3D PG_HMAC_ERROR_INTERNAL; + ctx->errreason =3D pg_cryptohash_error(hash_ctx); pg_cryptohash_free(hash_ctx); FREE(shrinkbuf); return -1; @@ -186,6 +204,8 @@ pg_hmac_init(pg_hmac_ctx *ctx, const uint8 *key, size_t= len) if (pg_cryptohash_init(ctx->hash) < 0 || pg_cryptohash_update(ctx->hash, ctx->k_ipad, ctx->block_size) < 0) { + ctx->error =3D PG_HMAC_ERROR_INTERNAL; + ctx->errreason =3D pg_cryptohash_error(ctx->hash); if (shrinkbuf) FREE(shrinkbuf); return -1; @@ -208,7 +228,11 @@ pg_hmac_update(pg_hmac_ctx *ctx, const uint8 *data, si= ze_t len) return -1; =20 if (pg_cryptohash_update(ctx->hash, data, len) < 0) + { + ctx->error =3D PG_HMAC_ERROR_INTERNAL; + ctx->errreason =3D pg_cryptohash_error(ctx->hash); return -1; + } =20 return 0; } @@ -228,11 +252,16 @@ pg_hmac_final(pg_hmac_ctx *ctx, uint8 *dest, size_t l= en) =20 h =3D ALLOC(ctx->digest_size); if (h =3D=3D NULL) + { + ctx->error =3D PG_HMAC_ERROR_OOM; return -1; + } memset(h, 0, ctx->digest_size); =20 if (pg_cryptohash_final(ctx->hash, h, ctx->digest_size) < 0) { + ctx->error =3D PG_HMAC_ERROR_INTERNAL; + ctx->errreason =3D pg_cryptohash_error(ctx->hash); FREE(h); return -1; } @@ -243,6 +272,8 @@ pg_hmac_final(pg_hmac_ctx *ctx, uint8 *dest, size_t len) pg_cryptohash_update(ctx->hash, h, ctx->digest_size) < 0 || pg_cryptohash_final(ctx->hash, dest, len) < 0) { + ctx->error =3D PG_HMAC_ERROR_INTERNAL; + ctx->errreason =3D pg_cryptohash_error(ctx->hash); FREE(h); return -1; } @@ -266,3 +297,36 @@ pg_hmac_free(pg_hmac_ctx *ctx) explicit_bzero(ctx, sizeof(pg_hmac_ctx)); FREE(ctx); } + +/* + * pg_hmac_error + * + * Returns a static string providing details about an error that happened + * during a HMAC computation. + */ +const char * +pg_hmac_error(pg_hmac_ctx *ctx) +{ + if (ctx =3D=3D NULL) + return _("out of memory"); + + /* + * If a reason is provided, rely on it, else fallback to any error code + * set. + */ + if (ctx->errreason) + return ctx->errreason; + + switch (ctx->error) + { + case PG_HMAC_ERROR_NONE: + return _("success"); + case PG_HMAC_ERROR_INTERNAL: + return _("internal error"); + case PG_HMAC_ERROR_OOM: + return _("out of memory"); + } + + Assert(false); /* cannot be reached */ + return _("success"); +} diff --git a/src/common/hmac_openssl.c b/src/common/hmac_openssl.c index d2cb5474bb..c352f9db9e 100644 --- a/src/common/hmac_openssl.c +++ b/src/common/hmac_openssl.c @@ -20,6 +20,8 @@ #include "postgres_fe.h" #endif =20 + +#include #include =20 #include "common/hmac.h" @@ -50,6 +52,14 @@ #define FREE(ptr) free(ptr) #endif /* FRONTEND */ =20 +/* Set of error states */ +typedef enum pg_hmac_errno +{ + PG_HMAC_ERROR_NONE =3D 0, + PG_HMAC_ERROR_DEST_LEN, + PG_HMAC_ERROR_OPENSSL +} pg_hmac_errno; + /* * Internal structure for pg_hmac_ctx->data with this implementation. */ @@ -57,12 +67,27 @@ struct pg_hmac_ctx { HMAC_CTX *hmacctx; pg_cryptohash_type type; + pg_hmac_errno error; + const char *errreason; =20 #ifndef FRONTEND ResourceOwner resowner; #endif }; =20 +static const char * +SSLerrmessage(unsigned long ecode) +{ + if (ecode =3D=3D 0) + return NULL; + + /* + * This may return NULL, but we would fall back to a default error path if + * that were the case. + */ + return ERR_reason_error_string(ecode); +} + /* * pg_hmac_create * @@ -80,6 +105,8 @@ pg_hmac_create(pg_cryptohash_type type) memset(ctx, 0, sizeof(pg_hmac_ctx)); =20 ctx->type =3D type; + ctx->error =3D PG_HMAC_ERROR_NONE; + ctx->errreason =3D NULL; =20 /* * Initialization takes care of assigning the correct type for OpenSSL. @@ -154,7 +181,11 @@ pg_hmac_init(pg_hmac_ctx *ctx, const uint8 *key, size_= t len) =20 /* OpenSSL internals return 1 on success, 0 on failure */ if (status <=3D 0) + { + ctx->errreason =3D SSLerrmessage(ERR_get_error()); + ctx->error =3D PG_HMAC_ERROR_OPENSSL; return -1; + } =20 return 0; } @@ -176,7 +207,11 @@ pg_hmac_update(pg_hmac_ctx *ctx, const uint8 *data, si= ze_t len) =20 /* OpenSSL internals return 1 on success, 0 on failure */ if (status <=3D 0) + { + ctx->errreason =3D SSLerrmessage(ERR_get_error()); + ctx->error =3D PG_HMAC_ERROR_OPENSSL; return -1; + } return 0; } =20 @@ -198,27 +233,45 @@ pg_hmac_final(pg_hmac_ctx *ctx, uint8 *dest, size_t l= en) { case PG_MD5: if (len < MD5_DIGEST_LENGTH) + { + ctx->error =3D PG_HMAC_ERROR_DEST_LEN; return -1; + } break; case PG_SHA1: if (len < SHA1_DIGEST_LENGTH) + { + ctx->error =3D PG_HMAC_ERROR_DEST_LEN; return -1; + } break; case PG_SHA224: if (len < PG_SHA224_DIGEST_LENGTH) + { + ctx->error =3D PG_HMAC_ERROR_DEST_LEN; return -1; + } break; case PG_SHA256: if (len < PG_SHA256_DIGEST_LENGTH) + { + ctx->error =3D PG_HMAC_ERROR_DEST_LEN; return -1; + } break; case PG_SHA384: if (len < PG_SHA384_DIGEST_LENGTH) + { + ctx->error =3D PG_HMAC_ERROR_DEST_LEN; return -1; + } break; case PG_SHA512: if (len < PG_SHA512_DIGEST_LENGTH) + { + ctx->error =3D PG_HMAC_ERROR_DEST_LEN; return -1; + } break; } =20 @@ -226,7 +279,11 @@ pg_hmac_final(pg_hmac_ctx *ctx, uint8 *dest, size_t le= n) =20 /* OpenSSL internals return 1 on success, 0 on failure */ if (status <=3D 0) + { + ctx->errreason =3D SSLerrmessage(ERR_get_error()); + ctx->error =3D PG_HMAC_ERROR_OPENSSL; return -1; + } return 0; } =20 @@ -254,3 +311,36 @@ pg_hmac_free(pg_hmac_ctx *ctx) explicit_bzero(ctx, sizeof(pg_hmac_ctx)); FREE(ctx); } + +/* + * pg_hmac_error + * + * Returns a static string providing details about an error that happened + * during a HMAC computation. + */ +const char * +pg_hmac_error(pg_hmac_ctx *ctx) +{ + if (ctx =3D=3D NULL) + return _("out of memory"); + + /* + * If a reason is provided, rely on it, else fallback to any error code + * set. + */ + if (ctx->errreason) + return ctx->errreason; + + switch (ctx->error) + { + case PG_HMAC_ERROR_NONE: + return _("success"); + case PG_HMAC_ERROR_DEST_LEN: + return _("destination buffer too small"); + case PG_HMAC_ERROR_OPENSSL: + return _("OpenSSL failure"); + } + + Assert(false); /* cannot be reached */ + return _("success"); +} diff --git a/src/common/scram-common.c b/src/common/scram-common.c index 23b68b14da..5f90397c66 100644 --- a/src/common/scram-common.c +++ b/src/common/scram-common.c @@ -33,7 +33,7 @@ int scram_SaltedPassword(const char *password, const char *salt, int saltlen, int iterations, - uint8 *result) + uint8 *result, const char **errstr) { int password_len =3D strlen(password); uint32 one =3D pg_hton32(1); @@ -58,6 +58,7 @@ scram_SaltedPassword(const char *password, pg_hmac_update(hmac_ctx, (uint8 *) &one, sizeof(uint32)) < 0 || pg_hmac_final(hmac_ctx, Ui_prev, sizeof(Ui_prev)) < 0) { + *errstr =3D pg_hmac_error(hmac_ctx); pg_hmac_free(hmac_ctx); return -1; } @@ -71,6 +72,7 @@ scram_SaltedPassword(const char *password, pg_hmac_update(hmac_ctx, (uint8 *) Ui_prev, SCRAM_KEY_LEN) < 0 || pg_hmac_final(hmac_ctx, Ui, sizeof(Ui)) < 0) { + *errstr =3D pg_hmac_error(hmac_ctx); pg_hmac_free(hmac_ctx); return -1; } @@ -90,18 +92,22 @@ scram_SaltedPassword(const char *password, * not included in the hash). Returns 0 on success, -1 on failure. */ int -scram_H(const uint8 *input, int len, uint8 *result) +scram_H(const uint8 *input, int len, uint8 *result, const char **errstr) { pg_cryptohash_ctx *ctx; =20 ctx =3D pg_cryptohash_create(PG_SHA256); if (ctx =3D=3D NULL) + { + *errstr =3D pg_cryptohash_error(NULL); /* returns OOM */ return -1; + } =20 if (pg_cryptohash_init(ctx) < 0 || pg_cryptohash_update(ctx, input, len) < 0 || pg_cryptohash_final(ctx, result, SCRAM_KEY_LEN) < 0) { + *errstr =3D pg_cryptohash_error(ctx); pg_cryptohash_free(ctx); return -1; } @@ -114,7 +120,8 @@ scram_H(const uint8 *input, int len, uint8 *result) * Calculate ClientKey. Returns 0 on success, -1 on failure. */ int -scram_ClientKey(const uint8 *salted_password, uint8 *result) +scram_ClientKey(const uint8 *salted_password, uint8 *result, + const char **errstr) { pg_hmac_ctx *ctx =3D pg_hmac_create(PG_SHA256); =20 @@ -125,6 +132,7 @@ scram_ClientKey(const uint8 *salted_password, uint8 *re= sult) pg_hmac_update(ctx, (uint8 *) "Client Key", strlen("Client Key")) < 0 || pg_hmac_final(ctx, result, SCRAM_KEY_LEN) < 0) { + *errstr =3D pg_hmac_error(ctx); pg_hmac_free(ctx); return -1; } @@ -137,17 +145,22 @@ scram_ClientKey(const uint8 *salted_password, uint8 *= result) * Calculate ServerKey. Returns 0 on success, -1 on failure. */ int -scram_ServerKey(const uint8 *salted_password, uint8 *result) +scram_ServerKey(const uint8 *salted_password, uint8 *result, + const char **errstr) { pg_hmac_ctx *ctx =3D pg_hmac_create(PG_SHA256); =20 if (ctx =3D=3D NULL) + { + *errstr =3D pg_hmac_error(NULL); /* returns OOM */ return -1; + } =20 if (pg_hmac_init(ctx, salted_password, SCRAM_KEY_LEN) < 0 || pg_hmac_update(ctx, (uint8 *) "Server Key", strlen("Server Key")) < 0 || pg_hmac_final(ctx, result, SCRAM_KEY_LEN) < 0) { + *errstr =3D pg_hmac_error(ctx); pg_hmac_free(ctx); return -1; } @@ -167,7 +180,7 @@ scram_ServerKey(const uint8 *salted_password, uint8 *re= sult) */ char * scram_build_secret(const char *salt, int saltlen, int iterations, - const char *password) + const char *password, const char **errstr) { uint8 salted_password[SCRAM_KEY_LEN]; uint8 stored_key[SCRAM_KEY_LEN]; @@ -185,15 +198,17 @@ scram_build_secret(const char *salt, int saltlen, int= iterations, =20 /* Calculate StoredKey and ServerKey */ if (scram_SaltedPassword(password, salt, saltlen, iterations, - salted_password) < 0 || - scram_ClientKey(salted_password, stored_key) < 0 || - scram_H(stored_key, SCRAM_KEY_LEN, stored_key) < 0 || - scram_ServerKey(salted_password, server_key) < 0) + salted_password, errstr) < 0 || + scram_ClientKey(salted_password, stored_key, errstr) < 0 || + scram_H(stored_key, SCRAM_KEY_LEN, stored_key, errstr) < 0 || + scram_ServerKey(salted_password, server_key, errstr) < 0) { + /* errstr is filled already here */ #ifdef FRONTEND return NULL; #else - elog(ERROR, "could not calculate stored key and server key"); + elog(ERROR, "could not calculate stored key and server key: %s", + *errstr); #endif } =20 @@ -215,7 +230,10 @@ scram_build_secret(const char *salt, int saltlen, int = iterations, #ifdef FRONTEND result =3D malloc(maxlen); if (!result) + { + *errstr =3D _("out of memory"); return NULL; + } #else result =3D palloc(maxlen); #endif @@ -226,11 +244,12 @@ scram_build_secret(const char *salt, int saltlen, int= iterations, encoded_result =3D pg_b64_encode(salt, saltlen, p, encoded_salt_len); if (encoded_result < 0) { + *errstr =3D _("could not encode salt"); #ifdef FRONTEND free(result); return NULL; #else - elog(ERROR, "could not encode salt"); + elog(ERROR, "%s", *errstr); #endif } p +=3D encoded_result; @@ -241,11 +260,12 @@ scram_build_secret(const char *salt, int saltlen, int= iterations, encoded_stored_len); if (encoded_result < 0) { + *errstr =3D _("could not encode stored key"); #ifdef FRONTEND free(result); return NULL; #else - elog(ERROR, "could not encode stored key"); + elog(ERROR, "%s", *errstr); #endif } =20 @@ -257,11 +277,12 @@ scram_build_secret(const char *salt, int saltlen, int= iterations, encoded_server_len); if (encoded_result < 0) { + *errstr =3D _("could not encode server key"); #ifdef FRONTEND free(result); return NULL; #else - elog(ERROR, "could not encode server key"); + elog(ERROR, "%s", *errstr); #endif } =20 diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe= -auth-scram.c index cc41440c4e..b173d7d502 100644 --- a/src/interfaces/libpq/fe-auth-scram.c +++ b/src/interfaces/libpq/fe-auth-scram.c @@ -80,10 +80,11 @@ static bool read_server_first_message(fe_scram_state *s= tate, char *input); static bool read_server_final_message(fe_scram_state *state, char *input); static char *build_client_first_message(fe_scram_state *state); static char *build_client_final_message(fe_scram_state *state); -static bool verify_server_signature(fe_scram_state *state, bool *match); +static bool verify_server_signature(fe_scram_state *state, bool *match, + const char **errstr); static bool calculate_client_proof(fe_scram_state *state, const char *client_final_message_without_proof, - uint8 *result); + uint8 *result, const char **errstr); =20 /* * Initialize SCRAM exchange status. @@ -211,6 +212,7 @@ scram_exchange(void *opaq, char *input, int inputlen, { fe_scram_state *state =3D (fe_scram_state *) opaq; PGconn *conn =3D state->conn; + const char *errstr =3D NULL; =20 *done =3D false; *success =3D false; @@ -273,10 +275,10 @@ scram_exchange(void *opaq, char *input, int inputlen, * Verify server signature, to make sure we're talking to the * genuine server. */ - if (!verify_server_signature(state, success)) + if (!verify_server_signature(state, success, &errstr)) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("could not verify server signature\n")); + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("could not verify server signature: %s\n"), errstr= ); goto error; } =20 @@ -469,6 +471,7 @@ build_client_final_message(fe_scram_state *state) uint8 client_proof[SCRAM_KEY_LEN]; char *result; int encoded_len; + const char *errstr =3D NULL; =20 initPQExpBuffer(&buf); =20 @@ -572,11 +575,12 @@ build_client_final_message(fe_scram_state *state) /* Append proof to it, to form client-final-message. */ if (!calculate_client_proof(state, state->client_final_message_without_proof, - client_proof)) + client_proof, &errstr)) { termPQExpBuffer(&buf); - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("could not calculate client proof\n")); + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("could not calculate client proof: %s\n"), + errstr); return NULL; } =20 @@ -787,7 +791,7 @@ read_server_final_message(fe_scram_state *state, char *= input) static bool calculate_client_proof(fe_scram_state *state, const char *client_final_message_without_proof, - uint8 *result) + uint8 *result, const char **errstr) { uint8 StoredKey[SCRAM_KEY_LEN]; uint8 ClientKey[SCRAM_KEY_LEN]; @@ -797,17 +801,27 @@ calculate_client_proof(fe_scram_state *state, =20 ctx =3D pg_hmac_create(PG_SHA256); if (ctx =3D=3D NULL) + { + *errstr =3D pg_hmac_error(NULL); /* returns OOM */ return false; + } =20 /* * Calculate SaltedPassword, and store it in 'state' so that we can reuse * it later in verify_server_signature. */ if (scram_SaltedPassword(state->password, state->salt, state->saltlen, - state->iterations, state->SaltedPassword) < 0 || - scram_ClientKey(state->SaltedPassword, ClientKey) < 0 || - scram_H(ClientKey, SCRAM_KEY_LEN, StoredKey) < 0 || - pg_hmac_init(ctx, StoredKey, SCRAM_KEY_LEN) < 0 || + state->iterations, state->SaltedPassword, + errstr) < 0 || + scram_ClientKey(state->SaltedPassword, ClientKey, errstr) < 0 || + scram_H(ClientKey, SCRAM_KEY_LEN, StoredKey, errstr) < 0) + { + /* errstr is already filled here */ + pg_hmac_free(ctx); + return false; + } + + if (pg_hmac_init(ctx, StoredKey, SCRAM_KEY_LEN) < 0 || pg_hmac_update(ctx, (uint8 *) state->client_first_message_bare, strlen(state->client_first_message_bare)) < 0 || @@ -821,6 +835,7 @@ calculate_client_proof(fe_scram_state *state, strlen(client_final_message_without_proof)) < 0 || pg_hmac_final(ctx, ClientSignature, sizeof(ClientSignature)) < 0) { + *errstr =3D pg_hmac_error(ctx); pg_hmac_free(ctx); return false; } @@ -839,7 +854,8 @@ calculate_client_proof(fe_scram_state *state, * false for a processing error. */ static bool -verify_server_signature(fe_scram_state *state, bool *match) +verify_server_signature(fe_scram_state *state, bool *match, + const char **errstr) { uint8 expected_ServerSignature[SCRAM_KEY_LEN]; uint8 ServerKey[SCRAM_KEY_LEN]; @@ -847,11 +863,20 @@ verify_server_signature(fe_scram_state *state, bool *= match) =20 ctx =3D pg_hmac_create(PG_SHA256); if (ctx =3D=3D NULL) + { + *errstr =3D pg_hmac_error(NULL); /* returns OOM */ return false; + } + + if (scram_ServerKey(state->SaltedPassword, ServerKey, errstr) < 0) + { + /* errstr is filled already */ + pg_hmac_free(ctx); + return false; + } =20 - if (scram_ServerKey(state->SaltedPassword, ServerKey) < 0 || /* calculate ServerSignature */ - pg_hmac_init(ctx, ServerKey, SCRAM_KEY_LEN) < 0 || + if (pg_hmac_init(ctx, ServerKey, SCRAM_KEY_LEN) < 0 || pg_hmac_update(ctx, (uint8 *) state->client_first_message_bare, strlen(state->client_first_message_bare)) < 0 || @@ -866,6 +891,7 @@ verify_server_signature(fe_scram_state *state, bool *ma= tch) pg_hmac_final(ctx, expected_ServerSignature, sizeof(expected_ServerSignature)) < 0) { + *errstr =3D pg_hmac_error(ctx); pg_hmac_free(ctx); return false; } @@ -885,7 +911,7 @@ verify_server_signature(fe_scram_state *state, bool *ma= tch) * Build a new SCRAM secret. */ char * -pg_fe_scram_build_secret(const char *password) +pg_fe_scram_build_secret(const char *password, const char **errstr) { char *prep_password; pg_saslprep_rc rc; @@ -899,20 +925,25 @@ pg_fe_scram_build_secret(const char *password) */ rc =3D pg_saslprep(password, &prep_password); if (rc =3D=3D SASLPREP_OOM) + { + *errstr =3D _("out of memory"); return NULL; + } if (rc =3D=3D SASLPREP_SUCCESS) password =3D (const char *) prep_password; =20 /* Generate a random salt */ if (!pg_strong_random(saltbuf, SCRAM_DEFAULT_SALT_LEN)) { + *errstr =3D _("failed to generate random salt"); if (prep_password) free(prep_password); return NULL; } =20 result =3D scram_build_secret(saltbuf, SCRAM_DEFAULT_SALT_LEN, - SCRAM_DEFAULT_ITERATIONS, password); + SCRAM_DEFAULT_ITERATIONS, password, + errstr); =20 if (prep_password) free(prep_password); diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c index 2edc3f48e2..f8f4111fef 100644 --- a/src/interfaces/libpq/fe-auth.c +++ b/src/interfaces/libpq/fe-auth.c @@ -1293,11 +1293,13 @@ PQencryptPasswordConn(PGconn *conn, const char *pas= swd, const char *user, */ if (strcmp(algorithm, "scram-sha-256") =3D=3D 0) { - crypt_pwd =3D pg_fe_scram_build_secret(passwd); - /* We assume the only possible failure is OOM */ + const char *errstr =3D NULL; + + crypt_pwd =3D pg_fe_scram_build_secret(passwd, &errstr); if (!crypt_pwd) - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("out of memory\n")); + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("could not encrypt password: %s\n"), + errstr); } else if (strcmp(algorithm, "md5") =3D=3D 0) { diff --git a/src/interfaces/libpq/fe-auth.h b/src/interfaces/libpq/fe-auth.h index f22b3fe648..049a8bb1a1 100644 --- a/src/interfaces/libpq/fe-auth.h +++ b/src/interfaces/libpq/fe-auth.h @@ -25,6 +25,7 @@ extern char *pg_fe_getauthname(PQExpBuffer errorMessage); =20 /* Mechanisms in fe-auth-scram.c */ extern const pg_fe_sasl_mech pg_scram_mech; -extern char *pg_fe_scram_build_secret(const char *password); +extern char *pg_fe_scram_build_secret(const char *password, + const char **errstr); =20 #endif /* FE_AUTH_H */ --=20 2.34.1 --nDAZZZ0rWjxYgQKL-- --TGqISen7ZWQtDCvg Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmHevEsACgkQnvQgOdby QH2MuRAAj3DWqah8ojc0ZYfrcpi0lvMprakCmKWvE1CnhJ4h2nVuGX4nwz0EVJHT o4PMKPwCWSWzDvBnDVY+0mt6pjhBBHjh3OPzrAcSqW28qtsPBjKOVP5lWElBtGDb LUI7KE3Rd/0Tv3n4ZQe6PlvK+Xg4q4CQ+XrkuJqGlgfFoY88KSPiIM8uCBX+Llzg Mr0Xy+FLBCAdmyHeNDxXB/ozrsvbBiamxsr/nnCcSn/UuejUHveIcKsY2Iu3ovHK VseXtBzZyyOrr5EtGJA8NCdctjbvUS7xWiQMsvygWFww8dw5eJhlnetyqyXQ9id/ WQ5D6tgsLgY0EOTXOMbAZ4UWBerUgutBY9eVUzPH/mYaqZUmi91+HQ4/TWs8XMZe ngNPLhCwDjltwV6XHZYdKmF5jgrx92sky/7N09nzqEFFL8mPNJuT+Q62PIUDxGYl PTfjbo5j4ldzCPA1kqnyc4nzNw9iAedb1pEBlJRAdZJaayArqulrpBZwp1YVl+Gu f/+d2GHudeZwS9+r6aT2RYQDtzcllYOYGitE3BsemtdYw002Y19y1haPHu3oH/7B A25qN5JwDK2hm57CAkPzaYLtJhkARpGHAnwGVJMVH5sAPejni9LPTf0sRQn0Nwtu igdNGRjUBu6dEkPHs60MB0Hd0lmhPYBH9ad6djCFAoE4Ts9S1HA= =sGbL -----END PGP SIGNATURE----- --TGqISen7ZWQtDCvg--