From 14226461c14472fd470a42357b5be5bd7bc134c7 Mon Sep 17 00:00:00 2001 From: Joey Castillo Date: Wed, 10 Jun 2015 17:13:14 -0400 Subject: Improved smart card error handling --- .../keychain/ui/NfcOperationActivity.java | 46 +++++-- .../keychain/ui/base/BaseNfcActivity.java | 137 +++++++++++++++++++-- OpenKeychain/src/main/res/values/strings.xml | 8 ++ 3 files changed, 168 insertions(+), 23 deletions(-) (limited to 'OpenKeychain/src') 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 51485cb16..c5fc9abe0 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java @@ -10,7 +10,6 @@ import android.content.Intent; import android.os.Bundle; import android.view.WindowManager; -import org.spongycastle.util.Arrays; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.pgp.CanonicalizedSecretKey; @@ -28,6 +27,7 @@ import org.sufficientlysecure.keychain.util.Preferences; import java.io.IOException; import java.nio.ByteBuffer; +import java.util.Arrays; /** * This class provides a communication interface to OpenPGP applications on ISO SmartCard compliant @@ -47,6 +47,8 @@ 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}; + @Override protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); @@ -126,17 +128,29 @@ public class NfcOperationActivity extends BaseNfcActivity { } if (key.canSign() || key.canCertify()) { - nfcPutKey(0xB6, key, passphrase); - nfcPutData(0xCE, timestampBytes); - nfcPutData(0xC7, key.getFingerprint()); + if (shouldPutKey(key.getFingerprint(), 0)) { + nfcPutKey(0xB6, key, passphrase); + nfcPutData(0xCE, timestampBytes); + nfcPutData(0xC7, key.getFingerprint()); + } else { + throw new IOException("Key slot occupied; card must be reset to put new signature key."); + } } else if (key.canEncrypt()) { - nfcPutKey(0xB8, key, passphrase); - nfcPutData(0xCF, timestampBytes); - nfcPutData(0xC8, key.getFingerprint()); + if (shouldPutKey(key.getFingerprint(), 1)) { + nfcPutKey(0xB8, key, passphrase); + nfcPutData(0xCF, timestampBytes); + nfcPutData(0xC8, key.getFingerprint()); + } else { + throw new IOException("Key slot occupied; card must be reset to put new decryption key."); + } } else if (key.canAuthenticate()) { - nfcPutKey(0xA4, key, passphrase); - nfcPutData(0xD0, timestampBytes); - nfcPutData(0xC9, key.getFingerprint()); + if (shouldPutKey(key.getFingerprint(), 2)) { + nfcPutKey(0xA4, key, passphrase); + nfcPutData(0xD0, timestampBytes); + nfcPutData(0xC9, key.getFingerprint()); + } else { + throw new IOException("Key slot occupied; card must be reset to put new authentication key."); + } } else { throw new IOException("Inappropriate key flags for smart card key."); } @@ -158,6 +172,18 @@ public class NfcOperationActivity extends BaseNfcActivity { finish(); } + 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 (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() { 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 3445107a6..3e9ed282e 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 @@ -95,6 +95,8 @@ public abstract class BaseNfcActivity extends BaseActivity { if (NfcAdapter.ACTION_TAG_DISCOVERED.equals(intent.getAction())) { try { handleNdefDiscoveredIntent(intent); + } catch (CardException e) { + handleNfcError(e); } catch (IOException e) { handleNfcError(e); } @@ -105,6 +107,83 @@ public abstract class BaseNfcActivity extends BaseActivity { Log.e(Constants.TAG, "nfc error", e); Notify.create(this, getString(R.string.error_nfc, e.getMessage()), Style.WARN).show(); + } + + public void handleNfcError(CardException e) { + Log.e(Constants.TAG, "card error", e); + + short status = e.getResponseCode(); + // When entering a PIN, a status of 63CX indicates X attempts remaining. + if ((status & (short)0xFFF0) == 0x63C0) { + Notify.create(this, getString(R.string.error_pin, status & 0x000F), Style.WARN).show(); + return; + } + + // Otherwise, all status codes are fixed values. + switch (status) { + // These errors should not occur in everyday use; if they are returned, it means we + // made a mistake sending data to the card. + case 0x6A80: + throw new AssertionError("Card returned 'Wrong Data' status; this is a programming error!"); + case 0x6883: + throw new AssertionError("Card expected last command in a chain; this is a programming error!"); + case 0x6B00: + throw new AssertionError("Card reported invalid P1/P2 parameter; this is a programming error!"); + case 0x6D00: + throw new AssertionError("Instruction (INS) not supported by smart card; this is a programming error!"); + case 0x6E00: + throw new AssertionError("Class (CLA) not supported by smart card; this is a programming error!"); + + // These errors might be encountered in everyday use, and should display a localized + // error message to the user. + case 0x6285: + { + Notify.create(this, getString(R.string.error_nfc, + getString(R.string.error_nfc_terminated)), Style.WARN).show(); + break; + } + case 0x6700: + { + Notify.create(this, getString(R.string.error_nfc, + getString(R.string.error_nfc_wrong_length)), Style.WARN).show(); + break; + } + case 0x6982: + { + Notify.create(this, getString(R.string.error_nfc, + getString(R.string.error_nfc_security_not_satisfied)), Style.WARN).show(); + break; + } + case 0x6983: + { + Notify.create(this, getString(R.string.error_nfc, + getString(R.string.error_nfc_authentication_blocked)), Style.WARN).show(); + break; + } + case 0x6985: + { + Notify.create(this, getString(R.string.error_nfc, + getString(R.string.error_nfc_conditions_not_satisfied)), Style.WARN).show(); + break; + } + case 0x6A88: + case 0x6A83: + { + Notify.create(this, getString(R.string.error_nfc, + getString(R.string.error_nfc_data_not_found)), Style.WARN).show(); + break; + } + // 6F00 is a JavaCard proprietary status code, SW_UNKNOWN, and usually represents an + // unhandled exception on the smart card. + case 0x6F00: + { + Notify.create(this, getString(R.string.error_nfc, + getString(R.string.error_nfc_unknown)), Style.WARN).show(); + break; + } + default: + Notify.create(this, getString(R.string.error_nfc, e.getMessage()), Style.WARN).show(); + } } @@ -223,8 +302,9 @@ public abstract class BaseNfcActivity extends BaseActivity { + "06" // Lc (number of bytes) + "D27600012401" // Data (6 bytes) + "00"; // Le - if ( ! nfcCommunicate(opening).endsWith(accepted)) { // activate connection - throw new IOException("Initialization failed!"); + String response = nfcCommunicate(opening); // activate connection + if ( ! response.endsWith(accepted) ) { + throw new CardException("Initialization failed!", parseCardStatus(response)); } byte[] pwStatusBytes = nfcGetPwStatusBytes(); @@ -439,7 +519,7 @@ public abstract class BaseNfcActivity extends BaseActivity { } if ( ! "9000".equals(status)) { - throw new IOException("Bad NFC response code: " + status); + throw new CardException("Bad NFC response code: " + status, parseCardStatus(response)); } // Make sure the signature we received is actually the expected number of bytes long! @@ -511,9 +591,10 @@ public abstract class BaseNfcActivity extends BaseActivity { + String.format("%02x", mode) // P2 + String.format("%02x", pin.length) // Lc + Hex.toHexString(pin); - if (!nfcCommunicate(login).equals(accepted)) { // login + String response = nfcCommunicate(login); // login + if (!response.equals(accepted)) { handlePinError(); - throw new IOException("Bad PIN!"); + throw new CardException("Bad PIN!", parseCardStatus(response)); } if (mode == 0x81) { @@ -567,9 +648,10 @@ public abstract class BaseNfcActivity extends BaseActivity { + String.format("%02x", pin.length + newPin.length) // Lc + getHex(pin) + getHex(newPin); - if (!nfcCommunicate(changeReferenceDataApdu).equals("9000")) { // Change reference data + String response = nfcCommunicate(changeReferenceDataApdu); // change PIN + if (!response.equals("9000")) { handlePinError(); - throw new IOException("Failed to change PIN"); + throw new CardException("Failed to change PIN", parseCardStatus(response)); } } @@ -600,12 +682,9 @@ public abstract class BaseNfcActivity extends BaseActivity { + String.format("%02x", data.length) // Lc + getHex(data); - String response = nfcCommunicate(putDataApdu); + String response = nfcCommunicate(putDataApdu); // put data if (!response.equals("9000")) { - throw new IOException("Failed to put data for tag " - + String.format("%02x", (dataObject & 0xFF00) >> 8) - + String.format("%02x", dataObject & 0xFF) - + ": " + response); + throw new CardException("Failed to put data.", parseCardStatus(response)); } } @@ -713,7 +792,7 @@ public abstract class BaseNfcActivity extends BaseActivity { } if (!response.endsWith("9000")) { - throw new IOException("Key export to card failed"); + throw new CardException("Key export to card failed", parseCardStatus(response)); } } @@ -721,6 +800,24 @@ public abstract class BaseNfcActivity extends BaseActivity { Arrays.fill(dataToSend, (byte) 0); } + /** + * Parses out the status word from a JavaCard response string. + * + * @param response A hex string with the response from the card + * @return A short indicating the SW1/SW2, or 0 if a status could not be determined. + */ + short parseCardStatus(String response) { + if (response.length() < 4) { + return 0; // invalid input + } + + try { + return Short.parseShort(response.substring(response.length() - 4), 16); + } catch (NumberFormatException e) { + return 0; + } + } + /** * Prints a message to the screen * @@ -790,4 +887,18 @@ public abstract class BaseNfcActivity extends BaseActivity { return new String(Hex.encode(raw)); } + public class CardException extends IOException { + private short mResponseCode; + + public CardException(String detailMessage, short responseCode) { + super(detailMessage); + mResponseCode = responseCode; + } + + public short getResponseCode() { + return mResponseCode; + } + + } + } diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index 7323d19cd..8e2f0f5e2 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -1309,6 +1309,14 @@ "Import" Different key stored on YubiKey! "NFC Error: %s" + "Incorrect PIN; %d tries remaining." + "Smart card in termination state" + "Wrong length for sent / received data" + "Conditions of use not satisfied" + "Security status not satisfied" + "PIN blocked after too many attempts" + "Key or object not found" + "Unknown Error" Default PIN was rejected! Error creating temporary file. Bluetooth sharing will fail. "Filenames are encrypted." -- cgit v1.2.3 From cda8b63bb4c4c38065f76b7db474188e478e5b23 Mon Sep 17 00:00:00 2001 From: Joey Castillo Date: Thu, 11 Jun 2015 11:52:09 -0400 Subject: Replace AssertionErrors with snackbar notifications, fix style issues. --- .../keychain/ui/base/BaseNfcActivity.java | 82 +++++++++++----------- OpenKeychain/src/main/res/values/strings.xml | 19 ++--- 2 files changed, 51 insertions(+), 50 deletions(-) (limited to 'OpenKeychain/src') 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 3e9ed282e..a28a5ea59 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 @@ -122,63 +122,61 @@ public abstract class BaseNfcActivity extends BaseActivity { // Otherwise, all status codes are fixed values. switch (status) { // These errors should not occur in everyday use; if they are returned, it means we - // made a mistake sending data to the card. - case 0x6A80: - throw new AssertionError("Card returned 'Wrong Data' status; this is a programming error!"); - case 0x6883: - throw new AssertionError("Card expected last command in a chain; this is a programming error!"); - case 0x6B00: - throw new AssertionError("Card reported invalid P1/P2 parameter; this is a programming error!"); - case 0x6D00: - throw new AssertionError("Instruction (INS) not supported by smart card; this is a programming error!"); - case 0x6E00: - throw new AssertionError("Class (CLA) not supported by smart card; this is a programming error!"); - - // These errors might be encountered in everyday use, and should display a localized - // error message to the user. - case 0x6285: - { - Notify.create(this, getString(R.string.error_nfc, - getString(R.string.error_nfc_terminated)), Style.WARN).show(); + // made a mistake sending data to the card, or the card is misbehaving. + case 0x6A80: { + Notify.create(this, getString(R.string.error_nfc_bad_data), Style.WARN).show(); break; } - case 0x6700: - { - Notify.create(this, getString(R.string.error_nfc, - getString(R.string.error_nfc_wrong_length)), Style.WARN).show(); + case 0x6883: { + Notify.create(this, getString(R.string.error_nfc_chaining_error), Style.WARN).show(); break; } - case 0x6982: - { - Notify.create(this, getString(R.string.error_nfc, - getString(R.string.error_nfc_security_not_satisfied)), Style.WARN).show(); + case 0x6B00: { + Notify.create(this, getString(R.string.error_nfc_header, "P1/P2"), Style.WARN).show(); break; } - case 0x6983: - { - Notify.create(this, getString(R.string.error_nfc, - getString(R.string.error_nfc_authentication_blocked)), Style.WARN).show(); + case 0x6D00: { + Notify.create(this, getString(R.string.error_nfc_header, "INS"), Style.WARN).show(); break; } - case 0x6985: - { - Notify.create(this, getString(R.string.error_nfc, - getString(R.string.error_nfc_conditions_not_satisfied)), Style.WARN).show(); + case 0x6E00: { + Notify.create(this, getString(R.string.error_nfc_header, "CLA"), Style.WARN).show(); break; } + // These error conditions are more likely to be experienced by an end user. + case 0x6285: { + Notify.create(this, getString(R.string.error_nfc_terminated), Style.WARN).show(); + break; + } + case 0x6700: { + Notify.create(this, getString(R.string.error_nfc_wrong_length), Style.WARN).show(); + break; + } + case 0x6982: { + Notify.create(this, getString(R.string.error_nfc_security_not_satisfied), + Style.WARN).show(); + break; + } + case 0x6983: { + Notify.create(this, getString(R.string.error_nfc_authentication_blocked), + Style.WARN).show(); + break; + } + case 0x6985: { + Notify.create(this, getString(R.string.error_nfc_conditions_not_satisfied), + Style.WARN).show(); + break; + } + // 6A88 is "Not Found" in the spec, but Yubikey also returns 6A83 for this in some cases. case 0x6A88: - case 0x6A83: - { - Notify.create(this, getString(R.string.error_nfc, - getString(R.string.error_nfc_data_not_found)), Style.WARN).show(); + case 0x6A83: { + Notify.create(this, getString(R.string.error_nfc_data_not_found), Style.WARN).show(); break; } // 6F00 is a JavaCard proprietary status code, SW_UNKNOWN, and usually represents an // unhandled exception on the smart card. - case 0x6F00: - { - Notify.create(this, getString(R.string.error_nfc, - getString(R.string.error_nfc_unknown)), Style.WARN).show(); + case 0x6F00: { + Notify.create(this, getString(R.string.error_nfc_unknown), Style.WARN).show(); break; } default: diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index 8e2f0f5e2..88d44d2e4 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -1309,14 +1309,17 @@ "Import" Different key stored on YubiKey! "NFC Error: %s" - "Incorrect PIN; %d tries remaining." - "Smart card in termination state" - "Wrong length for sent / received data" - "Conditions of use not satisfied" - "Security status not satisfied" - "PIN blocked after too many attempts" - "Key or object not found" - "Unknown Error" + "NFC: Incorrect PIN; %d tries remaining." + "NFC: Smart card in termination state" + "NFC: Wrong length for sent / received data" + "NFC: Conditions of use not satisfied" + "NFC: Security status not satisfied" + "NFC: PIN blocked after too many attempts" + "NFC: Key or object not found" + "NFC: Unknown Error" + "NFC: Card reported invalid data" + "NFC: Card expected last command in a chain" + "NFC: Card reported invalid %s byte" Default PIN was rejected! Error creating temporary file. Bluetooth sharing will fail. "Filenames are encrypted." -- cgit v1.2.3