From c89404edf3f3b35a4a599d71f407055bda8261b6 Mon Sep 17 00:00:00 2001 From: Magnus Hagander Date: Thu, 13 Nov 2008 09:45:25 +0000 Subject: [PATCH] Fix libpq certificate validation for SSL connections. Add config parameter "sslverify" to control the verification. Default is to do full verification. Clean up some old SSL code that never really worked. --- doc/src/sgml/libpq.sgml | 53 ++++++++++++-- doc/src/sgml/runtime.sgml | 16 +++-- src/interfaces/libpq/fe-connect.c | 29 +++++++- src/interfaces/libpq/fe-secure.c | 115 +++++++++++++----------------- src/interfaces/libpq/libpq-int.h | 3 +- 5 files changed, 139 insertions(+), 77 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 6f977f9083..0b220ab514 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -1,4 +1,4 @@ - + <application>libpq</application> - C Library @@ -259,6 +259,33 @@ + + sslverify + + + This option controls how libpq verifies the certificate on the + server when performing an SSL connection. There are + three options: none disables verification completely + (not recommended!); cert enables verification that + the certificate chains to a known CA only; cn will + both verify that the certificate chains to a known CA and that + the cn attribute of the certificate matches the + hostname the connection is being made to (default). + + + + It is always recommended to use the cn value for + this parameter, since this is the only option that prevents + man-in-the-middle attacks. Note that this requires the server + name on the certificate to match exactly with the host name + used for the connection, and therefore does not support connections + to aliased names. It can be used with pure IP address connections + only if the certificate also has just the IP address in the + cn field. + + + + requiressl @@ -5679,6 +5706,22 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough) + + + + PGSSLVERIFY + + PGSSLVERIFY controls how libpq verifies the certificate on the + server when performing an SSL connection. There are + three options: none disables verification completely + (not recommended!); cert enables verification that + the certificate chains to a known CA only; cn will + both verify that the certificate chains to a known CA and that + the cn attribute of the certificate matches the + hostname the connection is being made to (default). + + + @@ -6026,9 +6069,11 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough) - To verify the server certificate is trustworthy, place certificates of - the certificate authorities (CA) you trust in the - file ~/.postgresql/root.crt in the user's home directory. + When the sslverify parameter is set to cn or + cert, libpq will verify that the server certificate is + trustworthy by checking the certificate chain up to a CA. + For this to work, place the certificate of a trusted CA + in the file ~/.postgresql/root.crt in the user's home directory. (On Microsoft Windows the file is named %APPDATA%\postgresql\root.crt.) libpq will then verify that the server's diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml index a46f59cf1f..b04de288c3 100644 --- a/doc/src/sgml/runtime.sgml +++ b/doc/src/sgml/runtime.sgml @@ -1,4 +1,4 @@ - + Operating System Environment @@ -1418,9 +1418,9 @@ $ kill -INT `head -1 /usr/local/pgsql/data/postmaster.pid`server.key (key) and server.crt (certificate) files (). The TCP client must connect using - sslmode='require' () and have - a ~/.postgresql/root.crt SSL certificate (). + sslmode='require', specify sslverify='cn' + or sslverify='cert' and have the required certificate + files present (). @@ -1544,8 +1544,8 @@ $ kill -INT `head -1 /usr/local/pgsql/data/postmaster.pid` - It is possible for both the client and server to provide SSL keys - or certificates to each other. It takes some extra configuration + It is possible for both the client and server to provide SSL + certificates to each other. It takes some extra configuration on each side, but this provides stronger verification of identity than the mere use of passwords. It prevents a computer from pretending to be the server just long enough to read the password @@ -1757,7 +1757,9 @@ chmod og-rwx server.key A self-signed certificate can be used for testing, but a certificate signed by a certificate authority (CA) (either one of the global CAs or a local one) should be used in production - so the client can verify the server's identity. + so the client can verify the server's identity. If all the clients + are local to the organization, using a local CA is + recommended. diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index d8b243b8d6..e1376dc017 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/interfaces/libpq/fe-connect.c,v 1.367 2008/11/09 00:28:35 tgl Exp $ + * $PostgreSQL: pgsql/src/interfaces/libpq/fe-connect.c,v 1.368 2008/11/13 09:45:24 mha Exp $ * *------------------------------------------------------------------------- */ @@ -92,8 +92,10 @@ static int ldapServiceLookup(const char *purl, PQconninfoOption *options, #define DefaultPassword "" #ifdef USE_SSL #define DefaultSSLMode "prefer" +#define DefaultSSLVerify "cn" #else #define DefaultSSLMode "disable" +#define DefaultSSLVerify "none" #endif /* ---------- @@ -181,6 +183,9 @@ static const PQconninfoOption PQconninfoOptions[] = { {"sslmode", "PGSSLMODE", DefaultSSLMode, NULL, "SSL-Mode", "", 8}, /* sizeof("disable") == 8 */ + {"sslverify", "PGSSLVERIFY", DefaultSSLVerify, NULL, + "SSL-Verify", "", 5}, /* sizeof("chain") == 5 */ + #if defined(KRB5) || defined(ENABLE_GSS) || defined(ENABLE_SSPI) /* Kerberos and GSSAPI authentication support specifying the service name */ {"krbsrvname", "PGKRBSRVNAME", PG_KRB_SRVNAM, NULL, @@ -415,6 +420,8 @@ 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; #ifdef USE_SSL tmp = conninfo_getval(connOptions, "requiressl"); if (tmp && tmp[0] == '1') @@ -529,6 +536,24 @@ connectOptions2(PGconn *conn) else 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 @@ -2008,6 +2033,8 @@ freePGconn(PGconn *conn) free(conn->pgpass); if (conn->sslmode) free(conn->sslmode); + if (conn->sslverify) + free(conn->sslverify); #if defined(KRB5) || defined(ENABLE_GSS) || defined(ENABLE_SSPI) if (conn->krbsrvname) free(conn->krbsrvname); diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c index af85257a86..1cc7c5cbfb 100644 --- a/src/interfaces/libpq/fe-secure.c +++ b/src/interfaces/libpq/fe-secure.c @@ -11,7 +11,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.106 2008/10/24 12:29:11 mha Exp $ + * $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.107 2008/11/13 09:45:25 mha Exp $ * * NOTES * @@ -87,9 +87,7 @@ #define ERR_pop_to_mark() ((void) 0) #endif -#ifdef NOT_USED -static int verify_peer_name_matches_certificate(PGconn *); -#endif +static bool verify_peer_name_matches_certificate(PGconn *); static int verify_cb(int ok, X509_STORE_CTX *ctx); static int client_cert_cb(SSL *, X509 **, EVP_PKEY **); static int init_ssl_system(PGconn *conn); @@ -438,77 +436,44 @@ verify_cb(int ok, X509_STORE_CTX *ctx) return ok; } -#ifdef NOT_USED /* * Verify that common name resolves to peer. */ -static int +static bool verify_peer_name_matches_certificate(PGconn *conn) { - struct hostent *cn_hostentry = NULL; - struct sockaddr server_addr; - struct sockaddr_in *sin (struct sockaddr_in *) &server_addr; - ACCEPT_TYPE_ARG3 len; - char **s; - unsigned long l; - - /* Get the address on the other side of the socket. */ - len = sizeof(server_addr); - if (getpeername(conn->sock, &server_addr, &len) == -1) - { - char sebuf[256]; - - printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("error querying socket: %s\n"), - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); - return -1; - } + /* + * 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 (server_addr.sa_family != AF_INET) + if (conn->pghostaddr) { printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("unsupported protocol\n")); - return -1; + libpq_gettext("verified SSL connections are only supported when connecting to a hostname")); + return false; } - - /* Get the IP addresses of the certificate's common name (CN) */ + else { - struct hostent hpstr; - char buf[BUFSIZ]; - int herrno = 0; - /* - * Currently, pqGethostbyname() is used only on platforms that don't - * have getaddrinfo(). If you enable this function, you should - * convert the pqGethostbyname() function call to use getaddrinfo(). + * Connect by hostname. + * + * XXX: Should support alternate names here + * XXX: Should support wildcard certificates here */ - pqGethostbyname(conn->peer_cn, &hpstr, buf, sizeof(buf), - &cn_hostentry, &herrno); - } - - /* Did we get an IP address? */ - if (cn_hostentry == NULL) - { - printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("could not get information about host \"%s\": %s\n"), - conn->peer_cn, hstrerror(h_errno)); - return -1; + if (pg_strcasecmp(conn->peer_cn, conn->pghost) != 0) + { + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("server common name '%s' does not match hostname '%s'"), + conn->peer_cn, conn->pghost); + return false; + } + else + return true; } - - /* Does one of the CN's IP addresses match the server's IP address? */ - for (s = cn_hostentry->h_addr_list; *s != NULL; s++) - if (!memcmp(&sin->sin_addr.s_addr, *s, cn_hostentry->h_length)) - return 0; - - l = ntohl(sin->sin_addr.s_addr); - printfPQExpBuffer(&conn->errorMessage, - libpq_gettext( - "server common name \"%s\" does not resolve to %ld.%ld.%ld.%ld\n"), - conn->peer_cn, (l >> 24) % 0x100, (l >> 16) % 0x100, - (l >> 8) % 0x100, l % 0x100); - return -1; } -#endif /* NOT_USED */ /* * Callback used by SSL to load client cert and key. @@ -846,6 +811,12 @@ initialize_SSL(PGconn *conn) if (init_ssl_system(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. + */ + /* Set up to verify server cert, if root.crt is present */ if (pqGetHomeDirectory(homedir, sizeof(homedir))) { @@ -889,6 +860,24 @@ initialize_SSL(PGconn *conn) SSL_CTX_set_verify(SSL_context, SSL_VERIFY_PEER, verify_cb); } + else + { + if (strcmp(conn->sslverify, "none") != 0) + { + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("root certificate file (%s) not found"), fnbuf); + return -1; + } + } + } + else + { + if (strcmp(conn->sslverify, "none") != 0) + { + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("cannot find home directory to locate root certificate file")); + return -1; + } } /* set up mechanism to provide client certificate, if available */ @@ -1004,13 +993,11 @@ open_client_SSL(PGconn *conn) NID_commonName, conn->peer_cn, SM_USER); conn->peer_cn[SM_USER] = '\0'; -#ifdef NOT_USED - if (verify_peer_name_matches_certificate(conn) == -1) + if (!verify_peer_name_matches_certificate(conn)) { close_SSL(conn); return PGRES_POLLING_FAILED; } -#endif /* SSL handshake is complete */ return PGRES_POLLING_OK; diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index d5ec8ce13f..adeaa35d0b 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -12,7 +12,7 @@ * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/interfaces/libpq/libpq-int.h,v 1.136 2008/10/28 12:10:44 mha Exp $ + * $PostgreSQL: pgsql/src/interfaces/libpq/libpq-int.h,v 1.137 2008/11/13 09:45:25 mha Exp $ * *------------------------------------------------------------------------- */ @@ -291,6 +291,7 @@ 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) */ #if defined(KRB5) || defined(ENABLE_GSS) || defined(ENABLE_SSPI) char *krbsrvname; /* Kerberos service name */ #endif -- 2.40.0