From 051e4b8a7908a1dc28f341612569d2d7664c6f77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Mon, 31 Aug 2015 19:08:44 +0200 Subject: Improve NFC exception handling, fixes RuntimeExceptions --- .../keychain/ui/CreateKeyActivity.java | 4 +- .../keychain/ui/CreateYubiKeyImportFragment.java | 2 +- .../keychain/ui/ImportKeysActivity.java | 2 +- .../keychain/ui/NfcOperationActivity.java | 39 ++++++----- .../keychain/ui/ViewKeyActivity.java | 2 +- .../keychain/ui/base/BaseNfcActivity.java | 76 ++++++++++++---------- 6 files changed, 66 insertions(+), 59 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyActivity.java index bd355da19..94b5f4e4b 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyActivity.java @@ -155,7 +155,7 @@ public class CreateKeyActivity extends BaseNfcActivity { } @Override - protected void onNfcPostExecute() throws IOException { + protected void onNfcPostExecute() { if (mCurrentFragment instanceof NfcListenerFragment) { ((NfcListenerFragment) mCurrentFragment).onNfcPostExecute(); return; @@ -257,7 +257,7 @@ public class CreateKeyActivity extends BaseNfcActivity { interface NfcListenerFragment { void doNfcInBackground() throws IOException; - void onNfcPostExecute() throws IOException; + void onNfcPostExecute(); } @Override diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateYubiKeyImportFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateYubiKeyImportFragment.java index d88e6b9f9..f7925b7f4 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateYubiKeyImportFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateYubiKeyImportFragment.java @@ -208,7 +208,7 @@ public class CreateYubiKeyImportFragment } @Override - public void onNfcPostExecute() throws IOException { + public void onNfcPostExecute() { setData(); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysActivity.java index 4ef6c40dc..c578bcf15 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysActivity.java @@ -451,7 +451,7 @@ public class ImportKeysActivity extends BaseNfcActivity } @Override - protected void onNfcPostExecute() throws IOException { + protected void onNfcPostExecute() { // either way, finish after NFC AsyncTask finish(); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java index 1ff87541a..d9b31de98 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java @@ -27,6 +27,7 @@ import android.view.View; import android.view.WindowManager; import android.widget.Button; import android.widget.TextView; +import android.widget.Toast; import android.widget.ViewAnimator; import org.sufficientlysecure.keychain.Constants; @@ -72,7 +73,7 @@ public class NfcOperationActivity extends BaseNfcActivity { private RequiredInputParcel mRequiredInput; private Intent mServiceIntent; - private static final byte[] BLANK_FINGERPRINT = {0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0}; + private static final byte[] BLANK_FINGERPRINT = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; private CryptoInputParcel mInputParcel; @@ -245,7 +246,7 @@ public class NfcOperationActivity extends BaseNfcActivity { } @Override - protected void onNfcPostExecute() throws IOException { + protected void onNfcPostExecute() { if (mServiceIntent != null) { // if we're triggered by OpenPgpService // save updated cryptoInputParcel in cache @@ -276,6 +277,7 @@ public class NfcOperationActivity extends BaseNfcActivity { } } } + @Override protected void onPostExecute(Void result) { super.onPostExecute(result); @@ -292,27 +294,14 @@ public class NfcOperationActivity extends BaseNfcActivity { vAnimator.setDisplayedChild(3); } - private boolean shouldPutKey(byte[] fingerprint, int idx) throws IOException { - byte[] cardFingerprint = nfcGetFingerprint(idx); - - // Slot is empty, or contains this key already. PUT KEY operation is safe - if (cardFingerprint == null || - Arrays.equals(cardFingerprint, BLANK_FINGERPRINT) || - Arrays.equals(cardFingerprint, fingerprint)) { - return true; - } - - // Slot already contains a different key; don't overwrite it. - return false; - } - @Override - public void handlePinError() { + public void onPinError() { // avoid a loop Preferences prefs = Preferences.getPreferences(this); if (prefs.useDefaultYubiKeyPin()) { - toast(getString(R.string.error_pin_nodefault)); + // use Toast because activity is finished afterwards + Toast.makeText(this, R.string.error_pin_nodefault, Toast.LENGTH_LONG).show(); setResult(RESULT_CANCELED); finish(); return; @@ -323,4 +312,18 @@ public class NfcOperationActivity extends BaseNfcActivity { this, mRequiredInput.getMasterKeyId(), mRequiredInput.getSubKeyId()); } + private boolean shouldPutKey(byte[] fingerprint, int idx) throws IOException { + byte[] cardFingerprint = nfcGetFingerprint(idx); + + // Slot is empty, or contains this key already. PUT KEY operation is safe + if (cardFingerprint == null || + Arrays.equals(cardFingerprint, BLANK_FINGERPRINT) || + Arrays.equals(cardFingerprint, fingerprint)) { + return true; + } + + // Slot already contains a different key; don't overwrite it. + return false; + } + } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java index 930c1fc26..de859724b 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ViewKeyActivity.java @@ -554,7 +554,7 @@ public class ViewKeyActivity extends BaseNfcActivity implements } @Override - protected void onNfcPostExecute() throws IOException { + protected void onNfcPostExecute() { long yubiKeyId = KeyFormattingUtils.getKeyIdFromFingerprint(mNfcFingerprints); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseNfcActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseNfcActivity.java index 813ed07fc..03d0c8ec4 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseNfcActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/base/BaseNfcActivity.java @@ -103,7 +103,7 @@ public abstract class BaseNfcActivity extends BaseActivity { /** * Override to handle result of NFC operations (UI thread) */ - protected void onNfcPostExecute() throws IOException { + protected void onNfcPostExecute() { final long subKeyId = KeyFormattingUtils.getKeyIdFromFingerprint(mNfcFingerprints); @@ -134,9 +134,19 @@ public abstract class BaseNfcActivity extends BaseActivity { Notify.create(this, error, Style.WARN).show(); } + /** + * Override to do something when PIN is wrong, e.g., clear passphrases (UI thread) + */ + protected void onPinError() { + // use Toast because activity is finished afterwards + Toast.makeText(this, R.string.error_pin_wrong, Toast.LENGTH_LONG).show(); + setResult(RESULT_CANCELED); + finish(); + } + public void handleIntentInBackground(final Intent intent) { // Actual NFC operations are executed in doInBackground to not block the UI thread - new AsyncTask() { + new AsyncTask() { @Override protected void onPreExecute() { super.onPreExecute(); @@ -144,7 +154,7 @@ public abstract class BaseNfcActivity extends BaseActivity { } @Override - protected Exception doInBackground(Void... params) { + protected IOException doInBackground(Void... params) { try { handleTagDiscoveredIntent(intent); } catch (IOException e) { @@ -155,7 +165,7 @@ public abstract class BaseNfcActivity extends BaseActivity { } @Override - protected void onPostExecute(Exception exception) { + protected void onPostExecute(IOException exception) { super.onPostExecute(exception); if (exception != null) { @@ -163,11 +173,7 @@ public abstract class BaseNfcActivity extends BaseActivity { return; } - try { - onNfcPostExecute(); - } catch (IOException e) { - handleNfcError(e); - } + onNfcPostExecute(); } }.execute(); } @@ -219,22 +225,30 @@ public abstract class BaseNfcActivity extends BaseActivity { } } - private void handleNfcError(Exception e) { - Log.e(Constants.TAG, "nfc error", e); + private void handleNfcError(IOException e) { if (e instanceof TagLostException) { onNfcError(getString(R.string.error_nfc_tag_lost)); return; } + if (e instanceof IsoDepNotSupportedException) { + onNfcError(getString(R.string.error_nfc_iso_dep_not_supported)); + return; + } + short status; if (e instanceof CardException) { status = ((CardException) e).getResponseCode(); } else { status = -1; } - // When entering a PIN, a status of 63CX indicates X attempts remaining. - if ((status & (short)0xFFF0) == 0x63C0) { + + // Wrong PIN, a status of 63CX indicates X attempts remaining. + if ((status & (short) 0xFFF0) == 0x63C0) { + // hook to do something different when PIN is wrong + onPinError(); + int tries = status & 0x000F; onNfcError(getResources().getQuantityString(R.plurals.error_pin, tries, tries)); return; @@ -305,12 +319,6 @@ public abstract class BaseNfcActivity extends BaseActivity { } - public void handlePinError() { - toast("Wrong PIN!"); - setResult(RESULT_CANCELED); - finish(); - } - /** * Called when the system is about to start resuming a previous activity, * disables NFC Foreground Dispatch @@ -335,7 +343,7 @@ public abstract class BaseNfcActivity extends BaseActivity { protected void obtainYubiKeyPin(RequiredInputParcel requiredInput) { - // shortcut if we only use the default yubikey pin + // shortcut if we only use the default YubiKey pin Preferences prefs = Preferences.getPreferences(this); if (prefs.useDefaultYubiKeyPin()) { mPin = new Passphrase("123456"); @@ -343,10 +351,10 @@ public abstract class BaseNfcActivity extends BaseActivity { } try { - Passphrase phrase = PassphraseCacheService.getCachedPassphrase(this, + Passphrase passphrase = PassphraseCacheService.getCachedPassphrase(this, requiredInput.getMasterKeyId(), requiredInput.getSubKeyId()); - if (phrase != null) { - mPin = phrase; + if (passphrase != null) { + mPin = passphrase; return; } @@ -405,8 +413,7 @@ public abstract class BaseNfcActivity extends BaseActivity { // Connect to the detected tag, setting a couple of settings mIsoDep = IsoDep.get(detectedTag); if (mIsoDep == null) { - // TODO: better exception? - throw new IOException("Tag does not support ISO-DEP (ISO 14443-4)!"); + throw new IsoDepNotSupportedException("Tag does not support ISO-DEP (ISO 14443-4)"); } mIsoDep.setTimeout(TIMEOUT); // timeout is set to 100 seconds to avoid cancellation during calculation mIsoDep.connect(); @@ -684,7 +691,6 @@ public abstract class BaseNfcActivity extends BaseActivity { + Hex.toHexString(pin); String response = nfcCommunicate(login); // login if (!response.equals(accepted)) { - handlePinError(); throw new CardException("Bad PIN!", parseCardStatus(response)); } @@ -739,7 +745,6 @@ public abstract class BaseNfcActivity extends BaseActivity { + getHex(newPin); String response = nfcCommunicate(changeReferenceDataApdu); // change PIN if (!response.equals("9000")) { - handlePinError(); throw new CardException("Failed to change PIN", parseCardStatus(response)); } } @@ -907,15 +912,6 @@ public abstract class BaseNfcActivity extends BaseActivity { } } - /** - * Prints a message to the screen - * - * @param text the text which should be contained within the toast - */ - protected void toast(String text) { - Toast.makeText(this, text, Toast.LENGTH_LONG).show(); - } - /** * Receive new NFC Intents to this activity only by enabling foreground dispatch. * This can only be done in onResume! @@ -982,6 +978,14 @@ public abstract class BaseNfcActivity extends BaseActivity { return new String(Hex.encode(raw)); } + public class IsoDepNotSupportedException extends IOException { + + public IsoDepNotSupportedException(String detailMessage) { + super(detailMessage); + } + + } + public class CardException extends IOException { private short mResponseCode; -- cgit v1.2.3