Lars Kanis wrote:
>>> 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()?
> In the patch it is already there, isn't it?
Eh. So it is. Don't know where I got the idea that it didn't :-)
>> 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?
> That's right. I thought about it, but I don't know where the right place is.
>
>> Can you test that and verify that it doesn't break for you?
> It breaks with Segmentation fault in my smartcard-based setup. The
> pkey-handle
> is all we have from the engine, when client_cert_cb() is finished. Obviously
> the ref-counting of openssl does not take the pkey-handle into account, so we
> need to keep the engine_ptr for later freeing.
So we need to keep the engine initialized during this time? Ugh. We
don't currently carry around the engine pointer. I guess we have to.
> close_SSL() should be the right place for ENGINE_finish() and ENGINE_free() ?
Yup.
How about the attached patch? Does it work for you?
A question from that then, for others, is it Ok to add a field to the
PGconn structure during RC? :-) It's only in libpq-int.h, but? Comments?
Tom, perhaps?
--
Magnus Hagander
Self: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
*** a/src/interfaces/libpq/fe-secure.c
--- b/src/interfaces/libpq/fe-secure.c
***************
*** 669,683 **** client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
)
{
/* Colon, but not in second character, treat as engine:key */
- ENGINE *engine_ptr;
char *engine_str = strdup(conn->sslkey);
char *engine_colon = strchr(engine_str, ':');
*engine_colon = '\0'; /* engine_str now has engine name */
engine_colon++; /* engine_colon now has key name */
! engine_ptr = ENGINE_by_id(engine_str);
! if (engine_ptr == NULL)
{
char *err = SSLerrmessage();
--- 669,682 ----
)
{
/* Colon, but not in second character, treat as engine:key */
char *engine_str = strdup(conn->sslkey);
char *engine_colon = strchr(engine_str, ':');
*engine_colon = '\0'; /* engine_str now has engine name */
engine_colon++; /* engine_colon now has key name */
! conn->engine = ENGINE_by_id(engine_str);
! if (conn->engine == NULL)
{
char *err = SSLerrmessage();
***************
*** 690,696 **** client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
return 0;
}
! *pkey = ENGINE_load_private_key(engine_ptr, engine_colon,
NULL, NULL);
if (*pkey == NULL)
{
--- 689,710 ----
return 0;
}
! if (ENGINE_init(conn->engine) == 0)
! {
! char *err = SSLerrmessage();
!
! printfPQExpBuffer(&conn->errorMessage,
! libpq_gettext("could not initialize SSL engine \"%s\": %s\n"),
! engine_str, err);
! SSLerrfree(err);
! ENGINE_free(conn->engine);
! conn->engine = NULL;
! free(engine_str);
! ERR_pop_to_mark();
! return 0;
! }
!
! *pkey = ENGINE_load_private_key(conn->engine, engine_colon,
NULL, NULL);
if (*pkey == NULL)
{
***************
*** 700,705 **** client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
--- 714,722 ----
libpq_gettext("could not read private SSL key \"%s\" from engine \"%s\": %s\n"),
engine_colon, engine_str, err);
SSLerrfree(err);
+ ENGINE_finish(conn->engine);
+ ENGINE_free(conn->engine);
+ conn->engine = NULL;
free(engine_str);
ERR_pop_to_mark();
return 0;
***************
*** 1217,1222 **** close_SSL(PGconn *conn)
--- 1234,1246 ----
X509_free(conn->peer);
conn->peer = NULL;
}
+
+ if (conn->engine)
+ {
+ ENGINE_finish(conn->engine);
+ ENGINE_free(conn->engine);
+ conn->engine = NULL;
+ }
}
/*
*** a/src/interfaces/libpq/libpq-int.h
--- b/src/interfaces/libpq/libpq-int.h
***************
*** 383,388 **** struct pg_conn
--- 383,389 ----
X509 *peer; /* X509 cert of server */
char peer_dn[256 + 1]; /* peer distinguished name */
char peer_cn[SM_USER + 1]; /* peer common name */
+ ENGINE *engine; /* SSL engine, if any */
#endif
#ifdef ENABLE_GSS
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
