From c57355b24a33200b1d6c35bfcac92d4c5bdfa908 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Wed, 14 Jan 2015 00:00:04 +0100 Subject: [PATCH] actually import user attributes (though they are not shown anywhere yet) --- .../operations/results/OperationResult.java | 12 ++ .../keychain/pgp/PgpKeyOperation.java | 2 +- .../keychain/pgp/UncachedPublicKey.java | 30 ++++ .../keychain/pgp/WrappedSignature.java | 3 + .../keychain/pgp/WrappedUserAttribute.java | 21 ++- .../keychain/provider/KeychainProvider.java | 4 +- .../keychain/provider/ProviderHelper.java | 129 ++++++++++++++++-- OpenKeychain/src/main/res/values/strings.xml | 17 +++ 8 files changed, 202 insertions(+), 16 deletions(-) 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 9824173f5..556d70cb2 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 @@ -343,6 +343,18 @@ public abstract class OperationResult implements Parcelable { MSG_IP_UID_REORDER(LogLevel.DEBUG, R.string.msg_ip_uid_reorder), MSG_IP_UID_PROCESSING (LogLevel.DEBUG, R.string.msg_ip_uid_processing), MSG_IP_UID_REVOKED (LogLevel.DEBUG, R.string.msg_ip_uid_revoked), + MSG_IP_UAT_CLASSIFYING (LogLevel.DEBUG, R.string.msg_ip_uat_classifying), + MSG_IP_UAT_PROCESSING_IMAGE (LogLevel.DEBUG, R.string.msg_ip_uat_processing_image), + MSG_IP_UAT_PROCESSING_UNKNOWN (LogLevel.DEBUG, R.string.msg_ip_uat_processing_unknown), + MSG_IP_UAT_REVOKED (LogLevel.DEBUG, R.string.msg_ip_uat_revoked), + MSG_IP_UAT_CERT_BAD (LogLevel.WARN, R.string.msg_ip_uat_cert_bad), + MSG_IP_UAT_CERT_OLD (LogLevel.DEBUG, R.string.msg_ip_uat_cert_old), + MSG_IP_UAT_CERT_NONREVOKE (LogLevel.DEBUG, R.string.msg_ip_uat_cert_nonrevoke), + MSG_IP_UAT_CERT_NEW (LogLevel.DEBUG, R.string.msg_ip_uat_cert_new), + MSG_IP_UAT_CERT_ERROR (LogLevel.WARN, R.string.msg_ip_uat_cert_error), + MSG_IP_UAT_CERTS_UNKNOWN (LogLevel.DEBUG, R.plurals.msg_ip_uat_certs_unknown), + MSG_IP_UAT_CERT_GOOD_REVOKE (LogLevel.DEBUG, R.string.msg_ip_uat_cert_good_revoke), + MSG_IP_UAT_CERT_GOOD (LogLevel.DEBUG, R.string.msg_ip_uat_cert_good), // import secret MSG_IS(LogLevel.START, R.string.msg_is), diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java index 8facbfd2a..18a5410bf 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/PgpKeyOperation.java @@ -531,7 +531,7 @@ public class PgpKeyOperation { WrappedUserAttribute attribute = saveParcel.mAddUserAttribute.get(i); switch (attribute.getType()) { - case WrappedUserAttribute.UAT_UNKNOWN: + case WrappedUserAttribute.UAT_NONE: log.add(LogType.MSG_MF_UAT_ADD_UNKNOWN, indent); break; case WrappedUserAttribute.UAT_IMAGE: diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java index fe3ab96a5..9e3528515 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java @@ -24,6 +24,7 @@ import org.spongycastle.bcpg.sig.KeyFlags; import org.spongycastle.openpgp.PGPPublicKey; import org.spongycastle.openpgp.PGPSignature; import org.spongycastle.openpgp.PGPSignatureSubpacketVector; +import org.spongycastle.openpgp.PGPUserAttributeSubpacketVector; import org.spongycastle.openpgp.operator.jcajce.JcaPGPContentVerifierBuilderProvider; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.util.IterableIterator; @@ -215,6 +216,15 @@ public class UncachedPublicKey { return userIds; } + public ArrayList getUnorderedUserAttributes() { + ArrayList userAttributes = new ArrayList(); + for (PGPUserAttributeSubpacketVector userAttribute : + new IterableIterator(mPublicKey.getUserAttributes())) { + userAttributes.add(new WrappedUserAttribute(userAttribute)); + } + return userAttributes; + } + public boolean isElGamalEncrypt() { return getAlgorithm() == PGPPublicKey.ELGAMAL_ENCRYPT; } @@ -270,6 +280,25 @@ public class UncachedPublicKey { } } + public Iterator getSignaturesForUserAttribute(WrappedUserAttribute attribute) { + final Iterator it = mPublicKey.getSignaturesForUserAttribute(attribute.getVector()); + if (it != null) { + return new Iterator() { + public void remove() { + it.remove(); + } + public WrappedSignature next() { + return new WrappedSignature(it.next()); + } + public boolean hasNext() { + return it.hasNext(); + } + }; + } else { + return null; + } + } + /** Get all key usage flags. * If at least one key flag subpacket is present return these. If no * subpacket is present it returns null. @@ -299,4 +328,5 @@ public class UncachedPublicKey { } return mCacheUsage; } + } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSignature.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSignature.java index 3dc02d3ed..cb03970e2 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSignature.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSignature.java @@ -214,6 +214,9 @@ public class WrappedSignature { public boolean verifySignature(CanonicalizedPublicKey key, String uid) throws PgpGeneralException { return verifySignature(key.getPublicKey(), uid); } + public boolean verifySignature(UncachedPublicKey key, WrappedUserAttribute attribute) throws PgpGeneralException { + return verifySignature(key.getPublicKey(), attribute.getVector()); + } public static WrappedSignature fromBytes(byte[] data) { PGPObjectFactory factory = new PGPObjectFactory(data); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedUserAttribute.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedUserAttribute.java index 2359d48ac..248ef11aa 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedUserAttribute.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedUserAttribute.java @@ -33,7 +33,7 @@ import java.io.Serializable; public class WrappedUserAttribute implements Serializable { - public static final int UAT_UNKNOWN = 0; + public static final int UAT_NONE = 0; public static final int UAT_IMAGE = UserAttributeSubpacketTags.IMAGE_ATTRIBUTE; private PGPUserAttributeSubpacketVector mVector; @@ -47,8 +47,9 @@ public class WrappedUserAttribute implements Serializable { } public int getType() { - if (mVector.getSubpacket(UserAttributeSubpacketTags.IMAGE_ATTRIBUTE) != null) { - return UAT_IMAGE; + UserAttributeSubpacket[] subpackets = mVector.toSubpacketArray(); + if (subpackets.length > 0) { + return subpackets[0].getType(); } return 0; } @@ -62,6 +63,20 @@ public class WrappedUserAttribute implements Serializable { } + public byte[] getEncoded () throws IOException { + UserAttributeSubpacket[] subpackets = mVector.toSubpacketArray(); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + for (UserAttributeSubpacket subpacket : subpackets) { + subpacket.encode(out); + } + return out.toByteArray(); + } + + public static WrappedUserAttribute fromData (byte[] data) { + // TODO + return null; + } + /** Writes this object to an ObjectOutputStream. */ private void writeObject(java.io.ObjectOutputStream out) throws IOException { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainProvider.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainProvider.java index 13a923f03..29b9c5f9f 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainProvider.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainProvider.java @@ -486,10 +486,12 @@ public class KeychainProvider extends ContentProvider { // for now, we only respect user ids here, so TYPE must be NULL // TODO expand with KEY_RING_USER_PACKETS query type which lifts this restriction - qb.appendWhere(Tables.USER_PACKETS + "." + UserPackets.TYPE + " IS NULL AND "); + qb.appendWhere(Tables.USER_PACKETS + "." + UserPackets.TYPE + " IS NULL"); // If we are searching for a particular keyring's ids, add where if (match == KEY_RING_USER_IDS) { + // TODO remove with the thing above + qb.appendWhere(" AND "); qb.appendWhere(Tables.USER_PACKETS + "." + UserPackets.MASTER_KEY_ID + " = "); qb.appendWhereEscapeString(uri.getPathSegments().get(1)); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java index b3f98117d..c3990229d 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java @@ -31,6 +31,7 @@ import android.support.v4.util.LongSparseArray; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.operations.results.ImportKeyResult; +import org.sufficientlysecure.keychain.pgp.WrappedUserAttribute; import org.sufficientlysecure.keychain.provider.KeychainContract.UserPackets; import org.sufficientlysecure.keychain.ui.util.KeyFormattingUtils; import org.sufficientlysecure.keychain.util.ParcelableFileCache.IteratorWithSize; @@ -439,18 +440,18 @@ public class ProviderHelper { // classify and order user ids. primary are moved to the front, revoked to the back, // otherwise the order in the keyfile is preserved. + List uids = new ArrayList(); + if (trustedKeys.size() == 0) { log(LogType.MSG_IP_UID_CLASSIFYING_ZERO); } else { log(LogType.MSG_IP_UID_CLASSIFYING, trustedKeys.size()); } mIndent += 1; - List uids = new ArrayList(); - for (byte[] rawUserId : new IterableIterator( - masterKey.getUnorderedRawUserIds().iterator())) { + for (byte[] rawUserId : masterKey.getUnorderedRawUserIds()) { String userId = Utf8Util.fromUTF8ByteArrayReplaceBadEncoding(rawUserId); - UserIdItem item = new UserIdItem(); + UserPacketItem item = new UserPacketItem(); uids.add(item); item.userId = userId; @@ -533,6 +534,105 @@ public class ProviderHelper { } mIndent -= 1; + ArrayList userAttributes = masterKey.getUnorderedUserAttributes(); + // Don't spam the log if there aren't even any attributes + if ( ! userAttributes.isEmpty()) { + log(LogType.MSG_IP_UAT_CLASSIFYING); + } + + mIndent += 1; + for (WrappedUserAttribute userAttribute : userAttributes) { + + UserPacketItem item = new UserPacketItem(); + uids.add(item); + item.type = userAttribute.getType(); + item.attributeData = userAttribute.getEncoded(); + + int unknownCerts = 0; + + switch (item.type) { + case WrappedUserAttribute.UAT_IMAGE: + log(LogType.MSG_IP_UAT_PROCESSING_IMAGE); + break; + default: + log(LogType.MSG_IP_UAT_PROCESSING_UNKNOWN); + break; + } + mIndent += 1; + // look through signatures for this specific key + for (WrappedSignature cert : new IterableIterator( + masterKey.getSignaturesForUserAttribute(userAttribute))) { + long certId = cert.getKeyId(); + // self signature + if (certId == masterKeyId) { + + // NOTE self-certificates are already verified during canonicalization, + // AND we know there is at most one cert plus at most one revocation + if (!cert.isRevocation()) { + item.selfCert = cert; + } else { + item.isRevoked = true; + log(LogType.MSG_IP_UAT_REVOKED); + } + continue; + + } + + // do we have a trusted key for this? + if (trustedKeys.indexOfKey(certId) < 0) { + unknownCerts += 1; + continue; + } + + // verify signatures from known private keys + CanonicalizedPublicKey trustedKey = trustedKeys.get(certId); + + try { + cert.init(trustedKey); + // if it doesn't certify, leave a note and skip + if ( ! cert.verifySignature(masterKey, userAttribute)) { + log(LogType.MSG_IP_UAT_CERT_BAD); + continue; + } + + log(cert.isRevocation() + ? LogType.MSG_IP_UAT_CERT_GOOD_REVOKE + : LogType.MSG_IP_UAT_CERT_GOOD, + KeyFormattingUtils.convertKeyIdToHexShort(trustedKey.getKeyId()) + ); + + // check if there is a previous certificate + WrappedSignature prev = item.trustedCerts.get(cert.getKeyId()); + if (prev != null) { + // if it's newer, skip this one + if (prev.getCreationTime().after(cert.getCreationTime())) { + log(LogType.MSG_IP_UAT_CERT_OLD); + continue; + } + // if the previous one was a non-revokable certification, no need to look further + if (!prev.isRevocation() && !prev.isRevokable()) { + log(LogType.MSG_IP_UAT_CERT_NONREVOKE); + continue; + } + log(LogType.MSG_IP_UAT_CERT_NEW); + } + item.trustedCerts.put(cert.getKeyId(), cert); + + } catch (PgpGeneralException e) { + log(LogType.MSG_IP_UAT_CERT_ERROR, + KeyFormattingUtils.convertKeyIdToHex(cert.getKeyId())); + } + + } + + if (unknownCerts > 0) { + log(LogType.MSG_IP_UAT_CERTS_UNKNOWN, unknownCerts); + } + mIndent -= 1; + + } + mIndent -= 1; + progress.setProgress(LogType.MSG_IP_UID_REORDER.getMsgId(), 65, 100); log(LogType.MSG_IP_UID_REORDER); // primary before regular before revoked (see UserIdItem.compareTo) @@ -540,7 +640,7 @@ public class ProviderHelper { Collections.sort(uids); // iterate and put into db for (int userIdRank = 0; userIdRank < uids.size(); userIdRank++) { - UserIdItem item = uids.get(userIdRank); + UserPacketItem item = uids.get(userIdRank); operations.add(buildUserIdOperations(masterKeyId, item, userIdRank)); if (item.selfCert != null) { // TODO get rid of "self verified" status? this cannot even happen anymore! @@ -605,15 +705,23 @@ public class ProviderHelper { } - private static class UserIdItem implements Comparable { + private static class UserPacketItem implements Comparable { + Integer type; String userId; + byte[] attributeData; boolean isPrimary = false; boolean isRevoked = false; WrappedSignature selfCert; LongSparseArray trustedCerts = new LongSparseArray(); @Override - public int compareTo(UserIdItem o) { + public int compareTo(UserPacketItem o) { + // if one is a user id, but the other isn't, the user id always comes first. + // we compare for null values here, so != is the correct operator! + // noinspection NumberEquality + if (type != o.type) { + return type == null ? -1 : 1; + } // if one key is primary but the other isn't, the primary one always comes first if (isPrimary != o.isPrimary) { return isPrimary ? -1 : 1; @@ -1234,16 +1342,15 @@ public class ProviderHelper { * Build ContentProviderOperation to add PublicUserIds to database corresponding to a keyRing */ private ContentProviderOperation - buildUserIdOperations(long masterKeyId, UserIdItem item, int rank) { + buildUserIdOperations(long masterKeyId, UserPacketItem item, int rank) { ContentValues values = new ContentValues(); values.put(UserPackets.MASTER_KEY_ID, masterKeyId); + values.put(UserPackets.TYPE, item.type); values.put(UserPackets.USER_ID, item.userId); + values.put(UserPackets.ATTRIBUTE_DATA, item.attributeData); values.put(UserPackets.IS_PRIMARY, item.isPrimary); values.put(UserPackets.IS_REVOKED, item.isRevoked); values.put(UserPackets.RANK, rank); - // we explicitly set these to null here, to indicate that this is a user id, not an attribute - values.put(UserPackets.TYPE, (String) null); - values.put(UserPackets.ATTRIBUTE_DATA, (String) null); Uri uri = UserPackets.buildUserIdsUri(masterKeyId); diff --git a/OpenKeychain/src/main/res/values/strings.xml b/OpenKeychain/src/main/res/values/strings.xml index 6e2b84642..87de16f6a 100644 --- a/OpenKeychain/src/main/res/values/strings.xml +++ b/OpenKeychain/src/main/res/values/strings.xml @@ -683,6 +683,23 @@ "Re-ordering user IDs" "Processing user ID %s" "User ID is revoked" + + "Processing user attribute of type image" + "Processing user attribute of unknown type" + "Encountered bad certificate!" + "Error processing certificate!" + "Already have a non-revokable certificate, skipping." + "Certificate is older than previous, skipping." + "Certificate is more recent, replacing previous." + "Found good certificate by %1$s" + "Found good certificate revocation by %1$s" + + "Ignoring one certificate issued by an unknown public key" + "Ignoring %s certificates issued by unknown public keys" + + "Classifying user attributes" + "User attribute is revoked" + "Tried to import public keyring as secret. This is a bug, please file a report!" "Tried to import a keyring without canonicalization. This is a bug, please file a report!"