Router family fixes

Don't overwrite an existing family keystore file
Don't allow starting a new family with an existing family keystore file
Don't add family to RI until we have restarted and have a keystore file
Don't fail a netdb store for no family sig
Don't ever fail our own netdb store for family errors, to avoid rekey/restart

apps/routerconsole/java/src/net/i2p/router/web/helpers/ConfigFamilyHandler.java

@@ -37,11 +37,20 @@ public class ConfigFamilyHandler extends FormHandler {
             } else if (family.length() > 32) {
                 // let's enforce some sanity
                 addFormError("Family too long, 32 chars max: " + family);
-            } else if (_context.router().saveConfig(FamilyKeyCrypto.PROP_FAMILY_NAME, family.trim())) {
-                addFormNotice(_t("Configuration saved successfully."));
-                addFormError(_t("Restart required to take effect"));
             } else {
-                addFormError(_t("Error saving the configuration (applied but not saved) - please see the error logs"));
+                family = family.trim();
+                File ks = new SecureDirectory(_context.getConfigDir(), "keystore");
+                ks = new File(ks, FamilyKeyCrypto.KEYSTORE_PREFIX + family + FamilyKeyCrypto.KEYSTORE_SUFFIX);
+                if (ks.exists()) {
+                    addFormError("Keystore for family " + family + " already exists! Delete or rename it first: " + ks);
+                } else {
+                    if (_context.router().saveConfig(FamilyKeyCrypto.PROP_FAMILY_NAME, family.trim())) {
+                        addFormNotice(_t("Configuration saved successfully."));
+                        addFormError(_t("Restart required to take effect"));
+                    } else {
+                        addFormError(_t("Error saving the configuration (applied but not saved) - please see the error logs"));
+                    }
+                }
             }
         } else if (_action.equals(_t("Join Family"))) {
             InputStream in = _requestWrapper.getInputStream("file");
@@ -57,6 +66,11 @@ public class ConfigFamilyHandler extends FormHandler {
                 String family = CertUtil.getSubjectValue(certs.get(0), "CN");
                 if (family == null) {
                     addFormError("Bad certificate - No Subject CN");
+                    return;
+                }
+                if (family.contains("/") || family.contains("\\")) {
+                    addFormError("Bad characters in Family: " + family);
+                    return;
                 }
                 if (family.endsWith(FamilyKeyCrypto.CN_SUFFIX) && family.length() > FamilyKeyCrypto.CN_SUFFIX.length())
                     family = family.substring(0, family.length() - FamilyKeyCrypto.CN_SUFFIX.length());
@@ -65,6 +79,10 @@ public class ConfigFamilyHandler extends FormHandler {
                 if (!ks.exists())
                     ks.mkdirs();
                 ks = new File(ks, FamilyKeyCrypto.KEYSTORE_PREFIX + family + FamilyKeyCrypto.KEYSTORE_SUFFIX);
+                if (ks.exists()) {
+                    addFormError("Keystore for family " + family + " already exists! Delete or rename it first: " + ks);
+                    return;
+                }
                 String keypw = KeyStoreUtil.randomString();
                 KeyStoreUtil.storePrivateKey(ks, KeyStoreUtil.DEFAULT_KEYSTORE_PASSWORD, family, keypw, pk, certs);
                 // store certificate

router/java/src/net/i2p/router/StatisticsManager.java

@@ -159,7 +159,6 @@ public class StatisticsManager {
 
         String family = _context.getProperty(FamilyKeyCrypto.PROP_FAMILY_NAME);
         if (family != null) {
-            stats.setProperty(FamilyKeyCrypto.OPT_NAME, family);
             String sig = null;
             String key = null;
             RouterInfo oldRI = _context.router().getRouterInfo();
@@ -173,10 +172,12 @@ public class StatisticsManager {
                             // we changed the separator from ';' to ':'
                             key = null;
                         } else {
-                            stats.setProperty(FamilyKeyCrypto.OPT_KEY, key);
                             sig = oldRI.getOption(FamilyKeyCrypto.OPT_SIG);
-                            if (sig != null)
+                            if (sig != null) {
+                                stats.setProperty(FamilyKeyCrypto.OPT_NAME, family);
+                                stats.setProperty(FamilyKeyCrypto.OPT_KEY, key);
                                 stats.setProperty(FamilyKeyCrypto.OPT_SIG, sig);
+                            }
                         }
                     }
                 }

router/java/src/net/i2p/router/networkdb/kademlia/KademliaNetworkDatabaseFacade.java

@@ -1131,8 +1131,15 @@ public abstract class KademliaNetworkDatabaseFacade extends NetworkDatabaseFacad
             switch (r) {
                 case BAD_KEY:
                 case INVALID_SIG:
+                    Hash h = routerInfo.getHash();
+                    // never fail our own router, that would cause a restart and rekey
+                    if (h.equals(_context.routerHash()))
+                        break;
+                    return "Bad family " + r + ' ' + h;
+
                 case NO_SIG:
-                    return "Bad family " + r + ' ' + routerInfo.getHash();
+                    // Routers older than 0.9.54 that added a family and haven't restarted
+                    break;
 
                 case BAD_SIG:
                     // To be investigated