Skip to content

Commit

Permalink
TEIIDDES-2139 / 2237 - Adds a password token to connections and servers
Browse files Browse the repository at this point in the history 
* Since the url of the server is not unique, it is possible to overwrite
  passwords held in secure storage with values from other servers or
  jdbc connections.

* Adds a pass token to the servers and the connection properties which
  provides a unique reference to the password stored in secure storage

* Pass token generate with 1-way hash of the password and the url

* By carrying the pass token, each connection / server retain their unique
  password without actually holding it in the clear or revealing it until
  its actually used

* TeiidConnectionTest
 * Unit tests added for possible use cases involving password permutations
   hopefully ensuring that previous password methods of storage are taken
   into account and things stay backward compatible

* [*]SecureStorageProvider
 * Adds existence method to test a node keys existence since null is a
   valid password value

* TeiidConnectionInfo
 * Removes caching password entirely since it just confuses
 * Explicitly tests for storage password existence rather than just checking
   whether retrieved password is null
 * Only if there was a password when changing url fields is the password
   retrieved and updated with the new url
  • Loading branch information
Paul Richardson committed Aug 6, 2014
1 parent 1023f1c commit bbdeaf4
Show file tree
Hide file tree
Showing 14 changed files with 511 additions and 86 deletions.
14 changes: 11 additions & 3 deletions ...ols.connectivity.ui/src/org/teiid/datatools/connectivity/ui/TeiidDriverUIContributor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,10 +230,13 @@ public void setConnectionInformation() {

/*
* Avoid placing the password into properties that are persisted in the clear
* by securely storing the password against the connection's url
* by securely storing the password against the connection's url and a 1-way hash of the url and password
*/
String urlStorageKey = ConnectivityUtil.buildSecureStorageKey(TeiidJDBCConnection.class, url);
try {
String passToken = ConnectivityUtil.generateHashToken(url, this.passwordText.getText());
properties.setProperty(IJDBCDriverDefinitionConstants.PASSWORD_PROP_ID, passToken);
String urlStorageKey = ConnectivityUtil.buildSecureStorageKey(TeiidJDBCConnection.class, url, passToken);

ConnectivityUtil.getSecureStorageProvider().storeInSecureStorage(
urlStorageKey,
ConnectivityUtil.JDBC_PASSWORD,
Expand Down Expand Up @@ -381,7 +384,12 @@ public void loadProperties() {
updateURL();

String urlString = urlText.getText().trim();
String urlStorageKey = ConnectivityUtil.buildSecureStorageKey(TeiidJDBCConnection.class, urlString);
/*
* The pass token not the actual password is provided by the PASSWORD property. This provides a
* reference to a node key made from a hash of the url and original password.
*/
String passToken = this.properties.getProperty(IJDBCDriverDefinitionConstants.PASSWORD_PROP_ID);
String urlStorageKey = ConnectivityUtil.buildSecureStorageKey(TeiidJDBCConnection.class, urlString, passToken);
String password = null;

/* Retrieve the password from the secure storage */
Expand Down
1 change: 1 addition & 0 deletions plugins/org.teiid.datatools.connectivity/META-INF/MANIFEST.MF
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Require-Bundle: org.eclipse.ui;bundle-version="[3.103.0,4.0.0)",
org.eclipse.datatools.connectivity.sqm.core;bundle-version="[1.2.5,2.0.0)",
org.eclipse.datatools.sqltools.parsers.sql;bundle-version="[1.0.2,2.0.0)",
org.eclipse.datatools.connectivity.sqm.server.ui;bundle-version="[1.1.100,2.0.0)",
org.apache.commons.codec;bundle-version="[1.3.0,2.0.0)",
org.teiid.datatools.connectivity.model;bundle-version="[8.1.0,9.0.0)";visibility:=reexport,
org.eclipse.equinox.security;bundle-version="[1.1.100,2.0.0)",
org.teiid.designer.spi;bundle-version="[8.1.0,9.0.0)"
Expand Down
98 changes: 80 additions & 18 deletions ...g.teiid.datatools.connectivity/src/org/teiid/datatools/connectivity/ConnectivityUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
*/
package org.teiid.datatools.connectivity;

import java.security.MessageDigest;
import java.sql.Driver;
import java.util.Properties;
import org.apache.commons.codec.binary.Base64;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.IStatus;
Expand Down Expand Up @@ -63,6 +65,11 @@ public class ConnectivityUtil {
*/
public static final String JDBC_PASSWORD = "jdbc_password"; //$NON-NLS-1$

/**
* Prefix for identifying a hash token
*/
public static final String TOKEN_PREFIX = "TOK##!"; //$NON-NLS-1$

/**
* Tries to find the most appropriate driver definition id for the given teiid version.
* <p>
Expand Down Expand Up @@ -108,7 +115,7 @@ private static String getTeiidDriverDefinitionId(ITeiidServerVersion serverVersi
}

private static String createTeiidDriverInstance( ITeiidServerVersion serverVersion, String jarList,
String driverURL, String username ) {
String driverURL, String username, String passToken ) {
/* Create an ad-hoc version to avoid clashing ids with any built-in teiid runtime drivers */
String driverId = TEIID_ADHOC_DRIVER_ID_SKELETON
.replaceAll("MAJOR", serverVersion.getMajor()) //$NON-NLS-1$
Expand All @@ -128,6 +135,9 @@ private static String createTeiidDriverInstance( ITeiidServerVersion serverVersi
if (null != username) {
baseProperties.setProperty(IJDBCDriverDefinitionConstants.USERNAME_PROP_ID, username);
}
if (null != passToken) {
baseProperties.setProperty(IJDBCDriverDefinitionConstants.PASSWORD_PROP_ID, passToken);
}

baseProperties.setProperty(IJDBCDriverDefinitionConstants.DATABASE_VENDOR_PROP_ID, TEIID_DATABASE_VENDOR_NAME);
baseProperties.setProperty(IJDBCDriverDefinitionConstants.DATABASE_VERSION_PROP_ID, serverVersion.toString());
Expand All @@ -147,11 +157,16 @@ private static String createTeiidDriverInstance( ITeiidServerVersion serverVersi
* @param url
* @param password
*/
private static void storeJDBCPassword(String url, String password) {
private static void storeJDBCPassword(String url, String passToken, String password) {
if (password == null)
return;

String urlStorageKey = ConnectivityUtil.buildSecureStorageKey(TeiidJDBCConnection.class, url);
String urlStorageKey;
if (passToken == null)
urlStorageKey = ConnectivityUtil.buildSecureStorageKey(TeiidJDBCConnection.class, url);
else
urlStorageKey = ConnectivityUtil.buildSecureStorageKey(TeiidJDBCConnection.class, url, passToken);

try {
getSecureStorageProvider().storeInSecureStorage(urlStorageKey, ConnectivityUtil.JDBC_PASSWORD, password);
} catch (Exception ex) {
Expand All @@ -160,32 +175,34 @@ private static void storeJDBCPassword(String url, String password) {
}

private static String acquireDriverDefinition(ITeiidServerVersion serverVersion, String driverPath,
String connectionURL, String username, String password)
String connectionURL, String username, String passToken, String password)
throws Exception {
String driverDefinitionId = getTeiidDriverDefinitionId(serverVersion);
DriverInstance mDriver = DriverManager.getInstance().getDriverInstanceByID(driverDefinitionId);
if (mDriver == null) {
driverDefinitionId = createTeiidDriverInstance(serverVersion, driverPath, connectionURL, username);
driverDefinitionId = createTeiidDriverInstance(serverVersion, driverPath, connectionURL, username, passToken);
} else {
// JBIDE-7493 Eclipse updates can break profiles because the driverPath is plugin version specific.
String jarList = mDriver.getJarList();
if(jarList != driverPath) {
mDriver.getPropertySet().getBaseProperties().put(IDriverMgmtConstants.PROP_DEFN_JARLIST, driverPath);
}
mDriver.getPropertySet().getBaseProperties().put(IJDBCDriverDefinitionConstants.URL_PROP_ID, connectionURL);
mDriver.getPropertySet().getBaseProperties().put(IJDBCDriverDefinitionConstants.PASSWORD_PROP_ID, passToken);
}

if (driverDefinitionId == null)
throw new Exception(Messages.getString(Messages.ConnectivityUtil.noTeiidDriverDefinitionFound, serverVersion));

storeJDBCPassword(connectionURL, password);
storeJDBCPassword(connectionURL, passToken, password);
return driverDefinitionId;
}

private static Properties createDriverProps(ITeiidServerVersion serverVersion, String driverId,
String jarList,
String driverURL,
String username,
String passToken,
String vdbName ) {
Properties baseProperties = new Properties();
baseProperties.setProperty(IDriverMgmtConstants.PROP_DEFN_JARLIST, jarList);
Expand All @@ -194,6 +211,9 @@ private static Properties createDriverProps(ITeiidServerVersion serverVersion, S
if(null != username) {
baseProperties.setProperty(IJDBCDriverDefinitionConstants.USERNAME_PROP_ID, username);
}
if (null != passToken) {
baseProperties.setProperty(IJDBCDriverDefinitionConstants.PASSWORD_PROP_ID, passToken);
}
baseProperties.setProperty(IJDBCDriverDefinitionConstants.DATABASE_VENDOR_PROP_ID, TEIID_DATABASE_VENDOR_NAME);
baseProperties.setProperty(IJDBCDriverDefinitionConstants.DATABASE_VERSION_PROP_ID, serverVersion.toString());
baseProperties.setProperty(IJDBCDriverDefinitionConstants.DATABASE_NAME_PROP_ID, vdbName);
Expand Down Expand Up @@ -224,12 +244,14 @@ public static IConnectionProfile createTransientTeiidProfile( ITeiidServerVersio
ProfileManager pm = ProfileManager.getInstance();

try {
String driverDefinitionId = acquireDriverDefinition(serverVersion, driverPath, connectionURL, username, password);
String passToken = ConnectivityUtil.generateHashToken(connectionURL, password);
String driverDefinitionId = acquireDriverDefinition(serverVersion, driverPath, connectionURL, username, passToken, password);
return pm.createTransientProfile(TEIID_PROFILE_PROVIDER_ID, createDriverProps(serverVersion,
driverDefinitionId,
driverPath,
connectionURL,
username,
passToken,
vdbName));
} catch (Exception e) {
Status status = new Status(IStatus.ERROR, Activator.PLUGIN_ID, 0,
Expand Down Expand Up @@ -268,8 +290,9 @@ public static Properties createVDBTeiidProfileProperties(ITeiidServerVersion ser

String driverDefinitionId;
try {
driverDefinitionId = acquireDriverDefinition(serverVersion, driverPath, connectionURL, username, password);
return createDriverProps(serverVersion, driverDefinitionId, driverPath, connectionURL, username, vdbName);
String passToken = ConnectivityUtil.generateHashToken(connectionURL, password);
driverDefinitionId = acquireDriverDefinition(serverVersion, driverPath, connectionURL, username, passToken, password);
return createDriverProps(serverVersion, driverDefinitionId, driverPath, connectionURL, username, passToken, vdbName);
} catch (Exception ex) {
Status status = new Status(IStatus.ERROR, Activator.PLUGIN_ID, 0,
Messages.getString(Messages.ConnectivityUtil.errorGettingProfileProperties), ex);
Expand Down Expand Up @@ -297,7 +320,8 @@ public static IConnectionProfile createVDBTeiidProfile( ITeiidServerVersion serv
String profileName) throws CoreException {
ProfileManager pm = ProfileManager.getInstance();
try {
String driverDefinitionId = acquireDriverDefinition(serverVersion, driverPath, connectionURL, username, password);
String passToken = ConnectivityUtil.generateHashToken(connectionURL, password);
String driverDefinitionId = acquireDriverDefinition(serverVersion, driverPath, connectionURL, username, passToken, password);

IConnectionProfile existingCP = pm.getProfileByName(profileName);
if (existingCP != null) {
Expand All @@ -311,6 +335,7 @@ public static IConnectionProfile createVDBTeiidProfile( ITeiidServerVersion serv
driverPath,
connectionURL,
username,
passToken,
vdbName));
} catch (Exception e) {
Status status = new Status(IStatus.ERROR, Activator.PLUGIN_ID, 0,
Expand All @@ -321,22 +346,58 @@ public static IConnectionProfile createVDBTeiidProfile( ITeiidServerVersion serv

/**
* Assemble the secure storage node key based
* upon the given class and connection url
* upon the given class and the set of arguments.
*
* @param klazz
* @param connectURL
* @param args
*
* @return fully qualified key used in secure storage
*/
public static String buildSecureStorageKey(Class<?> klazz, String connectURL) {
String secureKey = new StringBuilder(SECURE_STORAGE_BASEKEY)
public static String buildSecureStorageKey(Class<?> klazz, String... args) {
StringBuilder secureKeyBuilder = new StringBuilder(SECURE_STORAGE_BASEKEY)
.append(IPath.SEPARATOR)
.append(klazz.getSimpleName())
.append(IPath.SEPARATOR)
.append(connectURL)
.append(IPath.SEPARATOR).toString();
.append(IPath.SEPARATOR);

if (args != null) {
for (String arg : args) {
if (arg == null)
continue;

secureKeyBuilder.append(arg).append(IPath.SEPARATOR);
}
}

return secureKeyBuilder.toString();
}

/**
* @param args
* @return the hash of the given string
* @throws Exception
*/
public static String generateHashToken(String... args) throws Exception {

return secureKey;
StringBuilder builder = new StringBuilder();
for (String arg : args) {
builder.append(arg);
builder.append(IPath.SEPARATOR);
}

MessageDigest sha = MessageDigest.getInstance("SHA-512"); //$NON-NLS-1$
sha.update(builder.toString().getBytes("UTF-8")); //$NON-NLS-1$
byte[] hashedByteArray = sha.digest();

// Use Base64 encoding here -->
return TOKEN_PREFIX + Base64.encodeBase64URLSafeString(hashedByteArray);
}

/**
* @param arg
* @return true if this given arg is a generated hash token
*/
public static boolean isPasswordToken(String arg) {
return arg != null && arg.startsWith(TOKEN_PREFIX);
}

/**
Expand Down Expand Up @@ -368,4 +429,5 @@ public static Driver getTeiidDriver(ITeiidServerVersion teiidServerVersion, Stri
String msg = Messages.getString(Messages.ConnectivityUtil.noTeiidDriverFound, driverClass, teiidServerVersion);
throw new IllegalStateException(msg);
}

}
7 changes: 6 additions & 1 deletion ...eiid.datatools.connectivity/src/org/teiid/datatools/connectivity/TeiidJDBCConnection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,12 @@ protected Object createConnection( ClassLoader classloader ) throws Throwable {
String connectURL = props.getProperty(IJDBCDriverDefinitionConstants.URL_PROP_ID);
String uid = props.getProperty(IJDBCDriverDefinitionConstants.USERNAME_PROP_ID);

String urlStorageKey = ConnectivityUtil.buildSecureStorageKey(getClass(), connectURL);
/*
* The pass token not the actual password is provided by the PASSWORD property. This provides a
* reference to a node key made from a hash of the url and original password.
*/
String passToken = props.getProperty(IJDBCDriverDefinitionConstants.PASSWORD_PROP_ID);
String urlStorageKey = ConnectivityUtil.buildSecureStorageKey(getClass(), connectURL, passToken);
String pwd = ConnectivityUtil.getSecureStorageProvider()
.getFromSecureStorage(urlStorageKey, ConnectivityUtil.JDBC_PASSWORD);

Expand Down
39 changes: 35 additions & 4 deletions ...vity/src/org/teiid/datatools/connectivity/security/impl/EquinoxSecureStorageProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

import java.io.UnsupportedEncodingException;
import java.net.URLEncoder;
import java.util.Arrays;
import java.util.List;
import org.eclipse.equinox.security.storage.EncodingUtils;
import org.eclipse.equinox.security.storage.ISecurePreferences;
import org.eclipse.equinox.security.storage.SecurePreferencesFactory;
Expand Down Expand Up @@ -39,6 +41,23 @@ public static EquinoxSecureStorageProvider getInstance() {

return instance;
}

@Override
public boolean existsInSecureStorage(String nodeKey, String key) throws Exception {
ISecurePreferences root = getRoot();
String encoded = encode(nodeKey);

if (! root.nodeExists(encoded))
return false;

ISecurePreferences node = root.node(encoded);
String[] keys = node.keys();
if (keys == null)
return false;

List<String> keyList = Arrays.asList(keys);
return keyList.contains(key);
}

@Override
public String getFromSecureStorage(String nodeKey, String key) throws Exception {
Expand All @@ -53,8 +72,19 @@ public String getFromSecureStorage(String nodeKey, String key) throws Exception
@Override
public void storeInSecureStorage(String nodeKey, String key, String value) throws Exception {
ISecurePreferences node = getNode(nodeKey);
if (value == null) node.put(key, value, true);
else node.put(key, EncodingUtils.encodeBase64(value.getBytes()), true /* encrypt */);
if (value == null)
node.put(key, value, true);
else
node.put(key, EncodingUtils.encodeBase64(value.getBytes()), true /* encrypt */);
}

private ISecurePreferences getRoot() {
return SecurePreferencesFactory.getDefault();
}

private String encode(String nodeKey) throws UnsupportedEncodingException {
String encoded = URLEncoder.encode(nodeKey, "UTF-8"); //$NON-NLS-1$
return encoded;
}

/**
Expand All @@ -68,8 +98,9 @@ public void storeInSecureStorage(String nodeKey, String key, String value) throw
* @throws UnsupportedEncodingException
*/
private ISecurePreferences getNode(String nodeKey) throws Exception {
ISecurePreferences root = SecurePreferencesFactory.getDefault();
String encoded = URLEncoder.encode(nodeKey, "UTF-8"); //$NON-NLS-1$
ISecurePreferences root = getRoot();
String encoded = encode(nodeKey);

return root.node(encoded);
}
}
16 changes: 7 additions & 9 deletions ...ins/org.teiid.designer.dqp.ui/src/org/teiid/designer/runtime/ui/vdb/ExecuteVdbWorker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import org.eclipse.core.resources.IFile;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.Status;
import org.eclipse.datatools.connectivity.IConnectionProfile;
import org.eclipse.datatools.connectivity.ProfileManager;
import org.eclipse.jface.dialogs.ErrorDialog;
Expand Down Expand Up @@ -193,14 +194,7 @@ public void processForDTP(ITeiidServer teiidServer, String vdbName)
TeiidCPWizardDialog wizardDialog = new TeiidCPWizardDialog(Display.getCurrent().getActiveShell(), wiz);
wizardDialog.setProperties(cpProps);
wizardDialog.setBlockOnOpen(true);
if (wizardDialog.open() == Window.OK) {
profile = wiz.getParentProfile();
try {
PlatformUI.getWorkbench().showPerspective(DTP_PERSPECTIVE,DqpUiPlugin.getDefault().getCurrentWorkbenchWindow());
} catch (Throwable e) {
DqpUiConstants.UTIL.log(e);
}
} else {
if (wizardDialog.open() != Window.OK) {
return;
}
// if we have all the info we create it w/o user interaction
Expand All @@ -214,7 +208,11 @@ public void processForDTP(ITeiidServer teiidServer, String vdbName)
profileName);
}
}
IStatus connectionStatus = profile.connectWithoutJob();

IStatus connectionStatus = Status.OK_STATUS;
if( profile != null) {
connectionStatus = profile.connectWithoutJob();
}
try {
PlatformUI.getWorkbench().showPerspective(DTP_PERSPECTIVE,
DqpUiPlugin.getDefault().getCurrentWorkbenchWindow());
Expand Down

0 comments on commit bbdeaf4

Please sign in to comment.