Lars Kanis wrote:
> The following bug has been logged online:
>
> Bug reference: 4869
> Logged by: Lars Kanis
> Email address: ka...@comcard.de
> PostgreSQL version: 8.4rc1
> Operating system: Linux c1170lx 2.6.24-23-generic #1 SMP Wed Apr 1
> 21:47:28 UTC 2009 i686 GNU/Linux
> Description: No proper initialization of OpenSSL-Engine in libpq
> Details:
>
> When using OpenSSL-engine pkcs11 with PGSSLKEY=pkcs11:id_45 the
> authentication to the PG-server fails with "engine not initialized".
>
> According to the OpenSSL-docs
> (http://www.openssl.org/docs/crypto/engine.html) the structural reference
> returned by ENGINE_by_id needs to be initialized first before use. The
> buildin engine doesn't need this, but most of external engines don't work
> otherwise.
>
> Moreover the structural and functional references should be freed in any
> case.
>
>
> The following patch solves the problem:
This looks good in generael to me. I remember looking at the engine code
wondering why we didn't do that, but since I don't have a good
environment to test that part in, I forgot about it :(
Shouldn't there be an ENGINE_free() in the error path of ENGINE_init()?
Should we not also call ENGINE_finish() and ENGINE_free() in the success
path of this code? Your patch adds it to the case where we didn't get
the private key, but what if we did? I assume they should also go
outside the error path, per the attached patch - or will that break
their usage?
Can you test that and verify that it doesn't break for you?
--
Magnus Hagander
Self: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index bfc0965..53ac526 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -690,6 +690,20 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
return 0;
}
+ if (ENGINE_init(engine_ptr) == 0)
+ {
+ char *err = SSLerrmessage();
+
+ printfPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("could not initialize SSL engine \"%s\": %s\n"),
+ engine_str, err);
+ SSLerrfree(err);
+ ENGINE_free(engine_ptr);
+ free(engine_str);
+ ERR_pop_to_mark();
+ return 0;
+ }
+
*pkey = ENGINE_load_private_key(engine_ptr, engine_colon,
NULL, NULL);
if (*pkey == NULL)
@@ -700,10 +714,14 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
libpq_gettext("could not read private SSL key \"%s\" from engine \"%s\": %s\n"),
engine_colon, engine_str, err);
SSLerrfree(err);
+ ENGINE_finish(engine_ptr);
+ ENGINE_free(engine_ptr);
free(engine_str);
ERR_pop_to_mark();
return 0;
}
+ ENGINE_finish(engine_ptr);
+ ENGINE_free(engine_ptr);
free(engine_str);
fnbuf[0] = '\0'; /* indicate we're not going to load from a
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
