Skip to content

Commit cdeeaca

Browse files
vlsivlsi
committed
security: implement SSL hostname verification for non-default (LibPQFactory) 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.
1 parent c2885dd commit cdeeaca

27 files changed

+1455
-880
lines changed

‎.travis/travis_configure_ssl.sh‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,7 @@ set_conf_property "ssl_cert_file" "server.crt"
2222
set_conf_property "ssl_key_file" "server.key"
2323
set_conf_property "ssl_ca_file" "root.crt"
2424

25-
enable_ssl_property "testsinglecertfactory"
26-
enable_ssl_property "sslhostnossl9"
27-
enable_ssl_property "sslhostgh9"
28-
enable_ssl_property "sslhostbh9"
29-
enable_ssl_property "sslhostsslgh9"
30-
enable_ssl_property "sslhostsslbh9"
31-
enable_ssl_property "sslhostsslcertgh9"
32-
enable_ssl_property "sslhostsslcertbh9"
33-
enable_ssl_property "sslcertgh9"
34-
enable_ssl_property "sslcertbh9"
25+
enable_ssl_property "enable_ssl_tests"
3526

3627
PG_DATA_DIR="/etc/postgresql/${PG_VERSION}/main/"
3728
sudo cp certdir/server/pg_hba.conf "/etc/postgresql/${PG_VERSION}/main/pg_hba.conf"

‎README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ In addition to the standard connection parameters the driver supports a number o
111111
| password | String | null | The database user's password. |
112112
| ssl | Boolean | false | Control use of SSL (true value causes SSL to be required) |
113113
| sslfactory | String | null | Provide a SSLSocketFactory class when using SSL. |
114-
| sslfactoryarg | String | null | Argument forwarded to constructor of SSLSocketFactory class. |
114+
| sslfactoryarg (deprecated) | String | null | Argument forwarded to constructor of SSLSocketFactory class. |
115115
| sslmode | String | null | Parameter governing the use of SSL. |
116116
| sslcert | String | null | The location of the client's SSL certificate |
117117
| 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
144144
| hostRecheckSeconds | Integer | 10 | Specifies period (seconds) after which the host status is checked again in case it has changed |
145145
| loadBalanceHosts | Boolean | false | If disabled hosts are connected in the given order. If enabled hosts are chosen randomly from the set of suitable candidates |
146146
| socketFactory | String | null | Specify a socket factory for socket creation |
147-
| socketFactoryArg | String | null | Argument forwarded to constructor of SocketFactory class. |
147+
| socketFactoryArg (deprecated) | String | null | Argument forwarded to constructor of SocketFactory class. |
148148
| autosave | String | never | Specifies what the driver should do if a query fails, possible values: always, never, conservative |
149149
| preferQueryMode | String | extended | Specifies which mode is used to execute queries to database, possible values: extended, extendedForPrepared, extendedCacheEverything, simple |
150150
| reWriteBatchedInserts | Boolean | false | Enable optimization to rewrite and collapse compatible INSERT statements that are batched. |

‎build.properties‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,4 @@ preparethreshold=5
1919
loggerLevel=OFF
2020
loggerFile=target/pgjdbc-tests.log
2121
protocolVersion=0
22+
sslpassword=sslpwd

‎certdir/README‎

Lines changed: 0 additions & 57 deletions
This file was deleted.

‎certdir/README.md‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
To run the SSL tests, the following properties are used:
2+
3+
* certdir: directory where the certificates and keys are store
4+
* enable_ssl_tests: enables SSL tests
5+
6+
In order to configure PostgreSQL for SSL tests, the following changes should be applied:
7+
8+
* Copy server/server.crt, server/server.key, and server/root.crt to $PGDATA directory
9+
* In $PGDATA directory: chmod 0600 server.crt server.key root.crt
10+
* Set ssl=on in postgresql.conf
11+
* Set ssl_cert_file=server.crt in postgresql.conf
12+
* Set ssl_key_file=server.key in postgresql.conf
13+
* Set ssl_ca_file=root.crt in postgresql.conf
14+
* Add databases for SSL tests. Note: sslinfo extension is used in tests to tell if connection is using SSL or not
15+
16+
for db in hostssldb hostnossldb certdb hostsslcertdb; do
17+
createdb $db
18+
psql $db -c "create extension sslinfo"
19+
done
20+
* Add test databases to pg_hba.conf. If you do not overwrite the pg_hba.conf then remember to comment out all lines
21+
starting with "host all".
22+
* Uncomment enable_ssl_tests=true in ssltests.properties
23+
* The username for connecting to postgres as specified in build.local.properties tests has to be "test".
24+
25+
This directory contains example certificates generated by the following
26+
commands:
27+
28+
openssl req -x509 -newkey rsa:1024 -days 3650 -keyout goodclient.key -out goodclient.crt
29+
#Common name is test, password is sslpwd
30+
31+
openssl req -x509 -newkey rsa:1024 -days 3650 -keyout badclient.key -out badclient.crt
32+
#Common name is test, password is sslpwd
33+
34+
openssl req -x509 -newkey rsa:1024 -days 3650 -nodes -keyout badroot.key -out badroot.crt
35+
#Common name is localhost
36+
rm badroot.key
37+
38+
openssl pkcs8 -topk8 -in goodclient.key -out goodclient.pk8 -outform DER -v1 PBE-MD5-DES
39+
openssl pkcs8 -topk8 -in badclient.key -out badclient.pk8 -outform DER -v1 PBE-MD5-DES
40+
cp goodclient.crt server/root.crt
41+
cd server
42+
openssl req -x509 -newkey rsa:1024 -nodes -days 3650 -keyout server.key -out server.crt
43+
cp server.crt ../goodroot.crt
44+
#Common name is localhost, no password

‎docs/documentation/head/connect.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ Connection conn = DriverManager.getConnection(url);
8888
establishing a SSL connection. For more information see the section
8989
called [“Custom SSLSocketFactory”](ssl-factory.html).
9090

91-
* **sslfactoryarg** = String
91+
* **sslfactoryarg** (deprecated) = String
9292

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

411-
* **socketFactoryArg** = String
411+
* **socketFactoryArg** (deprecated) = String
412412

413413
This value is an optional argument to the constructor of the socket factory
414414
class provided above.

‎pgjdbc/src/main/java/org/postgresql/PGProperty.java‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -173,17 +173,18 @@ public enum PGProperty {
173173
"Enable optimization that disables column name sanitiser"),
174174

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

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

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

193194
/**
194195
* The String argument to give to the constructor of the SSL Factory.
196+
* @deprecated use {@code ..Factory(Properties)} constructor.
195197
*/
198+
@Deprecated
196199
SSL_FACTORY_ARG("sslfactoryarg", null,
197200
"Argument forwarded to constructor of SSLSocketFactory class."),
198201

@@ -279,7 +282,9 @@ public enum PGProperty {
279282

280283
/**
281284
* The String argument to give to the constructor of the Socket Factory.
285+
* @deprecated use {@code ..Factory(Properties)} constructor.
282286
*/
287+
@Deprecated
283288
SOCKET_FACTORY_ARG("socketFactoryArg", null,
284289
"Argument forwarded to constructor of SocketFactory class."),
285290

‎pgjdbc/src/main/java/org/postgresql/core/PGStream.java‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.io.Writer;
2222
import java.net.InetSocketAddress;
2323
import java.net.Socket;
24+
import java.net.SocketTimeoutException;
2425
import java.sql.SQLException;
2526
import javax.net.SocketFactory;
2627

@@ -109,7 +110,19 @@ public SocketFactory getSocketFactory() {
109110
* @throws IOException if something wrong happens
110111
*/
111112
public boolean hasMessagePending() throws IOException {
112-
return pg_input.available() > 0 || connection.getInputStream().available() > 0;
113+
if (pg_input.available() > 0) {
114+
return true;
115+
}
116+
// In certain cases, available returns 0, yet there are bytes
117+
int soTimeout = getNetworkTimeout();
118+
setNetworkTimeout(1);
119+
try {
120+
return pg_input.peek() != -1;
121+
} catch (SocketTimeoutException e) {
122+
return false;
123+
} finally {
124+
setNetworkTimeout(soTimeout);
125+
}
113126
}
114127

115128
/**

‎pgjdbc/src/main/java/org/postgresql/core/SocketFactoryFactory.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package org.postgresql.core;
77

88
import org.postgresql.PGProperty;
9+
import org.postgresql.ssl.LibPQFactory;
910
import org.postgresql.util.GT;
1011
import org.postgresql.util.ObjectFactory;
1112
import org.postgresql.util.PSQLException;
@@ -14,6 +15,7 @@
1415
import java.util.Properties;
1516

1617
import javax.net.SocketFactory;
18+
import javax.net.ssl.SSLSocketFactory;
1719

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

49+
/**
50+
* Instantiates {@link SSLSocketFactory} based on the {@link PGProperty#SSL_FACTORY}.
51+
*
52+
* @param info connection properties
53+
* @return SSL socket factory
54+
* @throws PSQLException if something goes wrong
55+
*/
56+
public static SSLSocketFactory getSslSocketFactory(Properties info) throws PSQLException {
57+
String classname = PGProperty.SSL_FACTORY.get(info);
58+
if (classname == null
59+
|| "org.postgresql.ssl.jdbc4.LibPQFactory".equals(classname)
60+
|| "org.postgresql.ssl.LibPQFactory".equals(classname)) {
61+
return new LibPQFactory(info);
62+
}
63+
try {
64+
return (SSLSocketFactory) ObjectFactory.instantiate(classname, info, true,
65+
PGProperty.SSL_FACTORY_ARG.get(info));
66+
} catch (Exception e) {
67+
throw new PSQLException(
68+
GT.tr("The SSLSocketFactory class provided {0} could not be instantiated.", classname),
69+
PSQLState.CONNECTION_FAILURE, e);
70+
}
71+
}
72+
4773
}

0 commit comments

Comments
 (0)