From be58f2ff4caa1cf016dbfadb1f167e7ec8f3121a Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 13 Apr 2018 17:32:21 +0200 Subject: [PATCH 1/3] Add autocrypt key transfer api method --- OpenKeychain/src/main/AndroidManifest.xml | 4 + .../remote/ApiPendingIntentFactory.java | 9 ++ .../keychain/remote/OpenPgpService.java | 34 ++++++ .../ui/RemoteDisplayTransferCodeActivity.java | 114 ++++++++++++++++++ .../res/layout/api_display_transfer_code.xml | 52 ++++++++ extern/openpgp-api-lib | 2 +- 6 files changed, 214 insertions(+), 1 deletion(-) create mode 100644 OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ui/RemoteDisplayTransferCodeActivity.java create mode 100644 OpenKeychain/src/main/res/layout/api_display_transfer_code.xml diff --git a/OpenKeychain/src/main/AndroidManifest.xml b/OpenKeychain/src/main/AndroidManifest.xml index 6b7677a14..51ce60576 100644 --- a/OpenKeychain/src/main/AndroidManifest.xml +++ b/OpenKeychain/src/main/AndroidManifest.xml @@ -929,6 +929,10 @@ android:configChanges="keyboardHidden|keyboard" android:label="@string/title_backup" /> + + . + */ + +package org.sufficientlysecure.keychain.remote.ui; + + +import java.nio.CharBuffer; + +import android.annotation.SuppressLint; +import android.app.Activity; +import android.app.Dialog; +import android.content.DialogInterface; +import android.os.Bundle; +import android.support.annotation.NonNull; +import android.support.v4.app.DialogFragment; +import android.support.v4.app.FragmentActivity; +import android.view.ContextThemeWrapper; +import android.view.LayoutInflater; +import android.view.View; +import android.widget.TextView; + +import org.sufficientlysecure.keychain.R; +import org.sufficientlysecure.keychain.ui.dialog.CustomAlertDialogBuilder; +import org.sufficientlysecure.keychain.ui.util.ThemeChanger; +import org.sufficientlysecure.keychain.ui.widget.PrefixedEditText; +import org.sufficientlysecure.keychain.util.Numeric9x4PassphraseUtil; +import org.sufficientlysecure.keychain.util.Passphrase; + + +public class RemoteDisplayTransferCodeActivity extends FragmentActivity { + public static final String EXTRA_TRANSFER_CODE = "transfer_code"; + + @Override + protected void onCreate(Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + + if (savedInstanceState == null) { + DisplayTransferCodeDialogFragment frag = new DisplayTransferCodeDialogFragment(); + frag.setArguments(getIntent().getExtras()); + frag.show(getSupportFragmentManager(), "displayTransferCode"); + } + } + + public static class DisplayTransferCodeDialogFragment extends DialogFragment { + @NonNull + @Override + public Dialog onCreateDialog(Bundle savedInstanceState) { + final Activity activity = getActivity(); + + Passphrase transferCode = getArguments().getParcelable(EXTRA_TRANSFER_CODE); + + ContextThemeWrapper theme = ThemeChanger.getDialogThemeWrapper(activity); + CustomAlertDialogBuilder alert = new CustomAlertDialogBuilder(theme); + + @SuppressLint("InflateParams") + View view = LayoutInflater.from(theme).inflate(R.layout.api_display_transfer_code, null, false); + alert.setView(view); + alert.setPositiveButton(R.string.button_got_it, (dialog, which) -> dismiss()); + + TextView[] transferCodeTextViews = new TextView[9]; + transferCodeTextViews[0] = view.findViewById(R.id.transfer_code_block_1); + transferCodeTextViews[1] = view.findViewById(R.id.transfer_code_block_2); + transferCodeTextViews[2] = view.findViewById(R.id.transfer_code_block_3); + transferCodeTextViews[3] = view.findViewById(R.id.transfer_code_block_4); + transferCodeTextViews[4] = view.findViewById(R.id.transfer_code_block_5); + transferCodeTextViews[5] = view.findViewById(R.id.transfer_code_block_6); + transferCodeTextViews[6] = view.findViewById(R.id.transfer_code_block_7); + transferCodeTextViews[7] = view.findViewById(R.id.transfer_code_block_8); + transferCodeTextViews[8] = view.findViewById(R.id.transfer_code_block_9); + + setTransferCode(transferCodeTextViews, transferCode); + + return alert.create(); + } + + private void setTransferCode(TextView[] view, Passphrase transferCode) { + CharBuffer transferCodeChars = CharBuffer.wrap(transferCode.getCharArray()).asReadOnlyBuffer(); + if (!Numeric9x4PassphraseUtil.isNumeric9x4Passphrase(transferCodeChars)) { + throw new IllegalStateException("Illegal passphrase format!"); + } + + PrefixedEditText prefixedEditText = (PrefixedEditText) view[0]; + prefixedEditText.setHint("34"); + prefixedEditText.setPrefix(transferCodeChars.subSequence(0, 2)); + prefixedEditText.setText(transferCodeChars.subSequence(2, 4)); + + for (int i = 1; i < 9; i++) { + view[i].setText(transferCodeChars.subSequence(i*5, i*5+4)); + } + } + + @Override + public void onDismiss(DialogInterface dialog) { + super.onDismiss(dialog); + + getActivity().finish(); + } + } + +} diff --git a/OpenKeychain/src/main/res/layout/api_display_transfer_code.xml b/OpenKeychain/src/main/res/layout/api_display_transfer_code.xml new file mode 100644 index 000000000..adb970240 --- /dev/null +++ b/OpenKeychain/src/main/res/layout/api_display_transfer_code.xml @@ -0,0 +1,52 @@ + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/extern/openpgp-api-lib b/extern/openpgp-api-lib index bfb355c5b..3631265b3 160000 --- a/extern/openpgp-api-lib +++ b/extern/openpgp-api-lib @@ -1 +1 @@ -Subproject commit bfb355c5bfa57245f50efd747a4f297eda57254a +Subproject commit 3631265b348a02ea3c79f4f38f6c031a82298ed5 From a63aca623ddc53d1bfcb1957970cd760e21ead77 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Tue, 1 May 2018 14:21:03 +0200 Subject: [PATCH 2/3] make sure all keys are allowed for transfer call --- .../keychain/remote/ApiPendingIntentFactory.java | 2 +- .../keychain/remote/OpenPgpService.java | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ApiPendingIntentFactory.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ApiPendingIntentFactory.java index e665322f0..18a467299 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ApiPendingIntentFactory.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/ApiPendingIntentFactory.java @@ -121,7 +121,7 @@ public class ApiPendingIntentFactory { return createInternal(data, intent); } - PendingIntent createRequestKeyPermissionPendingIntent(Intent data, String packageName, long[] masterKeyIds) { + PendingIntent createRequestKeyPermissionPendingIntent(Intent data, String packageName, long... masterKeyIds) { Intent intent = new Intent(mContext, RequestKeyPermissionActivity.class); intent.putExtra(RequestKeyPermissionActivity.EXTRA_PACKAGE_NAME, packageName); intent.putExtra(RequestKeyPermissionActivity.EXTRA_REQUESTED_KEY_IDS, masterKeyIds); 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 cd0082f14..46475c015 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java @@ -816,6 +816,19 @@ public class OpenPgpService extends Service { try { long[] masterKeyIds = data.getLongArrayExtra(OpenPgpApi.EXTRA_KEY_IDS); + HashSet allowedKeyIds = getAllowedKeyIds(); + for (long masterKeyId : masterKeyIds) { + if (!allowedKeyIds.contains(masterKeyId)) { + Intent result = new Intent(); + String packageName = mApiPermissionHelper.getCurrentCallingPackage(); + result.putExtra(OpenPgpApi.RESULT_INTENT, + mApiPendingIntentFactory.createRequestKeyPermissionPendingIntent( + data, packageName, masterKeyId)); + result.putExtra(OpenPgpApi.RESULT_CODE, OpenPgpApi.RESULT_CODE_USER_INTERACTION_REQUIRED); + return result; + } + } + Passphrase autocryptTransferCode = Numeric9x4PassphraseUtil.generateNumeric9x4Passphrase(); CryptoInputParcel inputParcel = CryptoInputParcel.createCryptoInputParcel(autocryptTransferCode); From 8edd084212a8d269db8fad970f18a9d25bf16c04 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Thu, 3 May 2018 15:11:35 +0200 Subject: [PATCH 3/3] allow setting custom headers in autocrypt setup message --- .../keychain/operations/BackupOperation.java | 29 ++++++++++++++++--- .../keychain/remote/OpenPgpService.java | 4 ++- .../keychain/service/BackupKeyringParcel.java | 17 +++++++++-- .../operations/BackupOperationTest.java | 21 ++++++++++++-- extern/openpgp-api-lib | 2 +- 5 files changed, 62 insertions(+), 11 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/BackupOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/BackupOperation.java index 7f51a9e9b..f88731800 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/BackupOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/BackupOperation.java @@ -26,8 +26,10 @@ import java.io.OutputStream; import java.text.SimpleDateFormat; import java.util.Collections; import java.util.Date; +import java.util.List; import java.util.Locale; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.regex.Pattern; import android.content.Context; import android.database.Cursor; @@ -84,6 +86,9 @@ public class BackupOperation extends BaseOperation { private static final int INDEX_MASTER_KEY_ID = 0; private static final int INDEX_HAS_ANY_SECRET = 1; + // this is a very simple matcher, we only need basic sanitization + private static final Pattern HEADER_PATTERN = Pattern.compile("[a-zA-Z0-9_-]+: [^\\n]+"); + public BackupOperation(Context context, KeyRepository keyRepository, Progressable progressable) { super(context, keyRepository, progressable); @@ -130,7 +135,7 @@ public class BackupOperation extends BaseOperation { CountingOutputStream outStream = new CountingOutputStream(new BufferedOutputStream(plainOut)); boolean backupSuccess = exportKeysToStream(log, backupInput.getMasterKeyIds(), - backupInput.getExportSecret(), backupInput.getExportPublic(), outStream); + backupInput.getExportSecret(), backupInput.getExportPublic(), outStream, backupInput.getExtraHeaders()); if (!backupSuccess) { // if there was an error, it will be in the log so we just have to return @@ -215,7 +220,7 @@ public class BackupOperation extends BaseOperation { } boolean exportKeysToStream(OperationLog log, long[] masterKeyIds, boolean exportSecret, boolean exportPublic, - OutputStream outStream) { + OutputStream outStream, List extraSecretKeyHeaders) { // noinspection unused TODO use these in a log entry int okSecret = 0, okPublic = 0; @@ -253,9 +258,10 @@ public class BackupOperation extends BaseOperation { boolean hasSecret = cursor.getInt(INDEX_HAS_ANY_SECRET) > 0; if (exportSecret && hasSecret) { log.add(LogType.MSG_BACKUP_SECRET, 2, KeyFormattingUtils.beautifyKeyId(masterKeyId)); - if (writeSecretKeyToStream(masterKeyId, log, outStream)) { + if (writeSecretKeyToStream(masterKeyId, log, outStream, extraSecretKeyHeaders)) { okSecret += 1; } + extraSecretKeyHeaders = null; } } @@ -300,12 +306,17 @@ public class BackupOperation extends BaseOperation { return true; } - private boolean writeSecretKeyToStream(long masterKeyId, OperationLog log, OutputStream outStream) + private boolean writeSecretKeyToStream(long masterKeyId, OperationLog log, OutputStream outStream, + List extraSecretKeyHeaders) throws IOException { ArmoredOutputStream arOutStream = null; try { arOutStream = new ArmoredOutputStream(outStream); + if (extraSecretKeyHeaders != null) { + addExtraHeadersToStream(arOutStream, extraSecretKeyHeaders); + } + byte[] data = mKeyRepository.loadSecretKeyRingData(masterKeyId); UncachedKeyRing uncachedKeyRing = UncachedKeyRing.decodeFromData(data); CanonicalizedSecretKeyRing ring = (CanonicalizedSecretKeyRing) uncachedKeyRing.canonicalize(log, 2, true); @@ -320,6 +331,16 @@ public class BackupOperation extends BaseOperation { return true; } + private void addExtraHeadersToStream(ArmoredOutputStream arOutStream, List headers) { + for (String header : headers) { + if (!HEADER_PATTERN.matcher(header).matches()) { + throw new IllegalArgumentException("bad header format"); + } + int sep = header.indexOf(':'); + arOutStream.setHeader(header.substring(0, sep), header.substring(sep + 2)); + } + } + private Cursor queryForKeys(long[] masterKeyIds) { String selection = null, selectionArgs[] = null; 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 46475c015..01e8238e9 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/remote/OpenPgpService.java @@ -829,10 +829,12 @@ public class OpenPgpService extends Service { } } + List headerLines = data.getStringArrayListExtra(OpenPgpApi.EXTRA_CUSTOM_HEADERS); + Passphrase autocryptTransferCode = Numeric9x4PassphraseUtil.generateNumeric9x4Passphrase(); CryptoInputParcel inputParcel = CryptoInputParcel.createCryptoInputParcel(autocryptTransferCode); - BackupKeyringParcel input = BackupKeyringParcel.createExportAutocryptSetupMessage(masterKeyIds); + BackupKeyringParcel input = BackupKeyringParcel.createExportAutocryptSetupMessage(masterKeyIds, headerLines); BackupOperation op = new BackupOperation(this, mKeyRepository, null); ExportResult pgpResult = op.execute(input, inputParcel, outputStream); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/BackupKeyringParcel.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/BackupKeyringParcel.java index 78b67c2d4..f88580fe2 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/BackupKeyringParcel.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/service/BackupKeyringParcel.java @@ -18,6 +18,8 @@ package org.sufficientlysecure.keychain.service; +import java.util.List; + import android.net.Uri; import android.os.Parcelable; import android.support.annotation.Nullable; @@ -36,15 +38,24 @@ public abstract class BackupKeyringParcel implements Parcelable { public abstract boolean getEnableAsciiArmorOutput(); @Nullable public abstract Uri getOutputUri(); + @Nullable + public abstract List getExtraHeaders(); public static BackupKeyringParcel create(long[] masterKeyIds, boolean exportSecret, boolean isEncrypted, boolean enableAsciiArmorOutput, Uri outputUri) { return new AutoValue_BackupKeyringParcel( - masterKeyIds, exportSecret, true, isEncrypted, enableAsciiArmorOutput, outputUri); + masterKeyIds, exportSecret, true, isEncrypted, enableAsciiArmorOutput, outputUri, null); } - public static BackupKeyringParcel createExportAutocryptSetupMessage(long[] masterKeyIds) { + public static BackupKeyringParcel create(long[] masterKeyIds, boolean exportSecret, + boolean isEncrypted, boolean enableAsciiArmorOutput, Uri outputUri, List extraHeaders) { return new AutoValue_BackupKeyringParcel( - masterKeyIds, true, false, true, true, null); + masterKeyIds, exportSecret, true, isEncrypted, enableAsciiArmorOutput, outputUri, extraHeaders); + } + + public static BackupKeyringParcel createExportAutocryptSetupMessage(long[] masterKeyIds, + List extraHeaders) { + return new AutoValue_BackupKeyringParcel( + masterKeyIds, true, false, true, true, null, extraHeaders); } } \ No newline at end of file diff --git a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/BackupOperationTest.java b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/BackupOperationTest.java index 190072597..7417d4314 100644 --- a/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/BackupOperationTest.java +++ b/OpenKeychain/src/test/java/org/sufficientlysecure/keychain/operations/BackupOperationTest.java @@ -24,6 +24,7 @@ import java.io.PipedInputStream; import java.io.PipedOutputStream; import java.io.PrintStream; import java.security.Security; +import java.util.Arrays; import java.util.Iterator; import android.app.Application; @@ -157,7 +158,7 @@ public class BackupOperationTest { assertTrue("second keyring has local certification", checkForLocal(mStaticRing2)); ByteArrayOutputStream out = new ByteArrayOutputStream(); - boolean result = op.exportKeysToStream(new OperationLog(), null, false, true, out); + boolean result = op.exportKeysToStream(new OperationLog(), null, false, true, out, null); assertTrue("export must be a success", result); @@ -194,7 +195,7 @@ public class BackupOperationTest { } out = new ByteArrayOutputStream(); - result = op.exportKeysToStream(new OperationLog(), null, true, true, out); + result = op.exportKeysToStream(new OperationLog(), null, true, true, out, null); assertTrue("export must be a success", result); @@ -238,6 +239,22 @@ public class BackupOperationTest { } + @Test + public void testExportWithExtraHeaders() throws Exception { + BackupOperation op = new BackupOperation(RuntimeEnvironment.application, + KeyWritableRepository.create(RuntimeEnvironment.application), null); + + ByteArrayOutputStream out = new ByteArrayOutputStream(); + boolean result = op.exportKeysToStream( + new OperationLog(), new long[] { mStaticRing1.getMasterKeyId() }, true, false, + out, Arrays.asList("header: value")); + + assertTrue(result); + + String resultData = new String(out.toByteArray()); + assertTrue(resultData.startsWith("-----BEGIN PGP PRIVATE KEY BLOCK-----\nheader: value\n\n")); + } + @Test public void testExportUnencrypted() throws Exception { ContentResolver mockResolver = mock(ContentResolver.class); diff --git a/extern/openpgp-api-lib b/extern/openpgp-api-lib index 3631265b3..c2ddaa76b 160000 --- a/extern/openpgp-api-lib +++ b/extern/openpgp-api-lib @@ -1 +1 @@ -Subproject commit 3631265b348a02ea3c79f4f38f6c031a82298ed5 +Subproject commit c2ddaa76bbb8819dafff55ae4af00ac40c94e6fb