* KeyManager: Eliminate races, buffer I/O, eliminate periodic syncing

router/java/src/net/i2p/router/KeyManager.java

@@ -8,10 +8,14 @@ package net.i2p.router;
  *
  */
 
+import java.io.BufferedInputStream;
+import java.io.BufferedOutputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
+import java.io.InputStream;
 import java.io.IOException;
+import java.io.OutputStream;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
@@ -42,7 +46,6 @@ public class KeyManager {
     private SigningPrivateKey _signingPrivateKey;
     private SigningPublicKey _signingPublicKey;
     private final Map<Hash, LeaseSetKeys> _leaseSetKeys; // Destination --> LeaseSetKeys
-    private final SynchronizeKeysJob _synchronizeJob;
     
     public final static String PROP_KEYDIR = "router.keyBackupDir";
     public final static String DEFAULT_KEYDIR = "keyBackup";
@@ -50,49 +53,38 @@ public class KeyManager {
     private final static String KEYFILE_PUBLIC_ENC = "publicEncryption.key";
     private final static String KEYFILE_PRIVATE_SIGNING = "privateSigning.key";
     private final static String KEYFILE_PUBLIC_SIGNING = "publicSigning.key";
-    // Doesn't seem like we need to periodically back up,
-    // since we don't store leaseSet keys,
-    // but for now just make it a long time.
-    private final static long DELAY = 7*24*60*60*1000;
     
     public KeyManager(RouterContext context) {
         _context = context;
         _log = _context.logManager().getLog(KeyManager.class);	
-        _synchronizeJob = new SynchronizeKeysJob();
         _leaseSetKeys = new ConcurrentHashMap();
     }
     
     public void startup() {
-        queueWrite();
+        // run inline so keys are loaded immediately
+        (new SynchronizeKeysJob()).runJob();
     }
     
-    /** Configure the router's private key */
+    /**
-    public void setPrivateKey(PrivateKey key) { 
+     *  Configure the router's keys.
-        _privateKey = key; 
+     *  @since 0.9.4 replace individual setters
-        if (key != null)
+     */
-            queueWrite();
+    public void setKeys(PublicKey key1, PrivateKey key2, SigningPublicKey key3, SigningPrivateKey key4) { 
+        synchronized(this) {
+            _publicKey = key1; 
+            _privateKey = key2; 
+            _signingPublicKey = key3; 
+            _signingPrivateKey = key4; 
+        }
+        queueWrite();
     }
+
     public PrivateKey getPrivateKey() { return _privateKey; }
-    /** Configure the router's public key */
+
-    public void setPublicKey(PublicKey key) { 
-        _publicKey = key; 
-        if (key != null)
-            queueWrite();
-    }
     public PublicKey getPublicKey() { return _publicKey; }
-    /** Configure the router's signing private key */
+
-    public void setSigningPrivateKey(SigningPrivateKey key) { 
-        _signingPrivateKey = key; 
-        if (key != null)
-            queueWrite();
-    }
     public SigningPrivateKey getSigningPrivateKey() { return _signingPrivateKey; }
-    /** Configure the router's signing public key */
+
-    public void setSigningPublicKey(SigningPublicKey key) { 
-        _signingPublicKey = key; 
-        if (key != null)
-            queueWrite();
-    }
     public SigningPublicKey getSigningPublicKey() { return _signingPublicKey; }
     
     public void registerKeys(Destination dest, SigningPrivateKey leaseRevocationPrivateKey, PrivateKey endpointDecryptionKey) {
@@ -102,15 +94,10 @@ public class KeyManager {
     }
    
     /**
-     *  Wait one second, as this will get called 4 times in quick succession
+     *  Read/Write the router keys from/to disk
-     *  There is still a race here though, if a key is set while the sync job is running
      */
     private void queueWrite() {
-        Clock cl = _context.clock();
+        _context.jobQueue().addJob(new SynchronizeKeysJob());
-        JobQueue q = _context.jobQueue();
-        if ( (cl == null) || (q == null) ) return;
-        _synchronizeJob.getTiming().setStartAfter(cl.now() + 1000);
-        q.addJob(_synchronizeJob);
     }
 
     public LeaseSetKeys unregisterKeys(Destination dest) {
@@ -122,27 +109,36 @@ public class KeyManager {
     public LeaseSetKeys getKeys(Destination dest) {
         return getKeys(dest.calculateHash());
     }
+
     public LeaseSetKeys getKeys(Hash dest) {
             return _leaseSetKeys.get(dest);
     }
     
+    /**
+     *  Read/Write the 4 files in keyBackup/
+     *  As of 0.9.4 this is run on-demand only, there's no need to
+     *  periodically sync.
+     *  Actually, there's little need for this at all.
+     *  If router.keys is corrupt, we should just make a new router identity,
+     *  there's no real reason to try so hard to recover our old keys.
+     */
     private class SynchronizeKeysJob extends JobImpl {
         public SynchronizeKeysJob() {
             super(KeyManager.this._context);
         }
+
         public void runJob() {
             String keyDir = getContext().getProperty(PROP_KEYDIR, DEFAULT_KEYDIR);
             File dir = new SecureDirectory(getContext().getRouterDir(), keyDir);
             if (!dir.exists())
                 dir.mkdirs();
             if (dir.exists() && dir.isDirectory() && dir.canRead() && dir.canWrite()) {
-                syncKeys(dir);
+                synchronized(KeyManager.this) {
+                    syncKeys(dir);
+                }
             } else {
                 _log.log(Log.CRIT, "Unable to synchronize keys in " + keyDir + " - permissions problem?");
             }
-
-            getTiming().setStartAfter(KeyManager.this._context.clock().now()+DELAY);
-            KeyManager.this._context.jobQueue().addJob(this);
         }
         
         private void syncKeys(File keyDir) {
@@ -152,7 +148,7 @@ public class KeyManager {
             syncVerificationKey(keyDir);
         }
 
-        private synchronized void syncPrivateKey(File keyDir) {
+        private void syncPrivateKey(File keyDir) {
             DataStructure ds;
             File keyFile = new File(keyDir, KEYFILE_PRIVATE_ENC);
             boolean exists = (_privateKey != null);
@@ -165,7 +161,7 @@ public class KeyManager {
                 _privateKey = (PrivateKey) readin;
         }
 
-        private synchronized void syncPublicKey(File keyDir) {
+        private void syncPublicKey(File keyDir) {
             DataStructure ds;
             File keyFile = new File(keyDir, KEYFILE_PUBLIC_ENC);
             boolean exists = (_publicKey != null);
@@ -178,7 +174,7 @@ public class KeyManager {
                 _publicKey = (PublicKey) readin;
         }
 
-        private synchronized void syncSigningKey(File keyDir) {
+        private void syncSigningKey(File keyDir) {
             DataStructure ds;
             File keyFile = new File(keyDir, KEYFILE_PRIVATE_SIGNING);
             boolean exists = (_signingPrivateKey != null);
@@ -191,7 +187,7 @@ public class KeyManager {
                 _signingPrivateKey = (SigningPrivateKey) readin;
         }
 
-        private synchronized void syncVerificationKey(File keyDir) {
+        private void syncVerificationKey(File keyDir) {
             DataStructure ds;
             File keyFile = new File(keyDir, KEYFILE_PUBLIC_SIGNING);
             boolean exists = (_signingPublicKey != null);
@@ -205,16 +201,16 @@ public class KeyManager {
         }
 
         private DataStructure syncKey(File keyFile, DataStructure structure, boolean exists) {
-            FileOutputStream out = null;
+            OutputStream out = null;
-            FileInputStream in = null;
+            InputStream in = null;
             try {
                 if (exists) {
-                    out = new SecureFileOutputStream(keyFile);
+                    out = new BufferedOutputStream(new SecureFileOutputStream(keyFile));
                     structure.writeBytes(out);
                     return structure;
                 } else {
                     if (keyFile.exists()) {
-                        in = new FileInputStream(keyFile);
+                        in = new BufferedInputStream(new FileInputStream(keyFile));
                         structure.readBytes(in);
                         return structure;
                     } else {

router/java/src/net/i2p/router/startup/CreateRouterInfoJob.java

@@ -8,9 +8,11 @@ package net.i2p.router.startup;
  *
  */
 
+import java.io.BufferedOutputStream;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.io.OutputStream;
 import java.util.Properties;
 
 import net.i2p.data.Certificate;
@@ -50,12 +52,25 @@ public class CreateRouterInfoJob extends JobImpl {
     }
     
     /**
-     *  Caller must hold Router.routerInfoFileLock
+     *  Writes 6 files: router.info (standard RI format),
+     *  router,keys, and 4 individual key files under keyBackup/
+     *
+     *  router.keys file format: Note that this is NOT the
+     *  same "eepPriv.dat" format used by the client code.
+     *<pre>
+     *   - Private key (256 bytes)
+     *   - Signing Private key (20 bytes)
+     *   - Public key (256 bytes)
+     *   - Signing Public key (128 bytes)
+     *  Total 660 bytes
+     *</pre>
+     *
+     *  Caller must hold Router.routerInfoFileLock.
      */
     RouterInfo createRouterInfo() {
         RouterInfo info = new RouterInfo();
-        FileOutputStream fos1 = null;
+        OutputStream fos1 = null;
-        FileOutputStream fos2 = null;
+        OutputStream fos2 = null;
         try {
             info.setAddresses(getContext().commSystem().createAddresses());
             Properties stats = getContext().statPublisher().publishStatistics();
@@ -89,21 +104,18 @@ public class CreateRouterInfoJob extends JobImpl {
             
             String infoFilename = getContext().getProperty(Router.PROP_INFO_FILENAME, Router.PROP_INFO_FILENAME_DEFAULT);
             File ifile = new File(getContext().getRouterDir(), infoFilename);
-            fos1 = new SecureFileOutputStream(ifile);
+            fos1 = new BufferedOutputStream(new SecureFileOutputStream(ifile));
             info.writeBytes(fos1);
             
             String keyFilename = getContext().getProperty(Router.PROP_KEYS_FILENAME, Router.PROP_KEYS_FILENAME_DEFAULT);
             File kfile = new File(getContext().getRouterDir(), keyFilename);
-            fos2 = new SecureFileOutputStream(kfile);
+            fos2 = new BufferedOutputStream(new SecureFileOutputStream(kfile));
             privkey.writeBytes(fos2);
             signingPrivKey.writeBytes(fos2);
             pubkey.writeBytes(fos2);
             signingPubKey.writeBytes(fos2);
             
-            getContext().keyManager().setSigningPrivateKey(signingPrivKey);
+            getContext().keyManager().setKeys(pubkey, privkey, signingPubKey, signingPrivKey);
-            getContext().keyManager().setSigningPublicKey(signingPubKey);
-            getContext().keyManager().setPrivateKey(privkey);
-            getContext().keyManager().setPublicKey(pubkey);
             
             _log.info("Router info created and stored at " + ifile.getAbsolutePath() + " with private keys stored at " + kfile.getAbsolutePath() + " [" + info + "]");
         } catch (DataFormatException dfe) {

router/java/src/net/i2p/router/startup/LoadRouterInfoJob.java

@@ -8,8 +8,10 @@ package net.i2p.router.startup;
  *
  */
 
+import java.io.BufferedInputStream;
 import java.io.File;
 import java.io.FileInputStream;
+import java.io.InputStream;
 import java.io.IOException;
 
 import net.i2p.data.DataFormatException;
@@ -64,8 +66,8 @@ public class LoadRouterInfoJob extends JobImpl {
         if (rkf.exists())
             _keysExist = true;
         
-        FileInputStream fis1 = null;
+        InputStream fis1 = null;
-        FileInputStream fis2 = null;
+        InputStream fis2 = null;
         try {
             // if we have a routerinfo but no keys, things go bad in a hurry:
             // CRIT   ...rkdb.PublishLocalRouterInfoJob: Internal error - signing private key not known?  rescheduling publish for 30s
@@ -74,7 +76,7 @@ public class LoadRouterInfoJob extends JobImpl {
             // at net.i2p.router.transport.udp.PacketBuilder.buildSessionConfirmedPacket(PacketBuilder.java:574)
             // so pretend the RI isn't there if there is no keyfile
             if (_infoExists && _keysExist) {
-                fis1 = new FileInputStream(rif);
+                fis1 = new BufferedInputStream(new FileInputStream(rif));
                 info = new RouterInfo();
                 info.readBytes(fis1);
                 // Catch this here before it all gets worse
@@ -86,7 +88,7 @@ public class LoadRouterInfoJob extends JobImpl {
             }
             
             if (_keysExist) {
-                fis2 = new FileInputStream(rkf);
+                fis2 = new BufferedInputStream(new FileInputStream(rkf));
                 PrivateKey privkey = new PrivateKey();
                 privkey.readBytes(fis2);
                 SigningPrivateKey signingPrivKey = new SigningPrivateKey();
@@ -96,10 +98,7 @@ public class LoadRouterInfoJob extends JobImpl {
                 SigningPublicKey signingPubKey = new SigningPublicKey();
                 signingPubKey.readBytes(fis2);
                 
-                getContext().keyManager().setPrivateKey(privkey);
+                getContext().keyManager().setKeys(pubkey, privkey, signingPubKey, signingPrivKey);
-                getContext().keyManager().setSigningPrivateKey(signingPrivKey);
-                getContext().keyManager().setPublicKey(pubkey); //info.getIdentity().getPublicKey());
-                getContext().keyManager().setSigningPublicKey(signingPubKey); // info.getIdentity().getSigningPublicKey());
             }
         } catch (IOException ioe) {
             _log.log(Log.CRIT, "Error reading the router info from " + rif.getAbsolutePath() + " and the keys from " + rkf.getAbsolutePath(), ioe);