From d4007ab0650baef50487592c263823f9294edd71 Mon Sep 17 00:00:00 2001 From: Impyy Date: Tue, 16 Aug 2016 12:28:27 +0200 Subject: [PATCH] Multiple improvements to KeyInfo.java - Got rid of the setters as those won't be used anyway - Renamed 'algo' to 'algorithm' - KeyInfo.FromURL is now guaranteed to produce a valid result --- .../main/java/me/impy/aegis/MainActivity.java | 2 +- .../java/me/impy/aegis/crypto/KeyInfo.java | 108 ++++++++---------- app/src/test/java/me/impy/aegis/HOTPTest.java | 4 +- 3 files changed, 49 insertions(+), 65 deletions(-) diff --git a/app/src/main/java/me/impy/aegis/MainActivity.java b/app/src/main/java/me/impy/aegis/MainActivity.java index b04e85fc..7e5a6cbc 100644 --- a/app/src/main/java/me/impy/aegis/MainActivity.java +++ b/app/src/main/java/me/impy/aegis/MainActivity.java @@ -43,7 +43,7 @@ public class MainActivity extends AppCompatActivity { KeyInfo info = (KeyInfo)data.getSerializableExtra("Keyinfo"); String nowTimeString = (Long.toHexString(System.currentTimeMillis() / 1000 / info.getPeriod())); - String totp = TOTP.generateTOTP(info.getSecret(), nowTimeString, info.getDigits(), info.getAlgo()); + String totp = TOTP.generateTOTP(info.getSecret(), nowTimeString, info.getDigits(), info.getAlgorithm()); tvTotp.setText(totp); } diff --git a/app/src/main/java/me/impy/aegis/crypto/KeyInfo.java b/app/src/main/java/me/impy/aegis/crypto/KeyInfo.java index cdbc5320..0b52d979 100644 --- a/app/src/main/java/me/impy/aegis/crypto/KeyInfo.java +++ b/app/src/main/java/me/impy/aegis/crypto/KeyInfo.java @@ -11,105 +11,87 @@ public class KeyInfo implements Serializable { private String label; private byte[] secret; private String issuer; - private String algo; - private int digits; + private String algorithm = "HmacSHA1"; + private int digits = 6; private long counter; - private int period; + private int period = 30; public String getType() { return type; } - - public KeyInfo setType(String type) { - this.type = type; - return this; - } - public String getLabel() { return label; } - - public KeyInfo setLabel(String label) { - this.label = label; - return this; - } - public byte[] getSecret() { return secret; } - - public KeyInfo setSecret(byte[] secret) { - this.secret = secret; - return this; - } - public String getIssuer() { return issuer; } - - public KeyInfo setIssuer(String issuer) { - this.issuer = issuer; - return this; + public String getAlgorithm() { + return algorithm; } - - public String getAlgo() { - return algo; - } - - public KeyInfo setAlgo(String algo) { - this.algo = algo; - return this; - } - public int getDigits() { return digits; } - - public KeyInfo setDigits(int digits) { - this.digits = digits; - return this; - } - public long getCounter() { return counter; } - - public KeyInfo setCounter(long counter) { - this.counter = counter; - return this; - } - public int getPeriod() { return period; } - public KeyInfo setPeriod(int period) { - this.period = period; - return this; - } - - private KeyInfo() { - - } + private KeyInfo() { } public static KeyInfo FromURL(String s) throws Exception { final Uri url = Uri.parse(s); - if (!url.getScheme().equals("otpauth")) { throw new Exception("unsupported protocol"); } KeyInfo info = new KeyInfo(); - info.type = 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"); + } info.secret = Base32.decode(secret); - info.issuer = url.getQueryParameter("issuer"); - info.label = url.getPath(); - info.algo = url.getQueryParameter("algorithm") != null ? url.getQueryParameter("algorithm") : "HmacSHA1"; - info.period = url.getQueryParameter("period") != null ? Integer.getInteger(url.getQueryParameter("period")) : 30; - info.digits = url.getQueryParameter("digits") != null ? Integer.getInteger(url.getQueryParameter("digits")) : 6; - info.counter = url.getQueryParameter("counter") != null ? Long.getLong(url.getQueryParameter("counter")) : 0; + + // provider info used to disambiguate accounts + // these parameters are not required but I don't want them to be null either + String issuer = url.getQueryParameter("issuer"); + String label = url.getQueryParameter("label"); + info.issuer = issuer != null ? issuer : ""; + info.label = label != null ? label : ""; + + // just use the defaults if these parameters aren't set + String algorithm = url.getQueryParameter("algorithm"); + if (algorithm != null) { + info.algorithm = "Hmac" + algorithm; + } + String period = url.getQueryParameter("period"); + if (period != null) { + info.period = Integer.getInteger(period); + } + String digits = url.getQueryParameter("digits"); + if (digits != null) { + info.digits = Integer.getInteger(digits); + } + + // 'counter' is required if the type is 'hotp' + String counter = url.getQueryParameter("counter"); + if (counter != null) { + info.counter = Long.getLong(counter); + } else if (info.type.equals("hotp")) { + throw new Exception("'counter' was not set which is required for 'hotp'"); + } return info; } diff --git a/app/src/test/java/me/impy/aegis/HOTPTest.java b/app/src/test/java/me/impy/aegis/HOTPTest.java index f163a632..7679f624 100644 --- a/app/src/test/java/me/impy/aegis/HOTPTest.java +++ b/app/src/test/java/me/impy/aegis/HOTPTest.java @@ -26,7 +26,9 @@ public class HOTPTest { new testVector(){{ Count = 9; OTP = "520489"; }}, }; - private final byte[] _secret = new byte[] {0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x30}; + private final byte[] _secret = new byte[] { 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, + 0x39, 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, 0x39, 0x30 + }; @Test public void vectors_match() throws Exception {