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);
+        }
+    }
+}