TOMEE-1648: The password cipher is called each time the datasource is built even if we already have the clear password
Project: http://git-wip-us.apache.org/repos/asf/tomee/repo Commit: http://git-wip-us.apache.org/repos/asf/tomee/commit/436089b6 Tree: http://git-wip-us.apache.org/repos/asf/tomee/tree/436089b6 Diff: http://git-wip-us.apache.org/repos/asf/tomee/diff/436089b6 Branch: refs/heads/tomee-7.0.0-M1 Commit: 436089b658e239757e0a51b49be01974ca339627 Parents: 34c4cc7 Author: Jean-Louis Monteiro <jlmonte...@apache.org> Authored: Fri Oct 30 16:13:56 2015 -0700 Committer: Jean-Louis Monteiro <jlmonte...@apache.org> Committed: Fri Oct 30 16:13:56 2015 -0700 ---------------------------------------------------------------------- .../resource/jdbc/dbcp/BasicDataSource.java | 20 +- .../jdbc/dbcp/BasicManagedDataSource.java | 23 ++- .../jdbc/CipheredPasswordDataSourceTest.java | 193 +++++++++++++++++++ 3 files changed, 233 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tomee/blob/436089b6/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/BasicDataSource.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/BasicDataSource.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/BasicDataSource.java index ffb5bba..995221c 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/BasicDataSource.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/BasicDataSource.java @@ -55,6 +55,10 @@ public class BasicDataSource extends org.apache.commons.dbcp2.BasicDataSource im * not ciphered. The {@link org.apache.openejb.cipher.PlainTextPasswordCipher} can also be used. */ private String passwordCipher; + + // keep tracking the user configured password in case we need it to be decrypted again + private String initialPassword; + private JMXBasicDataSource jmxDs; private CommonDataSource delegate; private String name; @@ -128,6 +132,18 @@ public class BasicDataSource extends org.apache.commons.dbcp2.BasicDataSource im } } + @Override + public void setPassword(final String password) { + final ReentrantLock l = lock; + l.lock(); + try { + // keep the encrypted value if it's encrypted + this.initialPassword = password; + super.setPassword(password); + } finally { + l.unlock(); + } + } public String getUserName() { final ReentrantLock l = lock; @@ -226,7 +242,9 @@ public class BasicDataSource extends org.apache.commons.dbcp2.BasicDataSource im // check password codec if available if (null != passwordCipher && !"PlainText".equals(passwordCipher)) { final PasswordCipher cipher = PasswordCipherFactory.getPasswordCipher(passwordCipher); - final String plainPwd = cipher.decrypt(getPassword().toCharArray()); + + // always use the initial encrypted value + final String plainPwd = cipher.decrypt(initialPassword.toCharArray()); // override previous password value super.setPassword(plainPwd); http://git-wip-us.apache.org/repos/asf/tomee/blob/436089b6/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/BasicManagedDataSource.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/BasicManagedDataSource.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/BasicManagedDataSource.java index ec604b0..0846f21 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/BasicManagedDataSource.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/BasicManagedDataSource.java @@ -54,6 +54,10 @@ public class BasicManagedDataSource extends org.apache.commons.dbcp2.managed.Bas * not ciphered. The {@link org.apache.openejb.cipher.PlainTextPasswordCipher} can also be used. */ private String passwordCipher; + + // keep tracking the user configured password in case we need it to be decrypted again + private String initialPassword; + private JMXBasicDataSource jmxDs; public BasicManagedDataSource(final String name) { @@ -85,7 +89,7 @@ public class BasicManagedDataSource extends org.apache.commons.dbcp2.managed.Bas private void setJndiXaDataSource(final String xaDataSource) { setXaDataSourceInstance( // proxy cause we don't know if this datasource was created before or not the delegate - XADataSourceResource.proxy(getDriverClassLoader() != null ? getDriverClassLoader() : Thread.currentThread().getContextClassLoader(), xaDataSource)); + XADataSourceResource.proxy(getDriverClassLoader() != null ? getDriverClassLoader() : Thread.currentThread().getContextClassLoader(), xaDataSource)); if (getTransactionManager() == null) { setTransactionManager(OpenEJB.getTransactionManager()); @@ -133,6 +137,19 @@ public class BasicManagedDataSource extends org.apache.commons.dbcp2.managed.Bas } } + @Override + public void setPassword(final String password) { + final ReentrantLock l = lock; + l.lock(); + try { + // keep the encrypted value if it's encrypted + this.initialPassword = password; + super.setPassword(password); + } finally { + l.unlock(); + } + } + public String getUserName() { final ReentrantLock l = lock; l.lock(); @@ -229,7 +246,9 @@ public class BasicManagedDataSource extends org.apache.commons.dbcp2.managed.Bas // check password codec if available if (null != passwordCipher) { final PasswordCipher cipher = PasswordCipherFactory.getPasswordCipher(passwordCipher); - final String plainPwd = cipher.decrypt(getPassword().toCharArray()); + + // always use the initial encrypted value + final String plainPwd = cipher.decrypt(initialPassword.toCharArray()); // override previous password value super.setPassword(plainPwd); http://git-wip-us.apache.org/repos/asf/tomee/blob/436089b6/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/CipheredPasswordDataSourceTest.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/CipheredPasswordDataSourceTest.java b/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/CipheredPasswordDataSourceTest.java new file mode 100644 index 0000000..0f1224f --- /dev/null +++ b/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/CipheredPasswordDataSourceTest.java @@ -0,0 +1,193 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.openejb.resource.jdbc; + +import org.apache.commons.dbcp.PoolableConnection; +import org.apache.openejb.cipher.PasswordCipher; +import org.apache.openejb.jee.EjbJar; +import org.apache.openejb.jee.SingletonBean; +import org.apache.openejb.junit.ApplicationComposer; +import org.apache.openejb.resource.jdbc.dbcp.BasicManagedDataSource; +import org.apache.openejb.testing.Configuration; +import org.apache.openejb.testing.Module; +import org.apache.openejb.util.Strings; +import org.apache.openejb.util.reflection.Reflections; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; + +import javax.annotation.Resource; +import javax.ejb.EJB; +import javax.ejb.EJBContext; +import javax.ejb.LocalBean; +import javax.ejb.Singleton; +import javax.ejb.Startup; +import javax.ejb.TransactionAttribute; +import javax.ejb.TransactionAttributeType; +import javax.sql.DataSource; +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.DriverPropertyInfo; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.SQLFeatureNotSupportedException; +import java.sql.Statement; +import java.util.Properties; +import java.util.logging.Logger; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +@RunWith(ApplicationComposer.class) +public class CipheredPasswordDataSourceTest { + private static final String URL = "jdbc:fake://sthg:3306"; + private static final String USER = "pete"; + private static final String ENCRYPTED_PASSWORD = "This is the encrypted value."; + + @EJB + private Persister persistManager; + + @Resource + private DataSource ds; + + @Configuration + public Properties config() { + final Properties p = new Properties(); + p.put("openejb.jdbc.datasource-creator", "dbcp"); + + p.put("managed", "new://Resource?type=DataSource"); + p.put("managed.JdbcDriver", FakeDriver.class.getName()); + p.put("managed.JdbcUrl", URL); + p.put("managed.UserName", USER); + p.put("managed.Password", ENCRYPTED_PASSWORD); + p.put("managed.PasswordCipher", EmptyPasswordCipher.class.getName()); + p.put("managed.JtaManaged", "true"); + p.put("managed.initialSize", "10"); + p.put("managed.maxActive", "10"); + p.put("managed.maxIdle", "10"); + p.put("managed.minIdle", "10"); + p.put("managed.maxWait", "200"); + p.put("managed.defaultAutoCommit", "false"); + p.put("managed.defaultReadOnly", "false"); + p.put("managed.testOnBorrow", "true"); + p.put("managed.testOnReturn", "true"); + p.put("managed.testWhileIdle", "true"); + + return p; + } + + @Module + public EjbJar app() throws Exception { + return new EjbJar() + .enterpriseBean(new SingletonBean(Persister.class).localBean()); + + } + + @Before + public void createDatasource(){ + // This is to make sure TomEE created the data source. + persistManager.save(); + } + + @Test + public void rebuild() { + // because of the exception at startup, the pool creating aborted + // before fixing this bug, the password was already decrypted, but the data source field + // wasn't yet initialized, so this.data source == null + // but the next time we try to initialize it, it tries to decrypt and already decrypted password + + try { + ds.getConnection(); + + } catch (final SQLException e) { + System.out.println(e.getMessage()); + // success - it throws the SQLException from the connect + // but it does not fail with the IllegalArgumentException from the Password Cipher + } + } + + public static class EmptyPasswordCipher implements PasswordCipher { + + @Override + public char[] encrypt(String plainPassword) { + throw new RuntimeException("Should never be called in this test."); + } + + @Override + public String decrypt(char[] encryptedPassword) { + System.out.println(String.format(">>> Decrypt password '%s'", new String(encryptedPassword))); + + // we want to know if the password as already been called + if (encryptedPassword.length == 0) { + throw new IllegalArgumentException("Can only decrypt a non empty string."); + } + + return ""; + } + } + + public static class FakeDriver implements java.sql.Driver { + public boolean acceptsURL(final String url) throws SQLException { + return false; + } + + public Logger getParentLogger() throws SQLFeatureNotSupportedException { + return null; + } + + public Connection connect(final String url, final Properties info) throws SQLException { + throw new SQLException("Pete said it first fails here!"); + } + + public int getMajorVersion() { + return 0; + } + + public int getMinorVersion() { + return 0; + } + + public DriverPropertyInfo[] getPropertyInfo(final String url, final Properties info) throws SQLException { + return new DriverPropertyInfo[0]; + } + + public boolean jdbcCompliant() { + return false; + } + } + + @LocalBean + @Singleton + @TransactionAttribute(TransactionAttributeType.REQUIRES_NEW) + public static class Persister { + + @Resource(name = "managed") + private DataSource ds; + + public void save() { + try { + ds.getConnection(); + + } catch (SQLException e) { + System.err.println("Generated SQL error > " + e.getMessage()); + } + } + } + + +}
