NFC Yubikey: only pass through hash of message and not whole content

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
      * <p/>
      * Fingerprint is shown using lowercase characters. Studies have shown that humans can
      * better differentiate between numbers and letters when letters are lowercase.

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<Integer> 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;
+        OutputStream out;
-        if (mEnableAsciiArmorOutput && !useNfc) {
+        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 (enableEncryption) {
-        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) {
             /* 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 {
+                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();
         }
 

OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java

@@ -176,8 +176,9 @@ public class UncachedKeyRing {
         for (PGPSecretKey sub : new IterableIterator<PGPSecretKey>(
                 ((PGPSecretKeyRing) mRing).getSecretKeys())) {
             S2K s2k = sub.getS2K();
-            // Set to 1, except if the encryption type is GNU_DUMMY_S2K
+            // add key, except if the private key has been stripped (GNU extension)
-            if(s2k == null || s2k.getType() != S2K.GNU_DUMMY_S2K) {
+            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());

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<Integer> getSupportedHashAlgorithms() {
+        LinkedList<Integer> supported = new LinkedList<Integer>();
+
+        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();
         }
 
+        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
-        JcaPGPContentSignerBuilder contentSignerBuilder = new JcaPGPContentSignerBuilder(
+            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();
         }
 

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

extern/openpgp-card-nfc-lib

@@ -1 +1 @@
-Subproject commit d05ad01970f4f08cc747bc1c44a99255d2796c7f
+Subproject commit 35cd39bb46f2749d55fe327862826e4bdfe38f51

extern/spongycastle

@@ -1 +1 @@
-Subproject commit 968405ee5d4272330cffdf75f7eee4cd9f5c8646
+Subproject commit a68ebd1ffc5af880903b2905c17e4f6ac0f13988