Improve NFC exception handling, fixes RuntimeExceptions

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

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();
 

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();
     }

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,6 +294,24 @@ public class NfcOperationActivity extends BaseNfcActivity {
         vAnimator.setDisplayedChild(3);
     }
 
+    @Override
+    public void onPinError() {
+
+        // avoid a loop
+        Preferences prefs = Preferences.getPreferences(this);
+        if (prefs.useDefaultYubiKeyPin()) {
+            // use Toast because activity is finished afterwards
+            Toast.makeText(this, R.string.error_pin_nodefault, Toast.LENGTH_LONG).show();
+            setResult(RESULT_CANCELED);
+            finish();
+            return;
+        }
+
+        // clear (invalid) passphrase
+        PassphraseCacheService.clearCachedPassphrase(
+                this, mRequiredInput.getMasterKeyId(), mRequiredInput.getSubKeyId());
+    }
+
     private boolean shouldPutKey(byte[] fingerprint, int idx) throws IOException {
         byte[] cardFingerprint = nfcGetFingerprint(idx);
 
@@ -306,21 +326,4 @@ public class NfcOperationActivity extends BaseNfcActivity {
         return false;
     }
 
-    @Override
-    public void handlePinError() {
-
-        // avoid a loop
-        Preferences prefs = Preferences.getPreferences(this);
-        if (prefs.useDefaultYubiKeyPin()) {
-            toast(getString(R.string.error_pin_nodefault));
-            setResult(RESULT_CANCELED);
-            finish();
-            return;
-        }
-
-        // clear (invalid) passphrase
-        PassphraseCacheService.clearCachedPassphrase(
-                this, mRequiredInput.getMasterKeyId(), mRequiredInput.getSubKeyId());
-    }
-
 }

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);
 

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();
-                    onNfcPostExecute();
-                } catch (IOException e) {
-                    handleNfcError(e);
-                }
             }
         }.execute();
     }
@@ -219,22 +225,30 @@ public abstract class BaseNfcActivity extends BaseActivity {
         }
     }
 
-    private void handleNfcError(Exception e) {
+    private void handleNfcError(IOException e) {
-        Log.e(Constants.TAG, "nfc error", 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) {
+            if (passphrase != null) {
-                mPin = phrase;
+                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 IsoDepNotSupportedException("Tag does not support ISO-DEP (ISO 14443-4)");
-            throw new IOException("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;
 

OpenKeychain/src/main/res/values/strings.xml

@@ -1539,8 +1539,10 @@
     <string name="error_nfc_chaining_error">"YubiKey expected last command in a chain."</string>
     <string name="error_nfc_header">"YubiKey reported invalid %s byte."</string>
     <string name="error_nfc_tag_lost">"YubiKey has been taken off too early. Keep the YubiKey at the back until the operation finishes."</string>
+    <string name="error_nfc_iso_dep_not_supported">"Tag does not support ISO-DEP (ISO 14443-4)"</string>
     <string name="error_nfc_try_again">"Try again"</string>
     <string name="error_pin_nodefault">Default PIN was rejected!</string>
+    <string name="error_pin_wrong">PIN is wrong!</string>
     <string name="error_temp_file">Error creating temporary file.</string>
     <string name="btn_delete_original">Delete original file</string>