Hi Hackers,
When configuring SSL on the Postgres server side with the following
information:
ssl = on
ssl_ca_file = 'root_ca.crt'
ssl_cert_file = 'server-cn-only.crt'
ssl_key_file = 'server-cn-only.key'
If a user makes a mistake, for example, accidentally using 'root_ca.crl'
instead of 'root_ca.crt', Postgres will report an error like the one below:
FATAL: could not load root certificate file "root_ca.crl": SSL error
code 2147483650
Here, the error code 2147483650 is not very helpful for the user. The
reason is that Postgres is missing the initial SSL file check before
passing it to the OpenSSL API. For example:
if (ssl_ca_file[0])
{
STACK_OF(X509_NAME) * root_cert_list;
if (SSL_CTX_load_verify_locations(context, ssl_ca_file, NULL)
!= 1 ||
(root_cert_list = SSL_load_client_CA_file(ssl_ca_file)) ==
NULL)
{
ereport(isServerStart ? FATAL : LOG,
(errcode(ERRCODE_CONFIG_FILE_ERROR),
errmsg("could not load root certificate file
\"%s\": %s",
ssl_ca_file, SSLerrmessage(ERR_get_error()))));
goto error;
}
The SSL_CTX_load_verify_locations function in OpenSSL will return NULL
if there is a system error, such as "No such file or directory" in this
case:
const char *ERR_reason_error_string(unsigned long e)
{
ERR_STRING_DATA d, *p = NULL;
unsigned long l, r;
if (!RUN_ONCE(&err_string_init, do_err_strings_init)) {
return NULL;
}
/*
* ERR_reason_error_string() can't safely return system error strings,
* since openssl_strerror_r() needs a buffer for thread safety, and we
* haven't got one that would serve any sensible purpose.
*/
if (ERR_SYSTEM_ERROR(e))
return NULL;
It would be better to perform a simple SSL file check before passing the
SSL file to OpenSSL APIs so that the system error can be captured and a
meaningful message provided to the end user.
Attached is a simple patch to help address this issue for ssl_ca_file,
ssl_cert_file, and ssl_crl_file. With this patch, a similar test will
return something like the message below:
FATAL: could not access certificate file "root_ca.crl": No such file or
directory
I believe this can help end users quickly realize the mistake.
Thank you,
David
From bb6264791aab6a3e8150704fc8f1c8774c27018d Mon Sep 17 00:00:00 2001
From: David Zhang <idraw...@gmail.com>
Date: Wed, 6 Mar 2024 15:51:11 -0800
Subject: [PATCH] improve ssl files error code
---
src/backend/libpq/be-secure-common.c | 19 +++++++++++++++++++
src/backend/libpq/be-secure-openssl.c | 10 ++++++++++
src/include/libpq/libpq.h | 1 +
3 files changed, 30 insertions(+)
diff --git a/src/backend/libpq/be-secure-common.c
b/src/backend/libpq/be-secure-common.c
index 0582606192..01d567cbfc 100644
--- a/src/backend/libpq/be-secure-common.c
+++ b/src/backend/libpq/be-secure-common.c
@@ -102,7 +102,26 @@ error:
return len;
}
+/*
+ * Check SSL certificate files.
+ */
+bool
+check_ssl_file(const char *ssl_file, bool isServerStart)
+{
+ int loglevel = isServerStart ? FATAL : LOG;
+ struct stat buf;
+
+ if (stat(ssl_file, &buf) != 0)
+ {
+ ereport(loglevel,
+ (errcode_for_file_access(),
+ errmsg("could not access certificate file
\"%s\": %m",
+ ssl_file)));
+ return false;
+ }
+ return true;
+}
/*
* Check permissions for SSL key files.
*/
diff --git a/src/backend/libpq/be-secure-openssl.c
b/src/backend/libpq/be-secure-openssl.c
index e12b1cc9e3..c5d58545d9 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -144,6 +144,10 @@ be_tls_init(bool isServerStart)
/*
* Load and verify server's certificate and private key
*/
+
+ if (!check_ssl_file(ssl_cert_file, isServerStart))
+ goto error;
+
if (SSL_CTX_use_certificate_chain_file(context, ssl_cert_file) != 1)
{
ereport(isServerStart ? FATAL : LOG,
@@ -297,6 +301,9 @@ be_tls_init(bool isServerStart)
{
STACK_OF(X509_NAME) * root_cert_list;
+ if (!check_ssl_file(ssl_ca_file, isServerStart))
+ goto error;
+
if (SSL_CTX_load_verify_locations(context, ssl_ca_file, NULL)
!= 1 ||
(root_cert_list = SSL_load_client_CA_file(ssl_ca_file))
== NULL)
{
@@ -336,6 +343,9 @@ be_tls_init(bool isServerStart)
{
X509_STORE *cvstore = SSL_CTX_get_cert_store(context);
+ if (ssl_crl_file[0] && !check_ssl_file(ssl_crl_file,
isServerStart))
+ goto error;
+
if (cvstore)
{
/* Set the flags to check against the complete CRL
chain */
diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
index 6171a0d17a..a491f0caa9 100644
--- a/src/include/libpq/libpq.h
+++ b/src/include/libpq/libpq.h
@@ -140,5 +140,6 @@ extern int run_ssl_passphrase_command(const char *prompt,
bool is_server_start,
char
*buf, int size);
extern bool check_ssl_key_file_permissions(const char *ssl_key_file,
bool isServerStart);
+extern bool check_ssl_file(const char *ssl_file, bool isServerStart);
#endif /* LIBPQ_H */
--
2.34.1
