Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 6e34755

Browse files
committed
Avoid corner-case memory leak in SSL parameter processing.
After reading the root cert list from the ssl_ca_file, immediately install it as client CA list of the new SSL context. That gives the SSL context ownership of the list, so that SSL_CTX_free will free it. This avoids a permanent memory leak if we fail further down in be_tls_init(), which could happen if bogus CRL data is offered. The leak could only amount to something if the CRL parameters get broken after server start (else we'd just quit) and then the server is SIGHUP'd many times without fixing the CRL data. That's rather unlikely perhaps, but it seems worth fixing, if only because the code is clearer this way. While we're here, add some comments about the memory management aspects of this logic. Noted by Jelte Fennema and independently by Andres Freund. Back-patch to v10; before commit de41869 it doesn't matter, since we'd not re-execute this code during SIGHUP. Discussion: https://postgr.es/m/16160-18367e56e9a28264@postgresql.org
1 parent 60e5c2b commit 6e34755

File tree

1 file changed

+26
-21
lines changed

1 file changed

+26
-21
lines changed

src/backend/libpq/be-secure-openssl.c

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,6 @@ static int SSL_CTX_set_max_proto_version(SSL_CTX *ctx, int version);
8282
int
8383
be_tls_init(bool isServerStart)
8484
{
85-
STACK_OF(X509_NAME) *root_cert_list = NULL;
8685
SSL_CTX *context;
8786

8887
/* This stuff need be done only once. */
@@ -99,6 +98,10 @@ be_tls_init(bool isServerStart)
9998
}
10099

101100
/*
101+
* Create a new SSL context into which we'll load all the configuration
102+
* settings. If we fail partway through, we can avoid memory leakage by
103+
* freeing this context; we don't install it as active until the end.
104+
*
102105
* We use SSLv23_method() because it can negotiate use of the highest
103106
* mutually supported protocol version, while alternatives like
104107
* TLSv1_2_method() permit only one specific version. Note that we don't
@@ -254,6 +257,8 @@ be_tls_init(bool isServerStart)
254257
*/
255258
if (ssl_ca_file[0])
256259
{
260+
STACK_OF(X509_NAME) * root_cert_list;
261+
257262
if (SSL_CTX_load_verify_locations(context, ssl_ca_file, NULL) != 1 ||
258263
(root_cert_list = SSL_load_client_CA_file(ssl_ca_file)) == NULL)
259264
{
@@ -263,6 +268,25 @@ be_tls_init(bool isServerStart)
263268
ssl_ca_file, SSLerrmessage(ERR_get_error()))));
264269
goto error;
265270
}
271+
272+
/*
273+
* Tell OpenSSL to send the list of root certs we trust to clients in
274+
* CertificateRequests. This lets a client with a keystore select the
275+
* appropriate client certificate to send to us. Also, this ensures
276+
* that the SSL context will "own" the root_cert_list and remember to
277+
* free it when no longer needed.
278+
*/
279+
SSL_CTX_set_client_CA_list(context, root_cert_list);
280+
281+
/*
282+
* Always ask for SSL client cert, but don't fail if it's not
283+
* presented. We might fail such connections later, depending on what
284+
* we find in pg_hba.conf.
285+
*/
286+
SSL_CTX_set_verify(context,
287+
(SSL_VERIFY_PEER |
288+
SSL_VERIFY_CLIENT_ONCE),
289+
verify_cb);
266290
}
267291

268292
/*----------
@@ -302,26 +326,6 @@ be_tls_init(bool isServerStart)
302326
}
303327
}
304328

305-
if (ssl_ca_file[0])
306-
{
307-
/*
308-
* Always ask for SSL client cert, but don't fail if it's not
309-
* presented. We might fail such connections later, depending on what
310-
* we find in pg_hba.conf.
311-
*/
312-
SSL_CTX_set_verify(context,
313-
(SSL_VERIFY_PEER |
314-
SSL_VERIFY_CLIENT_ONCE),
315-
verify_cb);
316-
317-
/*
318-
* Tell OpenSSL to send the list of root certs we trust to clients in
319-
* CertificateRequests. This lets a client with a keystore select the
320-
* appropriate client certificate to send to us.
321-
*/
322-
SSL_CTX_set_client_CA_list(context, root_cert_list);
323-
}
324-
325329
/*
326330
* Success! Replace any existing SSL_context.
327331
*/
@@ -340,6 +344,7 @@ be_tls_init(bool isServerStart)
340344

341345
return 0;
342346

347+
/* Clean up by releasing working context. */
343348
error:
344349
if (context)
345350
SSL_CTX_free(context);

0 commit comments

Comments
 (0)