From 9e37522bddc7c5a048aca7fc60153b6f10ce8f44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Sch=C3=BCrmann?= Date: Sat, 13 Sep 2014 19:05:53 +0200 Subject: Get certificates by raw user ids, be more liberal about accepted user id encodings --- .../keychain/pgp/CanonicalizedKeyRing.java | 4 ++ .../keychain/pgp/UncachedKeyRing.java | 49 ++++++++++++---------- .../keychain/pgp/UncachedPublicKey.java | 34 ++++++++++----- .../keychain/pgp/WrappedSignature.java | 12 +++++- .../keychain/provider/ProviderHelper.java | 12 ++++-- 5 files changed, 72 insertions(+), 39 deletions(-) (limited to 'OpenKeychain/src/main/java/org/sufficientlysecure') diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedKeyRing.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedKeyRing.java index 0d3e5676a..554899843 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/CanonicalizedKeyRing.java @@ -59,6 +59,10 @@ public abstract class CanonicalizedKeyRing extends KeyRing { return getPublicKey().getPrimaryUserIdWithFallback(); } + public ArrayList getUnorderedRawUserIds() { + return getPublicKey().getUnorderedRawUserIds(); + } + public ArrayList getUnorderedUserIds() { return getPublicKey().getUnorderedUserIds(); } 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 fea314f41..768065f1c 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedKeyRing.java @@ -33,6 +33,7 @@ import org.spongycastle.openpgp.PGPSignature; import org.spongycastle.openpgp.PGPSignatureList; import org.spongycastle.openpgp.PGPUtil; import org.spongycastle.openpgp.operator.jcajce.JcaKeyFingerprintCalculator; +import org.spongycastle.util.Strings; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException; import org.sufficientlysecure.keychain.service.results.OperationResultParcel.LogLevel; @@ -361,23 +362,25 @@ public class UncachedKeyRing { } } - ArrayList processedUserIds = new ArrayList(); - for (String userId : new IterableIterator(masterKey.getUserIDs())) { + ArrayList processedUserIds = new ArrayList(); + for (byte[] rawUserId : new IterableIterator(masterKey.getRawUserIDs())) { + String userId = Strings.fromUTF8ByteArray(rawUserId); + // check for duplicate user ids - if (processedUserIds.contains(userId)) { + if (processedUserIds.contains(rawUserId)) { log.add(LogLevel.WARN, LogType.MSG_KC_UID_DUP, indent, userId); // strip out the first found user id with this name - modified = PGPPublicKey.removeCertification(modified, userId); + modified = PGPPublicKey.removeCertification(modified, rawUserId); } - processedUserIds.add(userId); + processedUserIds.add(rawUserId); PGPSignature selfCert = null; revocation = null; // look through signatures for this specific user id @SuppressWarnings("unchecked") - Iterator signaturesIt = masterKey.getSignaturesForID(userId); + Iterator signaturesIt = masterKey.getSignaturesForID(rawUserId); if (signaturesIt != null) { for (PGPSignature zert : new IterableIterator(signaturesIt)) { WrappedSignature cert = new WrappedSignature(zert); @@ -391,7 +394,7 @@ public class UncachedKeyRing { && type != PGPSignature.CERTIFICATION_REVOCATION) { log.add(LogLevel.WARN, LogType.MSG_KC_UID_BAD_TYPE, indent, "0x" + Integer.toString(zert.getSignatureType(), 16)); - modified = PGPPublicKey.removeCertification(modified, userId, zert); + modified = PGPPublicKey.removeCertification(modified, rawUserId, zert); badCerts += 1; continue; } @@ -399,7 +402,7 @@ public class UncachedKeyRing { if (cert.getCreationTime().after(now)) { // Creation date in the future? No way! log.add(LogLevel.WARN, LogType.MSG_KC_UID_BAD_TIME, indent); - modified = PGPPublicKey.removeCertification(modified, userId, zert); + modified = PGPPublicKey.removeCertification(modified, rawUserId, zert); badCerts += 1; continue; } @@ -407,7 +410,7 @@ public class UncachedKeyRing { if (cert.isLocal()) { // Creation date in the future? No way! log.add(LogLevel.WARN, LogType.MSG_KC_UID_BAD_LOCAL, indent); - modified = PGPPublicKey.removeCertification(modified, userId, zert); + modified = PGPPublicKey.removeCertification(modified, rawUserId, zert); badCerts += 1; continue; } @@ -418,7 +421,7 @@ public class UncachedKeyRing { if (isSecret()) { log.add(LogLevel.WARN, LogType.MSG_KC_UID_FOREIGN, indent, PgpKeyHelper.convertKeyIdToHex(certId)); - modified = PGPPublicKey.removeCertification(modified, userId, zert); + modified = PGPPublicKey.removeCertification(modified, rawUserId, zert); badCerts += 1; } continue; @@ -427,17 +430,17 @@ public class UncachedKeyRing { // Otherwise, first make sure it checks out try { cert.init(masterKey); - if (!cert.verifySignature(masterKey, userId)) { + if (!cert.verifySignature(masterKey, rawUserId)) { log.add(LogLevel.WARN, LogType.MSG_KC_UID_BAD, indent, userId); - modified = PGPPublicKey.removeCertification(modified, userId, zert); + modified = PGPPublicKey.removeCertification(modified, rawUserId, zert); badCerts += 1; continue; } } catch (PgpGeneralException e) { log.add(LogLevel.WARN, LogType.MSG_KC_UID_BAD_ERR, indent, userId); - modified = PGPPublicKey.removeCertification(modified, userId, zert); + modified = PGPPublicKey.removeCertification(modified, rawUserId, zert); badCerts += 1; continue; } @@ -452,13 +455,13 @@ public class UncachedKeyRing { } else if (selfCert.getCreationTime().before(cert.getCreationTime())) { log.add(LogLevel.DEBUG, LogType.MSG_KC_UID_CERT_DUP, indent, userId); - modified = PGPPublicKey.removeCertification(modified, userId, selfCert); + modified = PGPPublicKey.removeCertification(modified, rawUserId, selfCert); redundantCerts += 1; selfCert = zert; } else { log.add(LogLevel.DEBUG, LogType.MSG_KC_UID_CERT_DUP, indent, userId); - modified = PGPPublicKey.removeCertification(modified, userId, zert); + modified = PGPPublicKey.removeCertification(modified, rawUserId, zert); redundantCerts += 1; } // If there is a revocation certificate, and it's older than this, drop it @@ -466,7 +469,7 @@ public class UncachedKeyRing { && revocation.getCreationTime().before(selfCert.getCreationTime())) { log.add(LogLevel.DEBUG, LogType.MSG_KC_UID_REVOKE_OLD, indent, userId); - modified = PGPPublicKey.removeCertification(modified, userId, revocation); + modified = PGPPublicKey.removeCertification(modified, rawUserId, revocation); revocation = null; redundantCerts += 1; } @@ -477,7 +480,7 @@ public class UncachedKeyRing { if (selfCert != null && selfCert.getCreationTime().after(zert.getCreationTime())) { log.add(LogLevel.DEBUG, LogType.MSG_KC_UID_REVOKE_OLD, indent, userId); - modified = PGPPublicKey.removeCertification(modified, userId, zert); + modified = PGPPublicKey.removeCertification(modified, rawUserId, zert); redundantCerts += 1; continue; } @@ -488,13 +491,13 @@ public class UncachedKeyRing { } else if (revocation.getCreationTime().before(cert.getCreationTime())) { log.add(LogLevel.DEBUG, LogType.MSG_KC_UID_REVOKE_DUP, indent, userId); - modified = PGPPublicKey.removeCertification(modified, userId, revocation); + modified = PGPPublicKey.removeCertification(modified, rawUserId, revocation); redundantCerts += 1; revocation = zert; } else { log.add(LogLevel.DEBUG, LogType.MSG_KC_UID_REVOKE_DUP, indent, userId); - modified = PGPPublicKey.removeCertification(modified, userId, zert); + modified = PGPPublicKey.removeCertification(modified, rawUserId, zert); redundantCerts += 1; } break; @@ -506,7 +509,7 @@ public class UncachedKeyRing { if (selfCert == null && revocation == null) { log.add(LogLevel.ERROR, LogType.MSG_KC_UID_REMOVE, indent, userId); - modified = PGPPublicKey.removeCertification(modified, userId); + modified = PGPPublicKey.removeCertification(modified, rawUserId); } } @@ -817,9 +820,9 @@ public class UncachedKeyRing { } // Copy over all user id certificates - for (String userId : new IterableIterator(key.getUserIDs())) { + for (byte[] rawUserId : new IterableIterator(key.getRawUserIDs())) { @SuppressWarnings("unchecked") - Iterator signaturesIt = key.getSignaturesForID(userId); + Iterator signaturesIt = key.getSignaturesForID(rawUserId); // no signatures for this User ID, skip it if (signaturesIt == null) { continue; @@ -836,7 +839,7 @@ public class UncachedKeyRing { } newCerts += 1; certs.add(encoded); - modified = PGPPublicKey.addCertification(modified, userId, cert); + modified = PGPPublicKey.addCertification(modified, rawUserId, cert); } } // If anything changed, save the updated (sub)key 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 404dbc0fb..e27190bc7 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/UncachedPublicKey.java @@ -28,11 +28,13 @@ import org.spongycastle.openpgp.PGPPublicKey; import org.spongycastle.openpgp.PGPSignature; import org.spongycastle.openpgp.PGPSignatureSubpacketVector; import org.spongycastle.openpgp.operator.jcajce.JcaPGPContentVerifierBuilderProvider; +import org.spongycastle.util.Strings; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.util.IterableIterator; import org.sufficientlysecure.keychain.util.Log; import java.util.ArrayList; +import java.util.Arrays; import java.util.Calendar; import java.util.Date; import java.util.GregorianCalendar; @@ -126,14 +128,14 @@ public class UncachedPublicKey { * */ public String getPrimaryUserId() { - String found = null; + byte[] found = null; PGPSignature foundSig = null; // noinspection unchecked - for (String userId : new IterableIterator(mPublicKey.getUserIDs())) { + for (byte[] rawUserId : new IterableIterator(mPublicKey.getRawUserIDs())) { PGPSignature revocation = null; @SuppressWarnings("unchecked") - Iterator signaturesIt = mPublicKey.getSignaturesForID(userId); + Iterator signaturesIt = mPublicKey.getSignaturesForID(rawUserId); // no signatures for this User ID if (signaturesIt == null) { continue; @@ -147,10 +149,10 @@ public class UncachedPublicKey { // make sure it's actually valid sig.init(new JcaPGPContentVerifierBuilderProvider().setProvider( Constants.BOUNCY_CASTLE_PROVIDER_NAME), mPublicKey); - if (!sig.verifyCertification(userId, mPublicKey)) { + if (!sig.verifyCertification(rawUserId, mPublicKey)) { continue; } - if (found != null && found.equals(userId)) { + if (found != null && Arrays.equals(found, rawUserId)) { found = null; } revocation = sig; @@ -169,8 +171,8 @@ public class UncachedPublicKey { // make sure it's actually valid sig.init(new JcaPGPContentVerifierBuilderProvider().setProvider( Constants.BOUNCY_CASTLE_PROVIDER_NAME), mPublicKey); - if (sig.verifyCertification(userId, mPublicKey)) { - found = userId; + if (sig.verifyCertification(rawUserId, mPublicKey)) { + found = rawUserId; foundSig = sig; // this one can't be relevant anymore at this point revocation = null; @@ -182,7 +184,11 @@ public class UncachedPublicKey { } } } - return found; + if (found != null) { + return Strings.fromUTF8ByteArray(found); + } else { + return null; + } } /** @@ -204,6 +210,14 @@ public class UncachedPublicKey { return userIds; } + public ArrayList getUnorderedRawUserIds() { + ArrayList userIds = new ArrayList(); + for (byte[] userId : new IterableIterator(mPublicKey.getRawUserIDs())) { + userIds.add(userId); + } + return userIds; + } + public boolean isElGamalEncrypt() { return getAlgorithm() == PGPPublicKey.ELGAMAL_ENCRYPT; } @@ -320,8 +334,8 @@ public class UncachedPublicKey { }; } - public Iterator getSignaturesForId(String userId) { - final Iterator it = mPublicKey.getSignaturesForID(userId); + public Iterator getSignaturesForRawId(byte[] rawUserId) { + final Iterator it = mPublicKey.getSignaturesForID(rawUserId); if (it != null) { return new Iterator() { public void remove() { 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 f24259ba7..4d4c0e5d1 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSignature.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/WrappedSignature.java @@ -187,8 +187,16 @@ public class WrappedSignature { } } - public boolean verifySignature(UncachedPublicKey key, String uid) throws PgpGeneralException { - return verifySignature(key.getPublicKey(), uid); + boolean verifySignature(PGPPublicKey key, byte[] rawUserId) throws PgpGeneralException { + try { + return mSig.verifyCertification(rawUserId, key); + } catch (PGPException e) { + throw new PgpGeneralException("Error!", e); + } + } + + public boolean verifySignature(UncachedPublicKey key, byte[] rawUserId) throws PgpGeneralException { + return verifySignature(key.getPublicKey(), rawUserId); } public boolean verifySignature(CanonicalizedPublicKey key, String uid) throws PgpGeneralException { return verifySignature(key.getPublicKey(), uid); 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 f71fcbd80..36c2dcf9f 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/ProviderHelper.java @@ -28,6 +28,7 @@ import android.net.Uri; import android.os.RemoteException; import android.support.v4.util.LongSparseArray; +import org.spongycastle.util.Strings; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.helper.Preferences; @@ -434,8 +435,11 @@ public class ProviderHelper { } mIndent += 1; List uids = new ArrayList(); - for (String userId : new IterableIterator( - masterKey.getUnorderedUserIds().iterator())) { + for (byte[] rawUserId : new IterableIterator( + masterKey.getUnorderedRawUserIds().iterator())) { + String userId = Strings.fromUTF8ByteArray(rawUserId); + Log.d(Constants.TAG, "userId: "+userId); + UserIdItem item = new UserIdItem(); uids.add(item); item.userId = userId; @@ -446,7 +450,7 @@ public class ProviderHelper { mIndent += 1; // look through signatures for this specific key for (WrappedSignature cert : new IterableIterator( - masterKey.getSignaturesForId(userId))) { + masterKey.getSignaturesForRawId(rawUserId))) { long certId = cert.getKeyId(); try { // self signature @@ -469,7 +473,7 @@ public class ProviderHelper { if (trustedKeys.indexOfKey(certId) >= 0) { CanonicalizedPublicKey trustedKey = trustedKeys.get(certId); cert.init(trustedKey); - if (cert.verifySignature(masterKey, userId)) { + if (cert.verifySignature(masterKey, rawUserId)) { item.trustedCerts.add(cert); log(LogLevel.INFO, LogType.MSG_IP_UID_CERT_GOOD, PgpKeyHelper.convertKeyIdToHexShort(trustedKey.getKeyId()) -- cgit v1.2.3