Skip to content

Commit dd6000e

Browse files
Sehrope Sarkunivlsi
authored and
sehrope
committed
feat: Change AuthenticationPlugin interface to use char[] rather than String
Changes AuthenticationPlugin interface for dynamic passwords to supply passwords as a char[] rather than a String. This changes the currently unreleased public interface of AuthenticationPlugin and allows the driver to clear the user provided char[] array after it is finished using it for authentication. Users implementing that interface must ensure that each invocation of the method provides a new char[] array as the contents will be filled with zeroes by the driver after use. Call sites within the driver have been updated to use the char[] directly wherever possible. This includes direct usage in the GSS authentication code paths that internally were already converting the String password into a char[] for internal usage. The SASL (i.e. "SCRAM") internals have not been updated to use a char[] array as the entirety of that library uses String types for provided passwords. Assuming that it is not exposed in other parts of the driver, that could be updated as a standalone PR. For now the entrypoint from the ConnectionFactoryImpl into the SASL library simply converts the char[] array to a String at it's single usage point. Co-Authored-By: Vladimir Sitnikov <[email protected]>
1 parent 417c9a2 commit dd6000e

File tree

7 files changed

+198
-126
lines changed

7 files changed

+198
-126
lines changed

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

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

‎pgjdbc/src/main/java/org/postgresql/core/v3/AuthenticationPluginManager.java‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
/*
2+
* Copyright (c) 2021, PostgreSQL Global Development Group
3+
* See the LICENSE file in the project root for more information.
4+
*/
5+
6+
package org.postgresql.core.v3;
7+
8+
import org.postgresql.PGProperty;
9+
import org.postgresql.plugin.AuthenticationPlugin;
10+
import org.postgresql.plugin.AuthenticationRequestType;
11+
import org.postgresql.util.GT;
12+
import org.postgresql.util.ObjectFactory;
13+
import org.postgresql.util.PSQLException;
14+
import org.postgresql.util.PSQLState;
15+
16+
import org.checkerframework.checker.nullness.qual.Nullable;
17+
18+
import java.io.IOException;
19+
import java.nio.ByteBuffer;
20+
import java.nio.CharBuffer;
21+
import java.nio.charset.StandardCharsets;
22+
import java.util.Properties;
23+
import java.util.logging.Level;
24+
import java.util.logging.Logger;
25+
26+
class AuthenticationPluginManager {
27+
private static final Logger LOGGER = Logger.getLogger(AuthenticationPluginManager.class.getName());
28+
29+
@FunctionalInterface
30+
public interface PasswordAction<T, R> {
31+
R apply(T password) throws PSQLException, IOException;
32+
}
33+
34+
private AuthenticationPluginManager() {
35+
}
36+
37+
/**
38+
* If a password is requested by the server during connection initiation, this
39+
* method will be invoked to supply the password. This method will only be
40+
* invoked if the server actually requests a password, e.g. trust authentication
41+
* will skip it entirely.
42+
*
43+
* <p>The caller provides a action method that will be invoked with the {@code char[]}
44+
* password. After completion, for security reasons the {@code char[]} array will be
45+
* wiped by filling it with zeroes. Callers must not rely on being able to read
46+
* the password {@code char[]} after the action has completed.</p>
47+
*
48+
* @param type The authentication type that is being requested
49+
* @param info The connection properties for the connection
50+
* @param action The action to invoke with the password
51+
* @throws PSQLException Throws a PSQLException if the plugin class cannot be instantiated
52+
* @throws IOException Bubbles up any thrown IOException from the provided action
53+
*/
54+
public static <T> T withPassword(AuthenticationRequestType type, Properties info,
55+
PasswordAction<char @Nullable [], T> action) throws PSQLException, IOException {
56+
char[] password = null;
57+
58+
String authPluginClassName = PGProperty.AUTHENTICATION_PLUGIN_CLASS_NAME.get(info);
59+
60+
if (authPluginClassName == null || authPluginClassName.equals("")) {
61+
// Default auth plugin simply pulls password directly from connection properties
62+
String passwordText = PGProperty.PASSWORD.get(info);
63+
if (passwordText != null) {
64+
password = passwordText.toCharArray();
65+
}
66+
} else {
67+
AuthenticationPlugin authPlugin;
68+
try {
69+
authPlugin = (AuthenticationPlugin) ObjectFactory.instantiate(authPluginClassName, info,
70+
false, null);
71+
} catch (Exception ex) {
72+
LOGGER.log(Level.FINE, "Unable to load Authentication Plugin " + ex.toString());
73+
throw new PSQLException(ex.getMessage(), PSQLState.UNEXPECTED_ERROR);
74+
}
75+
76+
password = authPlugin.getPassword(type);
77+
}
78+
79+
try {
80+
return action.apply(password);
81+
} finally {
82+
if (password != null) {
83+
java.util.Arrays.fill(password, (char) 0);
84+
}
85+
}
86+
}
87+
88+
/**
89+
* Helper that wraps {@link #withPassword(AuthenticationRequestType, Properties, PasswordAction)}, checks that it is not-null, and encodes
90+
* it as a byte array. Used by internal code paths that require an encoded password
91+
* that may be an empty string, but not null.
92+
*
93+
* <p>The caller provides a callback method that will be invoked with the {@code byte[]}
94+
* encoded password. After completion, for security reasons the {@code byte[]} array will be
95+
* wiped by filling it with zeroes. Callers must not rely on being able to read
96+
* the password {@code byte[]} after the callback has completed.</p>
97+
98+
* @param type The authentication type that is being requested
99+
* @param info The connection properties for the connection
100+
* @param action The action to invoke with the encoded password
101+
* @throws PSQLException Throws a PSQLException if the plugin class cannot be instantiated or if the retrieved password is null.
102+
* @throws IOException Bubbles up any thrown IOException from the provided callback
103+
*/
104+
public static <T> T withEncodedPassword(AuthenticationRequestType type, Properties info,
105+
PasswordAction<byte[], T> action) throws PSQLException, IOException {
106+
byte[] encodedPassword = withPassword(type, info, password -> {
107+
if (password == null) {
108+
throw new PSQLException(
109+
GT.tr("The server requested password-based authentication, but no password was provided."),
110+
PSQLState.CONNECTION_REJECTED);
111+
}
112+
ByteBuffer buf = StandardCharsets.UTF_8.encode(CharBuffer.wrap(password));
113+
byte[] bytes = new byte[buf.limit()];
114+
buf.get(bytes);
115+
return bytes;
116+
});
117+
118+
try {
119+
return action.apply(encodedPassword);
120+
} finally {
121+
java.util.Arrays.fill(encodedPassword, (byte) 0);
122+
}
123+
}
124+
}

‎pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java‎

Lines changed: 49 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import static org.postgresql.util.internal.Nullness.castNonNull;
1010

1111
import org.postgresql.PGProperty;
12-
import org.postgresql.core.AuthenticationPluginManager;
1312
import org.postgresql.core.ConnectionFactory;
1413
import org.postgresql.core.PGStream;
1514
import org.postgresql.core.QueryExecutor;
@@ -497,12 +496,14 @@ private PGStream enableGSSEncrypted(PGStream pgStream, GSSEncMode gssEncMode, St
497496
case 'G':
498497
LOGGER.log(Level.FINEST, " <=BE GSSEncryptedOk");
499498
try {
500-
String password = AuthenticationPluginManager.getPassword(AuthenticationRequestType.GSS, info);
501-
org.postgresql.gss.MakeGSS.authenticate(true, pgStream, host, user, password,
502-
PGProperty.JAAS_APPLICATION_NAME.get(info),
503-
PGProperty.KERBEROS_SERVER_NAME.get(info), false, // TODO: fix this
504-
PGProperty.JAAS_LOGIN.getBoolean(info),
505-
PGProperty.LOG_SERVER_ERROR_DETAIL.getBoolean(info));
499+
AuthenticationPluginManager.withPassword(AuthenticationRequestType.GSS, info, password -> {
500+
org.postgresql.gss.MakeGSS.authenticate(true, pgStream, host, user, password,
501+
PGProperty.JAAS_APPLICATION_NAME.get(info),
502+
PGProperty.KERBEROS_SERVER_NAME.get(info), false, // TODO: fix this
503+
PGProperty.JAAS_LOGIN.getBoolean(info),
504+
PGProperty.LOG_SERVER_ERROR_DETAIL.getBoolean(info));
505+
return void.class;
506+
});
506507
return pgStream;
507508
} catch (PSQLException ex) {
508509
// allow the connection to proceed
@@ -660,17 +661,23 @@ private void doAuthentication(PGStream pgStream, String host, String user, Prope
660661
LOGGER.log(Level.FINEST, " <=BE AuthenticationReqMD5(salt={0})", Utils.toHexString(md5Salt));
661662
}
662663

663-
byte[] encodedPassword = AuthenticationPluginManager.getEncodedPassword(AuthenticationRequestType.MD5_PASSWORD, info);
664-
byte[] digest =
665-
MD5Digest.encode(user.getBytes(StandardCharsets.UTF_8), encodedPassword, md5Salt);
664+
byte[] digest = AuthenticationPluginManager.withEncodedPassword(
665+
AuthenticationRequestType.MD5_PASSWORD, info,
666+
encodedPassword -> MD5Digest.encode(user.getBytes(StandardCharsets.UTF_8),
667+
encodedPassword, md5Salt)
668+
);
666669

667670
if (LOGGER.isLoggable(Level.FINEST)) {
668671
LOGGER.log(Level.FINEST, " FE=> Password(md5digest={0})", new String(digest, StandardCharsets.US_ASCII));
669672
}
670673

671-
pgStream.sendChar('p');
672-
pgStream.sendInteger4(4 + digest.length + 1);
673-
pgStream.send(digest);
674+
try {
675+
pgStream.sendChar('p');
676+
pgStream.sendInteger4(4 + digest.length + 1);
677+
pgStream.send(digest);
678+
} finally {
679+
java.util.Arrays.fill(digest, (byte) 0);
680+
}
674681
pgStream.sendChar(0);
675682
pgStream.flush();
676683

@@ -681,11 +688,12 @@ private void doAuthentication(PGStream pgStream, String host, String user, Prope
681688
LOGGER.log(Level.FINEST, "<=BE AuthenticationReqPassword");
682689
LOGGER.log(Level.FINEST, " FE=> Password(password=<not shown>)");
683690

684-
byte[] encodedPassword = AuthenticationPluginManager.getEncodedPassword(AuthenticationRequestType.CLEARTEXT_PASSWORD, info);
685-
686-
pgStream.sendChar('p');
687-
pgStream.sendInteger4(4 + encodedPassword.length + 1);
688-
pgStream.send(encodedPassword);
691+
AuthenticationPluginManager.withEncodedPassword(AuthenticationRequestType.CLEARTEXT_PASSWORD, info, encodedPassword -> {
692+
pgStream.sendChar('p');
693+
pgStream.sendInteger4(4 + encodedPassword.length + 1);
694+
pgStream.send(encodedPassword);
695+
return void.class;
696+
});
689697
pgStream.sendChar(0);
690698
pgStream.flush();
691699

@@ -756,12 +764,14 @@ private void doAuthentication(PGStream pgStream, String host, String user, Prope
756764
castNonNull(sspiClient).startSSPI();
757765
} else {
758766
/* Use JGSS's GSSAPI for this request */
759-
String password = AuthenticationPluginManager.getPassword(AuthenticationRequestType.GSS, info);
760-
org.postgresql.gss.MakeGSS.authenticate(false, pgStream, host, user, password,
761-
PGProperty.JAAS_APPLICATION_NAME.get(info),
762-
PGProperty.KERBEROS_SERVER_NAME.get(info), usespnego,
763-
PGProperty.JAAS_LOGIN.getBoolean(info),
764-
PGProperty.LOG_SERVER_ERROR_DETAIL.getBoolean(info));
767+
AuthenticationPluginManager.withPassword(AuthenticationRequestType.GSS, info, password -> {
768+
org.postgresql.gss.MakeGSS.authenticate(false, pgStream, host, user, password,
769+
PGProperty.JAAS_APPLICATION_NAME.get(info),
770+
PGProperty.KERBEROS_SERVER_NAME.get(info), usespnego,
771+
PGProperty.JAAS_LOGIN.getBoolean(info),
772+
PGProperty.LOG_SERVER_ERROR_DETAIL.getBoolean(info));
773+
return void.class;
774+
});
765775
}
766776
break;
767777

@@ -775,20 +785,21 @@ private void doAuthentication(PGStream pgStream, String host, String user, Prope
775785
case AUTH_REQ_SASL:
776786
LOGGER.log(Level.FINEST, " <=BE AuthenticationSASL");
777787

778-
String password = AuthenticationPluginManager.getPassword(AuthenticationRequestType.SASL, info);
779-
if (password == null) {
780-
throw new PSQLException(
781-
GT.tr(
782-
"The server requested SCRAM-based authentication, but no password was provided."),
783-
PSQLState.CONNECTION_REJECTED);
784-
}
785-
if (password.equals("")) {
786-
throw new PSQLException(
787-
GT.tr(
788-
"The server requested SCRAM-based authentication, but the password is an empty string."),
789-
PSQLState.CONNECTION_REJECTED);
790-
}
791-
scramAuthenticator = new org.postgresql.jre7.sasl.ScramAuthenticator(user, castNonNull(password), pgStream);
788+
scramAuthenticator = AuthenticationPluginManager.withPassword(AuthenticationRequestType.SASL, info, password -> {
789+
if (password == null) {
790+
throw new PSQLException(
791+
GT.tr(
792+
"The server requested SCRAM-based authentication, but no password was provided."),
793+
PSQLState.CONNECTION_REJECTED);
794+
}
795+
if (password.length == 0) {
796+
throw new PSQLException(
797+
GT.tr(
798+
"The server requested SCRAM-based authentication, but the password is an empty string."),
799+
PSQLState.CONNECTION_REJECTED);
800+
}
801+
return new org.postgresql.jre7.sasl.ScramAuthenticator(user, String.valueOf(password), pgStream);
802+
});
792803
scramAuthenticator.processServerMechanismsAndInit();
793804
scramAuthenticator.sendScramClientFirstMessage();
794805
// This works as follows:

‎pgjdbc/src/main/java/org/postgresql/gss/GSSCallbackHandler.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@
2323
class GSSCallbackHandler implements CallbackHandler {
2424

2525
private final String user;
26-
private final @Nullable String password;
26+
private final char @Nullable [] password;
2727

28-
GSSCallbackHandler(String user, @Nullable String password) {
28+
GSSCallbackHandler(String user, char @Nullable [] password) {
2929
this.user = user;
3030
this.password = password;
3131
}
@@ -56,7 +56,7 @@ public void handle(Callback[] callbacks) throws IOException, UnsupportedCallback
5656
if (password == null) {
5757
throw new IOException("No cached kerberos ticket found and no password supplied.");
5858
}
59-
pc.setPassword(password.toCharArray());
59+
pc.setPassword(password);
6060
} else {
6161
throw new UnsupportedCallbackException(callback, "Unrecognized Callback");
6262
}

‎pgjdbc/src/main/java/org/postgresql/gss/MakeGSS.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public class MakeGSS {
2727
private static final Logger LOGGER = Logger.getLogger(MakeGSS.class.getName());
2828

2929
public static void authenticate(boolean encrypted,
30-
PGStream pgStream, String host, String user, @Nullable String password,
30+
PGStream pgStream, String host, String user, char @Nullable [] password,
3131
@Nullable String jaasApplicationName, @Nullable String kerberosServerName,
3232
boolean useSpnego, boolean jaasLogin,
3333
boolean logServerErrorDetail)

‎pgjdbc/src/main/java/org/postgresql/plugin/AuthenticationPlugin.java‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,24 @@
1010
import org.checkerframework.checker.nullness.qual.Nullable;
1111

1212
public interface AuthenticationPlugin {
13-
@Nullable
14-
String getPassword(AuthenticationRequestType type) throws PSQLException;
13+
14+
/**
15+
* Callback method to provide the password to use for authentication.
16+
*
17+
* <p>Implementers can also check the authentication type to ensure that the
18+
* authentication handshake is using a specific authentication method (e.g. SASL)
19+
* or avoiding a specific one (e.g. cleartext).</p>
20+
*
21+
* <p>For security reasons, the driver will wipe the contents of the array returned
22+
* by this method after it has been used for authentication.</p>
23+
*
24+
* <p><b>Implementers must provide a new array each time this method is invoked as
25+
* the previous contents will have been wiped.</b></p>
26+
*
27+
* @param type The authentication method that the server is requesting
28+
* @return The password to use or null if no password is available
29+
* @throws PSQLException if something goes wrong supplying the password
30+
*/
31+
char @Nullable [] getPassword(AuthenticationRequestType type) throws PSQLException;
1532

1633
}

0 commit comments

Comments
 (0)