Improve manual entry validation

app/src/main/java/me/impy/aegis/EditProfileActivity.java

@@ -16,9 +16,12 @@ import android.widget.EditText;
 import android.widget.ImageView;
 import android.widget.Spinner;
 
+import me.impy.aegis.crypto.CryptoUtils;
 import me.impy.aegis.crypto.KeyInfo;
+import me.impy.aegis.crypto.KeyInfoException;
 import me.impy.aegis.db.DatabaseEntry;
 import me.impy.aegis.encoding.Base32;
+import me.impy.aegis.helpers.AuthHelper;
 import me.impy.aegis.helpers.SpinnerHelper;
 
 public class EditProfileActivity extends AegisActivity {
@@ -89,7 +92,8 @@ public class EditProfileActivity extends AegisActivity {
 
         byte[] secretBytes = entry.getInfo().getSecret();
         if (secretBytes != null) {
-            _textSecret.setText(Base32.encodeOriginal(secretBytes));
+            char[] secretChars = Base32.encode(secretBytes);
+            _textSecret.setText(secretChars, 0, secretChars.length);
         }
 
         String type = entry.getInfo().getType();
@@ -154,6 +158,11 @@ public class EditProfileActivity extends AegisActivity {
     }
 
     private boolean onSave() {
+        if (_textSecret.length() == 0) {
+            onError("Secret is a required field.");
+            return false;
+        }
+
         int period;
         try {
             period = Integer.parseInt(_textPeriod.getText().toString());
@@ -176,12 +185,20 @@ public class EditProfileActivity extends AegisActivity {
         DatabaseEntry entry = _profile.getEntry();
         entry.setName(_textName.getText().toString());
         KeyInfo info = entry.getInfo();
-        info.setIssuer(_textIssuer.getText().toString());
+
-        info.setSecret(Base32.decode(_textSecret.getText().toString()));
+        try {
-        info.setPeriod(period);
+            char[] secret = AuthHelper.getEditTextChars(_textSecret);
-        info.setDigits(digits);
+            info.setSecret(secret);
-        info.setAlgorithm(algo);
+            CryptoUtils.zero(secret);
-        info.setType(type);
+            info.setIssuer(_textIssuer.getText().toString());
+            info.setPeriod(period);
+            info.setDigits(digits);
+            info.setAlgorithm(algo);
+            info.setType(type);
+        } catch (KeyInfoException e) {
+            onError("The entered info is incorrect: " + e.getMessage());
+            return false;
+        }
 
         Intent intent = new Intent();
         intent.putExtra("KeyProfile", _profile);

app/src/main/java/me/impy/aegis/crypto/KeyInfo.java

@@ -3,8 +3,10 @@ package me.impy.aegis.crypto;
 import android.net.Uri;
 
 import java.io.Serializable;
+import java.util.Arrays;
 
 import me.impy.aegis.encoding.Base32;
+import me.impy.aegis.encoding.Base32Exception;
 
 public class KeyInfo implements Serializable {
     private String _type = "totp";
@@ -24,7 +26,7 @@ public class KeyInfo implements Serializable {
         builder.appendQueryParameter("digits", Integer.toString(_digits));
         builder.appendQueryParameter("period", Integer.toString(_period));
         builder.appendQueryParameter("algorithm", _algorithm);
-        builder.appendQueryParameter("secret", Base32.encodeOriginal(_secret));
+        builder.appendQueryParameter("secret", new String(Base32.encode(_secret)));
         if (_type.equals("hotp")) {
             builder.appendQueryParameter("counter", Long.toString(_counter));
         }
@@ -44,30 +46,25 @@ public class KeyInfo implements Serializable {
         return p - (System.currentTimeMillis() % p);
     }
 
-    public static KeyInfo fromURL(String s) throws Exception {
+    public static KeyInfo fromURL(String s) throws KeyInfoException {
         final Uri url = Uri.parse(s);
         if (!url.getScheme().equals("otpauth")) {
-            throw new Exception("unsupported protocol");
+            throw new KeyInfoException("unsupported protocol");
         }
 
         KeyInfo info = new KeyInfo();
-
+        info.setType(url.getHost());
-        // only 'totp' and 'hotp' are supported
-        info._type = url.getHost();
-        if (info._type.equals("totp") && info._type.equals("hotp")) {
-            throw new Exception("unsupported type");
-        }
 
         // 'secret' is a required parameter
         String secret = url.getQueryParameter("secret");
         if (secret == null) {
-            throw new Exception("'secret' is not set");
+            throw new KeyInfoException("'secret' is not set");
         }
-        info._secret = Base32.decode(secret);
+        info.setSecret(secret.toCharArray());
 
         // provider info used to disambiguate accounts
         String path = url.getPath();
-        String label = path != null ? path.substring(1) : "";
+        String label = path != null && path.length() > 0 ? path.substring(1) : "";
 
         if (label.contains(":")) {
             // a label can only contain one colon
@@ -75,40 +72,40 @@ public class KeyInfo implements Serializable {
             String[] strings = label.split(":");
 
             if (strings.length == 2) {
-                info._issuer = strings[0];
+                info.setIssuer(strings[0]);
-                info._accountName = strings[1];
+                info.setAccountName(strings[1]);
             } else {
                 // at this point, just dump the whole thing into the accountName
-                info._accountName = label;
+                info.setAccountName(label);
             }
         } else {
             // label only contains the account name
             // grab the issuer's info from the 'issuer' parameter if it's present
             String issuer = url.getQueryParameter("issuer");
-            info._issuer = issuer != null ? issuer : "";
+            info.setIssuer(issuer != null ? issuer : "");
-            info._accountName = label;
+            info.setAccountName(label);
         }
 
         // just use the defaults if these parameters aren't set
         String algorithm = url.getQueryParameter("algorithm");
         if (algorithm != null) {
-            info._algorithm = algorithm;
+            info.setAlgorithm(algorithm);
         }
         String period = url.getQueryParameter("period");
         if (period != null) {
-            info._period = Integer.parseInt(period);
+            info.setPeriod(Integer.parseInt(period));
         }
         String digits = url.getQueryParameter("digits");
         if (digits != null) {
-            info._digits = Integer.parseInt(digits);
+            info.setDigits(Integer.parseInt(digits));
         }
 
         // 'counter' is required if the type is 'hotp'
         String counter = url.getQueryParameter("counter");
         if (counter != null) {
-            info._counter = Long.parseLong(counter);
+            info.setCounter(Long.parseLong(counter));
-        } else if (info._type.equals("hotp")) {
+        } else if (info.getType().equals("hotp")) {
-            throw new Exception("'counter' was not set which is required for 'hotp'");
+            throw new KeyInfoException("'counter' was not set which is required for 'hotp'");
         }
 
         return info;
@@ -117,56 +114,119 @@ public class KeyInfo implements Serializable {
     public String getType() {
         return _type;
     }
+
     public byte[] getSecret() {
         return _secret;
     }
+
     public String getAccountName() {
         return _accountName;
     }
+
     public String getIssuer() {
         return _issuer;
     }
+
     public String getAlgorithm(boolean java) {
         if (java) {
             return "Hmac" + _algorithm;
         }
         return _algorithm;
     }
+
     public int getDigits() {
         return _digits;
     }
+
     public long getCounter() {
         return _counter;
     }
+
     public int getPeriod() {
         return _period;
     }
 
-    public void setType(String type) {
+    public boolean isTypeValid(String type) {
+        return type.equals("totp") || type.equals("hotp");
+    }
+
+    public void setType(String type) throws KeyInfoException {
+        type = type.toLowerCase();
+        if (!isTypeValid(type)) {
+            throw new KeyInfoException(String.format("unsupported otp type: %s", type));
+        }
         _type = type.toLowerCase();
     }
+
+    public void setSecret(char[] base32) throws KeyInfoException {
+        byte[] secret;
+        try {
+            secret = Base32.decode(base32);
+        } catch (Base32Exception e) {
+            throw new KeyInfoException("bad secret", e);
+        }
+
+        setSecret(secret);
+    }
+
     public void setSecret(byte[] secret) {
         _secret = secret;
     }
+
     public void setAccountName(String accountName) {
         _accountName = accountName;
     }
+
     public void setIssuer(String issuer) {
         _issuer = issuer;
     }
-    public void setAlgorithm(String algorithm) {
+
+    public boolean isAlgorithmValid(String algorithm) {
+        return algorithm.equals("SHA1") || algorithm.equals("SHA256") || algorithm.equals("SHA512");
+    }
+
+    public void setAlgorithm(String algorithm) throws KeyInfoException {
         if (algorithm.startsWith("Hmac")) {
             algorithm = algorithm.substring(4);
         }
-        _algorithm = algorithm.toUpperCase();
+        algorithm = algorithm.toUpperCase();
+
+        if (!isAlgorithmValid(algorithm)) {
+            throw new KeyInfoException(String.format("unsupported algorithm: %s", algorithm));
+        }
+        _algorithm = algorithm;
     }
-    public void setDigits(int digits) {
+
+    public boolean isDigitsValid(int digits) {
+        return digits == 6 || digits == 8;
+    }
+
+    public void setDigits(int digits) throws KeyInfoException {
+        if (!isDigitsValid(digits)) {
+            throw new KeyInfoException(String.format("unsupported amount of digits: %d", digits));
+        }
         _digits = digits;
     }
-    public void setCounter(long count) {
+
+    public boolean isCounterValid(long count) {
+        return count >= 0;
+    }
+
+    public void setCounter(long count) throws KeyInfoException {
+        if (!isCounterValid(count)) {
+            throw new KeyInfoException(String.format("bad count: %d", count));
+        }
         _counter = count;
     }
-    public void setPeriod(int period) {
+
+    public boolean isPeriodValid(int period) {
+        return period > 0;
+    }
+
+    public void setPeriod(int period) throws KeyInfoException {
+        if (!isPeriodValid(period)) {
+            throw new KeyInfoException(String.format("bad period: %d", period));
+        }
         _period = period;
     }
 }

app/src/main/java/me/impy/aegis/crypto/KeyInfoException.java

@@ -0,0 +1,20 @@
+package me.impy.aegis.crypto;
+
+public class KeyInfoException extends Exception {
+    public KeyInfoException(String message) {
+        super(message);
+    }
+
+    public KeyInfoException(String message, Throwable cause) {
+        super(message, cause);
+    }
+
+    @Override
+    public String getMessage() {
+        Throwable cause = getCause();
+        if (cause == null) {
+            return super.getMessage();
+        }
+        return String.format("%s (%s)", super.getMessage(), cause.getMessage());
+    }
+}

app/src/main/java/me/impy/aegis/encoding/Base32.java

@@ -1,5 +1,7 @@
 package me.impy.aegis.encoding;
 
+// modified for use in Aegis
+
 /* (PD) 2001 The Bitzi Corporation
  * Please see http://bitzi.com/publicdomain for more info.
  *
@@ -20,7 +22,9 @@ package me.impy.aegis.encoding;
  * limitations under the License.
  */
 
-import java.io.UnsupportedEncodingException;
+import java.util.Arrays;
+
+import me.impy.aegis.crypto.CryptoUtils;
 
 /**
  * Base32 - encodes and decodes RFC3548 Base32
@@ -45,16 +49,6 @@ public class Base32 {
                     0x17,0x18,0x19,0xFF,0xFF,0xFF,0xFF,0xFF  // 'x', 'y', 'z', '{', '|', '}', '~', 'DEL'
             };
 
-    public static byte[] encode(byte[] data) throws UnsupportedEncodingException {
-        String lower = encodeOriginal(data).toLowerCase();
-        return lower.getBytes("US-ASCII");
-    }
-
-    public static byte[] decodeModified(String data) {
-        return decode(data.replace('8', 'L').replace('9', 'O'));
-    }
-
-
     /**
      * Encodes byte array to Base32 String.
      *
@@ -62,10 +56,10 @@ public class Base32 {
      * @return Encoded byte array <code>bytes</code> as a String.
      *
      */
-    static public String encodeOriginal(final byte[] bytes) {
+    public static char[] encode(final byte[] bytes) {
-        int i = 0, index = 0, digit = 0;
+        int i = 0, index = 0, digit = 0, j = 0;
         int currByte, nextByte;
-        StringBuffer base32 = new StringBuffer((bytes.length + 7) * 8 / 5);
+        char[] base32 = new char[(bytes.length + 7) * 8 / 5];
 
         while (i < bytes.length) {
             currByte = (bytes[i] >= 0) ? bytes[i] : (bytes[i] + 256); // unsign
@@ -90,10 +84,12 @@ public class Base32 {
                 if (index == 0)
                     i++;
             }
-            base32.append(base32Chars.charAt(digit));
+            base32[j++] = base32Chars.charAt(digit);
         }
 
-        return base32.toString();
+        char[] res = Arrays.copyOf(base32, j);
+        CryptoUtils.zero(base32);
+        return res;
     }
 
     /**
@@ -102,24 +98,13 @@ public class Base32 {
      * @param base32
      * @return Decoded <code>base32</code> String as a raw byte array.
      */
-    static public byte[] decode(final String base32) {
+    public static byte[] decode(final char[] base32) throws Base32Exception {
         int i, index, lookup, offset, digit;
-        byte[] bytes = new byte[base32.length() * 5 / 8];
+        byte[] bytes = new byte[base32.length * 5 / 8];
 
-        for (i = 0, index = 0, offset = 0; i < base32.length(); i++) {
+        for (i = 0, index = 0, offset = 0; i < base32.length; i++) {
-            lookup = base32.charAt(i) - '0';
+            lookup = base32[i] - '0';
-
+            digit = decodeDigit(lookup);
-            /* Skip chars outside the lookup table */
-            if (lookup < 0 || lookup >= base32Lookup.length) {
-                continue;
-            }
-
-            digit = base32Lookup[lookup];
-
-            /* If this digit is not in the table, ignore it */
-            if (digit == 0xFF) {
-                continue;
-            }
 
             if (index <= 3) {
                 index = (index + 5) % 8;
@@ -144,4 +129,20 @@ public class Base32 {
         }
         return bytes;
     }
+
+    private static int decodeDigit(int c) throws Base32Exception {
+        /* Skip chars outside the lookup table */
+        if (c < 0 || c >= base32Lookup.length) {
+            throw new Base32Exception("char not found in base32 lookup table");
+        }
+
+        int digit = base32Lookup[c];
+
+        /* If this digit is not in the table, ignore it */
+        if (digit == 0xFF) {
+            throw new Base32Exception("char not found in base32 lookup table");
+        }
+
+        return digit;
+    }
 }

app/src/main/java/me/impy/aegis/encoding/Base32Exception.java

@@ -1,8 +1,7 @@
 package me.impy.aegis.encoding;
 
-/**
+public class Base32Exception extends Exception {
- * Created by alex on 1/1/18.
+    public Base32Exception(String message) {
- */
+        super(message);
-
+    }
-public class Base32Exception {
 }

app/src/main/java/me/impy/aegis/importers/FreeOTPImporter.java

@@ -13,6 +13,7 @@ import java.util.ArrayList;
 import java.util.List;
 
 import me.impy.aegis.crypto.KeyInfo;
+import me.impy.aegis.crypto.KeyInfoException;
 import me.impy.aegis.db.DatabaseEntry;
 import me.impy.aegis.util.ByteInputStream;
 
@@ -40,7 +41,8 @@ public class FreeOTPImporter extends DatabaseImporter {
         return "FreeOTP";
     }
 
-    private static List<DatabaseEntry> parse(XmlPullParser parser) throws IOException, XmlPullParserException, JSONException {
+    private static List<DatabaseEntry> parse(XmlPullParser parser)
+            throws IOException, XmlPullParserException, JSONException, KeyInfoException {
         List<Entry> entries = new ArrayList<>();
 
         parser.require(XmlPullParser.START_TAG, null, "map");