From 7ac258c2f3c3a442b5a11b454c0bf9ed476cbb6a Mon Sep 17 00:00:00 2001 From: Tom Lane <tgl@sss.pgh.pa.us> Date: Sun, 26 Sep 2004 22:51:49 +0000 Subject: [PATCH] Fix multiple breakages in our support for SSL certificates. --- doc/src/sgml/libpq.sgml | 48 ++++++++++++---- doc/src/sgml/runtime.sgml | 30 +++++----- src/backend/libpq/be-secure.c | 25 +++++---- src/interfaces/libpq/fe-secure.c | 94 +++++++++++++++----------------- 4 files changed, 112 insertions(+), 85 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index e39302e178c..4691abb78d6 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -1,5 +1,5 @@ <!-- -$PostgreSQL: pgsql/doc/src/sgml/libpq.sgml,v 1.163 2004/09/23 13:31:09 momjian Exp $ +$PostgreSQL: pgsql/doc/src/sgml/libpq.sgml,v 1.164 2004/09/26 22:51:49 tgl Exp $ --> <chapter id="libpq"> @@ -233,22 +233,13 @@ PGconn *PQconnectdb(const char *conninfo); <para> If <productname>PostgreSQL</> is compiled without SSL support, - using option <literal>require</> will cause an error, and + using option <literal>require</> will cause an error, while options <literal>allow</> and <literal>prefer</> will be tolerated but <application>libpq</> will be unable to negotiate an <acronym>SSL</> connection.<indexterm><primary>SSL</><secondary sortas="libpq">with libpq</></indexterm> </para> - - <para> - Please note that <acronym>SSL</> support in libpq covers - encryption only. It will not verify the validity of the - certificate presented by the server that you are connecting to, - nor verify that the hostname matches that of the server's - certificate. Additionally, there is no support for client - certificates. - </para> </listitem> </varlistentry> @@ -3688,6 +3679,41 @@ If the permissions are less strict than this, the file will be ignored. </para> </sect1> + +<sect1 id="libpq-ssl"> +<title>SSL Support</title> + +<indexterm zone="libpq-ssl"> + <primary>SSL</primary> +</indexterm> + + <para> + <productname>PostgreSQL</> has native support for using + <acronym>SSL</> connections to encrypt client/server communications + for increased security. See <xref linkend="ssl-tcp"> for details + about the server-side <acronym>SSL</> functionality. + </para> + + <para> + If the server demands a client certificate, + <application>libpq</application> + will send the certificate stored in file + <filename>.postgresql/postgresql.crt</> within the user's home directory. + A matching private key file <filename>.postgresql/postgresql.key</> + must also be present, and must not be world-readable. + </para> + + <para> + If the file <filename>.postgresql/root.crt</> is present in the user's + home directory, + <application>libpq</application> will use the certificate list stored + therein to verify the server's certificate. The SSL connection will + fail if the server does not present a certificate; therefore, to + use this feature the server must also have a <filename>root.crt</> file. + </para> +</sect1> + + <sect1 id="libpq-threading"> <title>Behavior in Threaded Programs</title> diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml index ff437bad35a..59c4e325b90 100644 --- a/doc/src/sgml/runtime.sgml +++ b/doc/src/sgml/runtime.sgml @@ -1,5 +1,5 @@ <!-- -$PostgreSQL: pgsql/doc/src/sgml/runtime.sgml,v 1.283 2004/09/23 13:15:57 momjian Exp $ +$PostgreSQL: pgsql/doc/src/sgml/runtime.sgml,v 1.284 2004/09/26 22:51:49 tgl Exp $ --> <Chapter Id="runtime"> @@ -804,7 +804,7 @@ SET ENABLE_SEQSCAN TO OFF; <para> Enables <acronym>SSL</> connections. Please read <xref linkend="ssl-tcp"> before using this. The default - is off. + is off. This parameter can only be set at server start. </para> </listitem> </varlistentry> @@ -4324,8 +4324,8 @@ $ <userinput>kill -INT `head -1 /usr/local/pgsql/data/postmaster.pid`</userinput The server will listen for both standard and <acronym>SSL</> connections on the same TCP port, and will negotiate with any connecting client on whether to use <acronym>SSL</>. See <xref - linkend="auth-pg-hba-conf"> about how to force the server to - require use of <acronym>SSL</> for certain connections. + linkend="auth-pg-hba-conf"> about how to set up the server to + require use of <acronym>SSL</> for some or all connections. </para> <para> @@ -4361,20 +4361,24 @@ chmod og-rwx server.key <para> If verification of client certificates is required, place the - certificates of the <acronym>CA</acronym> you wish to check for in + certificates of the <acronym>CA</acronym>(s) you wish to check for in the file <filename>root.crt</filename> in the data directory. When present, a client certificate will be requested from the client - making the connection and it must have been signed by one of the - certificates present in <filename>root.crt</filename>. If no - certificate is presented, the connection will be allowed to proceed - anway. + during SSL connection startup, and it must have been signed by one of the + certificates present in <filename>root.crt</filename>. </para> <para> - The <filename>root.crt</filename> file is always checked for, and - its absence will be noted through a message in the log. This is - merely an informative message that client certificates will not be - requested. + When the <filename>root.crt</filename> file is not present, client + certificates will not be requested or checked. In this mode, SSL + provides communication security but not authentication. + </para> + + <para> + The files <filename>server.key</>, <filename>server.crt</>, + and <filename>root.crt</filename> are only examined during server + start; so you must restart the server to make changes in them take + effect. </para> </sect1> diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c index 43acba4473b..eee9ad28367 100644 --- a/src/backend/libpq/be-secure.c +++ b/src/backend/libpq/be-secure.c @@ -11,7 +11,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/libpq/be-secure.c,v 1.50 2004/09/23 20:27:50 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/libpq/be-secure.c,v 1.51 2004/09/26 22:51:49 tgl Exp $ * * Since the server static private key ($DataDir/server.key) * will normally be stored unencrypted so that the database @@ -117,7 +117,6 @@ static const char *SSLerrmessage(void); * (total in both directions) before we require renegotiation. */ #define RENEGOTIATION_LIMIT (512 * 1024 * 1024) -#define CA_PATH NULL static SSL_CTX *SSL_context = NULL; #endif @@ -412,12 +411,12 @@ static DH * load_dh_file(int keylength) { FILE *fp; - char fnbuf[2048]; + char fnbuf[MAXPGPATH]; DH *dh = NULL; int codes; /* attempt to open file. It's not an error if it doesn't exist. */ - snprintf(fnbuf, sizeof fnbuf, "%s/dh%d.pem", DataDir, keylength); + snprintf(fnbuf, sizeof(fnbuf), "%s/dh%d.pem", DataDir, keylength); if ((fp = fopen(fnbuf, "r")) == NULL) return NULL; @@ -694,20 +693,26 @@ initialize_SSL(void) if (SSL_CTX_set_cipher_list(SSL_context, "ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH") != 1) elog(FATAL, "could not set the cipher list (no valid ciphers available)"); - /* accept client certificates, but don't require them. */ + /* + * Require and check client certificates only if we have a root.crt file. + */ snprintf(fnbuf, sizeof(fnbuf), "%s/root.crt", DataDir); - if (!SSL_CTX_load_verify_locations(SSL_context, fnbuf, CA_PATH)) + if (!SSL_CTX_load_verify_locations(SSL_context, fnbuf, NULL)) { /* Not fatal - we do not require client certificates */ ereport(LOG, (errmsg("could not load root certificate file \"%s\": %s", fnbuf, SSLerrmessage()), errdetail("Will not verify client certificates."))); - return 0; } - SSL_CTX_set_verify(SSL_context, - SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE, - verify_cb); + else + { + SSL_CTX_set_verify(SSL_context, + (SSL_VERIFY_PEER | + SSL_VERIFY_FAIL_IF_NO_PEER_CERT | + SSL_VERIFY_CLIENT_ONCE), + verify_cb); + } return 0; } diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c index 2fba98d88b8..3fbeffe01e6 100644 --- a/src/interfaces/libpq/fe-secure.c +++ b/src/interfaces/libpq/fe-secure.c @@ -11,9 +11,11 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.51 2004/09/23 20:27:43 tgl Exp $ + * $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.52 2004/09/26 22:51:49 tgl Exp $ * * NOTES + * [ Most of these notes are wrong/obsolete, but perhaps not all ] + * * The client *requires* a valid server certificate. Since * SSH tunnels provide anonymous confidentiality, the presumption * is that sites that want endpoint authentication will use the @@ -527,7 +529,7 @@ verify_peer(PGconn *conn) if (h == NULL) { printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not get information about host (%s): %s\n"), + libpq_gettext("could not get information about host \"%s\": %s\n"), conn->peer_cn, hstrerror(h_errno)); return -1; } @@ -600,15 +602,15 @@ load_dh_file(int keylength) struct passwd pwdstr; struct passwd *pwd = NULL; FILE *fp; - char fnbuf[2048]; + char fnbuf[MAXPGPATH]; DH *dh = NULL; int codes; - if (pqGetpwuid(getuid(), &pwdstr, pwdbuf, sizeof(pwdbuf), &pwd) == 0) + if (pqGetpwuid(getuid(), &pwdstr, pwdbuf, sizeof(pwdbuf), &pwd) != 0) return NULL; /* attempt to open file. It's not an error if it doesn't exist. */ - snprintf(fnbuf, sizeof fnbuf, "%s/.postgresql/dh%d.pem", + snprintf(fnbuf, sizeof(fnbuf), "%s/.postgresql/dh%d.pem", pwd->pw_dir, keylength); if ((fp = fopen(fnbuf, "r")) == NULL) @@ -735,7 +737,7 @@ tmp_dh_cb(SSL *s, int is_export, int keylength) * This callback is only called when the server wants a * client cert. * - * Returns 1 on success, 0 on no data, -1 on error. + * Must return 1 on success, 0 on no data or error. */ static int client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey) @@ -748,52 +750,52 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey) struct passwd *pwd = NULL; struct stat buf, buf2; - char fnbuf[2048]; + char fnbuf[MAXPGPATH]; FILE *fp; PGconn *conn = (PGconn *) SSL_get_app_data(ssl); int (*cb) () = NULL; /* how to read user password */ char sebuf[256]; - if (pqGetpwuid(getuid(), &pwdstr, pwdbuf, sizeof(pwdbuf), &pwd) == 0) + if (pqGetpwuid(getuid(), &pwdstr, pwdbuf, sizeof(pwdbuf), &pwd) != 0) { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("could not get user information\n")); - return -1; + return 0; } /* read the user certificate */ - snprintf(fnbuf, sizeof fnbuf, "%s/.postgresql/postgresql.crt", + snprintf(fnbuf, sizeof(fnbuf), "%s/.postgresql/postgresql.crt", pwd->pw_dir); if (stat(fnbuf, &buf) == -1) return 0; if ((fp = fopen(fnbuf, "r")) == NULL) { printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not open certificate (%s): %s\n"), + libpq_gettext("could not open certificate file \"%s\": %s\n"), fnbuf, pqStrerror(errno, sebuf, sizeof(sebuf))); - return -1; + return 0; } if (PEM_read_X509(fp, x509, NULL, NULL) == NULL) { char *err = SSLerrmessage(); printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not read certificate (%s): %s\n"), + libpq_gettext("could not read certificate file \"%s\": %s\n"), fnbuf, err); SSLerrfree(err); fclose(fp); - return -1; + return 0; } fclose(fp); /* read the user key */ - snprintf(fnbuf, sizeof fnbuf, "%s/.postgresql/postgresql.key", + snprintf(fnbuf, sizeof(fnbuf), "%s/.postgresql/postgresql.key", pwd->pw_dir); if (stat(fnbuf, &buf) == -1) { printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("certificate present, but not private key (%s)\n"), + libpq_gettext("certificate present, but not private key file \"%s\"\n"), fnbuf); X509_free(*x509); return 0; @@ -802,37 +804,38 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey) buf.st_uid != getuid()) { printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("private key (%s) has wrong permissions\n"), fnbuf); + libpq_gettext("private key file \"%s\" has wrong permissions\n"), + fnbuf); X509_free(*x509); - return -1; + return 0; } if ((fp = fopen(fnbuf, "r")) == NULL) { printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not open private key file (%s): %s\n"), + libpq_gettext("could not open private key file \"%s\": %s\n"), fnbuf, pqStrerror(errno, sebuf, sizeof(sebuf))); X509_free(*x509); - return -1; + return 0; } if (fstat(fileno(fp), &buf2) == -1 || buf.st_dev != buf2.st_dev || buf.st_ino != buf2.st_ino) { printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("private key (%s) changed during execution\n"), fnbuf); + libpq_gettext("private key file \"%s\" changed during execution\n"), fnbuf); X509_free(*x509); - return -1; + return 0; } if (PEM_read_PrivateKey(fp, pkey, cb, NULL) == NULL) { char *err = SSLerrmessage(); printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not read private key (%s): %s\n"), + libpq_gettext("could not read private key file \"%s\": %s\n"), fnbuf, err); SSLerrfree(err); X509_free(*x509); fclose(fp); - return -1; + return 0; } fclose(fp); @@ -842,12 +845,12 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey) char *err = SSLerrmessage(); printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("certificate/private key mismatch (%s): %s\n"), + libpq_gettext("certificate does not match private key file \"%s\": %s\n"), fnbuf, err); SSLerrfree(err); X509_free(*x509); EVP_PKEY_free(*pkey); - return -1; + return 0; } return 1; @@ -952,46 +955,35 @@ initialize_SSL(PGconn *conn) char pwdbuf[BUFSIZ]; struct passwd pwdstr; struct passwd *pwd = NULL; - char fnbuf[2048]; + char fnbuf[MAXPGPATH]; #endif if (init_ssl_system(conn)) return -1; #ifndef WIN32 + /* Set up to verify server cert, if root.crt is present */ if (pqGetpwuid(getuid(), &pwdstr, pwdbuf, sizeof(pwdbuf), &pwd) == 0) { - snprintf(fnbuf, sizeof fnbuf, "%s/.postgresql/root.crt", + snprintf(fnbuf, sizeof(fnbuf), "%s/.postgresql/root.crt", pwd->pw_dir); - if (stat(fnbuf, &buf) == -1) + if (stat(fnbuf, &buf) == 0) { - return 0; -#ifdef NOT_USED - char sebuf[256]; + if (!SSL_CTX_load_verify_locations(SSL_context, fnbuf, NULL)) + { + char *err = SSLerrmessage(); - /* CLIENT CERTIFICATES NOT REQUIRED bjm 2002-09-26 */ - printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not read root certificate list (%s): %s\n"), - fnbuf, pqStrerror(errno, sebuf, sizeof(sebuf))); - return -1; -#endif - } - if (!SSL_CTX_load_verify_locations(SSL_context, fnbuf, 0)) - { - char *err = SSLerrmessage(); + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("could not read root certificate file \"%s\": %s\n"), + fnbuf, err); + SSLerrfree(err); + return -1; + } - printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not read root certificate list (%s): %s\n"), - fnbuf, err); - SSLerrfree(err); - return -1; + SSL_CTX_set_verify(SSL_context, SSL_VERIFY_PEER, verify_cb); } } - SSL_CTX_set_verify(SSL_context, - SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, verify_cb); - SSL_CTX_set_verify_depth(SSL_context, 1); - /* set up empheral DH keys */ SSL_CTX_set_tmp_dh_callback(SSL_context, tmp_dh_cb); SSL_CTX_set_options(SSL_context, SSL_OP_SINGLE_DH_USE); -- GitLab