From 261864a107553a6e2e214b20c2b6a789d5ee324e Mon Sep 17 00:00:00 2001 From: Sehrope Sarkuni Date: Fri, 28 Jan 2022 12:12:14 -0500 Subject: [PATCH] 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 --- .../core/AuthenticationPluginManager.java | 80 ----------- .../core/v3/AuthenticationPluginManager.java | 124 ++++++++++++++++++ .../core/v3/ConnectionFactoryImpl.java | 87 ++++++------ .../postgresql/gss/GSSCallbackHandler.java | 6 +- .../main/java/org/postgresql/gss/MakeGSS.java | 2 +- .../plugin/AuthenticationPlugin.java | 21 ++- .../test/plugin/AuthenticationPluginTest.java | 4 +- 7 files changed, 198 insertions(+), 126 deletions(-) delete mode 100644 pgjdbc/src/main/java/org/postgresql/core/AuthenticationPluginManager.java create mode 100644 pgjdbc/src/main/java/org/postgresql/core/v3/AuthenticationPluginManager.java diff --git a/pgjdbc/src/main/java/org/postgresql/core/AuthenticationPluginManager.java b/pgjdbc/src/main/java/org/postgresql/core/AuthenticationPluginManager.java deleted file mode 100644 index 8e883c066c..0000000000 --- a/pgjdbc/src/main/java/org/postgresql/core/AuthenticationPluginManager.java +++ /dev/null @@ -1,80 +0,0 @@ -/* - * Copyright (c) 2021, PostgreSQL Global Development Group - * See the LICENSE file in the project root for more information. - */ - -package org.postgresql.core; - -import org.postgresql.PGProperty; -import org.postgresql.plugin.AuthenticationPlugin; -import org.postgresql.plugin.AuthenticationRequestType; -import org.postgresql.util.GT; -import org.postgresql.util.ObjectFactory; -import org.postgresql.util.PSQLException; -import org.postgresql.util.PSQLState; - -import org.checkerframework.checker.nullness.qual.Nullable; - -import java.nio.charset.StandardCharsets; -import java.util.Properties; -import java.util.logging.Level; -import java.util.logging.Logger; - -public class AuthenticationPluginManager { - private static final Logger LOGGER = Logger.getLogger(AuthenticationPluginManager.class.getName()); - - private AuthenticationPluginManager() { - } - - /** - * If a password is requested by the server during connection initiation, this - * method will be invoked to supply the password. This method will only be - * invoked if the server actually requests a password, e.g. trust authentication - * will skip it entirely. - * - * @param type The authentication type that is being requested - * @param info The connection properties for the connection - * @return The password to use for authentication or null if none is available - * @throws PSQLException Throws a PSQLException if the plugin class cannot be instantiated - */ - public static @Nullable String getPassword(AuthenticationRequestType type, Properties info) throws PSQLException { - String authPluginClassName = PGProperty.AUTHENTICATION_PLUGIN_CLASS_NAME.get(info); - - if (authPluginClassName == null || authPluginClassName.equals("")) { - // Default auth plugin simply pulls password directly from connection properties - return PGProperty.PASSWORD.get(info); - } - - AuthenticationPlugin authPlugin; - try { - authPlugin = (AuthenticationPlugin) ObjectFactory.instantiate(authPluginClassName, info, - false, null); - } catch (Exception ex) { - LOGGER.log(Level.FINE, "Unable to load Authentication Plugin " + ex.toString()); - throw new PSQLException(ex.getMessage(), PSQLState.UNEXPECTED_ERROR); - } - return authPlugin.getPassword(type); - } - - /** - * Helper that wraps getPassword(...), checks that it is not-null, and encodes - * it as a byte array. Used by internal code paths that require an encoded password that may be an - * empty string, but not null. - * - * @param type The authentication type that is being requested - * @param info The connection properties for the connection - * @return The password to use for authentication encoded as a byte array - * @throws PSQLException Throws a PSQLException if the plugin class cannot be instantiated or if the retrieved password is null. - */ - public static byte[] getEncodedPassword(AuthenticationRequestType type, Properties info) throws PSQLException { - String password = getPassword(type, info); - - if (password == null) { - throw new PSQLException( - GT.tr("The server requested password-based authentication, but no password was provided."), - PSQLState.CONNECTION_REJECTED); - } - - return password.getBytes(StandardCharsets.UTF_8); - } -} diff --git a/pgjdbc/src/main/java/org/postgresql/core/v3/AuthenticationPluginManager.java b/pgjdbc/src/main/java/org/postgresql/core/v3/AuthenticationPluginManager.java new file mode 100644 index 0000000000..938f24632f --- /dev/null +++ b/pgjdbc/src/main/java/org/postgresql/core/v3/AuthenticationPluginManager.java @@ -0,0 +1,124 @@ +/* + * Copyright (c) 2021, PostgreSQL Global Development Group + * See the LICENSE file in the project root for more information. + */ + +package org.postgresql.core.v3; + +import org.postgresql.PGProperty; +import org.postgresql.plugin.AuthenticationPlugin; +import org.postgresql.plugin.AuthenticationRequestType; +import org.postgresql.util.GT; +import org.postgresql.util.ObjectFactory; +import org.postgresql.util.PSQLException; +import org.postgresql.util.PSQLState; + +import org.checkerframework.checker.nullness.qual.Nullable; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.CharBuffer; +import java.nio.charset.StandardCharsets; +import java.util.Properties; +import java.util.logging.Level; +import java.util.logging.Logger; + +class AuthenticationPluginManager { + private static final Logger LOGGER = Logger.getLogger(AuthenticationPluginManager.class.getName()); + + @FunctionalInterface + public interface PasswordAction { + R apply(T password) throws PSQLException, IOException; + } + + private AuthenticationPluginManager() { + } + + /** + * If a password is requested by the server during connection initiation, this + * method will be invoked to supply the password. This method will only be + * invoked if the server actually requests a password, e.g. trust authentication + * will skip it entirely. + * + *

The caller provides a action method that will be invoked with the {@code char[]} + * password. After completion, for security reasons the {@code char[]} array will be + * wiped by filling it with zeroes. Callers must not rely on being able to read + * the password {@code char[]} after the action has completed.

+ * + * @param type The authentication type that is being requested + * @param info The connection properties for the connection + * @param action The action to invoke with the password + * @throws PSQLException Throws a PSQLException if the plugin class cannot be instantiated + * @throws IOException Bubbles up any thrown IOException from the provided action + */ + public static T withPassword(AuthenticationRequestType type, Properties info, + PasswordAction action) throws PSQLException, IOException { + char[] password = null; + + String authPluginClassName = PGProperty.AUTHENTICATION_PLUGIN_CLASS_NAME.get(info); + + if (authPluginClassName == null || authPluginClassName.equals("")) { + // Default auth plugin simply pulls password directly from connection properties + String passwordText = PGProperty.PASSWORD.get(info); + if (passwordText != null) { + password = passwordText.toCharArray(); + } + } else { + AuthenticationPlugin authPlugin; + try { + authPlugin = (AuthenticationPlugin) ObjectFactory.instantiate(authPluginClassName, info, + false, null); + } catch (Exception ex) { + LOGGER.log(Level.FINE, "Unable to load Authentication Plugin " + ex.toString()); + throw new PSQLException(ex.getMessage(), PSQLState.UNEXPECTED_ERROR); + } + + password = authPlugin.getPassword(type); + } + + try { + return action.apply(password); + } finally { + if (password != null) { + java.util.Arrays.fill(password, (char) 0); + } + } + } + + /** + * Helper that wraps {@link #withPassword(AuthenticationRequestType, Properties, PasswordAction)}, checks that it is not-null, and encodes + * it as a byte array. Used by internal code paths that require an encoded password + * that may be an empty string, but not null. + * + *

The caller provides a callback method that will be invoked with the {@code byte[]} + * encoded password. After completion, for security reasons the {@code byte[]} array will be + * wiped by filling it with zeroes. Callers must not rely on being able to read + * the password {@code byte[]} after the callback has completed.

+ + * @param type The authentication type that is being requested + * @param info The connection properties for the connection + * @param action The action to invoke with the encoded password + * @throws PSQLException Throws a PSQLException if the plugin class cannot be instantiated or if the retrieved password is null. + * @throws IOException Bubbles up any thrown IOException from the provided callback + */ + public static T withEncodedPassword(AuthenticationRequestType type, Properties info, + PasswordAction action) throws PSQLException, IOException { + byte[] encodedPassword = withPassword(type, info, password -> { + if (password == null) { + throw new PSQLException( + GT.tr("The server requested password-based authentication, but no password was provided."), + PSQLState.CONNECTION_REJECTED); + } + ByteBuffer buf = StandardCharsets.UTF_8.encode(CharBuffer.wrap(password)); + byte[] bytes = new byte[buf.limit()]; + buf.get(bytes); + return bytes; + }); + + try { + return action.apply(encodedPassword); + } finally { + java.util.Arrays.fill(encodedPassword, (byte) 0); + } + } +} diff --git a/pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java b/pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java index 33c90eea95..6c81e8b9b2 100644 --- a/pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java +++ b/pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java @@ -9,7 +9,6 @@ import static org.postgresql.util.internal.Nullness.castNonNull; import org.postgresql.PGProperty; -import org.postgresql.core.AuthenticationPluginManager; import org.postgresql.core.ConnectionFactory; import org.postgresql.core.PGStream; import org.postgresql.core.QueryExecutor; @@ -497,12 +496,14 @@ private PGStream enableGSSEncrypted(PGStream pgStream, GSSEncMode gssEncMode, St case 'G': LOGGER.log(Level.FINEST, " <=BE GSSEncryptedOk"); try { - String password = AuthenticationPluginManager.getPassword(AuthenticationRequestType.GSS, info); - org.postgresql.gss.MakeGSS.authenticate(true, pgStream, host, user, password, - PGProperty.JAAS_APPLICATION_NAME.get(info), - PGProperty.KERBEROS_SERVER_NAME.get(info), false, // TODO: fix this - PGProperty.JAAS_LOGIN.getBoolean(info), - PGProperty.LOG_SERVER_ERROR_DETAIL.getBoolean(info)); + AuthenticationPluginManager.withPassword(AuthenticationRequestType.GSS, info, password -> { + org.postgresql.gss.MakeGSS.authenticate(true, pgStream, host, user, password, + PGProperty.JAAS_APPLICATION_NAME.get(info), + PGProperty.KERBEROS_SERVER_NAME.get(info), false, // TODO: fix this + PGProperty.JAAS_LOGIN.getBoolean(info), + PGProperty.LOG_SERVER_ERROR_DETAIL.getBoolean(info)); + return void.class; + }); return pgStream; } catch (PSQLException ex) { // allow the connection to proceed @@ -660,17 +661,23 @@ private void doAuthentication(PGStream pgStream, String host, String user, Prope LOGGER.log(Level.FINEST, " <=BE AuthenticationReqMD5(salt={0})", Utils.toHexString(md5Salt)); } - byte[] encodedPassword = AuthenticationPluginManager.getEncodedPassword(AuthenticationRequestType.MD5_PASSWORD, info); - byte[] digest = - MD5Digest.encode(user.getBytes(StandardCharsets.UTF_8), encodedPassword, md5Salt); + byte[] digest = AuthenticationPluginManager.withEncodedPassword( + AuthenticationRequestType.MD5_PASSWORD, info, + encodedPassword -> MD5Digest.encode(user.getBytes(StandardCharsets.UTF_8), + encodedPassword, md5Salt) + ); if (LOGGER.isLoggable(Level.FINEST)) { LOGGER.log(Level.FINEST, " FE=> Password(md5digest={0})", new String(digest, StandardCharsets.US_ASCII)); } - pgStream.sendChar('p'); - pgStream.sendInteger4(4 + digest.length + 1); - pgStream.send(digest); + try { + pgStream.sendChar('p'); + pgStream.sendInteger4(4 + digest.length + 1); + pgStream.send(digest); + } finally { + java.util.Arrays.fill(digest, (byte) 0); + } pgStream.sendChar(0); pgStream.flush(); @@ -681,11 +688,12 @@ private void doAuthentication(PGStream pgStream, String host, String user, Prope LOGGER.log(Level.FINEST, "<=BE AuthenticationReqPassword"); LOGGER.log(Level.FINEST, " FE=> Password(password=)"); - byte[] encodedPassword = AuthenticationPluginManager.getEncodedPassword(AuthenticationRequestType.CLEARTEXT_PASSWORD, info); - - pgStream.sendChar('p'); - pgStream.sendInteger4(4 + encodedPassword.length + 1); - pgStream.send(encodedPassword); + AuthenticationPluginManager.withEncodedPassword(AuthenticationRequestType.CLEARTEXT_PASSWORD, info, encodedPassword -> { + pgStream.sendChar('p'); + pgStream.sendInteger4(4 + encodedPassword.length + 1); + pgStream.send(encodedPassword); + return void.class; + }); pgStream.sendChar(0); pgStream.flush(); @@ -756,12 +764,14 @@ private void doAuthentication(PGStream pgStream, String host, String user, Prope castNonNull(sspiClient).startSSPI(); } else { /* Use JGSS's GSSAPI for this request */ - String password = AuthenticationPluginManager.getPassword(AuthenticationRequestType.GSS, info); - org.postgresql.gss.MakeGSS.authenticate(false, pgStream, host, user, password, - PGProperty.JAAS_APPLICATION_NAME.get(info), - PGProperty.KERBEROS_SERVER_NAME.get(info), usespnego, - PGProperty.JAAS_LOGIN.getBoolean(info), - PGProperty.LOG_SERVER_ERROR_DETAIL.getBoolean(info)); + AuthenticationPluginManager.withPassword(AuthenticationRequestType.GSS, info, password -> { + org.postgresql.gss.MakeGSS.authenticate(false, pgStream, host, user, password, + PGProperty.JAAS_APPLICATION_NAME.get(info), + PGProperty.KERBEROS_SERVER_NAME.get(info), usespnego, + PGProperty.JAAS_LOGIN.getBoolean(info), + PGProperty.LOG_SERVER_ERROR_DETAIL.getBoolean(info)); + return void.class; + }); } break; @@ -775,20 +785,21 @@ private void doAuthentication(PGStream pgStream, String host, String user, Prope case AUTH_REQ_SASL: LOGGER.log(Level.FINEST, " <=BE AuthenticationSASL"); - String password = AuthenticationPluginManager.getPassword(AuthenticationRequestType.SASL, info); - if (password == null) { - throw new PSQLException( - GT.tr( - "The server requested SCRAM-based authentication, but no password was provided."), - PSQLState.CONNECTION_REJECTED); - } - if (password.equals("")) { - throw new PSQLException( - GT.tr( - "The server requested SCRAM-based authentication, but the password is an empty string."), - PSQLState.CONNECTION_REJECTED); - } - scramAuthenticator = new org.postgresql.jre7.sasl.ScramAuthenticator(user, castNonNull(password), pgStream); + scramAuthenticator = AuthenticationPluginManager.withPassword(AuthenticationRequestType.SASL, info, password -> { + if (password == null) { + throw new PSQLException( + GT.tr( + "The server requested SCRAM-based authentication, but no password was provided."), + PSQLState.CONNECTION_REJECTED); + } + if (password.length == 0) { + throw new PSQLException( + GT.tr( + "The server requested SCRAM-based authentication, but the password is an empty string."), + PSQLState.CONNECTION_REJECTED); + } + return new org.postgresql.jre7.sasl.ScramAuthenticator(user, String.valueOf(password), pgStream); + }); scramAuthenticator.processServerMechanismsAndInit(); scramAuthenticator.sendScramClientFirstMessage(); // This works as follows: diff --git a/pgjdbc/src/main/java/org/postgresql/gss/GSSCallbackHandler.java b/pgjdbc/src/main/java/org/postgresql/gss/GSSCallbackHandler.java index 220b59e9dd..2d37e1c687 100644 --- a/pgjdbc/src/main/java/org/postgresql/gss/GSSCallbackHandler.java +++ b/pgjdbc/src/main/java/org/postgresql/gss/GSSCallbackHandler.java @@ -23,9 +23,9 @@ class GSSCallbackHandler implements CallbackHandler { private final String user; - private final @Nullable String password; + private final char @Nullable [] password; - GSSCallbackHandler(String user, @Nullable String password) { + GSSCallbackHandler(String user, char @Nullable [] password) { this.user = user; this.password = password; } @@ -56,7 +56,7 @@ public void handle(Callback[] callbacks) throws IOException, UnsupportedCallback if (password == null) { throw new IOException("No cached kerberos ticket found and no password supplied."); } - pc.setPassword(password.toCharArray()); + pc.setPassword(password); } else { throw new UnsupportedCallbackException(callback, "Unrecognized Callback"); } diff --git a/pgjdbc/src/main/java/org/postgresql/gss/MakeGSS.java b/pgjdbc/src/main/java/org/postgresql/gss/MakeGSS.java index b3e7922260..7dd299e077 100644 --- a/pgjdbc/src/main/java/org/postgresql/gss/MakeGSS.java +++ b/pgjdbc/src/main/java/org/postgresql/gss/MakeGSS.java @@ -27,7 +27,7 @@ public class MakeGSS { private static final Logger LOGGER = Logger.getLogger(MakeGSS.class.getName()); public static void authenticate(boolean encrypted, - PGStream pgStream, String host, String user, @Nullable String password, + PGStream pgStream, String host, String user, char @Nullable [] password, @Nullable String jaasApplicationName, @Nullable String kerberosServerName, boolean useSpnego, boolean jaasLogin, boolean logServerErrorDetail) diff --git a/pgjdbc/src/main/java/org/postgresql/plugin/AuthenticationPlugin.java b/pgjdbc/src/main/java/org/postgresql/plugin/AuthenticationPlugin.java index 461af5931a..6c699a22c1 100644 --- a/pgjdbc/src/main/java/org/postgresql/plugin/AuthenticationPlugin.java +++ b/pgjdbc/src/main/java/org/postgresql/plugin/AuthenticationPlugin.java @@ -10,7 +10,24 @@ import org.checkerframework.checker.nullness.qual.Nullable; public interface AuthenticationPlugin { - @Nullable - String getPassword(AuthenticationRequestType type) throws PSQLException; + + /** + * Callback method to provide the password to use for authentication. + * + *

Implementers can also check the authentication type to ensure that the + * authentication handshake is using a specific authentication method (e.g. SASL) + * or avoiding a specific one (e.g. cleartext).

+ * + *

For security reasons, the driver will wipe the contents of the array returned + * by this method after it has been used for authentication.

+ * + *

Implementers must provide a new array each time this method is invoked as + * the previous contents will have been wiped.

+ * + * @param type The authentication method that the server is requesting + * @return The password to use or null if no password is available + * @throws PSQLException if something goes wrong supplying the password + */ + char @Nullable [] getPassword(AuthenticationRequestType type) throws PSQLException; } diff --git a/pgjdbc/src/test/java/org/postgresql/test/plugin/AuthenticationPluginTest.java b/pgjdbc/src/test/java/org/postgresql/test/plugin/AuthenticationPluginTest.java index b726eb2a12..eaee9a04e1 100644 --- a/pgjdbc/src/test/java/org/postgresql/test/plugin/AuthenticationPluginTest.java +++ b/pgjdbc/src/test/java/org/postgresql/test/plugin/AuthenticationPluginTest.java @@ -32,11 +32,11 @@ public static class DummyAuthenticationPlugin implements AuthenticationPlugin { private static Consumer onGetPassword; @Override - public @Nullable String getPassword(AuthenticationRequestType type) throws PSQLException { + public @Nullable char[] getPassword(AuthenticationRequestType type) throws PSQLException { onGetPassword.accept(type); // Ex: "MD5" => "DUMMY-MD5" - return "DUMMY-" + type.toString(); + return ("DUMMY-" + type.toString()).toCharArray(); } }