From 3141b29053a4b48289ebb7ee26d22e395c07ff31 Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Tue, 15 Jun 2021 15:59:39 -0700 Subject: [PATCH 3/3] nss: clean up leaked resources The NSPR file descriptor and a handful of certificate references were preventing NSS from shutting down. At least one leak is still on the loose, when forcing TLSv1.2 or lower during client connections: $ psql 'host=localhost sslmode=require ssl_max_protocol_version=TLSv1.2' psql (14beta1) SSL connection (protocol: TLSv1.2, cipher: TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, bits: 128, compression: off) Type "help" for help. postgres=# \q unable to shut down NSS context: NSS could not shutdown. Objects are still in use. --- src/interfaces/libpq/fe-secure-nss.c | 37 ++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/src/interfaces/libpq/fe-secure-nss.c b/src/interfaces/libpq/fe-secure-nss.c index 90f57e0d99..7b92e89ecd 100644 --- a/src/interfaces/libpq/fe-secure-nss.c +++ b/src/interfaces/libpq/fe-secure-nss.c @@ -137,6 +137,22 @@ pgtls_init(PGconn *conn, bool do_ssl, bool do_crypto) void pgtls_close(PGconn *conn) { + /* All NSS references must be cleaned up before we close out the context. */ + if (conn->pr_fd) + { + PRStatus status; + + status = PR_Close(conn->pr_fd); + if (status != PR_SUCCESS) + { + pqInternalNotice(&conn->noticeHooks, + "unable to close NSPR fd: %s", + pg_SSLerrmessage(PR_GetError())); + } + + conn->pr_fd = NULL; + } + if (conn->nss_context) { SECStatus status; @@ -573,16 +589,16 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len) SECOidTag signature_tag; SECOidTag digest_alg; int digest_len; - PLArenaPool *arena; + PLArenaPool *arena = NULL; SECItem digest; - char *ret; + char *ret = NULL; SECStatus status; *len = 0; server_cert = SSL_PeerCertificate(conn->pr_fd); if (!server_cert) - return NULL; + goto cleanup; signature_tag = SECOID_GetAlgorithmTag(&server_cert->signature); if (!pg_find_signature_algorithm(signature_tag, &digest_alg, &digest_len)) @@ -590,7 +606,7 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len) printfPQExpBuffer(&conn->errorMessage, libpq_gettext("could not find digest for OID '%s'\n"), SECOID_FindOIDTagDescription(signature_tag)); - return NULL; + goto cleanup; } arena = PORT_NewArena(SEC_ASN1_DEFAULT_ARENA_SIZE); @@ -605,14 +621,18 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len) printfPQExpBuffer(&conn->errorMessage, libpq_gettext("unable to generate peer certificate digest: %s"), pg_SSLerrmessage(PR_GetError())); - PORT_FreeArena(arena, PR_TRUE); - return NULL; + goto cleanup; } ret = pg_malloc(digest.len); memcpy(ret, digest.data, digest.len); *len = digest_len; - PORT_FreeArena(arena, PR_TRUE); + +cleanup: + if (arena) + PORT_FreeArena(arena, PR_TRUE); + if (server_cert) + CERT_DestroyCertificate(server_cert); return ret; } @@ -712,6 +732,8 @@ done: /* san_list will be freed by freeing the arena it was allocated in */ if (arena) PORT_FreeArena(arena, PR_TRUE); + if (server_cert) + CERT_DestroyCertificate(server_cert); PR_Free(server_hostname); if (status == SECSuccess) @@ -826,6 +848,7 @@ pg_cert_auth_handler(void *arg, PRFileDesc *fd, PRBool checksig, PRBool isServer pg_SSLerrmessage(PR_GetError())); } + CERT_DestroyCertificate(server_cert); return status; } -- 2.25.1