From d882572f0d92f1cc5306dca7bbac689eed344553 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Mon, 21 Jul 2014 15:10:29 +0200 Subject: NFC Yubikey: only pass through hash of message and not whole content --- .../keychain/pgp/PgpKeyHelper.java | 2 +- .../keychain/pgp/PgpSignEncrypt.java | 76 ++++++++++----------- .../keychain/pgp/UncachedKeyRing.java | 5 +- .../keychain/pgp/WrappedSecretKey.java | 79 +++++++++++++++++++--- .../keychain/remote/OpenPgpService.java | 10 ++- 5 files changed, 119 insertions(+), 53 deletions(-) (limited to 'OpenKeychain') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyHelper.java index 1ba028006..20bb1f97c 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyHelper.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyHelper.java @@ -95,7 +95,7 @@ public class PgpKeyHelper { } /** - * Converts fingerprint to hex (optional: with whitespaces after 4 characters) + * Converts fingerprint to hex *

* Fingerprint is shown using lowercase characters. Studies have shown that humans can * better differentiate between numbers and letters when letters are lowercase. diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncrypt.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncrypt.java index 0750c2a3a..cc34e8737 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncrypt.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpSignEncrypt.java @@ -18,6 +18,7 @@ package org.sufficientlysecure.keychain.pgp; +import org.openkeychain.nfc.NfcHandler; import org.spongycastle.bcpg.ArmoredOutputStream; import org.spongycastle.bcpg.BCPGOutputStream; import org.spongycastle.bcpg.S2K; @@ -30,6 +31,7 @@ import org.spongycastle.openpgp.PGPSignatureGenerator; import org.spongycastle.openpgp.PGPV3SignatureGenerator; import org.spongycastle.openpgp.operator.jcajce.JcePBEKeyEncryptionMethodGenerator; import org.spongycastle.openpgp.operator.jcajce.JcePGPDataEncryptorBuilder; +import org.spongycastle.openpgp.operator.jcajce.NfcSyncPGPContentSignerBuilder; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; @@ -49,8 +51,10 @@ import java.nio.charset.Charset; import java.security.NoSuchAlgorithmException; import java.security.NoSuchProviderException; import java.security.SignatureException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Date; +import java.util.LinkedList; /** * This class uses a Builder pattern! @@ -73,7 +77,7 @@ public class PgpSignEncrypt { private String mSignaturePassphrase; private boolean mEncryptToSigner; private boolean mCleartextInput; - private String mNfcData; + private byte[] mNfcData; private static byte[] NEW_LINE; @@ -127,7 +131,7 @@ public class PgpSignEncrypt { private String mSignaturePassphrase = null; private boolean mEncryptToSigner = false; private boolean mCleartextInput = false; - private String mNfcData = null; + private byte[] mNfcData = null; public Builder(ProviderHelper providerHelper, String versionHeader, InputData data, OutputStream outStream) { this.mProviderHelper = providerHelper; @@ -212,7 +216,7 @@ public class PgpSignEncrypt { return this; } - public Builder setNfcData(String nfcData) { + public Builder setNfcData(byte[] nfcData) { mNfcData = nfcData; return this; } @@ -244,15 +248,20 @@ public class PgpSignEncrypt { } } + public static class WrongPassphraseException extends Exception { + public WrongPassphraseException() { + } + } + public static class NoSigningKeyException extends Exception { public NoSigningKeyException() { } } public static class NeedNfcDataException extends Exception { - public String mData; + public byte[] mData; - public NeedNfcDataException(String data) { + public NeedNfcDataException(byte[] data) { mData = data; } } @@ -268,7 +277,7 @@ public class PgpSignEncrypt { */ public void execute() throws IOException, PGPException, NoSuchProviderException, - NoSuchAlgorithmException, SignatureException, KeyExtractionException, NoSigningKeyException, NoPassphraseException, NeedNfcDataException { + NoSuchAlgorithmException, SignatureException, KeyExtractionException, NoSigningKeyException, NoPassphraseException, NeedNfcDataException, WrongPassphraseException { boolean enableSignature = mSignatureMasterKeyId != Constants.key.none; boolean enableEncryption = ((mEncryptionMasterKeyIds != null && mEncryptionMasterKeyIds.length > 0) @@ -308,10 +317,19 @@ public class PgpSignEncrypt { updateProgress(R.string.progress_extracting_signature_key, 0, 100); try { - signingKey.unlock(mSignaturePassphrase); + if (!signingKey.unlock(mSignaturePassphrase)) { + throw new WrongPassphraseException(); + } } catch (PgpGeneralException e) { throw new KeyExtractionException(); } + + // check if hash algo is supported + LinkedList supported = signingKey.getSupportedHashAlgorithms(); + if (!supported.contains(mSignatureHashAlgorithm)) { + // get most preferred + mSignatureHashAlgorithm = supported.getLast(); + } } updateProgress(R.string.progress_preparing_streams, 5, 100); @@ -350,17 +368,10 @@ public class PgpSignEncrypt { } } - // HACK - boolean useNfc = false; - if (signingKey.getSecretKey().getS2K().getType() == S2K.GNU_DUMMY_S2K - && signingKey.getSecretKey().getS2K().getProtectionMode() == 2) { - useNfc = true; - } - /* Initialize signature generator object for later usage */ PGPSignatureGenerator signatureGenerator = null; PGPV3SignatureGenerator signatureV3Generator = null; - if (enableSignature && !useNfc) { + if (enableSignature) { updateProgress(R.string.progress_preparing_signature, 10, 100); try { @@ -370,21 +381,17 @@ public class PgpSignEncrypt { mSignatureHashAlgorithm, cleartext); } else { signatureGenerator = signingKey.getSignatureGenerator( - mSignatureHashAlgorithm, cleartext); + mSignatureHashAlgorithm, cleartext, mNfcData); } } catch (PgpGeneralException e) { // TODO throw correct type of exception (which shouldn't be PGPException) throw new KeyExtractionException(); } } -// else if (enableSignature && useNfc) { -// -// } - ArmoredOutputStream armorOut = null; - OutputStream out = null; - if (mEnableAsciiArmorOutput && !useNfc) { + OutputStream out; + if (mEnableAsciiArmorOutput) { armorOut = new ArmoredOutputStream(mOutStream); armorOut.setHeader("Version", mVersionHeader); out = armorOut; @@ -397,19 +404,7 @@ public class PgpSignEncrypt { OutputStream encryptionOut = null; BCPGOutputStream bcpgOut; - // Barrier function! - if (useNfc && mNfcData == null) { - Log.d(Constants.TAG, "mNfcData is null"); - String nfcData = convertStreamToString(mData.getInputStream()); - Log.d(Constants.TAG, "nfcData: " + nfcData); - throw new NeedNfcDataException(nfcData); - } - - if (useNfc) { - Log.d(Constants.TAG, "mNfcData: " + mNfcData); - out.write(mNfcData.getBytes()); - out.flush(); - } else if (enableEncryption) { + if (enableEncryption) { /* actual encryption */ encryptionOut = cPk.open(out, new byte[1 << 16]); @@ -543,12 +538,17 @@ public class PgpSignEncrypt { Log.e(Constants.TAG, "not supported!"); } - if (enableSignature && !useNfc) { + if (enableSignature) { updateProgress(R.string.progress_generating_signature, 95, 100); if (mSignatureForceV3) { signatureV3Generator.generate().encode(pOut); } else { - signatureGenerator.generate().encode(pOut); + try { + signatureGenerator.generate().encode(pOut); + } catch (NfcSyncPGPContentSignerBuilder.NfcInteractionNeeded e) { + // this secret key diverts to a OpenPGP card, throw exception with to-be-signed hash + throw new NeedNfcDataException(e.hashToSign); + } } } @@ -562,7 +562,7 @@ public class PgpSignEncrypt { encryptionOut.close(); } - if (mEnableAsciiArmorOutput && !useNfc) { + if (mEnableAsciiArmorOutput) { armorOut.close(); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java index 1a8607e3f..a89cb6bfa 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java @@ -176,8 +176,9 @@ public class UncachedKeyRing { for (PGPSecretKey sub : new IterableIterator( ((PGPSecretKeyRing) mRing).getSecretKeys())) { S2K s2k = sub.getS2K(); - // Set to 1, except if the encryption type is GNU_DUMMY_S2K - if(s2k == null || s2k.getType() != S2K.GNU_DUMMY_S2K) { + // add key, except if the private key has been stripped (GNU extension) + if(s2k == null || (s2k.getType() == S2K.GNU_DUMMY_S2K + && s2k.getProtectionMode() != S2K.GNU_PROTECTION_MODE_NO_PRIVATE_KEY)) { result.add(sub.getKeyID()); } else { Log.d(Constants.TAG, "S2K GNU extension!, mode: " + s2k.getProtectionMode()); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSecretKey.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSecretKey.java index b928fd8cc..141f2d5eb 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSecretKey.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSecretKey.java @@ -1,5 +1,8 @@ package org.sufficientlysecure.keychain.pgp; +import org.spongycastle.bcpg.HashAlgorithmTags; +import org.spongycastle.bcpg.PublicKeyAlgorithmTags; +import org.spongycastle.bcpg.S2K; import org.spongycastle.openpgp.PGPException; import org.spongycastle.openpgp.PGPPrivateKey; import org.spongycastle.openpgp.PGPPublicKey; @@ -12,19 +15,26 @@ import org.spongycastle.openpgp.PGPSignatureSubpacketVector; import org.spongycastle.openpgp.PGPUtil; import org.spongycastle.openpgp.PGPV3SignatureGenerator; import org.spongycastle.openpgp.operator.PBESecretKeyDecryptor; +import org.spongycastle.openpgp.operator.PGPContentSignerBuilder; import org.spongycastle.openpgp.operator.PublicKeyDataDecryptorFactory; import org.spongycastle.openpgp.operator.jcajce.JcaPGPContentSignerBuilder; import org.spongycastle.openpgp.operator.jcajce.JcePBESecretKeyDecryptorBuilder; import org.spongycastle.openpgp.operator.jcajce.JcePublicKeyDataDecryptorFactoryBuilder; +import org.spongycastle.openpgp.operator.jcajce.NfcSyncPGPContentSignerBuilder; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralMsgIdException; import org.sufficientlysecure.keychain.util.IterableIterator; +import org.sufficientlysecure.keychain.util.Log; import java.security.NoSuchAlgorithmException; import java.security.NoSuchProviderException; import java.security.SignatureException; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.LinkedList; import java.util.List; +import java.util.Set; /** Wrapper for a PGPSecretKey. * @@ -42,6 +52,11 @@ public class WrappedSecretKey extends WrappedPublicKey { private final PGPSecretKey mSecretKey; private PGPPrivateKey mPrivateKey = null; + private int mPrivateKeyState = PRIVATE_KEY_STATE_LOCKED; + private static int PRIVATE_KEY_STATE_LOCKED = 0; + private static int PRIVATE_KEY_STATE_UNLOCKED = 1; + private static int PRIVATE_KEY_STATE_DIVERT_TO_CARD = 2; + WrappedSecretKey(WrappedSecretKeyRing ring, PGPSecretKey key) { super(ring, key.getPublicKey()); mSecretKey = key; @@ -59,11 +74,23 @@ public class WrappedSecretKey extends WrappedPublicKey { return mSecretKey; } + /** + * Returns true on right passphrase + */ public boolean unlock(String passphrase) throws PgpGeneralException { + // handle keys on OpenPGP cards like they were unlocked + if (mSecretKey.getS2K().getType() == S2K.GNU_DUMMY_S2K + && mSecretKey.getS2K().getProtectionMode() == S2K.GNU_PROTECTION_MODE_DIVERT_TO_CARD) { + mPrivateKeyState = PRIVATE_KEY_STATE_DIVERT_TO_CARD; + return true; + } + + // try to extract keys using the passphrase try { PBESecretKeyDecryptor keyDecryptor = new JcePBESecretKeyDecryptorBuilder().setProvider( Constants.BOUNCY_CASTLE_PROVIDER_NAME).build(passphrase.toCharArray()); mPrivateKey = mSecretKey.extractPrivateKey(keyDecryptor); + mPrivateKeyState = PRIVATE_KEY_STATE_UNLOCKED; } catch (PGPException e) { return false; } @@ -73,16 +100,46 @@ public class WrappedSecretKey extends WrappedPublicKey { return true; } - public PGPSignatureGenerator getSignatureGenerator(int hashAlgo, boolean cleartext) + // TODO: just a hack currently + public LinkedList getSupportedHashAlgorithms() { + LinkedList supported = new LinkedList(); + + if (mPrivateKeyState == PRIVATE_KEY_STATE_DIVERT_TO_CARD) { + // TODO: only works with SHA256 ?! + supported.add(HashAlgorithmTags.SHA256); + } else { + supported.add(HashAlgorithmTags.MD5); + supported.add(HashAlgorithmTags.RIPEMD160); + supported.add(HashAlgorithmTags.SHA1); + supported.add(HashAlgorithmTags.SHA224); + supported.add(HashAlgorithmTags.SHA256); + supported.add(HashAlgorithmTags.SHA384); + supported.add(HashAlgorithmTags.SHA512); // preferred is latest + } + + return supported; + } + + public PGPSignatureGenerator getSignatureGenerator(int hashAlgo, boolean cleartext, + byte[] nfcSignedHash) throws PgpGeneralException { - if(mPrivateKey == null) { + if (mPrivateKeyState == PRIVATE_KEY_STATE_LOCKED) { throw new PrivateKeyNotUnlockedException(); } - // content signer based on signing key algorithm and chosen hash algorithm - JcaPGPContentSignerBuilder contentSignerBuilder = new JcaPGPContentSignerBuilder( - mSecretKey.getPublicKey().getAlgorithm(), hashAlgo) - .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME); + PGPContentSignerBuilder contentSignerBuilder; + if (mPrivateKeyState == PRIVATE_KEY_STATE_DIVERT_TO_CARD) { + // use synchronous "NFC based" SignerBuilder + contentSignerBuilder = new NfcSyncPGPContentSignerBuilder( + mSecretKey.getPublicKey().getAlgorithm(), hashAlgo, + mSecretKey.getKeyID(), nfcSignedHash) + .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME); + } else { + // content signer based on signing key algorithm and chosen hash algorithm + contentSignerBuilder = new JcaPGPContentSignerBuilder( + mSecretKey.getPublicKey().getAlgorithm(), hashAlgo) + .setProvider(Constants.BOUNCY_CASTLE_PROVIDER_NAME); + } int signatureType; if (cleartext) { @@ -101,13 +158,15 @@ public class WrappedSecretKey extends WrappedPublicKey { signatureGenerator.setHashedSubpackets(spGen.generate()); return signatureGenerator; } catch(PGPException e) { + // TODO: simply throw PGPException! throw new PgpGeneralException("Error initializing signature!", e); } } public PGPV3SignatureGenerator getV3SignatureGenerator(int hashAlgo, boolean cleartext) throws PgpGeneralException { - if(mPrivateKey == null) { + // TODO: divert to card missing + if (mPrivateKeyState != PRIVATE_KEY_STATE_UNLOCKED) { throw new PrivateKeyNotUnlockedException(); } @@ -134,7 +193,8 @@ public class WrappedSecretKey extends WrappedPublicKey { } public PublicKeyDataDecryptorFactory getDecryptorFactory() { - if(mPrivateKey == null) { + // TODO: divert to card missing + if (mPrivateKeyState != PRIVATE_KEY_STATE_UNLOCKED) { throw new PrivateKeyNotUnlockedException(); } return new JcePublicKeyDataDecryptorFactoryBuilder() @@ -152,7 +212,8 @@ public class WrappedSecretKey extends WrappedPublicKey { throws PgpGeneralMsgIdException, NoSuchAlgorithmException, NoSuchProviderException, PGPException, SignatureException { - if(mPrivateKey == null) { + // TODO: divert to card missing + if (mPrivateKeyState != PRIVATE_KEY_STATE_UNLOCKED) { throw new PrivateKeyNotUnlockedException(); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java index f7ee0eab3..a00759180 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java @@ -136,10 +136,10 @@ public class OpenPgpService extends RemoteService { return result; } - private Intent getNfcIntent(Intent data, String in) { + private Intent getNfcIntent(Intent data, byte[] in) { // build PendingIntent for Yubikey NFC operations Intent intent = new Intent(getBaseContext(), NfcActivity.class); - intent.setAction(NfcActivity.ACTION_SIGN); + intent.setAction(NfcActivity.ACTION_SIGN_HASH); intent.putExtra(NfcActivity.EXTRA_NFC_DATA, in); intent.addFlags(Intent.FLAG_ACTIVITY_SINGLE_TOP | Intent.FLAG_ACTIVITY_CLEAR_TOP); // pass params through to activity that it can be returned again later to repeat pgp operation @@ -191,7 +191,7 @@ public class OpenPgpService extends RemoteService { return passphraseBundle; } - String nfcData = data.getStringExtra(OpenPgpApi.EXTRA_NFC_DATA); + byte[] nfcData = data.getByteArrayExtra(OpenPgpApi.EXTRA_NFC_DATA); // Get Input- and OutputStream from ParcelFileDescriptor InputStream is = new ParcelFileDescriptor.AutoCloseInputStream(input); @@ -223,6 +223,8 @@ public class OpenPgpService extends RemoteService { throw new Exception(getString(R.string.error_could_not_extract_private_key)); } catch (PgpSignEncrypt.NoPassphraseException e) { throw new Exception(getString(R.string.error_no_signature_passphrase)); + } catch (PgpSignEncrypt.WrongPassphraseException e) { + throw new Exception(getString(R.string.error_wrong_passphrase)); } catch (PgpSignEncrypt.NoSigningKeyException e) { throw new Exception(getString(R.string.error_no_signature_key)); } catch (PgpSignEncrypt.NeedNfcDataException e) { @@ -333,6 +335,8 @@ public class OpenPgpService extends RemoteService { throw new Exception(getString(R.string.error_could_not_extract_private_key)); } catch (PgpSignEncrypt.NoPassphraseException e) { throw new Exception(getString(R.string.error_no_signature_passphrase)); + } catch (PgpSignEncrypt.WrongPassphraseException e) { + throw new Exception(getString(R.string.error_wrong_passphrase)); } catch (PgpSignEncrypt.NoSigningKeyException e) { throw new Exception(getString(R.string.error_no_signature_key)); } -- cgit v1.2.3