From 7223abcf0cfa99a866a10483b017555fb768738f Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 2 Jan 2015 00:07:05 +0100 Subject: extend canonicalize to strip local certificates on export --- .../keychain/operations/ImportExportOperation.java | 27 +++++++---- .../operations/results/OperationResult.java | 14 +++--- .../keychain/pgp/UncachedKeyRing.java | 53 ++++++++++++++++++---- 3 files changed, 72 insertions(+), 22 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/ImportExportOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/ImportExportOperation.java index da532d2dc..6ca28142f 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/ImportExportOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/ImportExportOperation.java @@ -31,6 +31,7 @@ import org.sufficientlysecure.keychain.keyimport.Keyserver; import org.sufficientlysecure.keychain.keyimport.Keyserver.AddKeyException; import org.sufficientlysecure.keychain.keyimport.ParcelableKeyRing; import org.sufficientlysecure.keychain.operations.results.ExportResult; +import org.sufficientlysecure.keychain.pgp.CanonicalizedKeyRing; import org.sufficientlysecure.keychain.pgp.CanonicalizedPublicKeyRing; import org.sufficientlysecure.keychain.pgp.PgpHelper; import org.sufficientlysecure.keychain.pgp.Progressable; @@ -399,7 +400,7 @@ public class ImportExportOperation extends BaseOperation { } - private ExportResult exportKeyRings(OperationLog log, long[] masterKeyIds, boolean exportSecret, + ExportResult exportKeyRings(OperationLog log, long[] masterKeyIds, boolean exportSecret, OutputStream outStream) { /* TODO isn't this checked above, with the isStorageMounted call? @@ -469,12 +470,16 @@ public class ImportExportOperation extends BaseOperation { log.add(LogType.MSG_EXPORT_PUBLIC, 1, KeyFormattingUtils.beautifyKeyId(keyId)); - { // export public key part - byte[] data = cursor.getBlob(1); - arOutStream.write(data); + byte[] data = cursor.getBlob(1); + CanonicalizedKeyRing ring = + UncachedKeyRing.decodeFromData(data).canonicalize(log, 2, true); + ring.encode(arOutStream); - okPublic += 1; - } + okPublic += 1; + } catch (PgpGeneralException e) { + log.add(LogType.MSG_EXPORT_ERROR_KEY, 2); + updateProgress(progress++, numKeys); + continue; } finally { // make sure this is closed if (arOutStream != null) { @@ -491,12 +496,18 @@ public class ImportExportOperation extends BaseOperation { arOutStream.setHeader("Version", version); } - // export secret key part + // export secret key part log.add(LogType.MSG_EXPORT_SECRET, 2, KeyFormattingUtils.beautifyKeyId(keyId)); byte[] data = cursor.getBlob(2); - arOutStream.write(data); + CanonicalizedKeyRing ring = + UncachedKeyRing.decodeFromData(data).canonicalize(log, 2, true); + ring.encode(arOutStream); okSecret += 1; + } catch (PgpGeneralException e) { + log.add(LogType.MSG_EXPORT_ERROR_KEY, 2); + updateProgress(progress++, numKeys); + continue; } finally { // make sure this is closed if (arOutStream != null) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java index 426b0827e..bd73a9609 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/operations/results/OperationResult.java @@ -371,12 +371,13 @@ public abstract class OperationResult implements Parcelable { MSG_KC_ERROR_MASTER_ALGO (LogLevel.ERROR, R.string.msg_kc_error_master_algo), MSG_KC_ERROR_DUP_KEY (LogLevel.ERROR, R.string.msg_kc_error_dup_key), MSG_KC_MASTER (LogLevel.DEBUG, R.string.msg_kc_master), - MSG_KC_BAD_TYPE(LogLevel.WARN, R.string.msg_kc_bad_type), - MSG_KC_REVOKE_BAD_ERR (LogLevel.WARN, R.string.msg_kc_revoke_bad_err), - MSG_KC_REVOKE_BAD_LOCAL (LogLevel.WARN, R.string.msg_kc_revoke_bad_local), - MSG_KC_REVOKE_BAD_TIME (LogLevel.WARN, R.string.msg_kc_revoke_bad_time), - MSG_KC_REVOKE_BAD_TYPE_UID (LogLevel.WARN, R.string.msg_kc_revoke_bad_type_uid), - MSG_KC_REVOKE_BAD (LogLevel.WARN, R.string.msg_kc_revoke_bad), + MSG_KC_MASTER_BAD_TYPE(LogLevel.WARN, R.string.msg_kc_master_bad_type), + MSG_KC_MASTER_BAD_LOCAL(LogLevel.WARN, R.string.msg_kc_master_bad_local), + MSG_KC_MASTER_BAD_ERR(LogLevel.WARN, R.string.msg_kc_master_bad_err), + MSG_KC_MASTER_BAD_TIME(LogLevel.WARN, R.string.msg_kc_master_bad_time), + MSG_KC_MASTER_BAD_TYPE_UID(LogLevel.WARN, R.string.msg_kc_master_bad_type_uid), + MSG_KC_MASTER_BAD(LogLevel.WARN, R.string.msg_kc_master_bad), + MSG_KC_MASTER_LOCAL(LogLevel.WARN, R.string.msg_kc_master_local), MSG_KC_REVOKE_DUP (LogLevel.DEBUG, R.string.msg_kc_revoke_dup), MSG_KC_NOTATION_DUP (LogLevel.DEBUG, R.string.msg_kc_notation_dup), MSG_KC_NOTATION_EMPTY (LogLevel.DEBUG, R.string.msg_kc_notation_empty), @@ -618,6 +619,7 @@ public abstract class OperationResult implements Parcelable { MSG_EXPORT_ERROR_STORAGE (LogLevel.ERROR, R.string.msg_export_error_storage), MSG_EXPORT_ERROR_DB (LogLevel.ERROR, R.string.msg_export_error_db), MSG_EXPORT_ERROR_IO (LogLevel.ERROR, R.string.msg_export_error_io), + MSG_EXPORT_ERROR_KEY (LogLevel.ERROR, R.string.msg_export_error_key), MSG_EXPORT_SUCCESS (LogLevel.OK, R.string.msg_export_success), MSG_CRT_UPLOAD_SUCCESS (LogLevel.OK, R.string.msg_crt_upload_success), 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 5e5a28e83..c4cd0b3e5 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java @@ -266,6 +266,35 @@ public class UncachedKeyRing { */ @SuppressWarnings("ConstantConditions") public CanonicalizedKeyRing canonicalize(OperationLog log, int indent) { + return canonicalize(log, indent, false); + } + + + /** "Canonicalizes" a public key, removing inconsistencies in the process. + * + * More specifically: + * - Remove all non-verifying self-certificates + * - Remove all "future" self-certificates + * - Remove all certificates flagged as "local" + * - Remove all certificates which are superseded by a newer one on the same target, + * including revocations with later re-certifications. + * - Remove all certificates in other positions if not of known type: + * - key revocation signatures on the master key + * - subkey binding signatures for subkeys + * - certifications and certification revocations for user ids + * - If a subkey retains no valid subkey binding certificate, remove it + * - If a user id retains no valid self certificate, remove it + * - If the key is a secret key, remove all certificates by foreign keys + * - If no valid user id remains, log an error and return null + * + * This operation writes an OperationLog which can be used as part of an OperationResultParcel. + * + * @param forExport if this is true, non-exportable signatures will be removed + * @return A canonicalized key, or null on fatal error (log will include a message in this case) + * + */ + @SuppressWarnings("ConstantConditions") + public CanonicalizedKeyRing canonicalize(OperationLog log, int indent, boolean forExport) { log.add(isSecret() ? LogType.MSG_KC_SECRET : LogType.MSG_KC_PUBLIC, indent, KeyFormattingUtils.convertKeyIdToHex(getMasterKeyId())); @@ -311,7 +340,7 @@ public class UncachedKeyRing { || type == PGPSignature.CASUAL_CERTIFICATION || type == PGPSignature.POSITIVE_CERTIFICATION || type == PGPSignature.CERTIFICATION_REVOCATION) { - log.add(LogType.MSG_KC_REVOKE_BAD_TYPE_UID, indent); + log.add(LogType.MSG_KC_MASTER_BAD_TYPE_UID, indent); modified = PGPPublicKey.removeCertification(modified, zert); badCerts += 1; continue; @@ -320,7 +349,7 @@ public class UncachedKeyRing { if (type != PGPSignature.KEY_REVOCATION && type != PGPSignature.DIRECT_KEY) { // Unknown type, just remove - log.add(LogType.MSG_KC_BAD_TYPE, indent, "0x" + Integer.toString(type, 16)); + log.add(LogType.MSG_KC_MASTER_BAD_TYPE, indent, "0x" + Integer.toString(type, 16)); modified = PGPPublicKey.removeCertification(modified, zert); badCerts += 1; continue; @@ -328,7 +357,7 @@ public class UncachedKeyRing { if (cert.getCreationTime().after(nowPlusOneDay)) { // Creation date in the future? No way! - log.add(LogType.MSG_KC_REVOKE_BAD_TIME, indent); + log.add(LogType.MSG_KC_MASTER_BAD_TIME, indent); modified = PGPPublicKey.removeCertification(modified, zert); badCerts += 1; continue; @@ -337,23 +366,31 @@ public class UncachedKeyRing { try { cert.init(masterKey); if (!cert.verifySignature(masterKey)) { - log.add(LogType.MSG_KC_REVOKE_BAD, indent); + log.add(LogType.MSG_KC_MASTER_BAD, indent); modified = PGPPublicKey.removeCertification(modified, zert); badCerts += 1; continue; } } catch (PgpGeneralException e) { - log.add(LogType.MSG_KC_REVOKE_BAD_ERR, indent); + log.add(LogType.MSG_KC_MASTER_BAD_ERR, indent); modified = PGPPublicKey.removeCertification(modified, zert); badCerts += 1; continue; } - // special case: direct key signatures! + // if this is for export, we always remove any non-exportable certs + if (forExport && cert.isLocal()) { + // Remove revocation certs with "local" flag + log.add(LogType.MSG_KC_MASTER_LOCAL, indent); + modified = PGPPublicKey.removeCertification(modified, zert); + continue; + } + + // special case: non-exportable, direct key signatures for notations! if (cert.getSignatureType() == PGPSignature.DIRECT_KEY) { // must be local, otherwise strip! if (!cert.isLocal()) { - log.add(LogType.MSG_KC_BAD_TYPE, indent); + log.add(LogType.MSG_KC_MASTER_BAD_TYPE, indent); modified = PGPPublicKey.removeCertification(modified, zert); badCerts += 1; continue; @@ -376,7 +413,7 @@ public class UncachedKeyRing { continue; } else if (cert.isLocal()) { // Remove revocation certs with "local" flag - log.add(LogType.MSG_KC_REVOKE_BAD_LOCAL, indent); + log.add(LogType.MSG_KC_MASTER_BAD_LOCAL, indent); modified = PGPPublicKey.removeCertification(modified, zert); badCerts += 1; continue; -- cgit v1.2.3