Tom Lane wrote:
> Magnus Hagander <mag...@hagander.net> writes:
>> On 22 jun 2009, at 18.05, Tom Lane <t...@sss.pgh.pa.us> wrote:
>>> I'm also a bit concerned about wrapping a struct
>>> field inside such an #if, as that's likely to cause hard-to-debug
>>> problems if two compilations read libpq-int.h with different #define
>>> environments.
> 
>> Since it's a pointer we could just declare it as void, no? Or even #if  
>> between void and "real" pointer. Wouldn't that be safe?
> 
> Yeah, on reflection I think it'd be safe to do
> 
> #if (SSLEAY_VERSION_NUMBER >= 0x00907000L) && !defined(OPENSSL_NO_ENGINE)
>       ENGINE          *ptr;
> #else
>       /* dummy field to keep struct the same if OpenSSL version changes */
>       void            *ptr;
> #endif
> 
> (probably that #if should get wrapped up in an additional #define
> instead of being duplicated in several places...)

Attached is an updated patch that does both of these.


>> (we already have a similar issue with the whole #ifdef use_ssl, don't  
>> we? But having one isn't an excuse to create another of course…)
> 
> That's controlled by an entry in pg_config.h, though, and won't change
> if someone happens to update their openssl install and then recompile
> only parts of libpq.

True, agreed.

-- 
 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
***************
*** 31,36 ****
--- 31,37 ----
  #include "libpq-fe.h"
  #include "fe-auth.h"
  #include "pqsignal.h"
+ #include "libpq-int.h"
  
  #ifdef WIN32
  #include "win32.h"
***************
*** 62,68 ****
  #if (SSLEAY_VERSION_NUMBER >= 0x00907000L)
  #include <openssl/conf.h>
  #endif
! #if (SSLEAY_VERSION_NUMBER >= 0x00907000L) && !defined(OPENSSL_NO_ENGINE)
  #include <openssl/engine.h>
  #endif
  
--- 63,69 ----
  #if (SSLEAY_VERSION_NUMBER >= 0x00907000L)
  #include <openssl/conf.h>
  #endif
! #ifdef USE_SSL_ENGINE
  #include <openssl/engine.h>
  #endif
  
***************
*** 661,667 **** client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey)
  	 */
  	if (conn->sslkey && strlen(conn->sslkey) > 0)
  	{
! #if (SSLEAY_VERSION_NUMBER >= 0x00907000L) && !defined(OPENSSL_NO_ENGINE)
  		if (strchr(conn->sslkey, ':')
  #ifdef WIN32
  			&& conn->sslkey[1] != ':'
--- 662,668 ----
  	 */
  	if (conn->sslkey && strlen(conn->sslkey) > 0)
  	{
! #ifdef USE_SSL_ENGINE
  		if (strchr(conn->sslkey, ':')
  #ifdef WIN32
  			&& conn->sslkey[1] != ':'
***************
*** 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();
  
--- 670,683 ----
  			)
  		{
  			/* 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)
  			{
--- 690,711 ----
  				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)
--- 715,723 ----
  								  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)
--- 1235,1249 ----
  		X509_free(conn->peer);
  		conn->peer = NULL;
  	}
+ 
+ #ifdef USE_SSL_ENGINE
+ 	if (conn->engine)
+ 	{
+ 		ENGINE_finish(conn->engine);
+ 		ENGINE_free(conn->engine);
+ 		conn->engine = NULL;
+ 	}
+ #endif
  }
  
  /*
*** a/src/interfaces/libpq/libpq-int.h
--- b/src/interfaces/libpq/libpq-int.h
***************
*** 76,83 **** typedef struct
--- 76,88 ----
  #ifdef USE_SSL
  #include <openssl/ssl.h>
  #include <openssl/err.h>
+ 
+ #if (SSLEAY_VERSION_NUMBER >= 0x00907000L) && !defined(OPENSSL_NO_ENGINE)
+ #define USE_SSL_ENGINE
  #endif
  
+ #endif /* USE_SSL */
+ 
  /*
   * POSTGRES backend dependent Constants.
   */
***************
*** 383,389 **** struct pg_conn
--- 388,400 ----
  	X509	   *peer;			/* X509 cert of server */
  	char		peer_dn[256 + 1];		/* peer distinguished name */
  	char		peer_cn[SM_USER + 1];	/* peer common name */
+ #ifdef USE_SSL_ENGINE
+ 	ENGINE	   *engine;			/* SSL engine, if any */
+ #else
+ 	void	   *engine;			/* dummy field to keep struct the same
+ 								   if OpenSSL version changes */
  #endif
+ #endif /* USE_SSL */
  
  #ifdef ENABLE_GSS
  	gss_ctx_id_t gctx;			/* GSS context */
-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

Reply via email to