Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Peter Eisentraut wrote:
> On Friday 10 April 2009 08:39:33 Martin Pitt wrote:
> > Tom Lane [2009-04-10  1:15 -0400]:
> > > Martin Pitt  writesyuqhom#3:
> > > > The test suite detected one regression in libpq, though: Setting
> > > > $PGHOST now complains about a missing root.crt, although this is only
> > > > relevant on the server side (or did I misunderstood this?)
> > >
> > > No, that's a progression: the client wants to validate the server's
> > > cert, too.
> >
> > Indeed it is nice to see this feature (great to prevent spoofing), but
> > if I don't have a ~/.postgresql/root.crt at all, it shouldn't
> > certainly break completely? (which it does now).
> 
> I assume the server has the snakeoil certificate installed?  In that case, it 
> is correct that the client refuses to proceed, although the exact manner of 
> breaking could perhaps be improved.

I have developed a patch to more clearly explain the problem with a
missing client root.crt file:

	$ PGSSLVERIFY=cn sql -h localhost test
	psql: root certificate file "/u/postgres/.postgresql/root.crt" does not exist
	Either supply the file or set sslverify to "none" to disable server certificate verification.

	$ PGSSLVERIFY=none sql -h localhost test
	psql (8.4beta1)
	SSL connection (cipher: DHE-RSA-AES256-SHA, bits: 256)
	Type "help" for help.

I had to add a second error message line;  I didn't see us doing a
second line anywhere else in libpq, but it seemed to be the only
solution.  Should I use three lines?

-- 
  Bruce Momjian          http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: src/interfaces/libpq/fe-secure.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-secure.c,v
retrieving revision 1.122
diff -c -c -r1.122 fe-secure.c
*** src/interfaces/libpq/fe-secure.c	31 Mar 2009 01:41:27 -0000	1.122
--- src/interfaces/libpq/fe-secure.c	11 Apr 2009 19:32:47 -0000
***************
*** 1067,1073 ****
  		if (strcmp(conn->sslverify, "none") != 0)
  		{
  			printfPQExpBuffer(&conn->errorMessage,
! 							  libpq_gettext("root certificate file \"%s\" does not exist"), fnbuf);
  			return -1;
  		}
  	}
--- 1067,1074 ----
  		if (strcmp(conn->sslverify, "none") != 0)
  		{
  			printfPQExpBuffer(&conn->errorMessage,
! 							  libpq_gettext("root certificate file \"%s\" does not exist\n"
! 							  "Either supply the file or set sslverify to \"none\" to disable server certificate verification.\n"), fnbuf);
  			return -1;
  		}
  	}

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Bruce Momjian wrote:
> > That's the intention. When you're turning off something, I think it  
> > makes sense to use "no"....
> 
> But that doesn't scale:  sslmode currently has four options, soon
> perhaps to be six.   The idea is that the items should be of increasing
> security, and adding "no" in the middle doesn't allow that to be clear.
> 
> In fact there are too many sslmode options to list them in a paragraph; 
> it should be an SGML table;  I will work on that now.

OK, I have created an SGML table to show the sslmode options.  While
doing that I found that 'disable' was misstated as "attempt only an
unencrypted SSL connection".  Neither Magnus nor I know what an
unencrypted SSL connection is, so we assume it is an error.  I have
instead replaced it with "try a non-SSL connection".

When this was a paragraph it was hard to see that mistake; in an SGML
table it was obvious.

Patch attached and applied.

-- 
  Bruce Momjian          http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/libpq.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.283
diff -c -c -r1.283 libpq.sgml
*** doc/src/sgml/libpq.sgml	11 Apr 2009 16:46:54 -0000	1.283
--- doc/src/sgml/libpq.sgml	14 Apr 2009 20:37:02 -0000
***************
*** 254,276 ****
            
             This option determines whether or with what priority a
             SSL</> TCP/IP connection will be negotiated with the
!            server. There are four modes: disable</> will attempt
!            only an unencrypted SSL</> connection;
!            allow</> will negotiate, trying first a
!            non-SSL</> connection, then if that fails, trying an
!            SSL</> connection; prefer</> (the default)
!            will negotiate, trying first an SSL</> connection,
!            then if that fails, trying a regular non-SSL</>
!            connection; require</> will try only an
!            SSL</> connection.  sslmode</> is ignored
!            for Unix domain socket communication.
!           
  
            
             If PostgreSQL</> is compiled without SSL support,
             using option require</> will cause an error, while
             options allow</> and prefer</> will be
!            accepted but libpq</> will not in fact attempt
             an SSL</>
             connection.SSL</>with libpq</>
--- 254,308 ----
            
             This option determines whether or with what priority a
             SSL</> TCP/IP connection will be negotiated with the
!            server. There are four modes:
! 
!            
!             <literal>sslmode</literal> options
!             
!              
!               
!                Option
!                Description
!               
!              
!          
!              
!          
!               
!                disable</>
!                only try a non-SSL</> connection
!               
!          
!               
!                allow</>
!                first try a non-SSL</>
!                 connection;  if that fails, try an SSL</>
!                 connection
!               
!          
!               
!                prefer</> (default)
!                first try an SSL</> connection;  if
!                that fails, try a non-SSL</>
!                connection
!               
!          
!               
!                require</>
!                only try an SSL</> connection
!               
!          
!              
!             
!            
  
            
+            sslmode</> is ignored for Unix domain socket
+            communication.
             If PostgreSQL</> is compiled without SSL support,
             using option require</> will cause an error, while
             options allow</> and prefer</> will be
!            accepted but libpq</> will not actually attempt
             an SSL</>
             connection.SSL</>with libpq</>

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Bruce Momjian wrote:
> Magnus Hagander wrote:
>> On 14 apr 2009, at 04.33, Bruce Momjian  wrote:
>>
>>> Magnus Hagander wrote:
>>>>> I would actually call the two parameters 'verify-cert' and 'verify- 
>>>>> cn',
>>>>> and document that they also have "require" behavior.  Obviously you
>>>>> can't verify certificates unless you require SSL.
>>>> I would prefer having "verify", "verify-no-cn" and "no-verify" or
>>>> something like that. Making it the "default choice" to have  
>>>> verification
>>>> enabled, and very clear that you're turning something off if you're  
>>>> not.
>>>> And then just map require to verify. Or they could be "require-no-cn"
>>>> and "require-no-cert" perhaps?
>>>>
>>>> ("default choice" only for those using ssl of course - we'd still  
>>>> have
>>>> "disable" as the default *value* of the parameter)
>>> I think the "no" options are odd because they have _negative_
>>> designations.
>> That's the intention. When you're turning off something, I think it  
>> makes sense to use "no"....
> 
> But that doesn't scale:  sslmode currently has four options, soon
> perhaps to be six.   The idea is that the items should be of increasing
> security, and adding "no" in the middle doesn't allow that to be clear.

Here's a patch for this. Obviously, a lot needs to be done about the
docs here, I'm working on that.

I went with the names "require", "verify-ca" and "verify-full".

Patch also changes the default from "prefer" to "disable", per discussion.

Comments?

//Magnus
*** a/doc/src/sgml/libpq.sgml
--- b/doc/src/sgml/libpq.sgml
***************
*** 270,276 ****
              
          
               
!               disable</>
                only try a non-SSL</> connection
               
          
--- 270,276 ----
              
          
               
!               disable</> (default)
                only try a non-SSL</> connection
               
          
***************
*** 282,288 ****
               
          
               
!               prefer</> (default)
                first try an SSL</> connection;  if
                that fails, try a non-SSL</>
                connection
--- 282,288 ----
               
          
               
!               prefer</>
                first try an SSL</> connection;  if
                that fails, try a non-SSL</>
                connection
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
***************
*** 90,102 **** static int ldapServiceLookup(const char *purl, PQconninfoOption *options,
  #define DefaultOption	""
  #define DefaultAuthtype		  ""
  #define DefaultPassword		  ""
- #ifdef USE_SSL
- #define DefaultSSLMode	"prefer"
- #define DefaultSSLVerify "cn"
- #else
  #define DefaultSSLMode	"disable"
- #define DefaultSSLVerify "none"
- #endif
  
  /* ----------
   * Definition of the conninfo parameters and their fallback resources.
--- 90,96 ----
***************
*** 185,193 **** static const PQconninfoOption PQconninfoOptions[] = {
  	{"sslmode", "PGSSLMODE", DefaultSSLMode, NULL,
  	"SSL-Mode", "", 8},			/* sizeof("disable") == 8 */
  
- 	{"sslverify", "PGSSLVERIFY", DefaultSSLVerify, NULL,
- 	"SSL-Verify", "", 5},		/* sizeof("chain") == 5 */
- 
  	{"sslcert", "PGSSLCERT", NULL, NULL,
  	"SSL-Client-Cert", "", 64},
  
--- 179,184 ----
***************
*** 431,438 **** connectOptions1(PGconn *conn, const char *conninfo)
  	conn->connect_timeout = tmp ? strdup(tmp) : NULL;
  	tmp = conninfo_getval(connOptions, "sslmode");
  	conn->sslmode = tmp ? strdup(tmp) : NULL;
- 	tmp = conninfo_getval(connOptions, "sslverify");
- 	conn->sslverify = tmp ? strdup(tmp) : NULL;
  	tmp = conninfo_getval(connOptions, "sslkey");
  	conn->sslkey = tmp ? strdup(tmp) : NULL;
  	tmp = conninfo_getval(connOptions, "sslcert");
--- 422,427 ----
***************
*** 522,528 **** connectOptions2(PGconn *conn)
  		if (strcmp(conn->sslmode, "disable") != 0
  			&& strcmp(conn->sslmode, "allow") != 0
  			&& strcmp(conn->sslmode, "prefer") != 0
! 			&& strcmp(conn->sslmode, "require") != 0)
  		{
  			conn->status = CONNECTION_BAD;
  			printfPQExpBuffer(&conn->errorMessage,
--- 511,519 ----
  		if (strcmp(conn->sslmode, "disable") != 0
  			&& strcmp(conn->sslmode, "allow") != 0
  			&& strcmp(conn->sslmode, "prefer") != 0
! 			&& strcmp(conn->sslmode, "require") != 0
! 			&& strcmp(conn->sslmode, "verify-ca") != 0
! 			&& strcmp(conn->sslmode, "verify-full") != 0)
  		{
  			conn->status = CONNECTION_BAD;
  			printfPQExpBuffer(&conn->errorMessage,
***************
*** 544,549 **** connectOptions2(PGconn *conn)
--- 535,541 ----
  				break;
  
  			case 'r':			/* "require" */
+ 			case 'v':			/* "verify-ca" or "verify-full" */
  				conn->status = CONNECTION_BAD;
  				printfPQExpBuffer(&conn->errorMessage,
  								  libpq_gettext("sslmode value \"%s\" invalid when SSL support is not compiled in\n"),
***************
*** 556,579 **** connectOptions2(PGconn *conn)
  		conn->sslmode = strdup(DefaultSSLMode);
  
  	/*
- 	 * Validate sslverify option
- 	 */
- 	if (conn->sslverify)
- 	{
- 		if (strcmp(conn->sslverify, "none") != 0
- 			&& strcmp(conn->sslverify, "cert") != 0
- 			&& strcmp(conn->sslverify, "cn") != 0)
- 		{
- 			conn->status = CONNECTION_BAD;
- 			printfPQExpBuffer(&conn->errorMessage,
- 							libpq_gettext("invalid sslverify value: \"%s\"\n"),
- 							  conn->sslverify);
- 			return false;
- 		}
- 	}
- 
- 
- 	/*
  	 * Only if we get this far is it appropriate to try to connect. (We need a
  	 * state flag, rather than just the boolean result of this function, in
  	 * case someone tries to PQreset() the PGconn.)
--- 548,553 ----
***************
*** 1428,1434 **** keep_going:						/* We will come back to here until there is
  					}
  					else if (SSLok == 'N')
  					{
! 						if (conn->sslmode[0] == 'r')	/* "require" */
  						{
  							/* Require SSL, but server does not want it */
  							appendPQExpBuffer(&conn->errorMessage,
--- 1402,1409 ----
  					}
  					else if (SSLok == 'N')
  					{
! 						if (conn->sslmode[0] == 'r' ||	/* "require" */
! 							conn->sslmode[0] == 'v')    /* "verify-ca" or "verify-full" */
  						{
  							/* Require SSL, but server does not want it */
  							appendPQExpBuffer(&conn->errorMessage,
***************
*** 1445,1451 **** keep_going:						/* We will come back to here until there is
  						/* Received error - probably protocol mismatch */
  						if (conn->Pfdebug)
  							fprintf(conn->Pfdebug, "received error from server, attempting fallback to pre-7.0\n");
! 						if (conn->sslmode[0] == 'r')	/* "require" */
  						{
  							/* Require SSL, but server is too old */
  							appendPQExpBuffer(&conn->errorMessage,
--- 1420,1427 ----
  						/* Received error - probably protocol mismatch */
  						if (conn->Pfdebug)
  							fprintf(conn->Pfdebug, "received error from server, attempting fallback to pre-7.0\n");
! 						if (conn->sslmode[0] == 'r' ||	/* "require" */
! 							conn->sslmode[0] == 'v')    /* "verify-ca" or "verify-full" */
  						{
  							/* Require SSL, but server is too old */
  							appendPQExpBuffer(&conn->errorMessage,
***************
*** 2052,2059 **** freePGconn(PGconn *conn)
  		free(conn->pgpass);
  	if (conn->sslmode)
  		free(conn->sslmode);
- 	if (conn->sslverify)
- 		free(conn->sslverify);
  	if (conn->sslcert)
  		free(conn->sslcert);
  	if (conn->sslkey)
--- 2028,2033 ----
*** a/src/interfaces/libpq/fe-secure.c
--- b/src/interfaces/libpq/fe-secure.c
***************
*** 523,529 **** verify_peer_name_matches_certificate(PGconn *conn)
  	 * If told not to verify the peer name, don't do it. Return
  	 * 0 indicating that the verification was successful.
  	 */
! 	if(strcmp(conn->sslverify, "cn") != 0)
  		return true;
  
  	if (conn->pghostaddr)
--- 523,529 ----
  	 * If told not to verify the peer name, don't do it. Return
  	 * 0 indicating that the verification was successful.
  	 */
! 	if (strcmp(conn->sslmode, "verify-full") != 0)
  		return true;
  
  	if (conn->pghostaddr)
***************
*** 987,995 **** initialize_SSL(PGconn *conn)
  		return -1;
  
  	/*
! 	 * If sslverify is set to anything other than "none", perform certificate
! 	 * verification. If set to "cn" we will also do further verifications after
! 	 * the connection has been completed.
  	 *
  	 * If we are going to look for either root certificate or CRL in the home directory,
  	 * we need pqGetHomeDirectory() to succeed. In other cases, we don't need to
--- 987,995 ----
  		return -1;
  
  	/*
! 	 * If sslmode is set to one of the verify options, perform certificate
! 	 * verification. If set to "verify-full" we will also do further
! 	 * verification after the connection has been completed.
  	 *
  	 * If we are going to look for either root certificate or CRL in the home directory,
  	 * we need pqGetHomeDirectory() to succeed. In other cases, we don't need to
***************
*** 999,1005 **** initialize_SSL(PGconn *conn)
  	{
  		if (!pqGetHomeDirectory(homedir, sizeof(homedir)))
  		{
! 			if (strcmp(conn->sslverify, "none") != 0)
  			{
  				printfPQExpBuffer(&conn->errorMessage,
  								  libpq_gettext("could not get home directory to locate root certificate file"));
--- 999,1005 ----
  	{
  		if (!pqGetHomeDirectory(homedir, sizeof(homedir)))
  		{
! 			if (conn->sslmode[0] == 'v') /* "verify-ca" or "verify-full" */
  			{
  				printfPQExpBuffer(&conn->errorMessage,
  								  libpq_gettext("could not get home directory to locate root certificate file"));
***************
*** 1064,1070 **** initialize_SSL(PGconn *conn)
  	else
  	{
  		/* stat() failed; assume cert file doesn't exist */
! 		if (strcmp(conn->sslverify, "none") != 0)
  		{
  			printfPQExpBuffer(&conn->errorMessage,
  							  libpq_gettext("root certificate file \"%s\" does not exist\n"
--- 1064,1070 ----
  	else
  	{
  		/* stat() failed; assume cert file doesn't exist */
! 		if (conn->sslmode[0] == 'v') /* "verify-ca" or "verify-full" */
  		{
  			printfPQExpBuffer(&conn->errorMessage,
  							  libpq_gettext("root certificate file \"%s\" does not exist\n"
*** a/src/interfaces/libpq/libpq-int.h
--- b/src/interfaces/libpq/libpq-int.h
***************
*** 294,300 **** struct pg_conn
  	char	   *pguser;			/* Postgres username and password, if any */
  	char	   *pgpass;
  	char	   *sslmode;		/* SSL mode (require,prefer,allow,disable) */
- 	char	   *sslverify;		/* Verify server SSL certificate (none,chain,cn) */
  	char	   *sslkey;			/* client key filename */
  	char	   *sslcert;		/* client certificate filename */
  	char	   *sslrootcert;	/* root certificate filename */
--- 294,299 ----

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt