From ee15a614032b64cc21966691ed9eea4f3fe20e48 Mon Sep 17 00:00:00 2001 From: Alexander Bakker Date: Sun, 4 Dec 2022 18:44:26 +0100 Subject: [PATCH] Prevent the use of MD5 for anything other than mOTP This forcefully resets any HOTP/TOTP entries that were using MD5 back to SHA1, because users could only configure this by mistake. No website should be using it, as the HOTP algorithm was not made to be compatible with the hash length of MD5. --- .../beemdevelopment/aegis/otp/OtpInfo.java | 6 ++++++ .../aegis/ui/EditEntryActivity.java | 2 ++ .../aegis/otp/HotpInfoTest.java | 19 ++++++++++++++++++- 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/beemdevelopment/aegis/otp/OtpInfo.java b/app/src/main/java/com/beemdevelopment/aegis/otp/OtpInfo.java index 89b2cba4..3fbaa968 100644 --- a/app/src/main/java/com/beemdevelopment/aegis/otp/OtpInfo.java +++ b/app/src/main/java/com/beemdevelopment/aegis/otp/OtpInfo.java @@ -112,6 +112,12 @@ public abstract class OtpInfo implements Serializable { String algo = obj.getString("algo"); int digits = obj.getInt("digits"); + // Special case to work around a bug where a user could accidentally + // set the hash algorithm of a non-mOTP entry to MD5 + if (!type.equals(MotpInfo.ID) && algo.equals("MD5")) { + algo = DEFAULT_ALGORITHM; + } + switch (type) { case TotpInfo.ID: info = new TotpInfo(secret, algo, digits, obj.getInt("period")); diff --git a/app/src/main/java/com/beemdevelopment/aegis/ui/EditEntryActivity.java b/app/src/main/java/com/beemdevelopment/aegis/ui/EditEntryActivity.java index a6c12af5..05350118 100644 --- a/app/src/main/java/com/beemdevelopment/aegis/ui/EditEntryActivity.java +++ b/app/src/main/java/com/beemdevelopment/aegis/ui/EditEntryActivity.java @@ -291,11 +291,13 @@ public class EditEntryActivity extends AegisActivity { _textDigits.setText(String.valueOf(SteamInfo.DIGITS)); break; case TotpInfo.ID: + _dropdownAlgo.setText(OtpInfo.DEFAULT_ALGORITHM, false); _textPeriodCounterLayout.setHint(R.string.period_hint); _textPeriodCounter.setText(String.valueOf(TotpInfo.DEFAULT_PERIOD)); _textDigits.setText(String.valueOf(OtpInfo.DEFAULT_DIGITS)); break; case HotpInfo.ID: + _dropdownAlgo.setText(OtpInfo.DEFAULT_ALGORITHM, false); _textPeriodCounterLayout.setHint(R.string.counter); _textPeriodCounter.setText(String.valueOf(HotpInfo.DEFAULT_COUNTER)); _textDigits.setText(String.valueOf(OtpInfo.DEFAULT_DIGITS)); diff --git a/app/src/test/java/com/beemdevelopment/aegis/otp/HotpInfoTest.java b/app/src/test/java/com/beemdevelopment/aegis/otp/HotpInfoTest.java index dd079536..92f8a7dc 100644 --- a/app/src/test/java/com/beemdevelopment/aegis/otp/HotpInfoTest.java +++ b/app/src/test/java/com/beemdevelopment/aegis/otp/HotpInfoTest.java @@ -11,7 +11,24 @@ public class HotpInfoTest { public void testHotpInfoOtp() throws OtpInfoException { for (int i = 0; i < HOTPTest.VECTORS.length; i++) { HotpInfo info = new HotpInfo(HOTPTest.SECRET, OtpInfo.DEFAULT_ALGORITHM, OtpInfo.DEFAULT_DIGITS, i); - assertEquals(HOTPTest.VECTORS[i], info.getOtp()); + assertEquals(info.getOtp(), HOTPTest.VECTORS[i]); } } + + @Test + public void testHotpMd5Override() throws OtpInfoException { + final byte[] secret = new byte[]{1, 2, 3, 4}; + MotpInfo motpInfo = new MotpInfo(secret, "1234"); + motpInfo = (MotpInfo) OtpInfo.fromJson("motp", motpInfo.toJson()); + assertEquals("MD5", motpInfo.getAlgorithm(false)); + + HotpInfo info = new HotpInfo(secret); + info.setAlgorithm("MD5"); + info = (HotpInfo) OtpInfo.fromJson("hotp", info.toJson()); + assertEquals(OtpInfo.DEFAULT_ALGORITHM, info.getAlgorithm(false)); + + info.setAlgorithm("SHA256"); + info = (HotpInfo) OtpInfo.fromJson("hotp", info.toJson()); + assertEquals("SHA256", info.getAlgorithm(false)); + } }