Properly verify Device ID's entered by the user synchronously (fixes #159) (#160)

* Add Utils/Luhn.java for check rune calculation

* Update model/Device.java defaults

according to Device defaults in ConfigXml#getDevices

* Remove errorListener from ConfigRouter#addDevice

* Remove errorListener from RestApi#addDevice

Remove no longer used RestApi#normalizeDeviceId

* Add checkDeviceID to model/Device.java

and verify device ID's entered by the user before writing them to the config.

* Fix lint by using Locale.ROOT for internal strings

app/src/main/java/com/nutomic/syncthingandroid/activities/DeviceActivity.java

@@ -427,16 +427,17 @@ public class DeviceActivity extends SyncthingActivity
                             .show();
                     return true;
                 }
+                if (!mDevice.checkDeviceID()) {
+                    Toast.makeText(this, R.string.device_id_invalid, Toast.LENGTH_LONG)
+                            .show();
+                    return true;
+                }
                 if (isEmpty(mDevice.name)) {
                     Toast.makeText(this, R.string.device_name_required, Toast.LENGTH_LONG)
                             .show();
                     return true;
                 }
-                mConfig.addDevice(
-                    getApi(),
-                    mDevice,
-                    error -> Toast.makeText(this, error, Toast.LENGTH_LONG).show()
-                );
+                mConfig.addDevice(getApi(), mDevice);
                 finish();
                 return true;
             case R.id.share_device_id:

app/src/main/java/com/nutomic/syncthingandroid/model/Device.java

@@ -1,18 +1,26 @@
 package com.nutomic.syncthingandroid.model;
 
 import android.text.TextUtils;
+import java.util.Locale;
+// import android.util.Log;
 
+import com.google.common.io.BaseEncoding;
+
+import com.nutomic.syncthingandroid.util.Luhn;
+
+import java.lang.System;
+import java.util.Arrays;
 import java.util.List;
 
 public class Device {
     public String deviceID;
     public String name = "";
     public List<String> addresses;
-    public String compression;
+    public String compression = "metadata";
     public String certName;
     public String introducedBy = "";
     public boolean introducer = false;
-    public boolean paused;
+    public boolean paused = false;
     public List<PendingFolder> pendingFolders;
     public List<IgnoredFolder> ignoredFolders;
 
@@ -35,4 +43,73 @@ public class Device {
                 ? (TextUtils.isEmpty(deviceID) ? "" : deviceID.substring(0, 7))
                 : name;
     }
+
+    /**
+     * Returns if a syncthing device ID is correctly formatted.
+     */
+    public Boolean checkDeviceID() {
+        /**
+         * See https://github.com/syncthing/syncthing/blob/master/lib/protocol/deviceid.go
+         * how syncthing validates device IDs.
+         * Old dirty way to check was: return deviceID.matches("^([A-Z0-9]{7}-){7}[A-Z0-9]{7}$");
+         */
+        String deviceID = new String(this.deviceID);
+
+        // Trim "="
+        deviceID = deviceID.replaceAll("=", "");
+
+        // Convert to upper case.
+        deviceID = deviceID.toUpperCase(Locale.ROOT);
+
+        // untypeoify
+        deviceID = deviceID.replaceAll("1", "I");
+        deviceID = deviceID.replaceAll("0", "O");
+        deviceID = deviceID.replaceAll("8", "B");
+
+        // unchunkify
+        deviceID = deviceID.replaceAll("-", "");
+        deviceID = deviceID.replaceAll(" ", "");
+
+        // Check length.
+        switch(deviceID.length()) {
+            case 0:
+                // Log.w(TAG, "checkDeviceID: Empty device ID.");
+                return false;
+            case 56:
+                // unluhnify(deviceID)
+                byte bytesIn[] = deviceID.getBytes();
+                byte res[] = new byte[52];
+                for (int i = 0; i < 4; i++) {
+                    byte[] p = Arrays.copyOfRange(bytesIn, i*(13+1), (i+1)*(13+1)-1);
+                    System.arraycopy(p, 0, res, i*13, 13);
+
+                    // Generate check digit.
+                    Luhn luhn = new Luhn();
+                    String checkRune = luhn.generate(p);
+                    // Log.v(TAG, "checkDeviceID: luhn.generate(" + new String(p) + ") returned (" + checkRune + ")");
+                    if (checkRune == null) {
+                        // Log.w(TAG, "checkDeviceID: deviceID=(" + deviceID + "): invalid character");
+                        return false;
+                    }
+                    if (!deviceID.substring((i+1)*14-1, (i+1)*14-1+1).equals(checkRune)) {
+                        // Log.w(TAG, "checkDeviceID: deviceID=(" + deviceID + "): check digit incorrect");
+                        return false;
+                    }
+                }
+                deviceID = new String(res);
+                // Log.v(TAG, "isDeviceIdValid: unluhnify(deviceID)=" + deviceID);
+                // Fall-Through
+            case 52:
+                try {
+                    BaseEncoding.base32().decode(deviceID + "====");
+                    return true;
+                } catch (IllegalArgumentException e) {
+                    // Log.w(TAG, "checkDeviceID: deviceID=(" + deviceID + "): invalid character, base32 decode failed");
+                    return false;
+                }
+            default:
+                // Log.w(TAG, "checkDeviceID: Incorrect length (" + deviceID + ")");
+                return false;
+        }
+    }
 }

app/src/main/java/com/nutomic/syncthingandroid/service/RestApi.java

@@ -531,13 +531,11 @@ public class RestApi {
         throw new RuntimeException("RestApi.getLocalDevice: Failed to get the local device crucial to continuing execution.");
     }
 
-    public void addDevice(Device device, OnResultListener1<String> errorListener) {
-        normalizeDeviceId(device.deviceID, normalizedId -> {
-            synchronized (mConfigLock) {
-                mConfig.devices.add(device);
-                sendConfig();
-            }
-        }, errorListener);
+    public void addDevice(Device device) {
+        synchronized (mConfigLock) {
+            mConfig.devices.add(device);
+            sendConfig();
+        }
     }
 
     public void updateDevice(Device newDevice) {
@@ -760,24 +758,6 @@ public class RestApi {
         });
     }
 
-    /**
-     * Normalizes a given device ID.
-     */
-    private void normalizeDeviceId(String id, OnResultListener1<String> listener,
-                                   OnResultListener1<String> errorListener) {
-        new GetRequest(mContext, mUrl, GetRequest.URI_DEVICEID, mApiKey,
-                ImmutableMap.of("id", id), result -> {
-            JsonObject json = new JsonParser().parse(result).getAsJsonObject();
-            JsonElement normalizedId = json.get("id");
-            JsonElement error = json.get("error");
-            if (normalizedId != null)
-                listener.onResult(normalizedId.getAsString());
-            if (error != null)
-                errorListener.onResult(error.getAsString());
-        });
-    }
-
-
     /**
      * Updates cached folder and device completion info according to event data.
      */

app/src/main/java/com/nutomic/syncthingandroid/util/ConfigRouter.java

@@ -124,17 +124,17 @@ public class ConfigRouter {
         return restApi.getDevices(includeLocal);
     }
 
-    public void addDevice(RestApi restApi, Device device, OnResultListener1<String> errorListener) {
+    public void addDevice(RestApi restApi, Device device) {
         if (restApi == null || !restApi.isConfigLoaded()) {
             // Syncthing is not running or REST API is not (yet) available.
             configXml.loadConfig();
-            configXml.addDevice(device, error -> errorListener.onResult(error));
+            configXml.addDevice(device);
             configXml.saveChanges();
             return;
         }
 
         // Syncthing is running and REST API is available.
-        restApi.addDevice(device, error -> errorListener.onResult(error));       // This will send the config afterwards.
+        restApi.addDevice(device);       // This will send the config afterwards.
     }
 
     public void updateDevice(RestApi restApi, final Device device) {

app/src/main/java/com/nutomic/syncthingandroid/util/ConfigXml.java

@@ -154,7 +154,9 @@ public class ConfigXml {
         String localDeviceID = logOutput.replace("\n", "");
 
         // Verify that local device ID is correctly formatted.
-        if (!isDeviceIdValid(localDeviceID)) {
+        Device localDevice = new Device();
+        localDevice.deviceID = localDeviceID;
+        if (!localDevice.checkDeviceID()) {
             Log.w(TAG, "getLocalDeviceIDandStoreToPref: Syncthing core returned a bad formatted device ID \"" + localDeviceID + "\"");
             return "";
         }
@@ -682,12 +684,7 @@ public class ConfigXml {
         return devices;
     }
 
-    public void addDevice(final Device device, OnResultListener1<String> errorListener) {
-        if (!isDeviceIdValid(device.deviceID)) {
-            errorListener.onResult(mContext.getString(R.string.device_id_invalid));
-            return;
-        }
-
+    public void addDevice(final Device device) {
         Log.v(TAG, "addDevice: deviceID=" + device.deviceID);
         Node nodeConfig = mConfig.getDocumentElement();
         Node nodeDevice = mConfig.createElement("device");
@@ -861,13 +858,6 @@ public class ConfigXml {
         return sb.toString();
     }
 
-    /**
-     * Returns if a syncthing device ID is correctly formatted.
-     */
-    private Boolean isDeviceIdValid(final String deviceID) {
-        return deviceID.matches("^([A-Z0-9]{7}-){7}[A-Z0-9]{7}$");
-    }
-
     /**
      * Writes updated mConfig back to file.
      */

app/src/main/java/com/nutomic/syncthingandroid/util/Luhn.java

@@ -0,0 +1,45 @@
+package com.nutomic.syncthingandroid.util;
+
+// import android.util.Log;
+
+public final class Luhn {
+
+    private static final String TAG = "Luhn";
+
+    /**
+     * An alphabet is a string of N characters, representing the digits of a given
+     * base N.
+     */
+    private static final String LUHN_ALPHABET = "ABCDEFGHIJKLMNOPQRSTUVWXYZ234567";
+
+    /**
+     *
+     * generate returns a check digit for the string s, which should be composed
+     * of characters from the Alphabet a.
+     * Doesn't follow the actual Luhn algorithm
+     * see https://forum.syncthing.net/t/v0-9-0-new-node-id-format/478/6 for more.
+     */
+    public String generate (byte[] s) {
+        int factor = 1;
+        int sum = 0;
+        int n = LUHN_ALPHABET.length();
+
+        for (int i = 0; i < s.length; i++) {
+            int codepoint = LUHN_ALPHABET.indexOf(s[i]);
+            // Log.v(TAG, "generate: codepoint = " + codepoint);
+            if (codepoint == -1) {
+                // Error "Digit %q not valid in alphabet %q", s[i], a
+                return null;
+            }
+            int addend = factor * codepoint;
+            factor = (factor == 2 ? 1 : 2);
+            addend = (addend / n) + (addend % n);
+            sum += addend;
+        }
+        int remainder = sum % n;
+        int checkCodepoint = (n - remainder) % n;
+        // Log.v(TAG, "generate: checkCodepoint = " + checkCodepoint);
+        return LUHN_ALPHABET.substring(checkCodepoint, checkCodepoint+1);
+    }
+
+}