From 2535ecb91f99ef5db2cd373a28eddaf99d2c02a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Tue, 29 Sep 2015 14:38:30 +0200 Subject: Full 160 bit for phrase confirm, stuffed with ones --- .../keychain/experimental/BitInputStream.java | 17 +++++++++++------ .../keychain/experimental/SentenceConfirm.java | 18 +++++++++++------- .../keychain/ui/CertifyFingerprintFragment.java | 5 +++-- 3 files changed, 25 insertions(+), 15 deletions(-) (limited to 'OpenKeychain') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/experimental/BitInputStream.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/experimental/BitInputStream.java index b6ec7234e..111e0b366 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/experimental/BitInputStream.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/experimental/BitInputStream.java @@ -26,7 +26,7 @@ import java.io.InputStream; * general Java InputStream. * Like the various Stream-classes from Java, the BitInputStream * has to be created based on another Input stream. It provides - * a function to read the next bit from the sream, as well as to read multiple + * a function to read the next bit from the stream, as well as to read multiple * bits at once and write the resulting data into an integer value. *

* source: http://developer.nokia.com/Community/Wiki/Bit_Input/Output_Stream_utility_classes_for_efficient_data_transfer @@ -69,11 +69,11 @@ public class BitInputStream { * @return integer value containing the bits read from the stream. * @throws IOException */ - synchronized public int readBits(final int aNumberOfBits) + synchronized public int readBits(final int aNumberOfBits, boolean stuffIfEnd) throws IOException { int value = 0; for (int i = aNumberOfBits - 1; i >= 0; i--) { - value |= (readBit() << i); + value |= (readBit(stuffIfEnd) << i); } return value; } @@ -92,15 +92,20 @@ public class BitInputStream { * @return 0 if the bit is 0, 1 if the bit is 1. * @throws IOException */ - synchronized public int readBit() throws IOException { + synchronized public int readBit(boolean stuffIfEnd) throws IOException { if (iIs == null) throw new IOException("Already closed"); if (iNextBit == 8) { iBuffer = iIs.read(); - if (iBuffer == -1) - throw new EOFException(); + if (iBuffer == -1) { + if (stuffIfEnd) { + return 1; + } else { + throw new EOFException(); + } + } iNextBit = 0; } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/experimental/SentenceConfirm.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/experimental/SentenceConfirm.java index ead70b8f6..0374d878c 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/experimental/SentenceConfirm.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/experimental/SentenceConfirm.java @@ -99,8 +99,12 @@ public class SentenceConfirm { */ private EntropyString getWord(List words, BitInputStream bin) throws IOException { final int neededBits = log(words.size(), 2); - Log.d(Constants.TAG, "need " + neededBits + " bits of entropy"); - int bits = bin.readBits(neededBits); + Log.d(Constants.TAG, "need: " + neededBits + " bits of entropy"); + Log.d(Constants.TAG, "available: " + bin.available() + " bits"); + if (neededBits > bin.available()) { + Log.d(Constants.TAG, "stuffed with " + (neededBits - bin.available()) + " ones!"); + } + int bits = bin.readBits(neededBits, true); Log.d(Constants.TAG, "got word " + words.get(bits) + " with " + neededBits + " bits of entropy"); return new EntropyString(words.get(bits), neededBits); } @@ -108,7 +112,7 @@ public class SentenceConfirm { private EntropyString getNounPhrase(BitInputStream bits) throws IOException { final EntropyString phrase = new EntropyString(); phrase.append(getWord(art, bits)).append(" "); - if (bits.readBit() != 0) { + if (bits.readBit(true) != 0) { phrase.append(getWord(adj, bits)).append(" "); } phrase.incBits(); @@ -121,7 +125,7 @@ public class SentenceConfirm { EntropyString getSentence(BitInputStream bits) throws IOException { final EntropyString sentence = new EntropyString(); sentence.append(getNounPhrase(bits)); // Subject - if (bits.readBit() != 0) { + if (bits.readBit(true) != 0) { sentence.append(" ").append(getWord(vt, bits)); // Transitive verb sentence.append(" ").append(getNounPhrase(bits)); // Object of transitive verb } else { @@ -129,17 +133,17 @@ public class SentenceConfirm { } sentence.incBits(); - if (bits.readBit() != 0) { + if (bits.readBit(true) != 0) { sentence.append(" ").append(getWord(adv, bits)); // Adverb } sentence.incBits(); - if (bits.readBit() != 0) { + if (bits.readBit(true) != 0) { sentence.append(" ").append(getWord(p, bits)); // Preposition sentence.append(" ").append(getNounPhrase(bits)); // Object of preposition } sentence.incBits(); - Log.d(Constants.TAG, "got sentence " + sentence + " with " + sentence.getBits() + " bits of entropy"); + Log.d(Constants.TAG, "got sentence '" + sentence + "' with " + sentence.getBits() + " bits of entropy"); // uppercase first character, end with dot (without increasing the bits) sentence.getBuilder().replace(0, 1, diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CertifyFingerprintFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CertifyFingerprintFragment.java index 2409523bc..85be68505 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CertifyFingerprintFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CertifyFingerprintFragment.java @@ -191,9 +191,10 @@ public class CertifyFingerprintFragment extends LoaderFragment implements String fingerprint; try { - fingerprint = new SentenceConfirm(getActivity()).fromBytes(fingerprintBlob, 16); - } catch (IOException ioe) { + fingerprint = new SentenceConfirm(getActivity()).fromBytes(fingerprintBlob, 20); + } catch (IOException e) { fingerprint = "-"; + Log.e(Constants.TAG, "Problem when creating sentence!", e); } mFingerprint.setTextSize(18); -- cgit v1.2.3 From 1d0e9bf60a958dce3ee4605dd08c660331e2d171 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Mon, 6 Jul 2015 16:43:31 +0200 Subject: Enable YubiKey creation --- .../keychain/ui/CreateKeyActivity.java | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) (limited to 'OpenKeychain') 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 579a001cb..e56652d5f 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyActivity.java @@ -30,7 +30,6 @@ import org.sufficientlysecure.keychain.provider.KeychainContract.KeyRings; import org.sufficientlysecure.keychain.provider.ProviderHelper; import org.sufficientlysecure.keychain.ui.base.BaseNfcActivity; import org.sufficientlysecure.keychain.ui.util.KeyFormattingUtils; -import org.sufficientlysecure.keychain.ui.util.Notify; import org.sufficientlysecure.keychain.util.Passphrase; import org.sufficientlysecure.keychain.util.Preferences; @@ -120,13 +119,9 @@ public class CreateKeyActivity extends BaseNfcActivity { setTitle(R.string.title_import_keys); } else { -// Fragment frag = CreateYubiKeyBlankFragment.newInstance(); -// loadFragment(frag, FragAction.START); -// setTitle(R.string.title_manage_my_keys); - Notify.create(this, - "YubiKey key creation is currently not supported. Please follow our FAQ.", - Notify.Style.ERROR - ).show(); + Fragment frag = CreateYubiKeyBlankFragment.newInstance(); + loadFragment(frag, FragAction.START); + setTitle(R.string.title_manage_my_keys); } // done @@ -186,12 +181,8 @@ public class CreateKeyActivity extends BaseNfcActivity { loadFragment(frag, FragAction.TO_RIGHT); } } else { -// Fragment frag = CreateYubiKeyBlankFragment.newInstance(); -// loadFragment(frag, FragAction.TO_RIGHT); - Notify.create(this, - "YubiKey key creation is currently not supported. Please follow our FAQ.", - Notify.Style.ERROR - ).show(); + Fragment frag = CreateYubiKeyBlankFragment.newInstance(); + loadFragment(frag, FragAction.TO_RIGHT); } } -- cgit v1.2.3 From 08e25747da20e399096a2e0c1c4afcce67898718 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Sat, 22 Aug 2015 03:13:04 +0200 Subject: Fix crashes with Fluffys PGP applet --- .../keychain/pgp/PgpSecurityConstants.java | 2 +- .../keychain/ui/CreateKeyActivity.java | 10 ++++--- .../keychain/ui/NfcOperationActivity.java | 6 ++-- .../keychain/ui/base/BaseNfcActivity.java | 34 +++++++++++++--------- 4 files changed, 32 insertions(+), 20 deletions(-) (limited to 'OpenKeychain') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSecurityConstants.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSecurityConstants.java index cbd8ce47a..ee7e3b3eb 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSecurityConstants.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSecurityConstants.java @@ -206,7 +206,7 @@ public class PgpSecurityConstants { * TODO: Ed25519 * CITE: zooko's hash function table CITE: distinguishers on SHA-256 */ - public static final int DEFAULT_HASH_ALGORITHM = HashAlgorithmTags.SHA512; + public static final int DEFAULT_HASH_ALGORITHM = HashAlgorithmTags.SHA256; public interface OpenKeychainHashAlgorithmTags extends HashAlgorithmTags { int USE_DEFAULT = -1; 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 e56652d5f..bd355da19 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/CreateKeyActivity.java @@ -187,15 +187,17 @@ public class CreateKeyActivity extends BaseNfcActivity { } private boolean containsKeys(byte[] scannedFingerprints) { + if (scannedFingerprints == null) { + return false; + } + // If all fingerprint bytes are 0, the card contains no keys. - boolean cardContainsKeys = false; for (byte b : scannedFingerprints) { if (b != 0) { - cardContainsKeys = true; - break; + return true; } } - return cardContainsKeys; + return false; } @Override 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 b811b218e..1ff87541a 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java @@ -294,9 +294,11 @@ public class NfcOperationActivity extends BaseNfcActivity { 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)) { + if (cardFingerprint == null || + Arrays.equals(cardFingerprint, BLANK_FINGERPRINT) || + Arrays.equals(cardFingerprint, fingerprint)) { return true; } 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 972421abe..813ed07fc 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 @@ -147,8 +147,6 @@ public abstract class BaseNfcActivity extends BaseActivity { protected Exception doInBackground(Void... params) { try { handleTagDiscoveredIntent(intent); - } catch (CardException e) { - return e; } catch (IOException e) { return e; } @@ -406,6 +404,10 @@ 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)!"); + } mIsoDep.setTimeout(TIMEOUT); // timeout is set to 100 seconds to avoid cancellation during calculation mIsoDep.connect(); @@ -496,6 +498,9 @@ public abstract class BaseNfcActivity extends BaseActivity { */ public byte[] nfcGetFingerprint(int idx) throws IOException { byte[] data = nfcGetFingerprints(); + if (data == null) { + return null; + } // return the master key fingerprint ByteBuffer fpbuf = ByteBuffer.wrap(data); @@ -507,14 +512,11 @@ public abstract class BaseNfcActivity extends BaseActivity { } public byte[] nfcGetAid() throws IOException { - String info = "00CA004F00"; return mIsoDep.transceive(Hex.decode(info)); - } public String nfcGetUserId() throws IOException { - String info = "00CA006500"; return nfcGetHolderName(nfcCommunicate(info)); } @@ -952,14 +954,20 @@ public abstract class BaseNfcActivity extends BaseActivity { } public String nfcGetHolderName(String name) { - String slength; - int ilength; - name = name.substring(6); - slength = name.substring(0, 2); - ilength = Integer.parseInt(slength, 16) * 2; - name = name.substring(2, ilength + 2); - name = (new String(Hex.decode(name))).replace('<', ' '); - return (name); + try { + String slength; + int ilength; + name = name.substring(6); + slength = name.substring(0, 2); + ilength = Integer.parseInt(slength, 16) * 2; + name = name.substring(2, ilength + 2); + name = (new String(Hex.decode(name))).replace('<', ' '); + return name; + } catch (IndexOutOfBoundsException e) { + Log.e(Constants.TAG, "couldn't get holder name", e); + // try-catch for https://github.com/FluffyKaon/OpenPGP-Card + return ""; + } } private String nfcGetDataField(String output) { -- cgit v1.2.3 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 ++++++++++++---------- OpenKeychain/src/main/res/values/strings.xml | 2 + 7 files changed, 68 insertions(+), 59 deletions(-) (limited to 'OpenKeychain') 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; diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index b0e0f5734..e3359cdd9 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -1539,8 +1539,10 @@ "YubiKey expected last command in a chain." "YubiKey reported invalid %s byte." "YubiKey has been taken off too early. Keep the YubiKey at the back until the operation finishes." + "Tag does not support ISO-DEP (ISO 14443-4)" "Try again" Default PIN was rejected! + PIN is wrong! Error creating temporary file. Delete original file -- cgit v1.2.3 From 1dd61cab413b68ba632596624cd771e041b7877e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Mon, 31 Aug 2015 20:26:00 +0200 Subject: Cleanup in BaseNfcActivity --- .../sufficientlysecure/keychain/ui/base/BaseNfcActivity.java | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) (limited to 'OpenKeychain') 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 03d0c8ec4..dfb116173 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 @@ -369,10 +369,6 @@ public abstract class BaseNfcActivity extends BaseActivity { } - protected void setYubiKeyPin(Passphrase pin) { - mPin = pin; - } - @Override protected void onActivityResult(int requestCode, int resultCode, Intent data) { switch (requestCode) { @@ -478,7 +474,7 @@ public abstract class BaseNfcActivity extends BaseActivity { byte[] buf = mIsoDep.transceive(Hex.decode(data)); Iso7816TLV tlv = Iso7816TLV.readSingle(buf, true); - Log.d(Constants.TAG, "nfc tlv data:\n" + tlv.prettyPrint()); + Log.d(Constants.TAG, "nfcGetFingerprints() Iso7816TLV tlv data:\n" + tlv.prettyPrint()); Iso7816TLV fptlv = Iso7816TLV.findRecursive(tlv, 0xc5); if (fptlv == null) { @@ -657,8 +653,6 @@ public abstract class BaseNfcActivity extends BaseActivity { String decryptedSessionKey = nfcGetDataField(second); - Log.d(Constants.TAG, "decryptedSessionKey: " + decryptedSessionKey); - return Hex.decode(decryptedSessionKey); } @@ -928,12 +922,10 @@ public abstract class BaseNfcActivity extends BaseActivity { new IntentFilter(NfcAdapter.ACTION_TAG_DISCOVERED) }; - // https://code.google.com/p/android/issues/detail?id=62918 - // maybe mNfcAdapter.enableReaderMode(); ? try { mNfcAdapter.enableForegroundDispatch(this, nfcPendingIntent, writeTagFilters, null); } catch (IllegalStateException e) { - Log.i(Constants.TAG, "NfcForegroundDispatch Error!", e); + Log.i(Constants.TAG, "NfcForegroundDispatch Exception: Activity is not currently in the foreground?", e); } Log.d(Constants.TAG, "NfcForegroundDispatch has been enabled!"); } -- cgit v1.2.3 From 405b959fb8047d197b4447c568df87938a2a5638 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Mon, 31 Aug 2015 23:42:19 +0200 Subject: No show key button for RESULT_INVALID_SIGNATURE, Cleanup --- .../sufficientlysecure/keychain/pgp/PgpSecurityConstants.java | 2 +- .../sufficientlysecure/keychain/ui/NfcOperationActivity.java | 11 ++++++++--- .../sufficientlysecure/keychain/ui/base/BaseNfcActivity.java | 9 ++++++--- .../keychain/ui/util/KeyFormattingUtils.java | 8 +++++--- 4 files changed, 20 insertions(+), 10 deletions(-) (limited to 'OpenKeychain') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSecurityConstants.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSecurityConstants.java index ee7e3b3eb..cbd8ce47a 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSecurityConstants.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSecurityConstants.java @@ -206,7 +206,7 @@ public class PgpSecurityConstants { * TODO: Ed25519 * CITE: zooko's hash function table CITE: distinguishers on SHA-256 */ - public static final int DEFAULT_HASH_ALGORITHM = HashAlgorithmTags.SHA256; + public static final int DEFAULT_HASH_ALGORITHM = HashAlgorithmTags.SHA512; public interface OpenKeychainHashAlgorithmTags extends HashAlgorithmTags { int USE_DEFAULT = -1; 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 d9b31de98..7a2313e0f 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/NfcOperationActivity.java @@ -313,11 +313,16 @@ public class NfcOperationActivity extends BaseNfcActivity { } private boolean shouldPutKey(byte[] fingerprint, int idx) throws IOException { - byte[] cardFingerprint = nfcGetFingerprint(idx); + byte[] cardFingerprint = nfcGetMasterKeyFingerprint(idx); + + // Note: special case: This should not happen, but happens with + // https://github.com/FluffyKaon/OpenPGP-Card, thus for now assume true + if (cardFingerprint == null) { + return true; + } // Slot is empty, or contains this key already. PUT KEY operation is safe - if (cardFingerprint == null || - Arrays.equals(cardFingerprint, BLANK_FINGERPRINT) || + if (Arrays.equals(cardFingerprint, BLANK_FINGERPRINT) || Arrays.equals(cardFingerprint, fingerprint)) { return true; } 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 dfb116173..e68684c52 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 @@ -453,7 +453,7 @@ public abstract class BaseNfcActivity extends BaseActivity { * @return The long key id of the requested key, or null if not found. */ public Long nfcGetKeyId(int idx) throws IOException { - byte[] fp = nfcGetFingerprint(idx); + byte[] fp = nfcGetMasterKeyFingerprint(idx); if (fp == null) { return null; } @@ -499,7 +499,7 @@ public abstract class BaseNfcActivity extends BaseActivity { * @param idx Index of the key to return the fingerprint from. * @return The fingerprint of the requested key, or null if not found. */ - public byte[] nfcGetFingerprint(int idx) throws IOException { + public byte[] nfcGetMasterKeyFingerprint(int idx) throws IOException { byte[] data = nfcGetFingerprints(); if (data == null) { return null; @@ -952,8 +952,11 @@ public abstract class BaseNfcActivity extends BaseActivity { name = (new String(Hex.decode(name))).replace('<', ' '); return name; } catch (IndexOutOfBoundsException e) { - Log.e(Constants.TAG, "couldn't get holder name", e); // try-catch for https://github.com/FluffyKaon/OpenPGP-Card + // Note: This should not happen, but happens with + // https://github.com/FluffyKaon/OpenPGP-Card, thus return an empty string for now! + + Log.e(Constants.TAG, "Couldn't get holder name, returning empty string!", e); return ""; } } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/util/KeyFormattingUtils.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/util/KeyFormattingUtils.java index 8f5753dae..9ab0db03e 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/util/KeyFormattingUtils.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/util/KeyFormattingUtils.java @@ -571,8 +571,9 @@ public class KeyFormattingUtils { sigIcon = R.drawable.status_signature_invalid_cutout_24dp; sigColor = R.color.key_flag_red; - sigActionText = R.string.decrypt_result_action_show; - sigActionIcon = R.drawable.ic_vpn_key_grey_24dp; + // won't be used, but makes compiler happy + sigActionText = 0; + sigActionIcon = 0; break; } @@ -584,7 +585,8 @@ public class KeyFormattingUtils { holder.getSignatureStatusText().setText(sigText); holder.getSignatureStatusText().setTextColor(sigColorRes); - if (signatureResult.getResult() != OpenPgpSignatureResult.RESULT_NO_SIGNATURE) { + if (signatureResult.getResult() != OpenPgpSignatureResult.RESULT_NO_SIGNATURE + && signatureResult.getResult() != OpenPgpSignatureResult.RESULT_INVALID_SIGNATURE) { // has a signature, thus display layouts holder.getSignatureLayout().setVisibility(View.VISIBLE); -- cgit v1.2.3 From 4f13dc4fc5e9be78a73fcc4bbafbc025baaf3aa0 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 1 Oct 2015 17:08:50 +0200 Subject: spongy: use version which doesn't skip bad signatures --- .../java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java | 3 +++ 1 file changed, 3 insertions(+) (limited to 'OpenKeychain') diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java index 4cdcf0117..b870e5494 100644 --- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/pgp/UncachedKeyringTest.java @@ -28,6 +28,7 @@ import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; import org.robolectric.shadows.ShadowLog; import org.spongycastle.bcpg.sig.KeyFlags; +import org.spongycastle.jce.provider.BouncyCastleProvider; import org.sufficientlysecure.keychain.BuildConfig; import org.sufficientlysecure.keychain.WorkaroundBuildConfig; import org.sufficientlysecure.keychain.operations.results.PgpEditKeyResult; @@ -41,6 +42,7 @@ import org.sufficientlysecure.keychain.util.Passphrase; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.security.Security; import java.util.Iterator; import java.util.Random; @@ -53,6 +55,7 @@ public class UncachedKeyringTest { @BeforeClass public static void setUpOnce() throws Exception { + Security.insertProviderAt(new BouncyCastleProvider(), 1); ShadowLog.stream = System.out; SaveKeyringParcel parcel = new SaveKeyringParcel(); -- cgit v1.2.3