From aa463b37013770de0e1c2ef8714983410c11c00a Mon Sep 17 00:00:00 2001 From: M66B Date: Tue, 23 Apr 2019 21:21:06 +0200 Subject: [PATCH] Fixed encryption flow --- FAQ.md | 3 - .../eu/faircode/email/ActivityCompose.java | 1 + .../eu/faircode/email/FragmentCompose.java | 140 +++++++++--------- .../java/eu/faircode/email/MessageHelper.java | 5 +- 4 files changed, 75 insertions(+), 74 deletions(-) diff --git a/FAQ.md b/FAQ.md index b42cbcd700..9a6de72171 100644 --- a/FAQ.md +++ b/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. -
diff --git a/app/src/main/java/eu/faircode/email/ActivityCompose.java b/app/src/main/java/eu/faircode/email/ActivityCompose.java index 8379154772..8a308f2140 100644 --- a/app/src/main/java/eu/faircode/email/ActivityCompose.java +++ b/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) { diff --git a/app/src/main/java/eu/faircode/email/FragmentCompose.java b/app/src/main/java/eu/faircode/email/FragmentCompose.java index 069c61dbaf..be4a273465 100644 --- a/app/src/main/java/eu/faircode/email/FragmentCompose.java +++ b/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() { + new SimpleTask() { @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 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: - // Get public signature - Intent keyRequest = new Intent(); - 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 + if (BuildConfig.BETA_RELEASE) { + Log.i((sign ? "Signed " : "Encrypted ") + result); + Log.logExtras(result); + } + + 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 - for (EntityAttachment attachment : db.attachment().getAttachments(id)) - if (attachment.encryption != null) - db.attachment().deleteAttachment(attachment.id); - - int seq = db.attachment().getAttachmentSequence(id); - - 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); - try (OutputStream os2 = new BufferedOutputStream(new FileOutputStream(file2))) { - os2.write(bytes2); - - db.attachment().setDownloaded(attachment2.id, (long) bytes2.length); + EntityAttachment attachment = new EntityAttachment(); + attachment.message = id; + attachment.sequence = db.attachment().getAttachmentSequence(id) + 1; + attachment.name = (sign ? "signature.asc" : "encrypted.asc"); + attachment.type = "application/octet-stream"; + attachment.encryption = (sign ? EntityAttachment.PGP_SIGNATURE : EntityAttachment.PGP_MESSAGE); + attachment.id = db.attachment().insertAttachment(attachment); + + byte[] bytes = (sign + ? result.getByteArrayExtra(OpenPgpApi.RESULT_DETACHED_SIGNATURE) + : encrypted.toByteArray()); + + 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) { - if (pi == null) + protected void onExecuted(Bundle args, Object result) { + 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) diff --git a/app/src/main/java/eu/faircode/email/MessageHelper.java b/app/src/main/java/eu/faircode/email/MessageHelper.java index a9bd1e2f90..96b7af0349 100644 --- a/app/src/main/java/eu/faircode/email/MessageHelper.java +++ b/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 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 attachments = db.attachment().getAttachments(message.id); for (EntityAttachment attachment : attachments) if (attachment.available) available++;