Skip to content
Permalink
Browse files

security: implement SSL hostname verification for non-default (LibPQF…

…actory) SSL factories (CVE-2018-10936)

In order to configure full SLL verification, `sslmode=verify-full` should be used.
However, previous versions of pgjdbc missed hostname verification for non-default SSL factories,
so `sslmode=verify-full` was effectively the same as `sslmode=verify-ca`.

Default sslfactory (which is LibPQFactory) is not impacted.

Extra changes:
- support for sslmode=allow/prefer/require
- ssl=true is treated as verify-full
- sslfactoryarg and socketFactoryArg are deprecated (as constructors with Properties) can be used.
  • Loading branch information...
vlsi committed Aug 12, 2018
1 parent c2885dd commit cdeeaca47dc3bc6f727c79a582c9e4123099526e
Showing with 1,455 additions and 880 deletions.
  1. +1 −10 .travis/travis_configure_ssl.sh
  2. +2 −2 README.md
  3. +1 −0 build.properties
  4. +0 −57 certdir/README
  5. +44 −0 certdir/README.md
  6. +2 −2 docs/documentation/head/connect.md
  7. +11 −6 pgjdbc/src/main/java/org/postgresql/PGProperty.java
  8. +14 −1 pgjdbc/src/main/java/org/postgresql/core/PGStream.java
  9. +26 −0 pgjdbc/src/main/java/org/postgresql/core/SocketFactoryFactory.java
  10. +136 −80 pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java
  11. +1 −1 pgjdbc/src/main/java/org/postgresql/core/v3/QueryExecutorImpl.java
  12. +81 −0 pgjdbc/src/main/java/org/postgresql/jdbc/SslMode.java
  13. +20 −0 pgjdbc/src/main/java/org/postgresql/ssl/DefaultJavaSSLFactory.java
  14. +2 −1 pgjdbc/src/main/java/org/postgresql/ssl/{jdbc4 → }/LazyKeyManager.java
  15. +220 −0 pgjdbc/src/main/java/org/postgresql/ssl/LibPQFactory.java
  16. +30 −47 pgjdbc/src/main/java/org/postgresql/ssl/MakeSSL.java
  17. +264 −0 pgjdbc/src/main/java/org/postgresql/ssl/PGjdbcHostnameVerifier.java
  18. +39 −284 pgjdbc/src/main/java/org/postgresql/ssl/jdbc4/LibPQFactory.java
  19. +1 −1 pgjdbc/src/main/java/org/postgresql/util/ObjectFactory.java
  20. +33 −5 pgjdbc/src/test/java/org/postgresql/test/TestUtil.java
  21. +12 −10 pgjdbc/src/test/java/org/postgresql/test/jdbc2/NotifyTest.java
  22. +0 −1 pgjdbc/src/test/java/org/postgresql/test/jdbc4/Jdbc4TestSuite.java
  23. +51 −0 pgjdbc/src/test/java/org/postgresql/test/ssl/CommonNameVerifierTest.java
  24. +8 −2 pgjdbc/src/test/java/org/postgresql/test/{jdbc4 → ssl}/LibPQFactoryHostNameTest.java
  25. +445 −300 pgjdbc/src/test/java/org/postgresql/test/ssl/SslTest.java
  26. +10 −39 pgjdbc/src/test/java/org/postgresql/test/ssl/SslTestSuite.java
  27. +1 −31 ssltest.properties
@@ -22,16 +22,7 @@ set_conf_property "ssl_cert_file" "server.crt"
set_conf_property "ssl_key_file" "server.key"
set_conf_property "ssl_ca_file" "root.crt"

enable_ssl_property "testsinglecertfactory"
enable_ssl_property "sslhostnossl9"
enable_ssl_property "sslhostgh9"
enable_ssl_property "sslhostbh9"
enable_ssl_property "sslhostsslgh9"
enable_ssl_property "sslhostsslbh9"
enable_ssl_property "sslhostsslcertgh9"
enable_ssl_property "sslhostsslcertbh9"
enable_ssl_property "sslcertgh9"
enable_ssl_property "sslcertbh9"
enable_ssl_property "enable_ssl_tests"

PG_DATA_DIR="/etc/postgresql/${PG_VERSION}/main/"
sudo cp certdir/server/pg_hba.conf "/etc/postgresql/${PG_VERSION}/main/pg_hba.conf"
@@ -111,7 +111,7 @@ In addition to the standard connection parameters the driver supports a number o
| password | String | null | The database user's password. |
| ssl | Boolean | false | Control use of SSL (true value causes SSL to be required) |
| sslfactory | String | null | Provide a SSLSocketFactory class when using SSL. |
| sslfactoryarg | String | null | Argument forwarded to constructor of SSLSocketFactory class. |
| sslfactoryarg (deprecated) | String | null | Argument forwarded to constructor of SSLSocketFactory class. |
| sslmode | String | null | Parameter governing the use of SSL. |
| sslcert | String | null | The location of the client's SSL certificate |
| sslkey | String | null | The location of the client's PKCS#8 SSL key |
@@ -144,7 +144,7 @@ In addition to the standard connection parameters the driver supports a number o
| hostRecheckSeconds | Integer | 10 | Specifies period (seconds) after which the host status is checked again in case it has changed |
| loadBalanceHosts | Boolean | false | If disabled hosts are connected in the given order. If enabled hosts are chosen randomly from the set of suitable candidates |
| socketFactory | String | null | Specify a socket factory for socket creation |
| socketFactoryArg | String | null | Argument forwarded to constructor of SocketFactory class. |
| socketFactoryArg (deprecated) | String | null | Argument forwarded to constructor of SocketFactory class. |
| autosave | String | never | Specifies what the driver should do if a query fails, possible values: always, never, conservative |
| preferQueryMode | String | extended | Specifies which mode is used to execute queries to database, possible values: extended, extendedForPrepared, extendedCacheEverything, simple |
| reWriteBatchedInserts | Boolean | false | Enable optimization to rewrite and collapse compatible INSERT statements that are batched. |
@@ -19,3 +19,4 @@ preparethreshold=5
loggerLevel=OFF
loggerFile=target/pgjdbc-tests.log
protocolVersion=0
sslpassword=sslpwd

This file was deleted.

Oops, something went wrong.
@@ -0,0 +1,44 @@
To run the SSL tests, the following properties are used:

* certdir: directory where the certificates and keys are store
* enable_ssl_tests: enables SSL tests

In order to configure PostgreSQL for SSL tests, the following changes should be applied:

* Copy server/server.crt, server/server.key, and server/root.crt to $PGDATA directory
* In $PGDATA directory: chmod 0600 server.crt server.key root.crt
* Set ssl=on in postgresql.conf
* Set ssl_cert_file=server.crt in postgresql.conf
* Set ssl_key_file=server.key in postgresql.conf
* Set ssl_ca_file=root.crt in postgresql.conf
* Add databases for SSL tests. Note: sslinfo extension is used in tests to tell if connection is using SSL or not

for db in hostssldb hostnossldb certdb hostsslcertdb; do
createdb $db
psql $db -c "create extension sslinfo"
done
* Add test databases to pg_hba.conf. If you do not overwrite the pg_hba.conf then remember to comment out all lines
starting with "host all".
* Uncomment enable_ssl_tests=true in ssltests.properties
* The username for connecting to postgres as specified in build.local.properties tests has to be "test".

This directory contains example certificates generated by the following
commands:

openssl req -x509 -newkey rsa:1024 -days 3650 -keyout goodclient.key -out goodclient.crt
#Common name is test, password is sslpwd

openssl req -x509 -newkey rsa:1024 -days 3650 -keyout badclient.key -out badclient.crt
#Common name is test, password is sslpwd

openssl req -x509 -newkey rsa:1024 -days 3650 -nodes -keyout badroot.key -out badroot.crt
#Common name is localhost
rm badroot.key

openssl pkcs8 -topk8 -in goodclient.key -out goodclient.pk8 -outform DER -v1 PBE-MD5-DES
openssl pkcs8 -topk8 -in badclient.key -out badclient.pk8 -outform DER -v1 PBE-MD5-DES
cp goodclient.crt server/root.crt
cd server
openssl req -x509 -newkey rsa:1024 -nodes -days 3650 -keyout server.key -out server.crt
cp server.crt ../goodroot.crt
#Common name is localhost, no password
@@ -88,7 +88,7 @@ Connection conn = DriverManager.getConnection(url);
establishing a SSL connection. For more information see the section
called [“Custom SSLSocketFactory”](ssl-factory.html).

* **sslfactoryarg** = String
* **sslfactoryarg** (deprecated) = String

This value is an optional argument to the constructor of the sslfactory
class provided above. For more information see the section called [“Custom SSLSocketFactory”](ssl-factory.html).
@@ -408,7 +408,7 @@ Connection conn = DriverManager.getConnection(url);
This class must have a zero argument constructor or a single argument constructor taking a String argument.
This argument may optionally be supplied by `socketFactoryArg`.

* **socketFactoryArg** = String
* **socketFactoryArg** (deprecated) = String

This value is an optional argument to the constructor of the socket factory
class provided above.
@@ -173,17 +173,18 @@
"Enable optimization that disables column name sanitiser"),

/**
* Control use of SSL (any non-null value causes SSL to be required).
* Control use of SSL: empty or {@code true} values imply {@code sslmode==verify-full}
*/
SSL("ssl", null, "Control use of SSL (any non-null value causes SSL to be required)"),

/**
* Parameter governing the use of SSL. The allowed values are {@code require}, {@code verify-ca},
* {@code verify-full}, or {@code disable} ({@code allow} and {@code prefer} are not implemented)
* If not set, the {@code ssl} property may be checked to enable SSL mode.
* Parameter governing the use of SSL. The allowed values are {@code disable}, {@code allow},
* {@code prefer}, {@code require}, {@code verify-ca}, {@code verify-full}.
* If {@code ssl} property is empty or set to {@code true} it implies {@code verify-full}.
* Default mode is "require"
*/
SSL_MODE("sslmode", null, "Parameter governing the use of SSL",false,
"disable", "require", "verify-ca", "verify-full"),
SSL_MODE("sslmode", null, "Parameter governing the use of SSL", false,
"disable", "allow", "prefer", "require", "verify-ca", "verify-full"),

/**
* Classname of the SSL Factory to use (instance of {@code javax.net.ssl.SSLSocketFactory}).
@@ -192,7 +193,9 @@

/**
* The String argument to give to the constructor of the SSL Factory.
* @deprecated use {@code ..Factory(Properties)} constructor.
*/
@Deprecated
SSL_FACTORY_ARG("sslfactoryarg", null,
"Argument forwarded to constructor of SSLSocketFactory class."),

@@ -279,7 +282,9 @@

/**
* The String argument to give to the constructor of the Socket Factory.
* @deprecated use {@code ..Factory(Properties)} constructor.
*/
@Deprecated
SOCKET_FACTORY_ARG("socketFactoryArg", null,
"Argument forwarded to constructor of SocketFactory class."),

@@ -21,6 +21,7 @@
import java.io.Writer;
import java.net.InetSocketAddress;
import java.net.Socket;
import java.net.SocketTimeoutException;
import java.sql.SQLException;
import javax.net.SocketFactory;

@@ -109,7 +110,19 @@ public SocketFactory getSocketFactory() {
* @throws IOException if something wrong happens
*/
public boolean hasMessagePending() throws IOException {
return pg_input.available() > 0 || connection.getInputStream().available() > 0;
if (pg_input.available() > 0) {
return true;
}
// In certain cases, available returns 0, yet there are bytes
int soTimeout = getNetworkTimeout();
setNetworkTimeout(1);
try {
return pg_input.peek() != -1;
} catch (SocketTimeoutException e) {
return false;
} finally {
setNetworkTimeout(soTimeout);
}
}

/**
@@ -6,6 +6,7 @@
package org.postgresql.core;

import org.postgresql.PGProperty;
import org.postgresql.ssl.LibPQFactory;
import org.postgresql.util.GT;
import org.postgresql.util.ObjectFactory;
import org.postgresql.util.PSQLException;
@@ -14,6 +15,7 @@
import java.util.Properties;

import javax.net.SocketFactory;
import javax.net.ssl.SSLSocketFactory;

/**
* Instantiates {@link SocketFactory} based on the {@link PGProperty#SOCKET_FACTORY}.
@@ -44,4 +46,28 @@ public static SocketFactory getSocketFactory(Properties info) throws PSQLExcepti
}
}

/**
* Instantiates {@link SSLSocketFactory} based on the {@link PGProperty#SSL_FACTORY}.
*
* @param info connection properties
* @return SSL socket factory
* @throws PSQLException if something goes wrong
*/
public static SSLSocketFactory getSslSocketFactory(Properties info) throws PSQLException {
String classname = PGProperty.SSL_FACTORY.get(info);
if (classname == null
|| "org.postgresql.ssl.jdbc4.LibPQFactory".equals(classname)
|| "org.postgresql.ssl.LibPQFactory".equals(classname)) {
return new LibPQFactory(info);
}
try {
return (SSLSocketFactory) ObjectFactory.instantiate(classname, info, true,
PGProperty.SSL_FACTORY_ARG.get(info));
} catch (Exception e) {
throw new PSQLException(
GT.tr("The SSLSocketFactory class provided {0} could not be instantiated.", classname),
PSQLState.CONNECTION_FAILURE, e);
}
}

}
Oops, something went wrong.

0 comments on commit cdeeaca

Please sign in to comment.
You can’t perform that action at this time.