Fixed encryption flow

FAQ.md

@@ -29,7 +29,6 @@ For authorizing:
 * "*... Couldn't read row ...*" causes sometimes a crash. This could be caused by a bug in the [Room Persistence Library](https://developer.android.com/topic/libraries/architecture/room) but more likely indicates a corrupt database.
 * A [bug in Android](https://issuetracker.google.com/issues/119872129) "*... Bad notification posted ...*" lets FairEmail crash on some devices after updating FairEmail and tapping on a notification.
 * A [bug in Android](https://issuetracker.google.com/issues/62427912) "*... ActivityRecord not found for ...*" sometimes causes a crash after updating FairEmail.
-* Encryption with a token like [YubiKey](https://www.yubico.com/) results into an infinite loop. FairEmail follows the latest version of the [OpenKeychain API](https://github.com/open-keychain/openpgp-api), so this is likely being caused by an external bug.
 
 ## Planned features
 
@@ -392,8 +391,6 @@ If you want, you can verify a signature by opening the *signature.asc* attachmen
 
 Encryption/decryption is a pro feature.
 
-Please see the [known problems](#known-problems) for a problem with tokens like a YubiKey.
-
 <br />
 
 <a name="faq13"></a>

app/src/main/java/eu/faircode/email/ActivityCompose.java

@@ -47,6 +47,7 @@ public class ActivityCompose extends ActivityBilling implements FragmentManager.
     static final int REQUEST_IMAGE = 4;
     static final int REQUEST_ATTACHMENT = 5;
     static final int REQUEST_ENCRYPT = 6;
+    static final int REQUEST_SIGN = 7;
 
     @Override
     protected void onCreate(Bundle savedInstanceState) {

app/src/main/java/eu/faircode/email/FragmentCompose.java

@@ -1104,7 +1104,7 @@ public class FragmentCompose extends FragmentBase {
                     data.putExtra(OpenPgpApi.EXTRA_USER_IDS, tos);
                     data.putExtra(OpenPgpApi.EXTRA_REQUEST_ASCII_ARMOR, true);
 
-                    encrypt(data);
+                    encrypt(data, false);
                 } catch (Throwable ex) {
                     if (ex instanceof IllegalArgumentException)
                         Snackbar.make(view, ex.getMessage(), Snackbar.LENGTH_LONG).show();
@@ -1129,26 +1129,32 @@ public class FragmentCompose extends FragmentBase {
         }
     }
 
-    private void encrypt(Intent data) {
+    private void encrypt(Intent data, boolean sign) {
         final Bundle args = new Bundle();
         args.putLong("id", working);
         args.putParcelable("data", data);
+        args.putBoolean("sign", sign);
 
-        new SimpleTask<PendingIntent>() {
+        new SimpleTask<Object>() {
             @Override
-            protected PendingIntent onExecute(Context context, Bundle args) throws Throwable {
+            protected Object onExecute(Context context, Bundle args) throws Throwable {
                 // Get arguments
                 long id = args.getLong("id");
                 Intent data = args.getParcelable("data");
+                boolean sign = args.getBoolean("sign");
 
                 DB db = DB.getInstance(context);
 
-                // Get attachments
+                // Get data
                 EntityMessage message = db.message().getMessage(id);
+
                 List<EntityAttachment> attachments = db.attachment().getAttachments(id);
                 for (EntityAttachment attachment : new ArrayList<>(attachments))
-                    if (attachment.encryption != null)
+                    if (attachment.encryption != null) {
+                        if (!sign)
+                            db.attachment().deleteAttachment(attachment.id);
                         attachments.remove(attachment);
+                    }
 
                 EntityIdentity identity =
                         (message.identity == null ? null : db.identity().getIdentity(message.identity));
@@ -1157,73 +1163,52 @@ public class FragmentCompose extends FragmentBase {
                 Properties props = MessageHelper.getSessionProperties(Helper.AUTH_TYPE_PASSWORD, null, false);
                 Session isession = Session.getInstance(props, null);
                 MimeMessage imessage = new MimeMessage(isession);
-                MessageHelper.build(context, message, identity, imessage);
+                MessageHelper.build(context, message, attachments, identity, imessage);
 
                 // Serialize message
                 ByteArrayOutputStream os = new ByteArrayOutputStream();
                 imessage.writeTo(os);
                 ByteArrayInputStream decrypted = new ByteArrayInputStream(os.toByteArray());
-                ByteArrayOutputStream encrypted = new ByteArrayOutputStream();
+                ByteArrayOutputStream encrypted = (sign ? null : new ByteArrayOutputStream());
+
+                if (BuildConfig.BETA_RELEASE) {
+                    Log.i((sign ? "Sign " : "Encrypt ") + data);
+                    Log.logExtras(data);
+                }
 
                 // Encrypt message
                 OpenPgpApi api = new OpenPgpApi(context, pgpService.getService());
                 Intent result = api.executeApi(data, decrypted, encrypted);
-                switch (result.getIntExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR)) {
+
-                    case OpenPgpApi.RESULT_CODE_SUCCESS:
+                if (BuildConfig.BETA_RELEASE) {
-                        // Get public signature
+                    Log.i((sign ? "Signed " : "Encrypted ") + result);
-                        Intent keyRequest = new Intent();
+                    Log.logExtras(result);
-                        keyRequest.setAction(OpenPgpApi.ACTION_DETACHED_SIGN);
-                        keyRequest.putExtra(OpenPgpApi.EXTRA_SIGN_KEY_ID, data.getLongExtra(OpenPgpApi.EXTRA_SIGN_KEY_ID, -1));
-                        Intent key = api.executeApi(keyRequest, decrypted, null);
-                        int r = key.getIntExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR);
-                        if (r != OpenPgpApi.RESULT_CODE_SUCCESS) {
-                            OpenPgpError error = key.getParcelableExtra(OpenPgpApi.RESULT_ERROR);
-                            throw new IllegalArgumentException(error.getMessage());
                 }
 
-                        // Attach encrypted data
+                int resultCode = result.getIntExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_ERROR);
+                switch (resultCode) {
+                    case OpenPgpApi.RESULT_CODE_SUCCESS:
+                        // Attach encrypted data / signature
                         try {
                             db.beginTransaction();
 
-                            // Delete previously encrypted data
+                            EntityAttachment attachment = new EntityAttachment();
-                            for (EntityAttachment attachment : db.attachment().getAttachments(id))
+                            attachment.message = id;
-                                if (attachment.encryption != null)
+                            attachment.sequence = db.attachment().getAttachmentSequence(id) + 1;
-                                    db.attachment().deleteAttachment(attachment.id);
+                            attachment.name = (sign ? "signature.asc" : "encrypted.asc");
-
+                            attachment.type = "application/octet-stream";
-                            int seq = db.attachment().getAttachmentSequence(id);
+                            attachment.encryption = (sign ? EntityAttachment.PGP_SIGNATURE : EntityAttachment.PGP_MESSAGE);
-
+                            attachment.id = db.attachment().insertAttachment(attachment);
-                            EntityAttachment attachment1 = new EntityAttachment();
-                            attachment1.message = id;
-                            attachment1.sequence = seq + 1;
-                            attachment1.name = "encrypted.asc";
-                            attachment1.type = "application/octet-stream";
-                            attachment1.encryption = EntityAttachment.PGP_MESSAGE;
-                            attachment1.id = db.attachment().insertAttachment(attachment1);
-
-                            File file1 = attachment1.getFile(context);
-
-                            byte[] bytes1 = encrypted.toByteArray();
-                            try (OutputStream os1 = new BufferedOutputStream(new FileOutputStream(file1))) {
-                                os1.write(bytes1);
-
-                                db.attachment().setDownloaded(attachment1.id, (long) bytes1.length);
-                            }
-
-                            EntityAttachment attachment2 = new EntityAttachment();
-                            attachment2.message = id;
-                            attachment2.sequence = seq + 2;
-                            attachment2.name = "signature.asc";
-                            attachment2.type = "application/octet-stream";
-                            attachment2.encryption = EntityAttachment.PGP_SIGNATURE;
-                            attachment2.id = db.attachment().insertAttachment(attachment2);
-
-                            File file2 = attachment2.getFile(context);
 
-                            byte[] bytes2 = key.getByteArrayExtra(OpenPgpApi.RESULT_DETACHED_SIGNATURE);
+                            byte[] bytes = (sign
-                            try (OutputStream os2 = new BufferedOutputStream(new FileOutputStream(file2))) {
+                                    ? result.getByteArrayExtra(OpenPgpApi.RESULT_DETACHED_SIGNATURE)
-                                os2.write(bytes2);
+                                    : encrypted.toByteArray());
 
-                                db.attachment().setDownloaded(attachment2.id, (long) bytes2.length);
+                            File file = attachment.getFile(context);
+                            Log.i("Writing " + file + " size=" + bytes.length);
+                            try (OutputStream out = new BufferedOutputStream(new FileOutputStream(file))) {
+                                out.write(bytes);
+                                db.attachment().setDownloaded(attachment.id, (long) bytes.length);
                             }
 
                             db.setTransactionSuccessful();
@@ -1231,7 +1216,15 @@ public class FragmentCompose extends FragmentBase {
                             db.endTransaction();
                         }
 
-                        break;
+                        if (sign)
+                            return null; // send message
+                        else {
+                            // Sign message
+                            Intent signRequest = new Intent();
+                            signRequest.setAction(OpenPgpApi.ACTION_DETACHED_SIGN);
+                            signRequest.putExtra(OpenPgpApi.EXTRA_SIGN_KEY_ID, data.getLongExtra(OpenPgpApi.EXTRA_SIGN_KEY_ID, -1));
+                            return signRequest;
+                        }
 
                     case OpenPgpApi.RESULT_CODE_USER_INTERACTION_REQUIRED:
                         return result.getParcelableExtra(OpenPgpApi.RESULT_INTENT);
@@ -1239,20 +1232,26 @@ public class FragmentCompose extends FragmentBase {
                     case OpenPgpApi.RESULT_CODE_ERROR:
                         OpenPgpError error = result.getParcelableExtra(OpenPgpApi.RESULT_ERROR);
                         throw new IllegalArgumentException(error.getMessage());
-                }
 
-                return null;
+                    default:
+                        throw new IllegalArgumentException("Unknown result code=" + resultCode);
+                }
             }
 
             @Override
-            protected void onExecuted(Bundle args, PendingIntent pi) {
+            protected void onExecuted(Bundle args, Object result) {
-                if (pi == null)
+                if (result == null)
                     onAction(R.id.action_send);
-                else
+                else if (result instanceof Intent) {
+                    Intent data = (Intent) result;
+                    encrypt(data, true);
+                } else if (result instanceof PendingIntent)
                     try {
+                        boolean sign = args.getBoolean("sign");
+                        PendingIntent pi = (PendingIntent) result;
                         startIntentSenderForResult(
                                 pi.getIntentSender(),
-                                ActivityCompose.REQUEST_ENCRYPT,
+                                sign ? ActivityCompose.REQUEST_SIGN : ActivityCompose.REQUEST_ENCRYPT,
                                 null, 0, 0, 0, null);
                     } catch (IntentSender.SendIntentException ex) {
                         Log.e(ex);
@@ -1298,7 +1297,12 @@ public class FragmentCompose extends FragmentBase {
             } else if (requestCode == ActivityCompose.REQUEST_ENCRYPT) {
                 if (data != null) {
                     data.setAction(OpenPgpApi.ACTION_SIGN_AND_ENCRYPT);
-                    encrypt(data);
+                    encrypt(data, false);
+                }
+            } else if (requestCode == ActivityCompose.REQUEST_SIGN) {
+                if (data != null) {
+                    data.setAction(OpenPgpApi.ACTION_DETACHED_SIGN);
+                    encrypt(data, true);
                 }
             } else {
                 if (data != null)

app/src/main/java/eu/faircode/email/MessageHelper.java

@@ -284,12 +284,12 @@ public class MessageHelper {
                 return imessage;
             }
 
-        build(context, message, identity, imessage);
+        build(context, message, attachments, identity, imessage);
 
         return imessage;
     }
 
-    static void build(Context context, EntityMessage message, EntityIdentity identity, MimeMessage imessage) throws IOException, MessagingException {
+    static void build(Context context, EntityMessage message, List<EntityAttachment> attachments, EntityIdentity identity, MimeMessage imessage) throws IOException, MessagingException {
         DB db = DB.getInstance(context);
 
         if (message.receipt_request != null && message.receipt_request) {
@@ -350,7 +350,6 @@ public class MessageHelper {
         alternativePart.addBodyPart(htmlPart);
 
         int available = 0;
-        List<EntityAttachment> attachments = db.attachment().getAttachments(message.id);
         for (EntityAttachment attachment : attachments)
             if (attachment.available)
                 available++;