Improve NFC exception handling, fixes RuntimeExceptions

6 files changed, 66 insertions, 59 deletions

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<Void, Void, Exception>() {
+        new AsyncTask<Void, Void, IOException>() {
             @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));
         }
     }
@@ -908,15 +913,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;