diff options
author | Joel Beckmeyer <joel@thebeckmeyers.xyz> | 2018-09-25 13:45:54 -0400 |
---|---|---|
committer | حسين <zidhussein@gmail.com> | 2018-09-25 18:45:54 +0100 |
commit | eea0e68dda1eb7248c6d458f52baeedb318b466a (patch) | |
tree | 73ff2b2f121b8db3097671f0e0639906edc2abea /app/src/main/java | |
parent | ac889abdd3d71ffb7f064a384c375ec22e7734c4 (diff) |
Display HOTP code if password contains HOTP secret, unify HOTP and TOTP code (#413)
* Display HOTP code if password contains HOTP secret, unify HOTP and TOTP code
* Add ability to show HOTP instead of showing every decrypt
* Fix off by 1 error
* fix return intent logic so that edits and HOTP increments are properly committed
* fix linting errors
* Fix broken logic for case when a password is created
* add ability to choose if password entry will be updated on HOTP code calculation
Diffstat (limited to 'app/src/main/java')
-rw-r--r-- | app/src/main/java/com/zeapo/pwdstore/PasswordEntry.java | 69 | ||||
-rw-r--r-- | app/src/main/java/com/zeapo/pwdstore/PasswordStore.java | 12 | ||||
-rw-r--r-- | app/src/main/java/com/zeapo/pwdstore/UserPreference.kt | 7 | ||||
-rw-r--r-- | app/src/main/java/com/zeapo/pwdstore/crypto/PgpActivity.kt | 173 | ||||
-rw-r--r-- | app/src/main/java/com/zeapo/pwdstore/utils/Otp.java (renamed from app/src/main/java/com/zeapo/pwdstore/utils/Totp.java) | 11 |
5 files changed, 221 insertions, 51 deletions
diff --git a/app/src/main/java/com/zeapo/pwdstore/PasswordEntry.java b/app/src/main/java/com/zeapo/pwdstore/PasswordEntry.java index ba689fe9..590a779e 100644 --- a/app/src/main/java/com/zeapo/pwdstore/PasswordEntry.java +++ b/app/src/main/java/com/zeapo/pwdstore/PasswordEntry.java @@ -12,10 +12,14 @@ public class PasswordEntry { private static final String[] USERNAME_FIELDS = new String[]{"login", "username"}; - private final String extraContent; + private String extraContent; private final String password; private final String username; private final String totpSecret; + private final String hotpSecret; + private final Long hotpCounter; + private final String content; + private boolean isIncremented = false; public PasswordEntry(final ByteArrayOutputStream os) throws UnsupportedEncodingException { this(os.toString("UTF-8")); @@ -23,11 +27,14 @@ public class PasswordEntry { public PasswordEntry(final String decryptedContent) { final String[] passContent = decryptedContent.split("\n", 2); + content = decryptedContent; password = passContent[0]; - extraContent = passContent.length > 1 ? passContent[1] : ""; + totpSecret = findTotpSecret(content); + hotpSecret = findHotpSecret(content); + hotpCounter = findHotpCounter(content); + extraContent = findExtraContent(passContent); username = findUsername(); - totpSecret = findTotpSecret(decryptedContent); - } + } public String getPassword() { return password; @@ -45,6 +52,14 @@ public class PasswordEntry { return totpSecret; } + public Long getHotpCounter() { + return hotpCounter; + } + + public String getHotpSecret() { + return hotpSecret; + } + public boolean hasExtraContent() { return extraContent.length() != 0; } @@ -53,7 +68,24 @@ public class PasswordEntry { return username != null; } - public boolean hasTotp() { return totpSecret != null; } + public boolean hasTotp() { + return totpSecret != null; + } + + public boolean hasHotp() { + return hotpSecret != null && hotpCounter != null; + } + + public boolean hotpIsIncremented() { return isIncremented; } + + public void incrementHotp() { + for (String line : content.split("\n")) { + if (line.startsWith("otpauth://hotp/")) { + extraContent = extraContent.replaceFirst("counter=[0-9]+", "counter=" + Long.toString(hotpCounter + 1)); + isIncremented = true; + } + } + } private String findUsername() { final String[] extraLines = extraContent.split("\n"); @@ -75,4 +107,31 @@ public class PasswordEntry { } return null; } + + private String findHotpSecret(String decryptedContent) { + for (String line : decryptedContent.split("\n")) { + if (line.startsWith("otpauth://hotp/")) { + return Uri.parse(line).getQueryParameter("secret"); + } + } + return null; + } + + private Long findHotpCounter(String decryptedContent) { + for (String line : decryptedContent.split("\n")) { + if (line.startsWith("otpauth://hotp/")) { + return Long.parseLong(Uri.parse(line).getQueryParameter("counter")); + } + } + return null; + } + + private String findExtraContent(String [] passContent) { + String extraContent = passContent.length > 1 ? passContent[1] : ""; + // if there is a HOTP URI, we must return the extra content with the counter incremented + if (hasHotp()) { + return extraContent.replaceFirst("counter=[0-9]+", "counter=" + Long.toString(hotpCounter)); + } + return extraContent; + } } diff --git a/app/src/main/java/com/zeapo/pwdstore/PasswordStore.java b/app/src/main/java/com/zeapo/pwdstore/PasswordStore.java index b4029ba4..1a9d7fab 100644 --- a/app/src/main/java/com/zeapo/pwdstore/PasswordStore.java +++ b/app/src/main/java/com/zeapo/pwdstore/PasswordStore.java @@ -30,6 +30,7 @@ import android.view.Menu; import android.view.MenuItem; import android.view.View; import android.widget.TextView; +import android.widget.Toast; import com.zeapo.pwdstore.crypto.PgpActivity; import com.zeapo.pwdstore.git.GitActivity; @@ -606,6 +607,7 @@ public class PasswordStore extends AppCompatActivity { protected void onActivityResult(int requestCode, int resultCode, Intent data) { + if (resultCode == RESULT_OK) { switch (requestCode) { case GitActivity.REQUEST_CLONE: @@ -613,11 +615,15 @@ public class PasswordStore extends AppCompatActivity { settings.edit().putBoolean("repository_initialized", true).apply(); break; case REQUEST_CODE_DECRYPT_AND_VERIFY: - // if went from decrypt->edit and user saved changes, we need to commitChange + // if went from decrypt->edit and user saved changes or HOTP counter was incremented, we need to commitChange if (data != null && data.getBooleanExtra("needCommit", false)) { - commitChange(this.getResources().getString(R.string.edit_commit_text) + data.getExtras().getString("NAME")); - refreshListAdapter(); + if (data.getStringExtra("OPERATION").equals("EDIT")) { + commitChange(this.getResources().getString(R.string.edit_commit_text) + data.getExtras().getString("NAME")); + } else { + commitChange(this.getResources().getString(R.string.increment_commit_text) + data.getExtras().getString("NAME")); + } } + refreshListAdapter(); break; case REQUEST_CODE_ENCRYPT: commitChange(this.getResources().getString(R.string.add_commit_text) + data.getExtras().getString("NAME") + this.getResources().getString(R.string.from_store)); diff --git a/app/src/main/java/com/zeapo/pwdstore/UserPreference.kt b/app/src/main/java/com/zeapo/pwdstore/UserPreference.kt index 3e9b2c4c..94a81b8f 100644 --- a/app/src/main/java/com/zeapo/pwdstore/UserPreference.kt +++ b/app/src/main/java/com/zeapo/pwdstore/UserPreference.kt @@ -80,6 +80,12 @@ class UserPreference : AppCompatActivity() { true } + findPreference("hotp_remember_clear_choice").onPreferenceClickListener = Preference.OnPreferenceClickListener { + sharedPreferences.edit().putBoolean("hotp_remember_check", false).apply() + it.isEnabled = false + true + } + findPreference("git_server_info").onPreferenceClickListener = Preference.OnPreferenceClickListener { val intent = Intent(callingActivity, GitActivity::class.java) intent.putExtra("Operation", GitActivity.EDIT_SERVER) @@ -161,6 +167,7 @@ class UserPreference : AppCompatActivity() { findPreference("ssh_see_key").isEnabled = sharedPreferences.getBoolean("use_generated_key", false) findPreference("git_delete_repo").isEnabled = !sharedPreferences.getBoolean("git_external", false) findPreference("ssh_key_clear_passphrase").isEnabled = sharedPreferences.getString("ssh_key_passphrase", null)?.isNotEmpty() ?: false + findPreference("hotp_remember_clear_choice").isEnabled = sharedPreferences.getBoolean("hotp_remember_check", false) val keyPref = findPreference("openpgp_key_id_pref") val selectedKeys: Array<String> = ArrayList<String>(sharedPreferences.getStringSet("openpgp_key_ids_set", HashSet<String>())).toTypedArray() if (selectedKeys.isEmpty()) { diff --git a/app/src/main/java/com/zeapo/pwdstore/crypto/PgpActivity.kt b/app/src/main/java/com/zeapo/pwdstore/crypto/PgpActivity.kt index 168f92e1..b913fa91 100644 --- a/app/src/main/java/com/zeapo/pwdstore/crypto/PgpActivity.kt +++ b/app/src/main/java/com/zeapo/pwdstore/crypto/PgpActivity.kt @@ -2,6 +2,7 @@ package com.zeapo.pwdstore.crypto import android.annotation.SuppressLint import android.app.Activity +import android.app.AlertDialog import android.app.PendingIntent import android.content.* import android.graphics.Typeface @@ -17,12 +18,8 @@ import android.text.method.PasswordTransformationMethod import android.util.Log import android.view.* import android.widget.* -import com.zeapo.pwdstore.PasswordEntry -import com.zeapo.pwdstore.R -import com.zeapo.pwdstore.UserPreference -import com.zeapo.pwdstore.pwgenDialogFragment -import com.zeapo.pwdstore.utils.PasswordRepository -import com.zeapo.pwdstore.utils.Totp +import com.zeapo.pwdstore.* +import com.zeapo.pwdstore.utils.Otp import kotlinx.android.synthetic.main.decrypt_layout.* import kotlinx.android.synthetic.main.encrypt_layout.* import org.apache.commons.io.FileUtils @@ -44,6 +41,10 @@ class PgpActivity : AppCompatActivity(), OpenPgpServiceConnection.OnBound { private var passwordEntry: PasswordEntry? = null private var api: OpenPgpApi? = null + private var editName: String? = null + private var editPass: String? = null + private var editExtra: String? = null + private val operation: String by lazy { intent.getStringExtra("OPERATION") } private val repoPath: String by lazy { intent.getStringExtra("REPO_PATH") } @@ -102,6 +103,7 @@ class PgpActivity : AppCompatActivity(), OpenPgpServiceConnection.OnBound { } override fun onDestroy() { + checkAndIncrementHotp() super.onDestroy() mServiceConnection?.unbindFromService() } @@ -122,14 +124,21 @@ class PgpActivity : AppCompatActivity(), OpenPgpServiceConnection.OnBound { override fun onOptionsItemSelected(item: MenuItem?): Boolean { when (item?.itemId) { android.R.id.home -> { - setResult(RESULT_CANCELED) + if(passwordEntry?.hotpIsIncremented() == false) { + setResult(RESULT_CANCELED) + } finish() } R.id.copy_password -> copyPasswordToClipBoard() R.id.share_password_as_plaintext -> shareAsPlaintext() R.id.edit_password -> editPassword() R.id.crypto_confirm_add -> encrypt() - R.id.crypto_cancel_add -> setResult(RESULT_CANCELED) + R.id.crypto_cancel_add -> { + if(passwordEntry?.hotpIsIncremented() == false) { + setResult(RESULT_CANCELED) + } + finish() + } else -> return super.onOptionsItemSelected(item) } return true @@ -190,7 +199,7 @@ class PgpActivity : AppCompatActivity(), OpenPgpServiceConnection.OnBound { val iStream = FileUtils.openInputStream(File(fullPath)) val oStream = ByteArrayOutputStream() - api?.executeApiAsync(data, iStream, oStream, { result: Intent? -> + api?.executeApiAsync(data, iStream, oStream) { result: Intent? -> when (result?.getIntExtra(RESULT_CODE, RESULT_CODE_ERROR)) { RESULT_CODE_SUCCESS -> { try { @@ -253,27 +262,75 @@ class PgpActivity : AppCompatActivity(), OpenPgpServiceConnection.OnBound { } } - if (entry.hasTotp()) { + if (entry.hasTotp() || entry.hasHotp()) { crypto_extra_show_layout.visibility = View.VISIBLE crypto_extra_show.typeface = monoTypeface crypto_extra_show.text = entry.extraContent - crypto_totp_show.visibility = View.VISIBLE - crypto_totp_show_label.visibility = View.VISIBLE - crypto_copy_totp.visibility = View.VISIBLE + crypto_otp_show.visibility = View.VISIBLE + crypto_otp_show_label.visibility = View.VISIBLE + crypto_copy_otp.visibility = View.VISIBLE + + if (entry.hasTotp()) { + crypto_copy_otp.setOnClickListener { copyOtpToClipBoard(Otp.calculateCode(entry.totpSecret, Date().time / (1000 * Otp.TIME_WINDOW))) } + crypto_otp_show.text = Otp.calculateCode(entry.totpSecret, Date().time / (1000 * Otp.TIME_WINDOW)) + } else { + // we only want to calculate and show HOTP if the user requests it + crypto_copy_otp.setOnClickListener { + if (settings.getBoolean("hotp_remember_check", false)) { + if (settings.getBoolean("hotp_remember_choice", false)) { + calculateAndCommitHotp(entry) + } else { + calculateHotp(entry) + } + } else { + // show a dialog asking permission to update the HOTP counter in the entry + val checkInflater = LayoutInflater.from(this) + val checkLayout = checkInflater.inflate(R.layout.otp_confirm_layout, null) + val rememberCheck : CheckBox = checkLayout.findViewById(R.id.hotp_remember_checkbox) + val dialogBuilder = AlertDialog.Builder(this) + dialogBuilder.setView(checkLayout) + dialogBuilder.setMessage(R.string.dialog_update_body) + .setCancelable(false) + .setPositiveButton(R.string.dialog_update_positive, DialogInterface.OnClickListener { dialog, id -> + run { + calculateAndCommitHotp(entry) + if (rememberCheck.isChecked()) { + val editor = settings.edit() + editor.putBoolean("hotp_remember_check", true) + editor.putBoolean("hotp_remember_choice", true) + editor.commit() + } + } + }) + .setNegativeButton(R.string.dialog_update_negative, DialogInterface.OnClickListener { dialog, id -> + run { + calculateHotp(entry) + val editor = settings.edit() + editor.putBoolean("hotp_remember_check", true) + editor.putBoolean("hotp_remember_choice", false) + editor.commit() + } + }) + val updateDialog = dialogBuilder.create() + updateDialog.setTitle(R.string.dialog_update_title) + updateDialog.show() + } + } + crypto_otp_show.setText(R.string.hotp_pending) + } + crypto_otp_show.typeface = monoTypeface - crypto_copy_totp.setOnClickListener { copyTotpToClipBoard(Totp.calculateCode(entry.totpSecret, Date().time / 1000)) } - crypto_totp_show.typeface = monoTypeface - crypto_totp_show.text = Totp.calculateCode(entry.totpSecret, Date().time / 1000); } else { - crypto_totp_show.visibility = View.GONE - crypto_totp_show_label.visibility = View.GONE - crypto_copy_totp.visibility = View.GONE + crypto_otp_show.visibility = View.GONE + crypto_otp_show_label.visibility = View.GONE + crypto_copy_otp.visibility = View.GONE } if (settings.getBoolean("copy_on_decrypt", true)) { copyPasswordToClipBoard() } + } catch (e: Exception) { Log.e(TAG, "An Exception occurred", e) } @@ -282,23 +339,26 @@ class PgpActivity : AppCompatActivity(), OpenPgpServiceConnection.OnBound { RESULT_CODE_ERROR -> handleError(result) } - }) + } } /** * Encrypts the password and the extra content */ private fun encrypt() { - val name = crypto_password_file_edit.text.toString().trim() - val pass = crypto_password_edit.text.toString() - val extra = crypto_extra_edit.text.toString() + // if HOTP was incremented, we leave fields as is; they have already been set + if(intent.getStringExtra("OPERATION") != "INCREMENT") { + editName = crypto_password_file_edit.text.toString().trim() + editPass = crypto_password_edit.text.toString() + editExtra = crypto_extra_edit.text.toString() + } - if (name.isEmpty()) { + if (editName?.isEmpty() == true) { showToast(resources.getString(R.string.file_toast_text)) return } - if (pass.isEmpty() && extra.isEmpty()) { + if (editPass?.isEmpty() == true && editExtra?.isEmpty() == true) { showToast(resources.getString(R.string.empty_toast_text)) return } @@ -312,13 +372,12 @@ class PgpActivity : AppCompatActivity(), OpenPgpServiceConnection.OnBound { data.putExtra(OpenPgpApi.EXTRA_REQUEST_ASCII_ARMOR, true) // TODO Check if we could use PasswordEntry to generate the file - val iStream = ByteArrayInputStream("$pass\n$extra".toByteArray(Charset.forName("UTF-8"))) + val iStream = ByteArrayInputStream("$editPass\n$editExtra".toByteArray(Charset.forName("UTF-8"))) val oStream = ByteArrayOutputStream() - val path = if (intent.getStringExtra("OPERATION") == "EDIT") fullPath else "$fullPath/$name.gpg" + val path = if (intent.getBooleanExtra("fromDecrypt", false)) fullPath else "$fullPath/$editName.gpg" - api?.executeApiAsync(data, iStream, oStream, { result: Intent? -> - when (result?.getIntExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR)) { + api?.executeApiAsync(data, iStream, oStream, { result: Intent? -> when (result?.getIntExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR)) { OpenPgpApi.RESULT_CODE_SUCCESS -> { try { // TODO This might fail, we should check that the write is successful @@ -328,15 +387,16 @@ class PgpActivity : AppCompatActivity(), OpenPgpServiceConnection.OnBound { val returnIntent = Intent() returnIntent.putExtra("CREATED_FILE", path) - returnIntent.putExtra("NAME", name) + returnIntent.putExtra("NAME", editName) // if coming from decrypt screen->edit button if (intent.getBooleanExtra("fromDecrypt", false)) { - data.putExtra("needCommit", true) + returnIntent.putExtra("OPERATION", "EDIT") + returnIntent.putExtra("needCommit", true) } - setResult(RESULT_OK, returnIntent) finish() + } catch (e: Exception) { Log.e(TAG, "An Exception occurred", e) } @@ -379,6 +439,43 @@ class PgpActivity : AppCompatActivity(), OpenPgpServiceConnection.OnBound { } /** + * Writes updated HOTP counter to edit fields and encrypts + */ + private fun checkAndIncrementHotp() { + // we do not want to increment the HOTP counter if the user has edited the entry or has not + // generated an HOTP code + if(intent.getStringExtra("OPERATION") != "EDIT" && passwordEntry?.hotpIsIncremented() == true) { + editName = name.trim() + editPass = passwordEntry?.password + editExtra = passwordEntry?.extraContent + + val data = Intent(this, PgpActivity::class.java) + data.putExtra("OPERATION", "INCREMENT") + data.putExtra("fromDecrypt", true) + intent = data + encrypt() + } + } + + private fun calculateHotp(entry : PasswordEntry) { + copyOtpToClipBoard(Otp.calculateCode(entry.hotpSecret, entry.hotpCounter + 1)) + crypto_otp_show.text = Otp.calculateCode(entry.hotpSecret, entry.hotpCounter + 1) + crypto_extra_show.text = entry.extraContent + } + + private fun calculateAndCommitHotp(entry : PasswordEntry) { + calculateHotp(entry) + entry.incrementHotp() + // we must set the result before encrypt() is called, since in + // some cases it is called during the finish() sequence + val returnIntent = Intent() + returnIntent.putExtra("NAME", name.trim()) + returnIntent.putExtra("OPERATION", "INCREMENT") + returnIntent.putExtra("needCommit", true) + setResult(RESULT_OK, returnIntent) + } + + /** * Get the Key ids from OpenKeychain */ private fun getKeyIds(receivedIntent: Intent? = null) { @@ -500,13 +597,12 @@ class PgpActivity : AppCompatActivity(), OpenPgpServiceConnection.OnBound { showToast(resources.getString(R.string.clipboard_username_toast_text)) } - private fun copyTotpToClipBoard(code: String) { + private fun copyOtpToClipBoard(code: String) { val clip = ClipData.newPlainText("pgp_handler_result_pm", code) clipboard.primaryClip = clip - showToast(resources.getString(R.string.clipboard_totp_toast_text)) + showToast(resources.getString(R.string.clipboard_otp_toast_text)) } - private fun shareAsPlaintext() { if (findViewById<View>(R.id.share_password_as_plaintext) == null) return @@ -586,6 +682,7 @@ class PgpActivity : AppCompatActivity(), OpenPgpServiceConnection.OnBound { override fun onPostExecute(b: Boolean?) { if (skip) return + checkAndIncrementHotp() // only clear the clipboard if we automatically copied the password to it if (settings.getBoolean("copy_on_decrypt", true)) { @@ -602,13 +699,15 @@ class PgpActivity : AppCompatActivity(), OpenPgpServiceConnection.OnBound { } if (crypto_password_show != null) { - passwordEntry = null // clear password; if decrypt changed to encrypt layout via edit button, no need + if(passwordEntry?.hotpIsIncremented() == false) { + setResult(Activity.RESULT_CANCELED) + } + passwordEntry = null crypto_password_show.text = "" crypto_extra_show.text = "" crypto_extra_show_layout.visibility = View.INVISIBLE crypto_container_decrypt.visibility = View.INVISIBLE - setResult(Activity.RESULT_CANCELED) finish() } } diff --git a/app/src/main/java/com/zeapo/pwdstore/utils/Totp.java b/app/src/main/java/com/zeapo/pwdstore/utils/Otp.java index 5e4326e2..44e2aa64 100644 --- a/app/src/main/java/com/zeapo/pwdstore/utils/Totp.java +++ b/app/src/main/java/com/zeapo/pwdstore/utils/Otp.java @@ -13,18 +13,18 @@ import java.util.Arrays; import javax.crypto.Mac; import javax.crypto.spec.SecretKeySpec; -public class Totp { +public class Otp { + public static final int TIME_WINDOW = 30; private static final String ALGORITHM = "HmacSHA1"; - private static final int TIME_WINDOW = 30; private static final int CODE_DIGITS = 6; private static final Base32 BASE_32 = new Base32(); - private Totp() { + private Otp() { } - public static String calculateCode(String secret, long epochSeconds) { + public static String calculateCode(String secret, long counter) { SecretKeySpec signingKey = new SecretKeySpec(BASE_32.decode(secret), ALGORITHM); Mac mac = null; @@ -39,8 +39,7 @@ public class Totp { return null; } - long time = epochSeconds / TIME_WINDOW; - byte[] digest = mac.doFinal(ByteBuffer.allocate(8).putLong(time).array()); + byte[] digest = mac.doFinal(ByteBuffer.allocate(8).putLong(counter).array()); int offset = digest[digest.length - 1] & 0xf; byte[] code = Arrays.copyOfRange(digest, offset, offset + 4); code[0] = (byte) (0x7f & code[0]); |