You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomee.apache.org by Romain Manni-Bucau <rm...@gmail.com> on 2015/11/02 20:36:15 UTC
Re: Fwd: tomee git commit: TOMEE-1648: The password cipher is called
each time the datasource is built even if we already have the clear password
Ok just checked and we didnt expose the setPassword to JMX for dbcp so we
are fine, sorry for the noise!
Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> | Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Tomitriber
<http://www.tomitribe.com>
2015-10-30 17:39 GMT-07:00 Jean-Louis Monteiro <jl...@tomitribe.com>:
> Sure. Maybe the fix can be improved. Let's discuss
> Le 30 oct. 2015 17:37, "Romain Manni-Bucau" <rm...@gmail.com> a
> écrit :
>
> > It prevents to reset the pwd at runtime if i read it correctly. It should
> > be fixed since it is used at least through jmx.
> > ---------- Message transféré ----------
> > De : <jl...@apache.org>
> > Date : 30 oct. 2015 16:14
> > Objet : tomee git commit: TOMEE-1648: The password cipher is called each
> > time the datasource is built even if we already have the clear password
> > À : <co...@tomee.apache.org>
> > Cc :
> >
> > Repository: tomee
> > Updated Branches:
> > refs/heads/master 34c4cc7a2 -> 436089b65
> >
> >
> > 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/master
> > Commit: 436089b658e239757e0a51b49be01974ca339627
> > Parents: 34c4cc7
> > Author: Jean-Louis Monteiro <jl...@apache.org>
> > Authored: Fri Oct 30 16:13:56 2015 -0700
> > Committer: Jean-Louis Monteiro <jl...@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());
> > + }
> > + }
> > + }
> > +
> > +
> > +}
> >
>
Re: Fwd: tomee git commit: TOMEE-1648: The password cipher is called
each time the datasource is built even if we already have the clear password
Posted by Jean-Louis Monteiro <jl...@tomitribe.com>.
oh that is all perfect feedback
--
Jean-Louis Monteiro
http://twitter.com/jlouismonteiro
http://www.tomitribe.com
On Mon, Nov 2, 2015 at 11:36 AM, Romain Manni-Bucau <rm...@gmail.com>
wrote:
> Ok just checked and we didnt expose the setPassword to JMX for dbcp so we
> are fine, sorry for the noise!
>
>
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> <http://rmannibucau.wordpress.com> | Github <
> https://github.com/rmannibucau> |
> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Tomitriber
> <http://www.tomitribe.com>
>
> 2015-10-30 17:39 GMT-07:00 Jean-Louis Monteiro <jl...@tomitribe.com>:
>
> > Sure. Maybe the fix can be improved. Let's discuss
> > Le 30 oct. 2015 17:37, "Romain Manni-Bucau" <rm...@gmail.com> a
> > écrit :
> >
> > > It prevents to reset the pwd at runtime if i read it correctly. It
> should
> > > be fixed since it is used at least through jmx.
> > > ---------- Message transféré ----------
> > > De : <jl...@apache.org>
> > > Date : 30 oct. 2015 16:14
> > > Objet : tomee git commit: TOMEE-1648: The password cipher is called
> each
> > > time the datasource is built even if we already have the clear password
> > > À : <co...@tomee.apache.org>
> > > Cc :
> > >
> > > Repository: tomee
> > > Updated Branches:
> > > refs/heads/master 34c4cc7a2 -> 436089b65
> > >
> > >
> > > 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/master
> > > Commit: 436089b658e239757e0a51b49be01974ca339627
> > > Parents: 34c4cc7
> > > Author: Jean-Louis Monteiro <jl...@apache.org>
> > > Authored: Fri Oct 30 16:13:56 2015 -0700
> > > Committer: Jean-Louis Monteiro <jl...@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());
> > > + }
> > > + }
> > > + }
> > > +
> > > +
> > > +}
> > >
> >
>