Mercurial > hg > release > thermostat-2.0
changeset 2483:366dfa1b9f2d
Windows - insecure file-based keyring (using ServiceTracker)
Reviewed-by: sgehwolf
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2016-October/021253.html
author | Simon Tooke <stooke@redhat.com> |
---|---|
date | Thu, 20 Oct 2016 11:08:56 -0400 |
parents | e8cf2946b207 |
children | c6d0e7c16513 |
files | keyring/src/main/java/com/redhat/thermostat/utils/keyring/internal/Activator.java keyring/src/main/java/com/redhat/thermostat/utils/keyring/internal/InsecureFileBasedKeyringImpl.java keyring/src/test/java/com/redhat/thermostat/utils/keyring/internal/InsecureFileBasedKeyringImplTest.java |
diffstat | 3 files changed, 452 insertions(+), 25 deletions(-) [+] |
line wrap: on
line diff
--- a/keyring/src/main/java/com/redhat/thermostat/utils/keyring/internal/Activator.java Tue Oct 18 11:45:10 2016 +0200 +++ b/keyring/src/main/java/com/redhat/thermostat/utils/keyring/internal/Activator.java Thu Oct 20 11:08:56 2016 -0400 @@ -36,45 +36,76 @@ package com.redhat.thermostat.utils.keyring.internal; +import com.redhat.thermostat.shared.config.CommonPaths; import org.osgi.framework.BundleActivator; import org.osgi.framework.BundleContext; import com.redhat.thermostat.utils.keyring.Keyring; +import org.osgi.framework.ServiceReference; +import org.osgi.framework.ServiceRegistration; +import org.osgi.util.tracker.ServiceTracker; public class Activator implements BundleActivator { + private static final boolean IS_UNIX = !System.getProperty("os.name").toLowerCase().contains("win"); + + private ServiceRegistration reg; + + private static class DummyKeyringImpl implements Keyring { + + /* Trivial implementation just to keep the world from blowing up. + * Everything noop. + */ + + @Override + public void savePassword(String url, String username, char[] password) { + // NOOP + } + + @Override + public char[] getPassword(String url, String username) { + // NOOP + return new char[]{}; + } + + @Override + public void clearPassword(String url, String username) { + // NOOP + } + } + @Override public void start(BundleContext context) throws Exception { Keyring theKeyring = null; try { theKeyring = new KeyringImpl(); } catch (UnsatisfiedLinkError e) { - theKeyring = new Keyring() { - /* Trivial implementation just to keep the world from blowing up. - * Everything noop. - */ - - @Override - public void savePassword(String url, String username, - char[] password) { - // NOOP - } - - @Override - public char[] getPassword(String url, String username) { - // NOOP - return new char[]{}; - } - - @Override - public void clearPassword(String url, String username) { - // NOOP - } - - }; + if (IS_UNIX) { + theKeyring = new DummyKeyringImpl(); + } + else { + ServiceTracker<CommonPaths,CommonPaths> pathTracker = new ServiceTracker(context,CommonPaths.class.getName(), null) { + @Override + public Object addingService(ServiceReference reference) { + CommonPaths paths = (CommonPaths) super.addingService(reference); + final Keyring theKeyring = new InsecureFileBasedKeyringImpl(paths); + reg = context.registerService(Keyring.class.getName(), theKeyring, null); + return paths; + } + @Override + public void removedService(ServiceReference reference, Object service) { + if (reg != null) { + reg.unregister(); + reg = null; + } + super.removedService(reference, service); + } + }; + pathTracker.open(); + } } - context.registerService(Keyring.class.getName(), theKeyring, null); - + if (theKeyring != null) + context.registerService(Keyring.class.getName(), theKeyring, null); } @Override
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/keyring/src/main/java/com/redhat/thermostat/utils/keyring/internal/InsecureFileBasedKeyringImpl.java Thu Oct 20 11:08:56 2016 -0400 @@ -0,0 +1,206 @@ +/* + * Copyright 2012-2016 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * <http://www.gnu.org/licenses/>. + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.utils.keyring.internal; + +import com.redhat.thermostat.shared.config.CommonPaths; +import com.redhat.thermostat.shared.config.InvalidConfigurationException; +import com.redhat.thermostat.utils.keyring.Keyring; +import com.redhat.thermostat.utils.keyring.KeyringException; + +import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; +import java.util.logging.Logger; + +// A simple class to create a stub keyring; expect it to be replaced to use native keyrings in the future +// The code tries not to keep passwords in memory longer than it has to + +// I/O buffers are the weak spot here - they'll contain file contents. +// Currently the keyring file is unencrypted. + +class InsecureFileBasedKeyringImpl implements Keyring { + + private static final String DEFAULT_KEYRING_FILENAME = "keyring.bin"; + private static final Logger logger = Logger.getLogger(InsecureFileBasedKeyringImpl.class.getName()); + + private final File krFile; + + File getKeyringFile() throws InvalidConfigurationException { + return krFile; + } + + InsecureFileBasedKeyringImpl(final CommonPaths cp) { + final String keyRingFn = cp.getUserConfigurationDirectory() + File.separator + DEFAULT_KEYRING_FILENAME; + krFile = new File(keyRingFn); + init(); + } + + InsecureFileBasedKeyringImpl( final String fn ) { + this.krFile = new File(fn); + init(); + } + + private void init() { + logger.warning("InsecureFileBasedKeyringImpl will store passwords in plaintext"); + final File krf = getKeyringFile(); + logger.info("Using '"+krf+"' as keyring file"); + if (krf.isDirectory()) { + logger.severe("'"+krf+"' is an existing directory"); + throw new InvalidConfigurationException("'"+krf+"' is an existing directory"); + } + else if (!krf.isFile()) { + createKeyring(krf); + } + } + + private void createKeyring( final File krf ) { + final Map<String,Map<String,char[]>> emptyMap = new HashMap<>(); + persist(krf,emptyMap); + } + + @Override + public synchronized void savePassword(String url, String username, char[] password) { + final Map<String,Map<String,char[]>> pwMap = depersist(); + if (!pwMap.containsKey(url)) { + Map<String, char[]> upMap = new HashMap<>(); + pwMap.put(url, upMap); + upMap.put(username, password.clone()); + } + else { + Map<String, char[]> upMap = pwMap.get(url); + upMap.put(username, password.clone()); + } + persist(pwMap); + } + + @Override + public synchronized char[] getPassword(String url, String username) { + final Map<String,Map<String,char[]>> pwMap = depersist(); + Map<String, char[]> upMap = pwMap.get(url); + final char[] oldpwd = upMap == null ? null : upMap.get(username); + final char[] pwd = oldpwd == null ? null : oldpwd.clone(); + persist(pwMap); + return pwd; + } + + @Override + public synchronized void clearPassword(String url, String username) { + final Map<String,Map<String,char[]>> pwMap = depersist(); + Map<String, char[]> upMap = pwMap.get(url); + if (upMap != null) { + char[] pwd = upMap.get(username); + if (pwd != null) { + Arrays.fill(pwd, '\0'); + upMap.remove(username); + persist(pwMap); + } + else { + empty(pwMap); + } + } + else { + empty(pwMap); + } + } + + protected void empty(final Map<String,Map<String,char[]>> pwMap) { + for (final Map.Entry<String, Map<String, char[]>> entry : pwMap.entrySet()) { + for (final Map.Entry<String, char[]> up : entry.getValue().entrySet()) { + Arrays.fill(up.getValue(), '\0'); + } + entry.getValue().clear(); + } + pwMap.clear(); + } + + private void persist( final File krf, final Map<String,Map<String,char[]>> map ) { + try ( FileOutputStream fileOut = new FileOutputStream(krf); + ObjectOutputStream out = new ObjectOutputStream(fileOut);){ + krf.getParentFile().mkdirs(); + out.writeObject(map); + out.close(); + fileOut.close(); + logger.finest("persisted to " + krf + ":\n" + dump(map)); + } catch( IOException e ) { + e.printStackTrace(); + } + } + + private void depersist( final File krf, final Map<String,Map<String,char[]>> map ) { + try ( FileInputStream fileIn = new FileInputStream(krf); + ObjectInputStream in = new ObjectInputStream(fileIn);) { + final Map<String,Map<String,char[]>> newmap = (Map<String,Map<String,char[]>>) in.readObject(); + map.clear(); + map.putAll(newmap); + in.close(); + fileIn.close(); + logger.finest("loaded from " + krf + ":\n" + dump(map)); + } catch(IOException i) { + i.printStackTrace(); + } catch(ClassNotFoundException c) { + throw new KeyringException("Map class not found. Not sure how that could happen..."); + } + } + + protected void persist(final Map<String,Map<String,char[]>> pwMap) { + persist(getKeyringFile(), pwMap); + empty(pwMap); + } + + private Map<String,Map<String,char[]>> depersist() { + final Map<String,Map<String,char[]>> pwMap = new HashMap<>(); + depersist(getKeyringFile(), pwMap); + return pwMap; + } + + private static String dump(final Map<String,Map<String,char[]>> pwMap) { + final StringBuilder bb = new StringBuilder(); + for ( final Map.Entry<String,Map<String, char[]> > entry : pwMap.entrySet()) { + bb.append("url: ").append(entry.getKey()).append('\n'); + for ( final Map.Entry<String, char[]> up : entry.getValue().entrySet()) { + bb.append(" u=").append(up.getKey()).append(" p=").append(up.getValue()).append('\n'); + } + } + return bb.toString(); + } +}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/keyring/src/test/java/com/redhat/thermostat/utils/keyring/internal/InsecureFileBasedKeyringImplTest.java Thu Oct 20 11:08:56 2016 -0400 @@ -0,0 +1,190 @@ +/* + * Copyright 2012-2016 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * <http://www.gnu.org/licenses/>. + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.utils.keyring.internal; + +import com.redhat.thermostat.utils.keyring.Keyring; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import java.io.File; +import java.io.IOException; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + + +public class InsecureFileBasedKeyringImplTest { + + private Keyring keyring = null; + + // tests mostly copied from KeyringImplTest + + private String createTempKeyringKeyringFilename() { + + try { + final File tmpKeyRingFile = File.createTempFile("thermostat-test-keyring-", "dat"); + tmpKeyRingFile.delete(); // we don't want the file, just the name of the file + return tmpKeyRingFile.getCanonicalPath(); + } catch (IOException e) { + e.printStackTrace(); + Assert.fail(); + } + return null; + } + + @Before + public void createPersistantKeyring() { + // create one keyring for most tests to use + keyring = new InsecureFileBasedKeyringImpl(createTempKeyringKeyringFilename()); + } + + @Test + public void verifySavedPasswordIsRetrieveable() { + + final String url = "www.example1.com"; + final String user = "mike"; + final char[] pw = new char[] {'1', '2', '3'}; + + try { + keyring.savePassword(url, user, pw); + assertEquals(new String(pw), new String(keyring.getPassword(url, user))); + } finally { + // Cleanup + keyring.clearPassword(url, user); + } + } + + @Test + public void verifySavedPasswordIsRetrieveableLater() { + + final String url = "www.example1.com"; + final String user = "mike"; + final char[] pw = new char[] {'1', '2', '3'}; + Keyring k2 = null; + + try { + keyring.savePassword(url, user, pw); + assertEquals(new String(pw), new String(keyring.getPassword(url, user))); + k2 = new InsecureFileBasedKeyringImpl(((InsecureFileBasedKeyringImpl) keyring).getKeyringFile().getAbsolutePath()); + assertEquals(new String(pw), new String(k2.getPassword(url, user))); + } finally { + // Cleanup + keyring.clearPassword(url,user); + if (k2 != null) + k2.clearPassword(url, user); + } + + } + + @Test + public void verifySavePasswordReplacesExisting() { + + final String url = "www.example2.com"; + final String user = "mike"; + final char[] pw1 = new char[] {'1', '2', '3'}; + final char[] pw2 = new char[] {'4', '5', '6'}; + + try { + keyring.savePassword(url, user, pw1); + keyring.savePassword(url, user, pw2); + + assertEquals(new String(pw2), new String(keyring.getPassword(url, user))); + } finally { + // Cleanup + keyring.clearPassword(url, user); + } + } + + @Test + public void verifyClearedPasswordIsCleared() { + + String url = "www.example3.com"; + String user = "mike"; + char[] pw = new char[] {'1', '2', '3'}; + + keyring.savePassword(url, user, pw); + assertNotNull(keyring.getPassword(url, user)); + keyring.clearPassword(url, user); + assertNull(keyring.getPassword(url, user)); + } + + @Test + public void multipleServersSameUser() { + + String url1 = "www.example4.com"; + String url2 = "www.fake.com"; + String user = "mike"; + char[] pw1 = new char[] {'1', '2', '3'}; + char[] pw2 = new char[] {'4', '5', '6'}; + + try { + keyring.savePassword(url1, user, pw1); + keyring.savePassword(url2, user, pw2); + + assertEquals(new String(pw1), new String(keyring.getPassword(url1, user))); + assertEquals(new String(pw2), new String(keyring.getPassword(url2, user))); + } finally { + // Cleanup + keyring.clearPassword(url1, user); + keyring.clearPassword(url2, user); + } + } + + @Test + public void multipleUsersSameServer() { + + String url = "www.example5.com"; + String user1 = "mike"; + String user2 = "mary"; + char[] pw1 = new char[] {'1', '2', '3'}; + char[] pw2 = new char[] {'4', '5', '6'}; + + try { + keyring.savePassword(url, user1, pw1); + keyring.savePassword(url, user2, pw2); + + assertEquals(new String(pw1), new String(keyring.getPassword(url, user1))); + assertEquals(new String(pw2), new String(keyring.getPassword(url, user2))); + } finally { + // Cleanup + keyring.clearPassword(url, user1); + keyring.clearPassword(url, user2); + } + } +}