On Fri, Jan 07, 2022 at 05:40:09PM -0500, Tom Lane wrote:
> Hm, you still have cast-away-const in md5_crypt_verify and
> plain_crypt_verify. Can we adjust their APIs to make them
> return const char * as well (and then their API spec is that
> the caller must never free the string, rather than being
> vague about it)?
Okay. Hmm. This requires a couple of extra const markers in the area
of authentication and SASL for the backend, but not much actually.
I thought first that it would be more invasive.
> The other thing that bothers me slightly is that it looks like
> some code paths could end up passing a NULL string pointer to
> ereport or sprintf, since you don't positively guarantee that
> an error will return a string there. I suppose this is safe
> since 3779ac62d, but I don't really want to start making API
> specs depend on it.
Sounds fair to me in the long term, even for non-assert builds. I
have added a small-ish wrapper routine in crytohash_openssl.c for this
purpose, with a name copied from {be,fe}-secure-openssl.c to ease any
future refactoring lookups if that proves to be worth in the future.
--
Michael
From 0e7ceca421ab309ce64d8f6fea7a714da08df56c Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@xxxxxxxxxxx>
Date: Sat, 8 Jan 2022 14:56:39 +0900
Subject: [PATCH v3] Improve error reporting for cryptohashes
---
src/include/common/cryptohash.h | 1 +
src/include/common/md5.h | 9 ++-
src/include/libpq/crypt.h | 7 +-
src/include/libpq/sasl.h | 4 +-
src/backend/commands/user.c | 4 +-
src/backend/libpq/auth-sasl.c | 2 +-
src/backend/libpq/auth-scram.c | 5 +-
src/backend/libpq/auth.c | 33 ++++----
src/backend/libpq/crypt.c | 46 ++++++-----
src/backend/replication/backup_manifest.c | 9 ++-
src/backend/utils/adt/cryptohashfuncs.c | 25 +++---
src/common/cryptohash.c | 57 +++++++++++++-
src/common/cryptohash_openssl.c | 93 +++++++++++++++++++++++
src/common/md5_common.c | 20 +++--
src/interfaces/libpq/fe-auth.c | 22 +++++-
contrib/uuid-ossp/uuid-ossp.c | 18 +++--
16 files changed, 282 insertions(+), 73 deletions(-)
diff --git a/src/include/common/cryptohash.h b/src/include/common/cryptohash.h
index ea1300d5d4..8e339c83ad 100644
--- a/src/include/common/cryptohash.h
+++ b/src/include/common/cryptohash.h
@@ -34,5 +34,6 @@ extern int pg_cryptohash_init(pg_cryptohash_ctx *ctx);
extern int pg_cryptohash_update(pg_cryptohash_ctx *ctx, const uint8 *data, size_t len);
extern int pg_cryptohash_final(pg_cryptohash_ctx *ctx, uint8 *dest, size_t len);
extern void pg_cryptohash_free(pg_cryptohash_ctx *ctx);
+extern const char *pg_cryptohash_error(pg_cryptohash_ctx *ctx);
#endif /* PG_CRYPTOHASH_H */
diff --git a/src/include/common/md5.h b/src/include/common/md5.h
index bbd2ec0165..942ca4242c 100644
--- a/src/include/common/md5.h
+++ b/src/include/common/md5.h
@@ -26,9 +26,12 @@
#define MD5_PASSWD_LEN 35
/* Utilities common to all the MD5 implementations, as of md5_common.c */
-extern bool pg_md5_hash(const void *buff, size_t len, char *hexsum);
-extern bool pg_md5_binary(const void *buff, size_t len, void *outbuf);
+extern bool pg_md5_hash(const void *buff, size_t len, char *hexsum,
+ const char **errstr);
+extern bool pg_md5_binary(const void *buff, size_t len, void *outbuf,
+ const char **errstr);
extern bool pg_md5_encrypt(const char *passwd, const char *salt,
- size_t salt_len, char *buf);
+ size_t salt_len, char *buf,
+ const char **errstr);
#endif /* PG_MD5_H */
diff --git a/src/include/libpq/crypt.h b/src/include/libpq/crypt.h
index ee60772e94..3238cf66d3 100644
--- a/src/include/libpq/crypt.h
+++ b/src/include/libpq/crypt.h
@@ -35,12 +35,13 @@ extern PasswordType get_password_type(const char *shadow_pass);
extern char *encrypt_password(PasswordType target_type, const char *role,
const char *password);
-extern char *get_role_password(const char *role, char **logdetail);
+extern char *get_role_password(const char *role, const char **logdetail);
extern int md5_crypt_verify(const char *role, const char *shadow_pass,
const char *client_pass, const char *md5_salt,
- int md5_salt_len, char **logdetail);
+ int md5_salt_len, const char **logdetail);
extern int plain_crypt_verify(const char *role, const char *shadow_pass,
- const char *client_pass, char **logdetail);
+ const char *client_pass,
+ const char **logdetail);
#endif
diff --git a/src/include/libpq/sasl.h b/src/include/libpq/sasl.h
index 7ba3f5f5bc..71cc0dc251 100644
--- a/src/include/libpq/sasl.h
+++ b/src/include/libpq/sasl.h
@@ -126,11 +126,11 @@ typedef struct pg_be_sasl_mech
int (*exchange) (void *state,
const char *input, int inputlen,
char **output, int *outputlen,
- char **logdetail);
+ const char **logdetail);
} pg_be_sasl_mech;
/* Common implementation for auth.c */
extern int CheckSASLAuth(const pg_be_sasl_mech *mech, Port *port,
- char *shadow_pass, char **logdetail);
+ char *shadow_pass, const char **logdetail);
#endif /* PG_SASL_H */
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index c79c8d247f..c50cf2d051 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -355,7 +355,7 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
if (password)
{
char *shadow_pass;
- char *logdetail;
+ const char *logdetail;
/*
* Don't allow an empty password. Libpq treats an empty password the
@@ -775,7 +775,7 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
if (password)
{
char *shadow_pass;
- char *logdetail;
+ const char *logdetail;
/* Like in CREATE USER, don't allow an empty password. */
if (password[0] == '\0' ||
diff --git a/src/backend/libpq/auth-sasl.c b/src/backend/libpq/auth-sasl.c
index 6cc776edeb..a1d7dbb6d5 100644
--- a/src/backend/libpq/auth-sasl.c
+++ b/src/backend/libpq/auth-sasl.c
@@ -50,7 +50,7 @@
*/
int
CheckSASLAuth(const pg_be_sasl_mech *mech, Port *port, char *shadow_pass,
- char **logdetail)
+ const char **logdetail)
{
StringInfoData sasl_mechs;
int mtype;
diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c
index fdfc3d5067..7c9dee70ce 100644
--- a/src/backend/libpq/auth-scram.c
+++ b/src/backend/libpq/auth-scram.c
@@ -111,7 +111,8 @@ static void scram_get_mechanisms(Port *port, StringInfo buf);
static void *scram_init(Port *port, const char *selected_mech,
const char *shadow_pass);
static int scram_exchange(void *opaq, const char *input, int inputlen,
- char **output, int *outputlen, char **logdetail);
+ char **output, int *outputlen,
+ const char **logdetail);
/* Mechanism declaration */
const pg_be_sasl_mech pg_be_scram_mech = {
@@ -335,7 +336,7 @@ scram_init(Port *port, const char *selected_mech, const char *shadow_pass)
*/
static int
scram_exchange(void *opaq, const char *input, int inputlen,
- char **output, int *outputlen, char **logdetail)
+ char **output, int *outputlen, const char **logdetail)
{
scram_state *state = (scram_state *) opaq;
int result;
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index b0d6988aff..895db5b730 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -45,7 +45,7 @@
* Global authentication functions
*----------------------------------------------------------------
*/
-static void auth_failed(Port *port, int status, char *logdetail);
+static void auth_failed(Port *port, int status, const char *logdetail);
static char *recv_password_packet(Port *port);
static void set_authn_id(Port *port, const char *id);
@@ -54,10 +54,11 @@ static void set_authn_id(Port *port, const char *id);
* Password-based authentication methods (password, md5, and scram-sha-256)
*----------------------------------------------------------------
*/
-static int CheckPasswordAuth(Port *port, char **logdetail);
-static int CheckPWChallengeAuth(Port *port, char **logdetail);
+static int CheckPasswordAuth(Port *port, const char **logdetail);
+static int CheckPWChallengeAuth(Port *port, const char **logdetail);
-static int CheckMD5Auth(Port *port, char *shadow_pass, char **logdetail);
+static int CheckMD5Auth(Port *port, char *shadow_pass,
+ const char **logdetail);
/*----------------------------------------------------------------
@@ -247,7 +248,7 @@ ClientAuthentication_hook_type ClientAuthentication_hook = NULL;
* particular, if logdetail isn't NULL, we send that string to the log.
*/
static void
-auth_failed(Port *port, int status, char *logdetail)
+auth_failed(Port *port, int status, const char *logdetail)
{
const char *errstr;
char *cdetail;
@@ -383,7 +384,7 @@ void
ClientAuthentication(Port *port)
{
int status = STATUS_ERROR;
- char *logdetail = NULL;
+ const char *logdetail = NULL;
/*
* Get the authentication method to use for this frontend/database
@@ -769,7 +770,7 @@ recv_password_packet(Port *port)
* Plaintext password authentication.
*/
static int
-CheckPasswordAuth(Port *port, char **logdetail)
+CheckPasswordAuth(Port *port, const char **logdetail)
{
char *passwd;
int result;
@@ -804,7 +805,7 @@ CheckPasswordAuth(Port *port, char **logdetail)
* MD5 and SCRAM authentication.
*/
static int
-CheckPWChallengeAuth(Port *port, char **logdetail)
+CheckPWChallengeAuth(Port *port, const char **logdetail)
{
int auth_result;
char *shadow_pass;
@@ -866,7 +867,7 @@ CheckPWChallengeAuth(Port *port, char **logdetail)
}
static int
-CheckMD5Auth(Port *port, char *shadow_pass, char **logdetail)
+CheckMD5Auth(Port *port, char *shadow_pass, const char **logdetail)
{
char md5Salt[4]; /* Password salt */
char *passwd;
@@ -3085,6 +3086,8 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
md5trailer = packet->vector;
for (i = 0; i < encryptedpasswordlen; i += RADIUS_VECTOR_LENGTH)
{
+ const char *errstr = NULL;
+
memcpy(cryptvector + strlen(secret), md5trailer, RADIUS_VECTOR_LENGTH);
/*
@@ -3093,10 +3096,12 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
*/
md5trailer = encryptedpassword + i;
- if (!pg_md5_binary(cryptvector, strlen(secret) + RADIUS_VECTOR_LENGTH, encryptedpassword + i))
+ if (!pg_md5_binary(cryptvector, strlen(secret) + RADIUS_VECTOR_LENGTH,
+ encryptedpassword + i, &errstr))
{
ereport(LOG,
- (errmsg("could not perform MD5 encryption of password")));
+ (errmsg("could not perform MD5 encryption of password: %s",
+ errstr)));
pfree(cryptvector);
pg_freeaddrinfo_all(hint.ai_family, serveraddrs);
return STATUS_ERROR;
@@ -3181,6 +3186,7 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
struct timeval timeout;
struct timeval now;
int64 timeoutval;
+ const char *errstr;
gettimeofday(&now, NULL);
timeoutval = (endtime.tv_sec * 1000000 + endtime.tv_usec) - (now.tv_sec * 1000000 + now.tv_usec);
@@ -3299,10 +3305,11 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
if (!pg_md5_binary(cryptvector,
packetlength + strlen(secret),
- encryptedpassword))
+ encryptedpassword, &errstr))
{
ereport(LOG,
- (errmsg("could not perform MD5 encryption of received packet")));
+ (errmsg("could not perform MD5 encryption of received packet: %s",
+ errstr)));
pfree(cryptvector);
continue;
}
diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c
index 7ebcdd7123..573706e251 100644
--- a/src/backend/libpq/crypt.c
+++ b/src/backend/libpq/crypt.c
@@ -34,7 +34,7 @@
* sent to the client, to avoid giving away user information!
*/
char *
-get_role_password(const char *role, char **logdetail)
+get_role_password(const char *role, const char **logdetail)
{
TimestampTz vuntil = 0;
HeapTuple roleTup;
@@ -116,6 +116,7 @@ encrypt_password(PasswordType target_type, const char *role,
{
PasswordType guessed_type = get_password_type(password);
char *encrypted_password;
+ const char *errstr = NULL;
if (guessed_type != PASSWORD_TYPE_PLAINTEXT)
{
@@ -132,8 +133,8 @@ encrypt_password(PasswordType target_type, const char *role,
encrypted_password = palloc(MD5_PASSWD_LEN + 1);
if (!pg_md5_encrypt(password, role, strlen(role),
- encrypted_password))
- elog(ERROR, "password encryption failed");
+ encrypted_password, &errstr))
+ elog(ERROR, "password encryption failed: %s", errstr);
return encrypted_password;
case PASSWORD_TYPE_SCRAM_SHA_256:
@@ -159,17 +160,19 @@ encrypt_password(PasswordType target_type, const char *role,
* 'client_pass' is the response given by the remote user to the MD5 challenge.
* 'md5_salt' is the salt used in the MD5 authentication challenge.
*
- * In the error case, optionally store a palloc'd string at *logdetail
- * that will be sent to the postmaster log (but not the client).
+ * In the error case, optionally save a string at *logdetail that will be
+ * sent to the postmaster log (but not the client). Note that this may
+ * not be palloc()'d.
*/
int
md5_crypt_verify(const char *role, const char *shadow_pass,
const char *client_pass,
const char *md5_salt, int md5_salt_len,
- char **logdetail)
+ const char **logdetail)
{
int retval;
char crypt_pwd[MD5_PASSWD_LEN + 1];
+ const char *errstr = NULL;
Assert(md5_salt_len > 0);
@@ -183,16 +186,18 @@ md5_crypt_verify(const char *role, const char *shadow_pass,
/*
* Compute the correct answer for the MD5 challenge.
- *
- * We do not bother setting logdetail for any pg_md5_encrypt failure
- * below: the only possible error is out-of-memory, which is unlikely, and
- * if it did happen adding a psprintf call would only make things worse.
*/
/* stored password already encrypted, only do salt */
if (!pg_md5_encrypt(shadow_pass + strlen("md5"),
md5_salt, md5_salt_len,
- crypt_pwd))
+ crypt_pwd, &errstr))
{
+ /*
+ * Note that this is not palloc()'d. One possible outcome here
+ * is an out-of-memory error, so adding an extra psprintf call
+ * would only make things worse.
+ */
+ *logdetail = errstr;
return STATUS_ERROR;
}
@@ -215,15 +220,17 @@ md5_crypt_verify(const char *role, const char *shadow_pass,
* pg_authid.rolpassword.
* 'client_pass' is the password given by the remote user.
*
- * In the error case, optionally store a palloc'd string at *logdetail
- * that will be sent to the postmaster log (but not the client).
+ * In the error case, optionally store a string at *logdetail that will be
+ * sent to the postmaster log (but not the client). Note that this may not
+ * be palloc()'d.
*/
int
plain_crypt_verify(const char *role, const char *shadow_pass,
const char *client_pass,
- char **logdetail)
+ const char **logdetail)
{
char crypt_client_pass[MD5_PASSWD_LEN + 1];
+ const char *errstr = NULL;
/*
* Client sent password in plaintext. If we have an MD5 hash stored, hash
@@ -251,14 +258,15 @@ plain_crypt_verify(const char *role, const char *shadow_pass,
if (!pg_md5_encrypt(client_pass,
role,
strlen(role),
- crypt_client_pass))
+ crypt_client_pass,
+ &errstr))
{
/*
- * We do not bother setting logdetail for pg_md5_encrypt
- * failure: the only possible error is out-of-memory, which is
- * unlikely, and if it did happen adding a psprintf call would
- * only make things worse.
+ * Note that this is not palloc()'d. One possible outcome
+ * here is an out-of-memory error, so adding an extra
+ * psprintf call would only make things worse.
*/
+ *logdetail = errstr;
return STATUS_ERROR;
}
if (strcmp(crypt_client_pass, shadow_pass) == 0)
diff --git a/src/backend/replication/backup_manifest.c b/src/backend/replication/backup_manifest.c
index 1678b89266..d47ab4c41e 100644
--- a/src/backend/replication/backup_manifest.c
+++ b/src/backend/replication/backup_manifest.c
@@ -68,7 +68,8 @@ InitializeBackupManifest(backup_manifest_info *manifest,
manifest->buffile = BufFileCreateTemp(false);
manifest->manifest_ctx = pg_cryptohash_create(PG_SHA256);
if (pg_cryptohash_init(manifest->manifest_ctx) < 0)
- elog(ERROR, "failed to initialize checksum of backup manifest");
+ elog(ERROR, "failed to initialize checksum of backup manifest: %s",
+ pg_cryptohash_error(manifest->manifest_ctx));
}
manifest->manifest_size = UINT64CONST(0);
@@ -334,7 +335,8 @@ SendBackupManifest(backup_manifest_info *manifest, bbsink *sink)
manifest->still_checksumming = false;
if (pg_cryptohash_final(manifest->manifest_ctx, checksumbuf,
sizeof(checksumbuf)) < 0)
- elog(ERROR, "failed to finalize checksum of backup manifest");
+ elog(ERROR, "failed to finalize checksum of backup manifest: %s",
+ pg_cryptohash_error(manifest->manifest_ctx));
AppendStringToManifest(manifest, "\"Manifest-Checksum\": \"");
hex_encode((char *) checksumbuf, sizeof checksumbuf, checksumstringbuf);
@@ -391,7 +393,8 @@ AppendStringToManifest(backup_manifest_info *manifest, char *s)
if (manifest->still_checksumming)
{
if (pg_cryptohash_update(manifest->manifest_ctx, (uint8 *) s, len) < 0)
- elog(ERROR, "failed to update checksum of backup manifest");
+ elog(ERROR, "failed to update checksum of backup manifest: %s",
+ pg_cryptohash_error(manifest->manifest_ctx));
}
BufFileWrite(manifest->buffile, s, len);
manifest->manifest_size += len;
diff --git a/src/backend/utils/adt/cryptohashfuncs.c b/src/backend/utils/adt/cryptohashfuncs.c
index 7e43c0e01d..03d84ea217 100644
--- a/src/backend/utils/adt/cryptohashfuncs.c
+++ b/src/backend/utils/adt/cryptohashfuncs.c
@@ -35,15 +35,17 @@ md5_text(PG_FUNCTION_ARGS)
text *in_text = PG_GETARG_TEXT_PP(0);
size_t len;
char hexsum[MD5_HASH_LEN + 1];
+ const char *errstr = NULL;
/* Calculate the length of the buffer using varlena metadata */
len = VARSIZE_ANY_EXHDR(in_text);
/* get the hash result */
- if (pg_md5_hash(VARDATA_ANY(in_text), len, hexsum) == false)
+ if (pg_md5_hash(VARDATA_ANY(in_text), len, hexsum, &errstr) == false)
ereport(ERROR,
- (errcode(ERRCODE_OUT_OF_MEMORY),
- errmsg("out of memory")));
+ (errcode(ERRCODE_INTERNAL_ERROR),
+ errmsg("could not compute %s hash: %s", "MD5",
+ errstr)));
/* convert to text and return it */
PG_RETURN_TEXT_P(cstring_to_text(hexsum));
@@ -58,12 +60,14 @@ md5_bytea(PG_FUNCTION_ARGS)
bytea *in = PG_GETARG_BYTEA_PP(0);
size_t len;
char hexsum[MD5_HASH_LEN + 1];
+ const char *errstr = NULL;
len = VARSIZE_ANY_EXHDR(in);
- if (pg_md5_hash(VARDATA_ANY(in), len, hexsum) == false)
+ if (pg_md5_hash(VARDATA_ANY(in), len, hexsum, &errstr) == false)
ereport(ERROR,
- (errcode(ERRCODE_OUT_OF_MEMORY),
- errmsg("out of memory")));
+ (errcode(ERRCODE_INTERNAL_ERROR),
+ errmsg("could not compute %s hash: %s", "MD5",
+ errstr)));
PG_RETURN_TEXT_P(cstring_to_text(hexsum));
}
@@ -111,12 +115,15 @@ cryptohash_internal(pg_cryptohash_type type, bytea *input)
ctx = pg_cryptohash_create(type);
if (pg_cryptohash_init(ctx) < 0)
- elog(ERROR, "could not initialize %s context", typestr);
+ elog(ERROR, "could not initialize %s context: %s", typestr,
+ pg_cryptohash_error(ctx));
if (pg_cryptohash_update(ctx, data, len) < 0)
- elog(ERROR, "could not update %s context", typestr);
+ elog(ERROR, "could not update %s context: %s", typestr,
+ pg_cryptohash_error(ctx));
if (pg_cryptohash_final(ctx, (unsigned char *) VARDATA(result),
digest_len) < 0)
- elog(ERROR, "could not finalize %s context", typestr);
+ elog(ERROR, "could not finalize %s context: %s", typestr,
+ pg_cryptohash_error(ctx));
pg_cryptohash_free(ctx);
SET_VARSIZE(result, digest_len + VARHDRSZ);
diff --git a/src/common/cryptohash.c b/src/common/cryptohash.c
index b13815dd98..8ff9eeb875 100644
--- a/src/common/cryptohash.c
+++ b/src/common/cryptohash.c
@@ -40,10 +40,18 @@
#define FREE(ptr) free(ptr)
#endif
+/* Set of error states */
+typedef enum pg_cryptohash_errno
+{
+ PG_CRYPTOHASH_ERROR_NONE = 0,
+ PG_CRYPTOHASH_ERROR_DEST_LEN
+} pg_cryptohash_errno;
+
/* Internal pg_cryptohash_ctx structure */
struct pg_cryptohash_ctx
{
pg_cryptohash_type type;
+ pg_cryptohash_errno error;
union
{
@@ -76,9 +84,10 @@ pg_cryptohash_create(pg_cryptohash_type type)
ctx = ALLOC(sizeof(pg_cryptohash_ctx));
if (ctx == NULL)
return NULL;
+
memset(ctx, 0, sizeof(pg_cryptohash_ctx));
ctx->type = type;
-
+ ctx->error = PG_CRYPTOHASH_ERROR_NONE;
return ctx;
}
@@ -174,32 +183,50 @@ pg_cryptohash_final(pg_cryptohash_ctx *ctx, uint8 *dest, size_t len)
{
case PG_MD5:
if (len < MD5_DIGEST_LENGTH)
+ {
+ ctx->error = PG_CRYPTOHASH_ERROR_DEST_LEN;
return -1;
+ }
pg_md5_final(&ctx->data.md5, dest);
break;
case PG_SHA1:
if (len < SHA1_DIGEST_LENGTH)
+ {
+ ctx->error = PG_CRYPTOHASH_ERROR_DEST_LEN;
return -1;
+ }
pg_sha1_final(&ctx->data.sha1, dest);
break;
case PG_SHA224:
if (len < PG_SHA224_DIGEST_LENGTH)
+ {
+ ctx->error = PG_CRYPTOHASH_ERROR_DEST_LEN;
return -1;
+ }
pg_sha224_final(&ctx->data.sha224, dest);
break;
case PG_SHA256:
if (len < PG_SHA256_DIGEST_LENGTH)
+ {
+ ctx->error = PG_CRYPTOHASH_ERROR_DEST_LEN;
return -1;
+ }
pg_sha256_final(&ctx->data.sha256, dest);
break;
case PG_SHA384:
if (len < PG_SHA384_DIGEST_LENGTH)
+ {
+ ctx->error = PG_CRYPTOHASH_ERROR_DEST_LEN;
return -1;
+ }
pg_sha384_final(&ctx->data.sha384, dest);
break;
case PG_SHA512:
if (len < PG_SHA512_DIGEST_LENGTH)
+ {
+ ctx->error = PG_CRYPTOHASH_ERROR_DEST_LEN;
return -1;
+ }
pg_sha512_final(&ctx->data.sha512, dest);
break;
}
@@ -221,3 +248,31 @@ pg_cryptohash_free(pg_cryptohash_ctx *ctx)
explicit_bzero(ctx, sizeof(pg_cryptohash_ctx));
FREE(ctx);
}
+
+/*
+ * pg_cryptohash_error
+ *
+ * Returns a static string providing errors about an error that
+ * happened during a computation.
+ */
+const char *
+pg_cryptohash_error(pg_cryptohash_ctx *ctx)
+{
+ /*
+ * This implementation would never fail because of an out-of-memory
+ * error, except when creating the context.
+ */
+ if (ctx == NULL)
+ return _("out of memory");
+
+ switch (ctx->error)
+ {
+ case PG_CRYPTOHASH_ERROR_NONE:
+ return _("success");
+ case PG_CRYPTOHASH_ERROR_DEST_LEN:
+ return _("destination buffer too small");
+ }
+
+ Assert(false);
+ return _("success");
+}
diff --git a/src/common/cryptohash_openssl.c b/src/common/cryptohash_openssl.c
index f7b390ed54..360deec258 100644
--- a/src/common/cryptohash_openssl.c
+++ b/src/common/cryptohash_openssl.c
@@ -21,6 +21,7 @@
#include "postgres_fe.h"
#endif
+#include <openssl/err.h>
#include <openssl/evp.h>
#include "common/cryptohash.h"
@@ -46,6 +47,14 @@
#define FREE(ptr) free(ptr)
#endif
+/* Set of error states */
+typedef enum pg_cryptohash_errno
+{
+ PG_CRYPTOHASH_ERROR_NONE = 0,
+ PG_CRYPTOHASH_ERROR_DEST_LEN,
+ PG_CRYPTOHASH_ERROR_OPENSSL
+} pg_cryptohash_errno;
+
/*
* Internal pg_cryptohash_ctx structure.
*
@@ -55,6 +64,8 @@
struct pg_cryptohash_ctx
{
pg_cryptohash_type type;
+ pg_cryptohash_errno error;
+ const char *errreason;
EVP_MD_CTX *evpctx;
@@ -63,6 +74,19 @@ struct pg_cryptohash_ctx
#endif
};
+static const char *
+SSLerrmessage(unsigned long ecode)
+{
+ if (ecode == 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_cryptohash_create
*
@@ -88,6 +112,8 @@ pg_cryptohash_create(pg_cryptohash_type type)
return NULL;
memset(ctx, 0, sizeof(pg_cryptohash_ctx));
ctx->type = type;
+ ctx->error = PG_CRYPTOHASH_ERROR_NONE;
+ ctx->errreason = NULL;
/*
* Initialization takes care of assigning the correct type for OpenSSL.
@@ -153,7 +179,11 @@ pg_cryptohash_init(pg_cryptohash_ctx *ctx)
/* OpenSSL internals return 1 on success, 0 on failure */
if (status <= 0)
+ {
+ ctx->errreason = SSLerrmessage(ERR_get_error());
+ ctx->error = PG_CRYPTOHASH_ERROR_OPENSSL;
return -1;
+ }
return 0;
}
@@ -174,7 +204,11 @@ pg_cryptohash_update(pg_cryptohash_ctx *ctx, const uint8 *data, size_t len)
/* OpenSSL internals return 1 on success, 0 on failure */
if (status <= 0)
+ {
+ ctx->errreason = SSLerrmessage(ERR_get_error());
+ ctx->error = PG_CRYPTOHASH_ERROR_OPENSSL;
return -1;
+ }
return 0;
}
@@ -195,27 +229,45 @@ pg_cryptohash_final(pg_cryptohash_ctx *ctx, uint8 *dest, size_t len)
{
case PG_MD5:
if (len < MD5_DIGEST_LENGTH)
+ {
+ ctx->error = PG_CRYPTOHASH_ERROR_DEST_LEN;
return -1;
+ }
break;
case PG_SHA1:
if (len < SHA1_DIGEST_LENGTH)
+ {
+ ctx->error = PG_CRYPTOHASH_ERROR_DEST_LEN;
return -1;
+ }
break;
case PG_SHA224:
if (len < PG_SHA224_DIGEST_LENGTH)
+ {
+ ctx->error = PG_CRYPTOHASH_ERROR_DEST_LEN;
return -1;
+ }
break;
case PG_SHA256:
if (len < PG_SHA256_DIGEST_LENGTH)
+ {
+ ctx->error = PG_CRYPTOHASH_ERROR_DEST_LEN;
return -1;
+ }
break;
case PG_SHA384:
if (len < PG_SHA384_DIGEST_LENGTH)
+ {
+ ctx->error = PG_CRYPTOHASH_ERROR_DEST_LEN;
return -1;
+ }
break;
case PG_SHA512:
if (len < PG_SHA512_DIGEST_LENGTH)
+ {
+ ctx->error = PG_CRYPTOHASH_ERROR_DEST_LEN;
return -1;
+ }
break;
}
@@ -223,7 +275,11 @@ pg_cryptohash_final(pg_cryptohash_ctx *ctx, uint8 *dest, size_t len)
/* OpenSSL internals return 1 on success, 0 on failure */
if (status <= 0)
+ {
+ ctx->errreason = SSLerrmessage(ERR_get_error());
+ ctx->error = PG_CRYPTOHASH_ERROR_OPENSSL;
return -1;
+ }
return 0;
}
@@ -248,3 +304,40 @@ pg_cryptohash_free(pg_cryptohash_ctx *ctx)
explicit_bzero(ctx, sizeof(pg_cryptohash_ctx));
FREE(ctx);
}
+
+/*
+ * pg_cryptohash_error
+ *
+ * Returns a static string providing errors about an error that
+ * happened during a computation.
+ */
+const char *
+pg_cryptohash_error(pg_cryptohash_ctx *ctx)
+{
+ /*
+ * This implementation would never fail because of an out-of-memory
+ * error, except when creating the context.
+ */
+ if (ctx == 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_CRYPTOHASH_ERROR_NONE:
+ return _("success");
+ case PG_CRYPTOHASH_ERROR_DEST_LEN:
+ return _("destination buffer too small");
+ case PG_CRYPTOHASH_ERROR_OPENSSL:
+ return _("OpenSSL failure");
+ }
+
+ Assert(false); /* cannot be reached */
+ return _("success");
+}
diff --git a/src/common/md5_common.c b/src/common/md5_common.c
index 65b0be4b66..edc935eeb8 100644
--- a/src/common/md5_common.c
+++ b/src/common/md5_common.c
@@ -67,7 +67,7 @@ bytesToHex(uint8 b[16], char *s)
*/
bool
-pg_md5_hash(const void *buff, size_t len, char *hexsum)
+pg_md5_hash(const void *buff, size_t len, char *hexsum, const char **errstr)
{
uint8 sum[MD5_DIGEST_LENGTH];
pg_cryptohash_ctx *ctx;
@@ -80,6 +80,7 @@ pg_md5_hash(const void *buff, size_t len, char *hexsum)
pg_cryptohash_update(ctx, buff, len) < 0 ||
pg_cryptohash_final(ctx, sum, sizeof(sum)) < 0)
{
+ *errstr = pg_cryptohash_error(ctx);
pg_cryptohash_free(ctx);
return false;
}
@@ -90,18 +91,23 @@ pg_md5_hash(const void *buff, size_t len, char *hexsum)
}
bool
-pg_md5_binary(const void *buff, size_t len, void *outbuf)
+pg_md5_binary(const void *buff, size_t len, void *outbuf, const char **errstr)
{
pg_cryptohash_ctx *ctx;
+ *errstr = NULL;
ctx = pg_cryptohash_create(PG_MD5);
if (ctx == NULL)
+ {
+ *errstr = pg_cryptohash_error(NULL); /* returns OOM */
return false;
+ }
if (pg_cryptohash_init(ctx) < 0 ||
pg_cryptohash_update(ctx, buff, len) < 0 ||
pg_cryptohash_final(ctx, outbuf, MD5_DIGEST_LENGTH) < 0)
{
+ *errstr = pg_cryptohash_error(ctx);
pg_cryptohash_free(ctx);
return false;
}
@@ -118,11 +124,12 @@ pg_md5_binary(const void *buff, size_t len, void *outbuf)
* Output format is "md5" followed by a 32-hex-digit MD5 checksum.
* Hence, the output buffer "buf" must be at least 36 bytes long.
*
- * Returns true if okay, false on error (out of memory).
+ * Returns true if okay, false on error with *errstr providing some
+ * error context.
*/
bool
pg_md5_encrypt(const char *passwd, const char *salt, size_t salt_len,
- char *buf)
+ char *buf, const char **errstr)
{
size_t passwd_len = strlen(passwd);
@@ -131,7 +138,10 @@ pg_md5_encrypt(const char *passwd, const char *salt, size_t salt_len,
bool ret;
if (!crypt_buf)
+ {
+ *errstr = _("out of memory");
return false;
+ }
/*
* Place salt at the end because it may be known by users trying to crack
@@ -141,7 +151,7 @@ pg_md5_encrypt(const char *passwd, const char *salt, size_t salt_len,
memcpy(crypt_buf + passwd_len, salt, salt_len);
strcpy(buf, "md5");
- ret = pg_md5_hash(crypt_buf, passwd_len + salt_len, buf + 3);
+ ret = pg_md5_hash(crypt_buf, passwd_len + salt_len, buf + 3, errstr);
free(crypt_buf);
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 8d2e4e5db4..c8965b345f 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -790,6 +790,7 @@ pg_password_sendauth(PGconn *conn, const char *password, AuthRequest areq)
case AUTH_REQ_MD5:
{
char *crypt_pwd2;
+ const char *errstr;
/* Allocate enough space for two MD5 hashes */
crypt_pwd = malloc(2 * (MD5_PASSWD_LEN + 1));
@@ -802,14 +803,21 @@ pg_password_sendauth(PGconn *conn, const char *password, AuthRequest areq)
crypt_pwd2 = crypt_pwd + MD5_PASSWD_LEN + 1;
if (!pg_md5_encrypt(password, conn->pguser,
- strlen(conn->pguser), crypt_pwd2))
+ strlen(conn->pguser), crypt_pwd2,
+ &errstr))
{
+ appendPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("could not encrypt password: %s\n"),
+ errstr);
free(crypt_pwd);
return STATUS_ERROR;
}
if (!pg_md5_encrypt(crypt_pwd2 + strlen("md5"), md5Salt,
- 4, crypt_pwd))
+ 4, crypt_pwd, &errstr))
{
+ appendPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("could not encrypt password: %s\n"),
+ errstr);
free(crypt_pwd);
return STATUS_ERROR;
}
@@ -1175,12 +1183,13 @@ char *
PQencryptPassword(const char *passwd, const char *user)
{
char *crypt_pwd;
+ const char *errstr = NULL;
crypt_pwd = malloc(MD5_PASSWD_LEN + 1);
if (!crypt_pwd)
return NULL;
- if (!pg_md5_encrypt(passwd, user, strlen(user), crypt_pwd))
+ if (!pg_md5_encrypt(passwd, user, strlen(user), crypt_pwd, &errstr))
{
free(crypt_pwd);
return NULL;
@@ -1287,8 +1296,13 @@ PQencryptPasswordConn(PGconn *conn, const char *passwd, const char *user,
crypt_pwd = malloc(MD5_PASSWD_LEN + 1);
if (crypt_pwd)
{
- if (!pg_md5_encrypt(passwd, user, strlen(user), crypt_pwd))
+ const char *errstr = NULL;
+
+ if (!pg_md5_encrypt(passwd, user, strlen(user), crypt_pwd, &errstr))
{
+ appendPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("could not encrypt password: %s\n"),
+ errstr);
free(crypt_pwd);
crypt_pwd = NULL;
}
diff --git a/contrib/uuid-ossp/uuid-ossp.c b/contrib/uuid-ossp/uuid-ossp.c
index 4859bd1933..7c0fb812fd 100644
--- a/contrib/uuid-ossp/uuid-ossp.c
+++ b/contrib/uuid-ossp/uuid-ossp.c
@@ -319,14 +319,17 @@ uuid_generate_internal(int v, unsigned char *ns, const char *ptr, int len)
pg_cryptohash_ctx *ctx = pg_cryptohash_create(PG_MD5);
if (pg_cryptohash_init(ctx) < 0)
- elog(ERROR, "could not initialize %s context", "MD5");
+ elog(ERROR, "could not initialize %s context: %s", "MD5",
+ pg_cryptohash_error(ctx));
if (pg_cryptohash_update(ctx, ns, sizeof(uu)) < 0 ||
pg_cryptohash_update(ctx, (unsigned char *) ptr, len) < 0)
- elog(ERROR, "could not update %s context", "MD5");
+ elog(ERROR, "could not update %s context: %s", "MD5",
+ pg_cryptohash_error(ctx));
/* we assume sizeof MD5 result is 16, same as UUID size */
if (pg_cryptohash_final(ctx, (unsigned char *) &uu,
sizeof(uu)) < 0)
- elog(ERROR, "could not finalize %s context", "MD5");
+ elog(ERROR, "could not finalize %s context: %s", "MD5",
+ pg_cryptohash_error(ctx));
pg_cryptohash_free(ctx);
}
else
@@ -335,12 +338,15 @@ uuid_generate_internal(int v, unsigned char *ns, const char *ptr, int len)
unsigned char sha1result[SHA1_DIGEST_LENGTH];
if (pg_cryptohash_init(ctx) < 0)
- elog(ERROR, "could not initialize %s context", "SHA1");
+ elog(ERROR, "could not initialize %s context: %s", "SHA1",
+ pg_cryptohash_error(ctx));
if (pg_cryptohash_update(ctx, ns, sizeof(uu)) < 0 ||
pg_cryptohash_update(ctx, (unsigned char *) ptr, len) < 0)
- elog(ERROR, "could not update %s context", "SHA1");
+ elog(ERROR, "could not update %s context: %s", "SHA1",
+ pg_cryptohash_error(ctx));
if (pg_cryptohash_final(ctx, sha1result, sizeof(sha1result)) < 0)
- elog(ERROR, "could not finalize %s context", "SHA1");
+ elog(ERROR, "could not finalize %s context: %s", "SHA1",
+ pg_cryptohash_error(ctx));
pg_cryptohash_free(ctx);
memcpy(&uu, sha1result, sizeof(uu));
--
2.34.1
Attachment:
signature.asc
Description: PGP signature
